libvirt might misdetect guests as not properly shut down when using plenty of hostdev ressources

Bug #1788226 reported by Christian Ehrhardt  on 2018-08-21
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
libvirt (Ubuntu)
Undecided
Unassigned
Bionic
Undecided
Unassigned
Cosmic
Undecided
Unassigned

Bug Description

[Impact]

 * Systems using plenty of PCI Host Devices can have lost the Qemu PID
   from /proc/<pid> but the kernel still holds it quite a while while
   cleaning up resources.
   That leads to libvirtd giving up on the process and keeping the guest
   forever in the state "in shutdown" as it can't know what is going on.

 * It turns out the kernel can need quite some time, especially if it hits
   a settling per pci subbus reset per device.

 * We have tested and upstreamed changes to adapt the time libvirt waits
   in those cases. Backporting those to Cosmic/Bionic would be great for
   Ubuntu to be ready for that many PCI device passthroughs - further back
   I think the backport noise and regression-risk tradeoff is no more
   worth - also in terms of requests I only got it for 18.04.

[Test Case]

 * If a certain PCI device needs the bus reset or not depends on the
   kernel driver. One that I could prove having this issue are Nvidia 3d
   Controllers of the type "3D controller: NVIDIA Corporation Device
   1db8". It is a general concept, so other PCI devices might work to
   reproduce the case, but I can't guarantee as drivers can implement
   smarter reset code. Keep that in mind when following the repro below

 * The former default delay was ~15 second, with the guest needing ~1-2
   seconds. So we need at least 14, better 16+ affected PCI devices.
   Pass those devices to the guest with libvirt snippet like:
    <hostdev mode='subsystem' type='pci' managed='yes'> <source> <address
    domain='0x0000' bus='0x34' slot='0x0' function='0x0'/> </source>
    </hostdev>

 * Let the guest start up as you'd normally

 * Now run virsh shutdown <guestname>"

 * You'll see shortly after that the qemu process is gone from
   /proc/<pid>, but libvirt will still spin on its shutdown. That is due
   to signal 0 still being able to reach it as it only guarantees the
   existence of the PID (not the process behind it)

 * After the timeout expires libvirt would give up and leavev the guest in
   the half-dead state like:
      libvirtd: virProcessKillPainfully:401 : Failed to terminate process
      5114 with SIGKILL: Device or resource busy

 * With the fix the maximum delay is scaled if needed and the shutdown
   will work e.g. with 16 devices about ~18 seconds.
   If one wants to check, log entries at debuglevel will contain the extra
   delay scaling:
      #40 devices attached, getting the proper plus 80 seconds:
      debug : virProcessKillPainfullyDelay:361 : vpid=34896 force=1
      extradelay=80

[Regression Potential]

 * I can think of two potential regressions, but none is on the level to
   stop this fix to improve the affected cases.

 * R1: For forced shutdowns (at the end of life of a guest) the worst case
   timeout after delivering the sigkill when the process was not going
   down due to sigterm got bumped - that said really hanging cases will
   need a few seconds more until libvirt gives up on them.
   Those cases would never recover anyway, but the time until giving up
   can be visible to e.g. higher level tools/admins in that longer
   duration.

 * R2: The scaling per Hostdevice was derived out of explanations by
   Kernel developers that there is some base work plus a 1 second settling
   period. Out of that the increased max timeout can go really high if you
   will one day have hundreds of hostdevices passed through. But libvirt
   in this case is on the lower layer, if/once the kernel has code that
   does no more scale linearly we can also make this extra delay less
   linear - but so far nothing like that is on the horizon.

  * Please keep in mind that both Risks that I spelled out are only a
    problem if the guest Process is not going away to sigterm/sigkill.
    That means it already is in a really bad spot and code path, we just
    wait longer to give it a chance to clean up. 99+% of the cases will
    have guests that go away as they should, and those cases are not
    waiting the full - now extended - delay.

[Other Info]

 * n/a

---

I had an upstream discussion at [1]

