systemd kmod builtin uses out of date kmod context

note that the test script in the description must be run as root on a system that does have a nvme drive that isn't in use (because the nvme module can't be removed if any nvme drive is in use).

https://github.com/systemd/systemd/issues/6723

slight update to test script, add a check to make sure the nvme module was unloaded - if any nvme partitions are mounted it would cause a false positive result.

#!/bin/bash
MOD_DIR=/lib/modules/$( uname -r )/kernel/drivers/nvme/host
modprobe -rq nvme
mv $MOD_DIR/nvme.ko .
depmod -a
sleep 3
udevadm trigger
sleep 1
mv nvme.ko $MOD_DIR/
depmod -a
grep -q nvme /proc/partitions && echo FAIL nvme driver still loaded unmount all nvme partitions && exit 1
udevadm trigger
sleep 3
grep -q nvme /proc/partitions && echo PASS || echo FAIL

grr, please note line wrap in above comment; script should be:

#!/bin/bash
MOD_DIR=/lib/modules/$( uname -r )/kernel/drivers/nvme/host
modprobe -rq nvme
mv $MOD_DIR/nvme.ko .
depmod -a
sleep 3
udevadm trigger
sleep 1
mv nvme.ko $MOD_DIR/
depmod -a
grep -q nvme /proc/partitions \
  && echo FAIL nvme driver still loaded unmount all nvme partitions \
  && exit 1
udevadm trigger
sleep 3
grep -q nvme /proc/partitions && echo PASS || echo FAIL

I guess this is related to the fact that udevd main loop re-reads configs at most every 3 seconds. I find that 3 seconds is a really long time these days.

I wonder if the 3s timeout around manager_reload(manager) should be removed in udevd.

yep, having up to 3 seconds worth of stale system data is too much time. Also, I think the builtins can just validate themselves - for example the kmod builtin can trivially call kmod_validate_resource(ctx) every time it's asked to load a module, and I think the other builtins that implement .validate probably also can self-validate too, and remove the udev_builtin_validate() call entirely.

I have a small draft patch just to the kmod builtin that I'm testing:
https://code.launchpad.net/~ddstreet/+git/systemd/+ref/sf149300

I'll take another look next week to see if it can be expanded to cover all the builtins that do validation, and submit something upstream for discussion.

opened upstream merge request 6814:
https://github.com/systemd/systemd/pull/6814

I think we should fix .udeb installation and the d-i to call `udevadm control --reload` after .udeb installation and before (re-)triggering udev.

Here is a sample patch against debian-installer-utils to try.

Do you need a d-i build for testing? If yes, for which release(s)?

> Changed in systemd:
> status: New → Fix Released

for clarification, this is not Fix Released upstream, it has been rejected as Won't Fix upstream, systemd/udev believes it isn't their problem.

> Here is a sample patch against debian-installer-utils to try.

yes i think it at least needs to go here, to handle this very specific case. It may be better to put it into the if $TRIGGER section, as I don't think it is needed for only the settle case, but either way should work.

This was reported to me, so I'll ask the reporter to test with this change to verify it fixes for them; I have not been able to reproduce it myself.

Also, this upstream udev bug has caused trouble before, e.g.:
https://bugs.launchpad.net/ubuntu/+source/hw-detect/+bug/1549456

that does a udev reload too, but only when specific devices are found. Hopefully this change to update-dev will cover more cases where this upstream udev bug will cause failures, but since it's a racy 0-3 second window of stale data that udev uses, and they refuse to fix it upstream, it will be really hard to test for and/or reproduce.

@xnox, the person that reported this to me and who can reproduce it, tested with a initrd including your update-dev patch and it does fix the problem. Can you go ahead and merge it and SRU please?

This bug was fixed in the package debian-installer-utils - 1.113ubuntu2

---------------
debian-installer-utils (1.113ubuntu2) artful; urgency=medium

  * Make sure udevd internal state is up-to-date before calling trigger &
    settle. This insures that newly anna-installed udev rules and kernel
    modules are loaded by udevd for correct processing of the udev
    rules. LP: #1714505.

 -- Dimitri John Ledkov <email address hidden> Sun, 17 Sep 2017 00:41:44 +0100

Whilst we can upload fixes for zesty and trusty; neither of those will have a point release anymore to include a fix for this.

What is the rationale behind requesting a fix for trusty? How do you expect it to get onto an .iso? A customized iso?

> What is the rationale behind requesting a fix for trusty? How do you expect it to get onto
> an .iso? A customized iso?

Customers use the initrd provided in our archive, not just isos, e.g.:

http://archive.ubuntu.com/ubuntu/dists/trusty-updates/main/installer-amd64/current/images/netboot/ubuntu-installer/amd64/initrd.gz

