allow repeating hot-unplug requests

[Christian Ehrhardt ]

[ Impact ]

 * In the past one could unplug a device, but if that didn't work
   it could be tried again. Changes in q35 backend for hotplug
   now will only queue one. But if that unplug was very early
   the guest will clean the GPEx.status and thereby never see
   the event.

 * The fix makes ACPI PCI behave the same as pcie which means
   allowing to requeue them, but under a rate controlling limit.

[ Test Plan ]

First, let's prepare an LXD VM to serve as our testbed. In this Test Plan we'll be using a Jammy VM.

physical-machine$ lxc launch ubuntu:jammy qemu-bug2018733-jammy --vm -c limits.memory=8GB
physical-machine$ lxc shell qemu-bug2018733-jammy
host-vm# apt update
host-vm# apt install -y libvirt-daemon-system libguestfs-tools
host-vm# usermod -a -G libvirt,kvm ubuntu
host-vm# su - ubuntu

In order to reproduce the issue, we will need to quickly attach and detach a disk into/from a VM. To do that, let's use an Ubuntu Cloud image and adjust its kernel's "boot_delay" parameter to give us time to perform the necessary operations.

host-vm$ wget https://cloud-images.ubuntu.com/lunar/current/lunar-server-cloudimg-amd64.img
host-vm$ qemu-img create disk.img 1G
host-vm$ sudo chown libvirt-qemu:kvm lunar-server-cloudimg-amd64.img disk.img
host-vm$ sudo chmod +x /home/ubuntu
host-vm$ sudo virt-customize -a lunar-server-cloudimg-amd64.img --root-password password:1234
host-vm$ cat > test-vm.xml << __EOF__
<domain type='kvm' id='3'>
  <name>test-vm</name>
  <memory unit='GiB'>1</memory>
  <currentMemory unit='GiB'>1</currentMemory>
  <vcpu placement='static'>1</vcpu>
  <resource>
    <partition>/machine</partition>
  </resource>
  <os>
    <type arch='x86_64' machine='pc-i440fx-4.2'>hvm</type>
    <boot dev='hd'/>
  </os>
  <features>
    <acpi/>
    <apic/>
    <vmcoreinfo state='on'/>
  </features>
  <cpu mode='custom' match='exact' check='full'>
    <model fallback='forbid'>Nehalem</model>
    <topology sockets='1' cores='1' threads='1'/>
    <feature policy='require' name='vme'/>
    <feature policy='require' name='x2apic'/>
    <feature policy='require' name='hypervisor'/>
  </cpu>
  <clock offset='utc'>
    <timer name='pit' tickpolicy='delay'/>
    <timer name='rtc' tickpolicy='catchup'/>
    <timer name='hpet' present='no'/>
  </clock>
  <on_poweroff>destroy</on_poweroff>
  <on_reboot>restart</on_reboot>
  <on_crash>destroy</on_crash>
  <devices>
    <emulator>/usr/bin/qemu-system-x86_64</emulator>
    <disk type='file' device='disk'>
      <driver name='qemu' type='qcow2' cache='none'/>
      <source file='/home/ubuntu/lunar-server-cloudimg-amd64.img' index='2'/>
      <target dev='vda' bus='virtio'/>
      <alias name='virtio-disk0'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
    </disk>
    <controller type='usb' index='0' model='none'>
      <alias name='usb'/>
    </controller>
    <controller type='pci' index='0' model='pci-root'>
      <alias name='pci.0'/>
    </controller>
    <controller type='ide' index='0'>
      <alias name='ide'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
    </controller>
    <input type='mouse' bus='ps2'>
      <alias name='input0'/>
    </input>
    <input type='keyboard' bus='ps2'>
      <alias name='input1'/>
    </input>
    <serial type='pty'>
      <source path='/dev/pts/0'/>
      <target type='isa-serial' port='0'>
        <model name='isa-serial'/>
      </target>
      <alias name='serial0'/>
    </serial>
    <console type='pty' tty='/dev/pts/0'>
      <source path='/dev/pts/0'/>
      <target type='serial' port='0'/>
      <alias name='serial0'/>
    </console>
    <memballoon model='virtio'>
      <stats period='10'/>
      <alias name='balloon0'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
    </memballoon>
    <rng model='virtio'>
      <backend model='random'>/dev/urandom</backend>
      <alias name='rng0'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/>
    </rng>
  </devices>