The TL;DR is that if using more than a few hostdevices as pass through then on shutting down of the guest there might be an odd time.

In that time the qemu process is already gone from /proc/<pid> but still reachable via singal-0.

That makes libvirt believe the process would not die (think of zombie processes due to e.g. failed NFS files).

But what really happens is that depending on the hostdev the kernel might need up to 1-2 seconds extra time to unallocate all the pci ressources.

We came up with patches that scale the allowed time depending on the number of hostdevs as well a s a general bump for the bad case (sigkill) following the recommendation of kernel engineers who said that we are in a bad path anyway, so we could as well provide a bit more time to let it clean up.

We should add these changes at least back into Bionic IMHO.

[1]: https://www.redhat.com/archives/libvir-list/2018-August/msg01295.html

Test builds of the Cosmic version of libvirt with the upstream accepted patches are building in [1]
Once I verified that from the PPA and through some regression-pre-checks we can upload to Cosmic.

[1]: https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/3370

dann frazier (dannf) on 2018-08-21
Changed in libvirt (Ubuntu Bionic):
status: New → Triaged
status: Triaged → New

The final form is working as expected on the case just as much as all the former versions I had tested.

40 devices attached, getting the proper plus 80 seconds:
debug : virProcessKillPainfullyDelay:361 : vpid=34896 force=1 extradelay=80

Uploaded to Cosmic, tracking migration there and prepping SRU here ...

description: updated
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package libvirt - 4.6.0-2ubuntu2