And updated isos can be produced by debian-installer, e.g.:

http://archive.ubuntu.com/ubuntu/dists/trusty-updates/main/installer-amd64/current/images/netboot/mini.iso

So are you going to SRU this or should I?

To clarify - if we don't SRU this, what do we tell Ubuntu users (including Canonical customers) who are encountering this problem on trusty, xenial, or zesty? Sorry, we aren't fixing this, you can't use your automated preseed install. Or, sorry, you'll have to upgrade to artful, or wait for the next xenial point release.

Neither sound very friendly to the Ubuntu user base.

Hello Dan, or anyone else affected,

Accepted debian-installer-utils into zesty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/debian-installer-utils/1.113ubuntu1.17.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-zesty to verification-done-zesty. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-zesty. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Hello Dan, or anyone else affected,

Accepted debian-installer-utils into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/debian-installer-utils/1.113ubuntu1.16.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-xenial to verification-done-xenial. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-xenial. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

in zesty lxd container built initrd without -proposed di-utils and initrd with -proposed di-utils:

ubuntu@build-zesty:~/debian-installer-20101020ubuntu504/build$ make USE_UDEBS_FROM_EXTRA="zesty-security zesty-updates" build_netboot
...
ubuntu@build-zesty:~/debian-installer-20101020ubuntu504/build$ cp dest/netboot/ubuntu-installer/amd64/initrd.gz ~/initrd-fail.gz

ubuntu@build-zesty:~/debian-installer-20101020ubuntu504/build$ make reallyclean
...
ubuntu@build-zesty:~/debian-installer-20101020ubuntu504/build$ make build_netboot
...
ubuntu@build-zesty:~/debian-installer-20101020ubuntu504/build$ cp dest/netboot/ubuntu-installer/amd64/initrd.gz ~/initrd-fixed.gz

person who reported this to me tested both; automated install using their preseed configuration fails on system with nvme drive using initrd-fail.gz, succeeds using initrd-fixed.gz

reporter noted xenial fixed initrd also works.

Dan, I'm not talking about hypothetical users who hypothetically are encountering any bugs. Of course, we fix all the things.

But, specifically about this bug and trusty, are there actual users who are encountering this bug on trusty?

Specifically, since the hardware in question was released after trusty. And there will be no more point releases for trusty - meaning users of iso's will not get an update. When I say users of ISOs I mean the server.iso from cdimage.ubuntu.com, not the d-i by-products that are published on archive.ubuntu.com those can be updated. It worries me, that we have users reinstalling trusty today, when xenial has been out for so long, and bionic will be generally available in less than 4 months time.

This bug was fixed in the package debian-installer-utils - 1.113ubuntu1.17.04.1

---------------
debian-installer-utils (1.113ubuntu1.17.04.1) zesty; urgency=medium

  * Make sure udevd internal state is up-to-date before calling trigger &
    settle. This insures that newly anna-installed udev rules and kernel
    modules are loaded by udevd for correct processing of the udev
    rules. LP: #1714505.

 -- Dimitri John Ledkov <email address hidden> Sun, 17 Sep 2017 00:41:44 +0100

The verification of the Stable Release Update for debian-installer-utils has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

> But, specifically about this bug and trusty, are there actual users who are encountering this
> bug on trusty?

No, the user who reported this does not encounter it with trusty, but that of course doesn't mean it does not exist there.

I am not able to reproduce it on my nvme systems with any release, as this is a specific race condition (with the poorly-designed 3 second window where udev uses stale kernel module info) when using fully automated preseeded install onto a nvme system behind a firewall with a proxy.

> When I say users of ISOs I mean the server.iso from cdimage.ubuntu.com

When installing to multiple systems, manual ISO installation is rarely used (by experienced sysadmins), for obvious reasons.

> It worries me, that we have users reinstalling trusty today

Many users - including large companies - still use trusty. In fact some users/companies still use precise, which is why Precise ESM exists.

This bug was fixed in the package debian-installer-utils - 1.113ubuntu1.16.04.1

---------------
debian-installer-utils (1.113ubuntu1.16.04.1) xenial; urgency=medium

  * Make sure udevd internal state is up-to-date before calling trigger &
    settle. This insures that newly anna-installed udev rules and kernel
    modules are loaded by udevd for correct processing of the udev
    rules. LP: #1714505.

 -- Dimitri John Ledkov <email address hidden> Sun, 17 Sep 2017 00:41:44 +0100

xenial debian-installer has been rebuilt and archive.ubuntu.com updated 2018-01-02 and includes this fix:
http://archive.ubuntu.com/ubuntu/dists/xenial-updates/main/installer-amd64/20101020ubuntu451.18/