</domain>
__EOF__
host-vm$ virsh define test-vm.xml
host-vm$ virsh start test-vm
host-vm$ virsh console test-vm

Wait for the VM to boot, log into it (user is "root", password is "1234"), and execute:

nested-vm# sed -i 's/^GRUB_CMDLINE_LINUX_DEFAULT="\(.*\)"$/GRUB_CMDLINE_LINUX_DEFAULT="\1 boot_delay=100"/' /etc/default/grub.d/50-cloudimg-settings.cfg
nested-vm# update-grub
nested-vm# reboot

Keep this terminal open, and quickly open another terminal, log into the host VM (running on LXD, named qemu-bug2018733-jammy in this Test Plan).

physical-machine$ lxc shell qemu-bug2018733-jammy
host-vm# su - ubuntu

Closely monitor the reboot process of the nested VM on the first terminal. When the VM starts booting again, switch to the second terminal (inside the host VM) and issue:

host-vm$ virsh attach-disk test-vm /home/ubuntu/disk.img vdx --live --persistent && sleep 1 && virsh detach-disk test-vm --live vdx

You will notice that the detach operation apparently succeeded, but you can confirm that it did not by doing:

host-vm$ virsh domblklist test-vm

You will notice that the new disk (named "vpx") is still attached to the VM. If you try to detach it again, you will get an error:

host-vm$ virsh detach-disk test-vm --live vdx
error: Failed to detach disk
error: internal error: unable to execute QEMU command 'device_del': Device virtio-disk23 is already in the process of unplug

[ Previous Test Plan ]

1. Modify the Ubuntu cloudguest image to have the boot_delay=100 added to the kernel args to simulate a slowly host

2. Start the Ubuntu domain and connect to the serial console to see it boot

3. Wait until the first messages appear in the console. This is around T+50sec from the virsh start. But note that the guest boot is slowed down with the boot_delay=100 kernel arg.

4. From a second terminal attach an additional disk to the guest. It succeeds.

5. Wait a second

6. Detach the additional disk from the guest. The virsh command hangs for couple of seconds, but then succeeds.

7. Check the domain XML, the disk is still attached

8. Check the lsblk command from the guest (after it is fully booted). The disk is still attached.

9. Check the virsh domblklist output. The disk is still attached.

10. Try to detach the disk again. It fails with "error: device not found: no target device"

An flow of these with commands and example output can be seen at:
https://gitlab.com/libvirt/libvirt/uploads/651523ecd79419a6dac574504fb4d531/reproduction-logs.txt

[ Where problems could occur ]

 * Depending how far we backport this (at least v6.2 in Jammy)
   we need to double check if the used callbacks and settings work
   the same back then. While this can be just "tested" it should
   also get a review of related changes to be sure.

 * The change and thereby regressions are limited to acpi PCI
   hotplug and for a software so complex as qemu it is always good
   to be able to clearly point to a small subset of the use cases
   to know what to look out for.

[ Other Info ]

 * n/a

-----------

This was kindly reported by ~kashyapc
I only convert this into a bug for tracking.

---

Report:

This [1] QEMU patch solves a genuine bug [2] involving disk hot-
unplug. More details in the commit message, and also in the bug that is
linked here[2].

I have also flagged the fix for QEMU 8.0 stable[3], and tested that the
fix itself works[4].

Please pick up the fix[1] once it merges.

[1] https://lists.nongnu.org/archive/html/qemu-devel/2023-04/msg02473.html
    — acpi: pcihp: allow repeating hot-unplug requests
[2] https://gitlab.com/libvirt/libvirt/-/issues/309
[3] https://lists.nongnu.org/archive/html/qemu-devel/2023-04/msg04994.html
[4] https://lists.nongnu.org/archive/html/qemu-devel/2023-05/msg01070.html

