Re: [libvirt] [libvirt-tck PATCH] Fix PCI device hotplug tests
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. 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? If so, then maybe we should expose a proper libvirt event
for the underlying 'guest agent available' event (aka VSERPORT_CHANGE)
that recent qemu added, and wait for that event to occur within a
reasonable time rather than blindly hard-coding a fixed sleep time.
While here, also fix a small typo from copy and paste of disk tests.
It would be better to split off the obvious trivial fixes and push them
now, while waiting on discussion for the rest of the patch.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
scripts/domain/250-pci-host-hotplug.t | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/scripts/domain/250-pci-host-hotplug.t
b/scripts/domain/250-pci-host-hotplug.t
index 4579477..e128de7 100644
--- a/scripts/domain/250-pci-host-hotplug.t
+++ b/scripts/domain/250-pci-host-hotplug.t
@@ -52,6 +52,8 @@ my $xml = $tck->generic_domain(name =>
"tck")->as_xml;
diag "Creating a new transient domain";
my $dom;
ok_domain(sub { $dom = $conn->create_domain($xml) }, "created transient domain
object");
+diag "Waiting 10 seconds for guest to initialize";
+sleep(10);
So this is the controversial hunk...
my ($domain, $bus, $slot, $function) = $tck->get_host_pci_device();
@@ -86,7 +88,7 @@ SKIP: {
ok(defined $nodedev, "found PCI device $domain:$bus:$slot.$function on
host");
- lives_ok(sub { $nodedev->dettach() }, "detached device from host OS");
+ lives_ok(sub { $nodedev->dettach(undef, 0) }, "detached device from host
OS");
lives_ok(sub { $nodedev->reset() }, "reset the host PCI device");
my $devxml =
@@ -112,6 +114,6 @@ SKIP: {
my $finalxml = $dom->get_xml_description;
- is($initialxml, $finalxml, "final XML has removed the disk")
+ is($initialxml, $finalxml, "final XML has removed the PCI device")
...but ACK to these two.
}
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 539 bytes)