Re: [libvirt] [libvirt-tck PATCH] Fix PCI device hotplug tests

Daniel P. Berrange wrote: > On Fri, Oct 24, 2014 at 09:15:42AM -0600, Jim Fehlig wrote: > >> Eric Blake wrote: >> >>> On 10/23/2014 03:21 PM, Jim Fehlig wrote: >>> >>> >>>> This patch fixes a few issues noted while enabling the TCK PCI device >>>> hotplug tests. >>>> >>>> First, the call to node device dettach function misses parameters, resulting >>>> in the following failure >>>> >>>> Failed test 'detached device from host OS' >>>> at /usr/share/libvirt-tck/tests/domain/250-pci-host-hotplug.t line 90. >>>> died: Usage: Sys::Virt::NodeDevice::dettach(dev, driversv, flags=0) >>>> at /usr/share/libvirt-tck/tests/domain/250-pci-host-hotplug.t line 90. >>>> >>>> Trivally fixed by adding the missing parameters when calling node device >>>> dettach function. >>>> >>>> >>> That part is fine to fix (note that 'dettach' is a typo but part of the >>> API, so we are stuck with it; it's nice to use 'detach' where possible). >>> >>> >>> >>>> Second, it appears qemu needs to reach some state of initialization before >>>> host device attach/detach works properly. Currently, the test fails when >>>> detaching the device from the guest, resetting it, and reattaching it to >>>> the host >>>> >>>> Failed test 'reset the host PCI device' >>>> at /usr/share/libvirt-tck/tests/domain/250-pci-host-hotplug.t line 110. >>>> died: Sys::Virt::Error (libvirt error code: 1, message: internal error: >>>> Not resetting active device 0000:03:00.1) >>>> >>>> Failed test 'reattached device to host OS' >>>> at /usr/share/libvirt-tck/tests/domain/250-pci-host-hotplug.t line 111. >>>> died: Sys::Virt::Error (libvirt error code: 55, message: Requested operation >>>> is not valid: PCI device 0000:03:00.1 is still in use by driver QEMU, domain >>>> tck) >>>> >>>> I've found that sleeping for a bit after creating the guest allows the >>>> tests to pass. >>>> >>>> >>> But that papers over the bug. Ideally, the libvirt API should be >>> waiting longer for the guest to relinquish control of the device before >>> returning success on the first call, so that the second call cannot hit >>> a collision. >>> >> Even with this delay, the guest has not completely booted when the test >> successfully completes. So it is not clear to me how much guest >> cooperation is needed. >> >> >>> I'm not so sure that this is the right patch to make to >>> TCK, and wonder if we should instead be patching libvirt proper. Or is >>> it a case that because detach requires guest cooperation, we need to >>> wait long enough for the guest to boot to the point that it can >>> cooperate? >>> >> Perhaps. Maybe acpiphp needs to be loaded in the guest kernel before >> this will work. >> > > Yeah, I'm a little fuzzy on just how functiuonal the guest OS has to > be. I suspect the kernel mod is sufficient, so whether it finishes > booting is probably not important as long as the mod is loaded. > Right. IMO, sleeping in this test is not papering over a bug. The bug is attempting to hotplug/hotunplug devices before the guest has an opportunity to boot. As currently written, the attempt to attach/detach occurs before seabios finishes loading :-).