Skip to content

core/unit: sort the dropin paths of a unit#35711

Closed
wenjianhn wants to merge 1 commit intosystemd:mainfrom
wenjianhn:runc/daemon-reload
Closed

core/unit: sort the dropin paths of a unit#35711
wenjianhn wants to merge 1 commit intosystemd:mainfrom
wenjianhn:runc/daemon-reload

Conversation

@wenjianhn
Copy link
Copy Markdown
Contributor

Fixes: ab932a6 ("core: simplify unit_need_daemon_reload() a bit")
Fixes #35710

@github-actions github-actions bot added tests please-review PR is ready for (re-)review by a maintainer labels Dec 21, 2024
@wenjianhn wenjianhn force-pushed the runc/daemon-reload branch 2 times, most recently from 533830a to 8f00aa1 Compare December 21, 2024 10:51
Fixes: ab932a6 ("core: simplify unit_need_daemon_reload() a bit")
Fixes systemd#35710
Copy link
Copy Markdown
Member

@YHNdnzj YHNdnzj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I figure the current logic is flawed in other sense too: we shall not be heedlessly inserting to dropin_paths. Apart from not sorted properly, if a drop-in under the same name already exists under e.g. /run/, we'd be spuriously showing the one in /etc/ alongside that, while it should be replaced. Hence probably it's better to call unit_find_dropin_paths afterwards?

q = NULL;

strv_uniq(u->dropin_paths);
strv_sort_uniq(u->dropin_paths);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems wrong. We need to sort this based on the actual loading priorities (/etc/, /run/, /usr/local/, /usr/), not lexical order.

@YHNdnzj YHNdnzj added pid1 reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed please-review PR is ready for (re-)review by a maintainer labels Dec 21, 2024
@github-actions github-actions bot removed the reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks label Jan 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

Every unit created by runc need daemon reload since systemd v230

2 participants