Re: [PATCH v4] acpi: pcihp: allow repeating hot-unplug requests

[Kashyap Chamarthy <kchamart@redhat.com>]

On Wed, Apr 26, 2023 at 07:20:08PM +0200, Kashyap Chamarthy wrote:
> On Wed, Apr 26, 2023 at 07:40:02PM +0300, Michael Tokarev wrote:
> > 18.04.2023 12:04, Igor Mammedov wrote:
> > > with Q35 using ACPI PCI hotplug by default, user's request to unplug
> > > device is ignored when it's issued before guest OS has been booted.
> > > And any additional attempt to request device hot-unplug afterwards
> > > results in following error:
> > > 
> > >    "Device XYZ is already in the process of unplug"
> > > 
> > > arguably it can be considered as a regression introduced by [2],
> > > before which it was possible to issue unplug request multiple
> > > times.
> > 
> > Stable-8.0 material?
> 
> FWIW, I'd say, yes. This fix is useful for stable releases.  As this
> solves a real problem for upper-management tools.
> 
> I have tested this fix; and it works.  I'll post my testing notes /
> reproducer in a follow-up email.  In short, I followed the
> reproducer steps from here[1].

Tested-by: Kashyap Chamarthy <kchamart@redhat.com>    

It solves the device-detach bug noted here[1].  As promised, here are my
reproducer notes (expanded from[1]):

Disk image prep
---------------

(1) Download an Ubuntu "Jammy" guest image from here:
    https://cloud-images.ubuntu.com/jammy/current/jammy-server-cloudimg-amd64.img

(2) Update the above disk image's kernel command-line to have the
    guest-boot slowed down by 100 seconds; use "boot_delay=100".

(3) Have an additional image ("disk2.img") ready for hot-plug/un-plug.

Test
----

(1) Build QEMU with the patch in question:

      $ git describe
      7.2.94v8.0.0-rc4-1-gfa6650df6d7

(2) Use the above QEMU binary to launch the Ubuntu "Jammy" guest:

      $ virsh dumpxml jammy1 | grep emulator
      <emulator>/home/kashyapc/tinker-space/qemu-upstream/build/qemu-system-x86_64</emulator>

(3) Have a split `tmux` ready; start the guest in the first pane, with
    the serial console logs rolling:

      $ virsh start jammy1 --console

(4) Wait until the guest consoles messages start rolling.  Once they do,
    on the other `tmux` pane, issue the below command (it's a live
    attach, followed by a detach):

      $ virsh attach-disk jammy1 ./disk2.img vdb --live --persistent \
          && sleep 1 \
          && virsh detach-disk jammy1 --live ./disk2.img
  
      Disk attached successfully
                                                     
      Disk detached successfully

(5) Enumerate the attached block devices to the guest.  We still see the
    second disk, "disk2.img":

      $> virsh domblklist jammy1
       Target   Source                         
      -------------------------------------------
       vda      /data/images/jammy-ubuntu.qcow2      
       vdb      /data/images/disk2.img

(6) Now detach the disk from the inactive guest XML (that affects
    next boot) by using "--persistent" flag; and enumerate the live
    block devices (we still see the second disk)

      $> virsh detach-disk jammy1 --persistent /data/images/disk2.img
      Disk detached successfully
  
      $> virsh domblklist jammy1
       Target   Source                         
      -------------------------------------------
       vda      /data/images/jammy-ubuntu.qcow2      
       vdb      /data/images/disk2.img
 
(NOTE: We're using two separate calls to `virsh detach-disk`, one with
"--live" and the other with "--persistent" based on upstream libvirt
recommendation in [1].)

(7) Again, re-issue the detach command with just "--live" flag:

      $> virsh detach-disk jammy1 --live /data/images/disk2.img                                       
      Disk detached successfully

(8) Re-enumerate the attached block devices: 

      $> virsh domblklist jammy1                                                                      
       Target   Source
      -------------------------------------------
       vda      /data/images/jammy-ubuntu.qcow2

Now we see the second device is detached "for real".  Overall, we were
able to successfully re-issue `device detach` while the guest is still
booting, and see through the actual detach to its logical conclusion.


[1] https://gitlab.com/libvirt/libvirt/-/issues/309 -- Disk detach is
    unsuccessfull while the guest is still booting

-- 
/kashyap