Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

udevadm: introduce 'wait' command #22872

Merged
merged 9 commits into from
Apr 1, 2022
Merged

udevadm: introduce 'wait' command #22872

merged 9 commits into from
Apr 1, 2022

Conversation

yuwata
Copy link
Member

@yuwata yuwata commented Mar 25, 2022

Prompted by #22717 (comment).

@yuwata
Copy link
Member Author

yuwata commented Mar 26, 2022

TODO:

  • support syspath
  • check initialized, and add --initialized=BOOL
  • add --removed

@yuwata
Copy link
Member Author

yuwata commented Mar 27, 2022

TODO:

* support syspath

* check initialized, and add --initialized=BOOL

* add --removed

done.

Copy link
Member

@poettering poettering left a comment

Choose a reason for hiding this comment

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

so, the concept is great, but i am very sure we shouldn#t use inotify for this, but use proper sd-event apis, i.e. the device monitor.

i.e. more along the lines of device_wait_for_devlink().

i.e. it is crucial for this tool to be useful that the complete udev ruleset ran for the device, i.e. we should only report ready once the udev db exists.

src/libsystemd/sd-device/sd-device.c Outdated Show resolved Hide resolved
src/udev/udevadm-wait.c Outdated Show resolved Hide resolved
src/udev/udevadm-wait.c Outdated Show resolved Hide resolved
src/udev/udevadm-wait.c Outdated Show resolved Hide resolved
@poettering poettering added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed please-review labels Mar 28, 2022
@yuwata
Copy link
Member Author

yuwata commented Mar 28, 2022

@poettering Thank you for the review. All comments are addressed. PTAL.

@yuwata yuwata added please-review and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Mar 28, 2022
src/libsystemd/sd-device/sd-device.c Outdated Show resolved Hide resolved
src/udev/udevadm-wait.c Outdated Show resolved Hide resolved
src/udev/udevadm-wait.c Show resolved Hide resolved
src/udev/udevadm-wait.c Outdated Show resolved Hide resolved
@poettering poettering added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed please-review labels Mar 28, 2022
@yuwata yuwata added please-review and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Mar 29, 2022
@yuwata
Copy link
Member Author

yuwata commented Mar 29, 2022

@poettering Thank you for the review. Hopefully, all comments are addressed. PTAL.

@yuwata yuwata added this to the v251 milestone Mar 29, 2022
@yuwata
Copy link
Member Author

yuwata commented Mar 30, 2022

The CI failures are by the tests for homed or dissect, should not be related.

Copy link
Member

@poettering poettering left a comment

Choose a reason for hiding this comment

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

looks really good

src/libsystemd/sd-device/sd-device.c Outdated Show resolved Hide resolved
src/libsystemd/sd-device/sd-device.c Show resolved Hide resolved
src/libsystemd/sd-device/sd-device.c Outdated Show resolved Hide resolved
src/udev/udev-node.c Outdated Show resolved Hide resolved
src/udev/udevd.c Outdated Show resolved Hide resolved
src/libsystemd/sd-device/sd-device.c Show resolved Hide resolved
src/udev/udevadm-wait.c Outdated Show resolved Hide resolved
src/udev/udevadm-wait.c Outdated Show resolved Hide resolved
src/udev/udevadm-wait.c Outdated Show resolved Hide resolved
helper_check_device_symlinks "/dev/disk"
# Reconnect the iSCSI devices and check if everything get detected correctly
iscsiadm --mode discoverydb --type sendtargets --portal "$target_ip" --discover
iscsiadm --mode node --targetname "$target_name" --portal "$target_ip:$target_port" --login
udevadm settle
udevadm wait --settle --timeout=30 "${expected_symlinks[@]}"
Copy link
Member

Choose a reason for hiding this comment

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

do we really need --settle on these?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. Not only 'add' but 'change' event may be already queued for the devices.

@poettering poettering added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed please-review labels Mar 30, 2022
@poettering
Copy link
Member

Another candidate for sd_device_open() is src/gpt-auto-generator/gpt-auto-generator.c's open_parent_block_device()...

@yuwata yuwata force-pushed the udevadm-wait branch 2 times, most recently from 50a6c37 to 148fb2f Compare March 30, 2022 22:26
@yuwata
Copy link
Member Author

yuwata commented Mar 30, 2022

@poettering Thank you for the review. Most comments are addressed. PTAL.

Another candidate for sd_device_open() is src/gpt-auto-generator/gpt-auto-generator.c's open_parent_block_device()...

Let's update it in later PR.

@yuwata yuwata added please-review and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Mar 30, 2022
Copy link
Member

@poettering poettering left a comment

Choose a reason for hiding this comment

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

lgtm

src/libsystemd/sd-device/sd-device.c Show resolved Hide resolved
src/libsystemd/sd-device/sd-device.c Show resolved Hide resolved
and sd_device_new_from_path() which takes devname or syspath.
Prompted by systemd#22717 (comment).

The new command 'udevadm wait' waits for device or device symlink being
created. This may be useful to wait for a device is processed by udevd
after e.g. formatting or partitioning the device.
As it is defined at linux/fs.h.
And move it from loop-util.[ch] -> fd-util.[ch]
We usually open() device node obtained by sd_device_get_devname().
However, the device node corresponds to the sd-device object may be
already removed, and another device node with the same path may be
created, hence an unexpected device may be opened.

The sd_device_open() opens device node, and checks the devnum and
diskseq of opened devnum, to avoid the above possibility.

Prompted by systemd#22906 (comment).
@yuwata
Copy link
Member Author

yuwata commented Apr 1, 2022

@poettering Thank you. The above two comments are addressed. Upgrading the green label.

@yuwata yuwata added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed good-to-merge/with-minor-suggestions labels Apr 1, 2022
@yuwata yuwata merged commit b8529cf into systemd:main Apr 1, 2022
@yuwata yuwata deleted the udevadm-wait branch April 1, 2022 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed tests udev
Development

Successfully merging this pull request may close these issues.

None yet

3 participants