Skip to content

rfkill: don't fail if the device cannot be initialized in time#6528

Closed
sfanxiang wants to merge 1 commit intosystemd:masterfrom
sfanxiang:rfkill-device-init-fail
Closed

rfkill: don't fail if the device cannot be initialized in time#6528
sfanxiang wants to merge 1 commit intosystemd:masterfrom
sfanxiang:rfkill-device-init-fail

Conversation

@sfanxiang
Copy link
Contributor

@sfanxiang sfanxiang commented Aug 3, 2017

Currently rfkill load/save fails when wait_for_initialized() returns an
error. It would be more reasonable to fall back to a generic state file
in this situation.

@sfanxiang
rfkill: don't fail if the device cannot be initialized in time
4679709 
Currently rfkill load/save fails when wait_for_initialized() returns an
error. It would be more reasonable to fall back to a generic state file
in this situation.
@keszybz
Copy link
Member

keszybz commented Aug 5, 2017

I'm not convinced this is the right change. Why should we want to set rfkill state for a device which doesn't exist (or at least udev doesn't know about)? Is there a specific situation you are trying to fix?

@sfanxiang
Copy link
Contributor Author

sfanxiang commented Aug 5, 2017 •
edited
Loading

@keszybz
When an event is received from /dev/rfkill, the device is guaranteed to exist. We should track the state for an existing device even if polling udev fails (mostly times out).

@keszybz
Copy link
Member

keszybz commented Aug 6, 2017

When would polling udev fail?

@sfanxiang
Copy link
Contributor Author

sfanxiang commented Aug 6, 2017

@keszybz
It happens to me that some devices created from udev_device_new_from_subsystem_sysname() are never initialized, while the same devices returned from polling udev are. Since wait_for_initialized() uses the former to determine initialization, it will wait for an event that has already been issued. These devices are in "/sys/devices/virtual/".

Is it possible that a device is created, but not initializable?

@poettering
Copy link
Member

poettering commented Aug 7, 2017 •
edited
Loading

This appears strange to me, too.

is it possible that you have some rfkill devices that are not marked with any property or tag or so? udev will not report them as initialized ever, as you appear to have found out in PR #6551... So far we assumed the right fix would be to add the a property to make sure this pitfall isn't an issue.

Also see: 32eae3c

Did you drop that udev rule from one of your devices? how can it happen that your rfkill device is never reported initialized by udev?

@sfanxiang
Copy link
Contributor Author

sfanxiang commented Aug 7, 2017

@poettering
The problem-causing device is located in /sys/devices/virtual/, for which builtin_path_id refuses to compose the path id, so SUBSYSTEM=="rfkill", ENV{SYSTEMD_RFKILL}="1", IMPORT{builtin}="path_id" fails before SYSTEMD_RFKILL=1 is added.

@poettering
Copy link
Member

poettering commented Aug 7, 2017

@poettering
The problem-causing device is located in /sys/devices/virtual/, for which builtin_path_id refuses to compose the path id, so SUBSYSTEM=="rfkill", ENV{SYSTEMD_RFKILL}="1", IMPORT{builtin}="path_id" fails before SYSTEMD_RFKILL=1 is added.

Ah, ouch!

Can you prep a patch that splits the rule in two, so that the SYSTEMD_RFKILL env var is applied even if the path_id part fails?

sfanxiang added a commit to sfanxiang/systemd that referenced this pull request Aug 7, 2017
@sfanxiang
rules: split the rfkill subsystem rule
8912568 
This patch makes sure both rules are applied to rfkill devices.
Otherwise the ENV rule may be skipped if path_id fails.

Fixes: systemd#6528
@sfanxiang sfanxiang mentioned this pull request Aug 7, 2017
Merged
@sfanxiang
Copy link
Contributor Author

sfanxiang commented Aug 7, 2017

Will this do? #6556

poettering pushed a commit that referenced this pull request Aug 8, 2017
@sfanxiang @poettering
rules: split the rfkill subsystem rule (#6556)
5aece00 
This patch makes sure both rules are applied to rfkill devices.
Otherwise the ENV rule may be skipped if path_id fails.

Fixes: #6528
@poettering
Copy link
Member

poettering commented Aug 8, 2017

Will this do? #6556

Yes it will! Thanks!

@sfanxiang sfanxiang deleted the rfkill-device-init-fail branch August 8, 2017 07:48
andir pushed a commit to andir/systemd that referenced this pull request Sep 7, 2017
@sfanxiang @andir
rules: split the rfkill subsystem rule (systemd#6556)
b2e3288 
This patch makes sure both rules are applied to rfkill devices.
Otherwise the ENV rule may be skipped if path_id fails.

Fixes: systemd#6528
andir pushed a commit to andir/systemd that referenced this pull request Sep 22, 2017
@sfanxiang @andir
rules: split the rfkill subsystem rule (systemd#6556)
ae8f4ac 
This patch makes sure both rules are applied to rfkill devices.
Otherwise the ENV rule may be skipped if path_id fails.

Fixes: systemd#6528
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

needs-discussion 🤔 rfkill

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sfanxiang @keszybz @poettering