--- ^^ report
--- vv extra context

Note:

- [2] + [4] have tests steps we can use for SRU verification.
- This has landed upstream by now
  https://gitlab.com/qemu-project/qemu/-/commit/0f689cf5ada4
- Also landed in 8.0 stable staging as
  https://gitlab.com/qemu-project/qemu/-/commit/76326210e439

[Kashyap Chamarthy (kashyapc)]

(Thanks for filing this, Christian!)

My expanded version of successful test result with the patch is here:

https://lists.nongnu.org/archive/html/qemu-devel/2023-05/msg01070.html

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[Launchpad Janitor (janitor)]

This bug was fixed in the package qemu - 1:7.2+dfsg-5ubuntu3

---------------
qemu (1:7.2+dfsg-5ubuntu3) mantic; urgency=medium

  * d/p/u/allow-repeating-hot-unplug-requests.patch: Allow repeating
    hot-unplug requests by making ACPI PCI able to requeue them.
    (LP: #2018733)

 -- Sergio Durigan Junior <email address hidden> Thu, 18 May 2023 15:13:14 -0400

[Steve Langasek (vorlon)]

Hello Christian, or anyone else affected,

Accepted qemu into lunar-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/qemu/1:7.2+dfsg-5ubuntu2.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, what testing has been performed on the package and change the tag from verification-needed-lunar to verification-done-lunar. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-lunar. 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 for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Ubuntu SRU Bot (ubuntu-sru-bot)]

All autopkgtests for the newly accepted qemu (1:7.2+dfsg-5ubuntu2.1) for lunar have finished running.
The following regressions have been reported in tests triggered by the package:

ganeti/3.0.2-3 (arm64, ppc64el)
systemd/252.5-2ubuntu3 (amd64, arm64, ppc64el, s390x)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/lunar/update_excuses.html#qemu

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

[Łukasz Zemczak (sil2100)]

Hello Christian, or anyone else affected,

Accepted qemu into jammy-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/qemu/1:6.2+dfsg-2ubuntu6.10 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, what testing has been performed on the package and change the tag from verification-needed-jammy to verification-done-jammy. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-jammy. 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 for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Łukasz Zemczak (sil2100)]

Hello Christian, or anyone else affected,

Accepted qemu into kinetic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/qemu/1:7.0+dfsg-7ubuntu2.5 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, what testing has been performed on the package and change the tag from verification-needed-kinetic to verification-done-kinetic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-kinetic. 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 for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Ubuntu SRU Bot (ubuntu-sru-bot)]

All autopkgtests for the newly accepted qemu (1:7.0+dfsg-7ubuntu2.5) for kinetic have finished running.
The following regressions have been reported in tests triggered by the package:

ganeti/3.0.2-1ubuntu1 (armhf)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/kinetic/update_excuses.html#qemu

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

[Ubuntu SRU Bot (ubuntu-sru-bot)]

All autopkgtests for the newly accepted qemu (1:6.2+dfsg-2ubuntu6.10) for jammy have finished running.
The following regressions have been reported in tests triggered by the package:

initramfs-tools/0.140ubuntu13.1 (amd64)
sbuild/0.81.2ubuntu6 (arm64, ppc64el, s390x)
systemd/249.11-0ubuntu3.9 (amd64, arm64, ppc64el, s390x)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/jammy/update_excuses.html#qemu

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

[Sergio Durigan Junior (sergiodj)]

Performing the verification for Jammy.

First, make sure that we can reproduce the problem following the instructions laid out in the Test Plan.

$ apt policy qemu-system-x86
qemu-system-x86:
  Installed: 1:6.2+dfsg-2ubuntu6.9
  Candidate: 1:6.2+dfsg-2ubuntu6.9
  Version table:
 *** 1:6.2+dfsg-2ubuntu6.9 500 (phased 20%)
        500 http://archive.ubuntu.com/ubuntu jammy-updates/main amd64 Packages
        100 /var/lib/dpkg/status
     1:6.2+dfsg-2ubuntu6.6 500
        500 http://security.ubuntu.com/ubuntu jammy-security/main amd64 Packages
     1:6.2+dfsg-2ubuntu6 500
        500 http://archive.ubuntu.com/ubuntu jammy/main amd64 Packages