---------------
libvirt (4.6.0-2ubuntu2) cosmic; urgency=medium

  * Fix an issue where guests with plenty of hostdevs attached where detected
    as not shut down due to the kernel needing more time to free up
    resources (LP: #1788226)
    - d/p/ubuntu/lp-1788226-wait-longer-5-30s-on-hard-shutdown.patch
    - d/p/ubuntu/lp-1788226-wait-longer-on-kill-per-assigned-Hostdev.patch

 -- Christian Ehrhardt <email address hidden> Tue, 21 Aug 2018 17:51:43 +0200

Changed in libvirt (Ubuntu Cosmic):
status: New → Fix Released
Changed in libvirt (Ubuntu Bionic):
status: New → Triaged

Migration of the fix into Cosmic worked and testing from there works as planned.
The SRU Template for Bionic is ready.
I'm doing a quick final run to verify from a PPA against Bionic and then push it into the SRU queue for Bionic.

Download full text (4.2 KiB)

Final PPA (final patches as accepted upstream onto Bionic): https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/3379

Upgrade log:
Reading package lists... Done
Building dependency tree
Reading state information... Done
Calculating upgrade... Done
The following packages were automatically installed and are no longer required:
  linux-headers-4.15.0-29 linux-headers-4.15.0-29-generic linux-image-4.15.0-29-generic linux-modules-4.15.0-29-generic linux-modules-extra-4.15.0-29-generic linux-tools-4.15.0-29
  linux-tools-4.15.0-29-generic
Use 'sudo apt autoremove' to remove them.
The following packages will be upgraded:
  libspice-server1 libvirt-clients libvirt-daemon libvirt-daemon-system libvirt0
5 upgraded, 0 newly installed, 0 to remove and 0 not upgraded.
Need to get 4.446 kB of archives.
After this operation, 0 B of additional disk space will be used.
Do you want to continue? [Y/n] Y
Get:1 http://security.ubuntu.com/ubuntu bionic-security/main amd64 libspice-server1 amd64 0.14.0-1ubuntu2.2 [345 kB]
Get:2 http://ppa.launchpad.net/ci-train-ppa-service/3379/ubuntu bionic/main amd64 libvirt-daemon-system amd64 4.0.0-1ubuntu8.4 [80,7 kB]
Get:3 http://ppa.launchpad.net/ci-train-ppa-service/3379/ubuntu bionic/main amd64 libvirt-clients amd64 4.0.0-1ubuntu8.4 [596 kB]
Get:4 http://ppa.launchpad.net/ci-train-ppa-service/3379/ubuntu bionic/main amd64 libvirt-daemon amd64 4.0.0-1ubuntu8.4 [2.173 kB]
Get:5 http://ppa.launchpad.net/ci-train-ppa-service/3379/ubuntu bionic/main amd64 libvirt0 amd64 4.0.0-1ubuntu8.4 [1.251 kB]
Fetched 4.446 kB in 23s (192 kB/s)
Preconfiguring packages ...
(Reading database ... 168374 files and directories currently installed.)
Preparing to unpack .../libvirt-daemon-system_4.0.0-1ubuntu8.4_amd64.deb ...
Unpacking libvirt-daemon-system (4.0.0-1ubuntu8.4) over (4.0.0-1ubuntu8.3) ...
Preparing to unpack .../libvirt-clients_4.0.0-1ubuntu8.4_amd64.deb ...
Unpacking libvirt-clients (4.0.0-1ubuntu8.4) over (4.0.0-1ubuntu8.3) ...
Preparing to unpack .../libvirt-daemon_4.0.0-1ubuntu8.4_amd64.deb ...
Unpacking libvirt-daemon (4.0.0-1ubuntu8.4) over (4.0.0-1ubuntu8.3) ...
Preparing to unpack .../libvirt0_4.0.0-1ubuntu8.4_amd64.deb ...
Unpacking libvirt0:amd64 (4.0.0-1ubuntu8.4) over (4.0.0-1ubuntu8.3) ...
Preparing to unpack .../libspice-server1_0.14.0-1ubuntu2.2_amd64.deb ...
Unpacking libspice-server1:amd64 (0.14.0-1ubuntu2.2) over (0.14.0-1ubuntu2.1) ...
Setting up libspice-server1:amd64 (0.14.0-1ubuntu2.2) ...
Setting up libvirt0:amd64 (4.0.0-1ubuntu8.4) ...
Setting up libvirt-daemon (4.0.0-1ubuntu8.4) ...
Processing triggers for libc-bin (2.27-3ubuntu1) ...
Processing triggers for systemd (237-3ubuntu10.3) ...
Processing triggers for man-db (2.8.3-2) ...
Setting up libvirt-clients (4.0.0-1ubuntu8.4) ...
Setting up libvirt-daemon-system (4.0.0-1ubuntu8.4) ...
virtlockd.service is a disabled or a static unit, not starting it.
Setting up libvirt-daemon dnsmasq configuration.

Testcase (as outlined be...

Read more...

Hello , or anyone else affected,

Accepted libvirt into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/libvirt/4.0.0-1ubuntu8.4 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-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

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

Changed in libvirt (Ubuntu Bionic):
status: Triaged → Fix Committed
tags: added: verification-needed verification-needed-bionic
Download full text (3.6 KiB)

Upgrade to proposed:
$ sudo apt install libvirt0 libvirt-daemon-system libvirt-daemon libvirt-clients
Reading package lists... Done
Building dependency tree
Reading state information... Done
The following packages were automatically installed and are no longer required:
  linux-headers-4.15.0-29 linux-headers-4.15.0-29-generic linux-image-4.15.0-29-generic linux-modules-4.15.0-29-generic linux-modules-extra-4.15.0-29-generic linux-tools-4.15.0-29
  linux-tools-4.15.0-29-generic
Use 'sudo apt autoremove' to remove them.
Suggested packages:
  libvirt-daemon-driver-storage-gluster libvirt-daemon-driver-storage-sheepdog libvirt-daemon-driver-storage-zfs numad radvd auditd systemtap zfsutils pm-utils
Recommended packages:
  libxml2-utils libvirt-daemon-driver-storage-rbd
The following packages will be upgraded:
  libvirt-clients libvirt-daemon libvirt-daemon-system libvirt0
4 upgraded, 0 newly installed, 0 to remove and 32 not upgraded.
Need to get 4.100 kB of archives.
After this operation, 0 B of additional disk space will be used.
Get:1 http://archive.ubuntu.com/ubuntu bionic-proposed/main amd64 libvirt-daemon-system amd64 4.0.0-1ubuntu8.4 [80,7 kB]
Get:2 http://archive.ubuntu.com/ubuntu bionic-proposed/main amd64 libvirt-clients amd64 4.0.0-1ubuntu8.4 [596 kB]
Get:3 http://archive.ubuntu.com/ubuntu bionic-proposed/main amd64 libvirt-daemon amd64 4.0.0-1ubuntu8.4 [2.174 kB]
Get:4 http://archive.ubuntu.com/ubuntu bionic-proposed/main amd64 libvirt0 amd64 4.0.0-1ubuntu8.4 [1.249 kB]
Fetched 4.100 kB in 2s (2.099 kB/s)
Preconfiguring packages ...
(Reading database ... 168385 files and directories currently installed.)
Preparing to unpack .../libvirt-daemon-system_4.0.0-1ubuntu8.4_amd64.deb ...
Unpacking libvirt-daemon-system (4.0.0-1ubuntu8.4) over (4.0.0-1ubuntu8.3) ...
Preparing to unpack .../libvirt-clients_4.0.0-1ubuntu8.4_amd64.deb ...
Unpacking libvirt-clients (4.0.0-1ubuntu8.4) over (4.0.0-1ubuntu8.3) ...
Preparing to unpack .../libvirt-daemon_4.0.0-1ubuntu8.4_amd64.deb ...
Unpacking libvirt-daemon (4.0.0-1ubuntu8.4) over (4.0.0-1ubuntu8.3) ...
Preparing to unpack .../libvirt0_4.0.0-1ubuntu8.4_amd64.deb ...
Unpacking libvirt0:amd64 (4.0.0-1ubuntu8.4) over (4.0.0-1ubuntu8.3) ...
Setting up libvirt0:amd64 (4.0.0-1ubuntu8.4) ...
Setting up libvirt-daemon (4.0.0-1ubuntu8.4) ...
Processing triggers for libc-bin (2.27-3ubuntu1) ...
Processing triggers for systemd (237-3ubuntu10.3) ...
Processing triggers for man-db (2.8.3-2) ...
Setting up libvirt-clients (4.0.0-1ubuntu8.4) ...
Setting up libvirt-daemon-system (4.0.0-1ubuntu8.4) ...
virtlockd.service is a disabled or a static unit, not starting it.
Setting up libvirt-daemon dnsmasq configuration.

Shutting down a 40*passthrough guest works finally!

$ sudo tail -f /var/log/libvirt/libvirtd.log | grep ainful
2018-08-23 13:03:33.315+0000: 73192: debug : virProcessKillPainfullyDelay:361 : vpid=73025 force=1 extradelay=80
2018-08-23 13:03:43.320+0000: 73192: debug : virProcessKillPainfullyDelay:380 : Timed out waiting after SIGTERM to process 73025, sending SIGKILL

Afterwards list is empty and the guest correctly gone:
$ virsh list
 Id Name State ...

Read more...

tags: added: verification-done verification-done-bionic
removed: verification-needed verification-needed-bionic
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package libvirt - 4.0.0-1ubuntu8.4

---------------
libvirt (4.0.0-1ubuntu8.4) bionic; urgency=medium

  * Fix an issue where guests with plenty of hostdevs attached where detected
    as not shut down due to the kernel needing more time to free up
    resources (LP: #1788226)
    - d/p/ubuntu/lp-1788226-wait-longer-5-30s-on-hard-shutdown.patch
    - d/p/ubuntu/lp-1788226-wait-longer-on-kill-per-assigned-Hostdev.patch

 -- Christian Ehrhardt <email address hidden> Thu, 23 Aug 2018 07:36:04 +0200

Changed in libvirt (Ubuntu Bionic):
status: Fix Committed → Fix Released

The verification of the Stable Release Update for libvirt 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.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers