udev: modernize code and propagate critical errors #12700

yuwata · 2019-05-30T04:20:13Z

Replaces #12416. Closes #12291.

@keszybz I cannot split the 3rd commit. Sorry. Could you take a look?

keszybz

Looks good in general. One pattern that I think should be improved is the if (r < 0 && r != -ENOENT) ... if (r == -ENOENT) checks (see one of the comments below). I think it'd be nice to standarize on a single form that uses if ... else if ... to avoid repeated checks of the same condition.

I think we should merge this with some minor changes, and work on follow-up cleanups later.

test/test-network/systemd-networkd-tests.py

src/basic/fs-util.h

src/udev/udev-event.c

src/udev/udev-rules.c

src/udev/udev-event.c

src/udev/udev-rules.c

src/udev/udev-event.c

keszybz · 2019-05-30T08:49:27Z

Is the first patch here by mistake? I see it also in #12699.

yuwata · 2019-05-30T09:07:52Z

Is the first patch here by mistake? I see it also in #12699.

Seems so. Sorry. Will drop.

yuwata · 2019-05-30T12:35:05Z

@keszybz Thank you for your review. I've force-pushed an updated version. Most of your comments are addressed. Exceptions are MODE_TO_PTR and udev_rule_token_free() things. PTAL.

yuwata · 2019-05-30T16:48:05Z

@mrc0mmand Is it possible to obtain udevd's logs during running systemd-networkd-tests.py? It seems some lines are ignored...

mrc0mmand · 2019-05-30T16:52:18Z

@yuwata will the Arch VM's journal help? https://ci.centos.org/job/systemd-pr-build/6666/artifact//systemd-centos-ci/artifacts_3_e2Yh/testsuite-logs-upstream.ner/journald-testsuite_PASS.log (it's the journal from the Arch KVM machine, not from the QEMU VMs for test/TEST-??-*)

https://ci.centos.org/job/systemd-pr-build/6666/artifact//systemd-centos-ci/index.html

Also, nice build ID :-)

yuwata · 2019-05-30T16:58:21Z

Thanks!

May 30 17:40:25 n3.pufty.ci.centos.org systemd-udevd[2827]: /usr/lib/udev/rules.d/84-nm-drivers.rules:10 Invalid value for PROGRAM.
May 30 17:40:25 n3.pufty.ci.centos.org systemd-udevd[2827]: /usr/lib/udev/rules.d/98-kexec.rules:1 The line takes no effect, ignoring.
May 30 17:40:25 n3.pufty.ci.centos.org systemd-udevd[2827]: /usr/lib/udev/rules.d/98-kexec.rules:2 The line takes no effect, ignoring.
May 30 17:40:25 n3.pufty.ci.centos.org systemd-udevd[2827]: /usr/lib/udev/rules.d/98-kexec.rules:3 The line takes no effect, ignoring.
May 30 17:40:25 n3.pufty.ci.centos.org systemd-udevd[2827]: /usr/lib/udev/rules.d/98-kexec.rules:4 The line takes no effect, ignoring.

Also, nice build ID :-)

Haha :-)

yuwata · 2019-05-30T17:31:11Z

Hmm, the warnings seem irrelevant... And my laptop works fine with this PR...

yuwata · 2019-05-31T06:47:13Z

Hmm. @mrc0mmand Could you enable debug logging of udevd? Adding udev.log_priority=debug to kernel command line is enough.

mrc0mmand · 2019-05-31T08:06:25Z

@yuwata https://ci.centos.org/job/systemd-ci-build/632/artifact//artifacts_all/artifacts_5tCa5U/testsuite-logs-upstream.KsK/journalctl-testsuite_PASS.log

All logs: https://ci.centos.org/job/systemd-ci-build/632/artifact//index.html

yuwata · 2019-05-31T08:32:12Z

Hmm...

Failed to run builtin 'hwdb --subsystem=pci': Invalid argument

yuwata · 2019-05-31T08:33:01Z

Ah, hwdb.bin does not exist, please run 'systemd-hwdb update'

This does the following: - rename enum udev_builtin_cmd -> UdevBuiltinCmd - rename struct udev_builtin -> UdevBuiltin - move type definitions to udev-rules.h - move prototypes of functions defined in udev-rules.c to udev-rules.h - drop to use strbuf - propagate critical errors in applying rules, - drop limitation for number of tokens per line.

Also, this makes the function refuse invalid formatting and propagate error in substituing variable.

Closes systemd#12291.

yuwata · 2019-06-02T23:43:25Z

I force-pushed a revised version. I hope all CIs will be green. PTAL.

yuwata · 2019-06-03T02:41:19Z

Yey! All green now!

keszybz · 2019-06-03T13:29:33Z

LGTM.

haraldh · 2020-02-25T11:48:38Z

This broke dracut in legacy network mode.

#14935

The original behavior of $env{INTERFACE} was undocumented and changed in the recent udev versions, breaking the ability to bring up networking reliably. Switching to $name directive should fix this issue. Related links: - systemd/systemd#12700 (udev PR) - systemd/systemd#12291 (related udev issue) - systemd/systemd#14935 (this issue, udev side) - dracutdevs#732 (this issue, dracut side) Fixes: dracutdevs#732

The original behavior of $env{INTERFACE} was undocumented and changed in the recent udev versions, breaking the ability to bring up networking reliably. Switching to $name directive should fix this issue. Related links: - systemd/systemd#12700 (udev PR) - systemd/systemd#12291 (related udev issue) - systemd/systemd#14935 (this issue, udev side) - #732 (this issue, dracut side) Fixes: #732 (cherry picked from commit a8ba1c4)

yuwata added the udev label May 30, 2019

yuwata mentioned this pull request May 30, 2019

udev-test seems to fail (in lxc containers on Semaphore and docker containers on Travis CI) #12698

Closed

yuwata force-pushed the udev-propagate-critical-errors branch from 2777705 to d46e716 Compare May 30, 2019 04:25

keszybz requested changes May 30, 2019

View reviewed changes

keszybz added the reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks label May 30, 2019

yuwata force-pushed the udev-propagate-critical-errors branch from d46e716 to fc63ba8 Compare May 30, 2019 09:22

yuwata added 2 commits May 30, 2019 21:08

util: introduce PTR_TO_MODE and MODE_TO_PTR macros

961189a

util: introduce strnpcpy() and strnscpy()

7e4831d

yuwata force-pushed the udev-propagate-critical-errors branch from fc63ba8 to 5ac9ba5 Compare May 30, 2019 12:31

yuwata added ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels May 30, 2019

yuwata force-pushed the udev-propagate-critical-errors branch from 5ac9ba5 to cb85e7b Compare May 30, 2019 15:18

yuwata mentioned this pull request May 31, 2019

udevadm: add --wait-for-initialization option and use it in systemd-networkd-tests.py #12708

Merged

yuwata force-pushed the udev-propagate-critical-errors branch from cb85e7b to 5d9f5ff Compare May 31, 2019 05:34

mrc0mmand mentioned this pull request May 31, 2019

debug-only: udev debug systemd/systemd-centos-ci#124

Closed

mrc0mmand mentioned this pull request May 31, 2019

Introduce pre/post tasks systemd/systemd-centos-ci#128

Closed

yuwata mentioned this pull request Jun 1, 2019

[DO NOT MERGE] #12718

Closed

yuwata force-pushed the udev-propagate-critical-errors branch 2 times, most recently from e72c444 to e5de67d Compare June 2, 2019 00:58

yuwata added 4 commits June 2, 2019 14:15

udev: rename enum subst_type -> FormatSubstitutionType

7e9c23d

udev: pass format type to udev_event_subst_format()

ef315ad

udev: comment that $tempnode and $sysfs formats are deprecated

3c209d6

yuwata force-pushed the udev-propagate-critical-errors branch 4 times, most recently from 7eed9bb to fd3eb83 Compare June 2, 2019 12:15

yuwata added 6 commits June 3, 2019 08:35

udev: split udev_event_apply_format() into small pieces

13cd553

Also, this makes the function refuse invalid formatting and propagate error in substituing variable.

udev: make parser for attribute of $result harder

ce4f94b

udev: replace xsprintf() + strpcpy() by strpcpyf()

1b65f1e

udev: use delete_trailing_chars() at one more place

7280153

udev: evaluate formatting in RUN= key earlier

1448820

Closes systemd#12291.

udev: check formatting of attribute or value earlier

d7aee41

yuwata force-pushed the udev-propagate-critical-errors branch from fd3eb83 to d7aee41 Compare June 2, 2019 23:38

yuwata removed the ci-fails/needs-rework 🔥 Please rework this, the CI noticed an issue with the PR label Jun 2, 2019

keszybz merged commit 2bb2488 into systemd:master Jun 3, 2019

yuwata deleted the udev-propagate-critical-errors branch June 3, 2019 13:43

haraldh mentioned this pull request Feb 25, 2020

network-legacy module seems to be broken in F31+ dracutdevs/dracut#732

Closed

mrc0mmand mentioned this pull request Feb 26, 2020

35network-legacy: use $name instead of $env{INTERFACE} dracutdevs/dracut#739

Closed

tblume mentioned this pull request May 28, 2020

network-legacy/net-genrules.sh: use $name instead of $env{INTERFACE} openSUSE/dracut.old#29

Merged

danimo mentioned this pull request Aug 20, 2020

network-legacy/net-genrules.sh: use $name instead of $env{INTERFACE} openSUSE/dracut#11

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

udev: modernize code and propagate critical errors #12700

udev: modernize code and propagate critical errors #12700

yuwata commented May 30, 2019 •

edited

keszybz left a comment

keszybz commented May 30, 2019

yuwata commented May 30, 2019

yuwata commented May 30, 2019

yuwata commented May 30, 2019

mrc0mmand commented May 30, 2019

yuwata commented May 30, 2019

yuwata commented May 30, 2019

yuwata commented May 31, 2019

mrc0mmand commented May 31, 2019 •

edited

yuwata commented May 31, 2019

yuwata commented May 31, 2019

yuwata commented Jun 2, 2019

yuwata commented Jun 3, 2019

keszybz commented Jun 3, 2019

haraldh commented Feb 25, 2020 •

edited

udev: modernize code and propagate critical errors #12700

udev: modernize code and propagate critical errors #12700

Conversation

yuwata commented May 30, 2019 • edited

keszybz left a comment

Choose a reason for hiding this comment

keszybz commented May 30, 2019

yuwata commented May 30, 2019

yuwata commented May 30, 2019

yuwata commented May 30, 2019

mrc0mmand commented May 30, 2019

yuwata commented May 30, 2019

yuwata commented May 30, 2019

yuwata commented May 31, 2019

mrc0mmand commented May 31, 2019 • edited

yuwata commented May 31, 2019

yuwata commented May 31, 2019

yuwata commented Jun 2, 2019

yuwata commented Jun 3, 2019

keszybz commented Jun 3, 2019

haraldh commented Feb 25, 2020 • edited

yuwata commented May 30, 2019 •

edited

mrc0mmand commented May 31, 2019 •

edited

haraldh commented Feb 25, 2020 •

edited