$ virsh detach-disk test-vm --live vdx
error: Failed to detach disk
error: internal error: unable to execute QEMU command 'device_del': Device virtio-disk23 is already in the process of unplug

As expected, the error is reproducible. Now, let's update qemu using the -proposed pocket and check that the new version fixes the bug.

$ apt policy qemu-system-x86
qemu-system-x86:
  Installed: 1:6.2+dfsg-2ubuntu6.10
  Candidate: 1:6.2+dfsg-2ubuntu6.10
  Version table:
 *** 1:6.2+dfsg-2ubuntu6.10 500
        500 http://archive.ubuntu.com/ubuntu jammy-proposed/main amd64 Packages
        100 /var/lib/dpkg/status
     1:6.2+dfsg-2ubuntu6.9 500 (phased 20%)
        500 http://archive.ubuntu.com/ubuntu jammy-updates/main amd64 Packages
     1:6.2+dfsg-2ubuntu6.6 500
        500 http://security.ubuntu.com/ubuntu jammy-security/main amd64 Packages
     1:6.2+dfsg-2ubuntu6 500
        500 http://archive.ubuntu.com/ubuntu jammy/main amd64 Packages

$ virsh detach-disk test-vm --live vdx
Disk detached successfully

$ virsh domblklist test-vm
 Target Source
--------------------------------------------------------
 vda /home/ubuntu/lunar-server-cloudimg-amd64.img

As expected, the detach-disk operation now works.

This concludes the verification for Jammy.

[Sergio Durigan Junior (sergiodj)]

Performing the verification for Kinetic.

First, make sure that we can reproduce the problem following the instructions laid out in the Test Plan.

$ apt policy qemu-system-x86
qemu-system-x86:
  Installed: 1:7.0+dfsg-7ubuntu2.4
  Candidate: 1:7.0+dfsg-7ubuntu2.4
  Version table:
 *** 1:7.0+dfsg-7ubuntu2.4 500 (phased 20%)
        500 http://archive.ubuntu.com/ubuntu kinetic-updates/main amd64 Packages
        100 /var/lib/dpkg/status
     1:7.0+dfsg-7ubuntu2.1 500
        500 http://security.ubuntu.com/ubuntu kinetic-security/main amd64 Packages
     1:7.0+dfsg-7ubuntu2 500
        500 http://archive.ubuntu.com/ubuntu kinetic/main amd64 Packages

$ virsh detach-disk test-vm --live vdx
error: Failed to detach disk
error: internal error: unable to execute QEMU command 'device_del': Device virtio-disk23 is already in the process of unplug

As expected, the error is reproducible. Now, let's update qemu using the -proposed pocket and check that the new version fixes the bug.

$ apt policy qemu-system-x86
qemu-system-x86:
  Installed: 1:7.0+dfsg-7ubuntu2.5
  Candidate: 1:7.0+dfsg-7ubuntu2.5
  Version table:
 *** 1:7.0+dfsg-7ubuntu2.5 500
        500 http://archive.ubuntu.com/ubuntu kinetic-proposed/main amd64 Packages
        100 /var/lib/dpkg/status
     1:7.0+dfsg-7ubuntu2.4 500 (phased 20%)
        500 http://archive.ubuntu.com/ubuntu kinetic-updates/main amd64 Packages
     1:7.0+dfsg-7ubuntu2.1 500
        500 http://security.ubuntu.com/ubuntu kinetic-security/main amd64 Packages
     1:7.0+dfsg-7ubuntu2 500
        500 http://archive.ubuntu.com/ubuntu kinetic/main amd64 Packages

$ virsh detach-disk test-vm --live vdx
Disk detached successfully

$ virsh domblklist test-vm
 Target Source
--------------------------------------------------------
 vda /home/ubuntu/lunar-server-cloudimg-amd64.img

As expected, the detach-disk operation now works.

This concludes the verification for Kinetic.

[Sergio Durigan Junior (sergiodj)]

Performing the verification for Lunar.

First, make sure that we can reproduce the problem following the instructions laid out in the Test Plan.

$ apt policy qemu-system-x86
qemu-system-x86:
  Installed: 1:7.2+dfsg-5ubuntu2
  Candidate: 1:7.2+dfsg-5ubuntu2
  Version table:
 *** 1:7.2+dfsg-5ubuntu2 500
        500 http://archive.ubuntu.com/ubuntu lunar/main amd64 Packages
        100 /var/lib/dpkg/status

$ virsh detach-disk test-vm --live vdx
error: Failed to detach disk
error: internal error: unable to execute QEMU command 'device_del': Device virtio-disk23 is already in the process of unplug

As expected, the error is reproducible. Now, let's update qemu using the -proposed pocket and check that the new version fixes the bug.

$ apt policy qemu-system-x86
qemu-system-x86:
  Installed: 1:7.2+dfsg-5ubuntu2.1
  Candidate: 1:7.2+dfsg-5ubuntu2.1
  Version table:
 *** 1:7.2+dfsg-5ubuntu2.1 100
        100 http://archive.ubuntu.com/ubuntu lunar-proposed/main amd64 Packages
        100 /var/lib/dpkg/status
     1:7.2+dfsg-5ubuntu2 500
        500 http://archive.ubuntu.com/ubuntu lunar/main amd64 Packages

$ virsh domblklist test-vm
 Target Source
--------------------------------------------------------
 vda /home/ubuntu/lunar-server-cloudimg-amd64.img
 vdx /home/ubuntu/disk.img

$ virsh detach-disk test-vm --live vdx
Disk detached successfully

$ virsh domblklist test-vm
 Target Source
--------------------------------------------------------
 vda /home/ubuntu/lunar-server-cloudimg-amd64.img
As expected, the detach-disk operation now works.

This concludes the verification for Lunar.

[Robie Basak (racb)]

SRU release is blocked on comment 8. autopkgtest failures for the other two releases seem to have been resolved, but Lunar is still outstanding.

[Sergio Durigan Junior (sergiodj)]

I'm retrying the Lunar dep8 test now.

[Sergio Durigan Junior (sergiodj)]

I remember seeing the Lunar tests passing before, but apparently there's been a re-run of the systemd dep8 which reintroduced the flaky failure. Anyway, it's all good (again), and the package is ready to be accepted.

[Launchpad Janitor (janitor)]

This bug was fixed in the package qemu - 1:7.2+dfsg-5ubuntu2.1

---------------
qemu (1:7.2+dfsg-5ubuntu2.1) lunar; urgency=medium

  * d/p/u/allow-repeating-hot-unplug-requests.patch: Allow repeating
    hot-unplug requests by making ACPI PCI able to requeue them.
    (LP: #2018733)

 -- Sergio Durigan Junior <email address hidden> Fri, 26 May 2023 15:57:03 -0400

[Andreas Hasenack (ahasenack)]

The verification of the Stable Release Update for qemu has completed successfully and the package is now being 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.

[Launchpad Janitor (janitor)]

This bug was fixed in the package qemu - 1:7.0+dfsg-7ubuntu2.5

---------------
qemu (1:7.0+dfsg-7ubuntu2.5) kinetic; urgency=medium

  * d/p/u/allow-repeating-hot-unplug-requests.patch: Allow repeating
    hot-unplug requests by making ACPI PCI able to requeue them.
    (LP: #2018733)

 -- Sergio Durigan Junior <email address hidden> Fri, 26 May 2023 17:38:19 -0400

[Launchpad Janitor (janitor)]

This bug was fixed in the package qemu - 1:6.2+dfsg-2ubuntu6.10

---------------
qemu (1:6.2+dfsg-2ubuntu6.10) jammy; urgency=medium

  * d/p/u/allow-repeating-hot-unplug-requests.patch: Allow repeating
    hot-unplug requests by making ACPI PCI able to requeue them.
    (LP: #2018733)

 -- Sergio Durigan Junior <email address hidden> Fri, 26 May 2023 17:40:31 -0400