[libvirt] [PATCH] qemu: Do not fail if detachment succeeded but failed on resetting device
It makes one curious to see error "Failed to detach device" while the device is detached successfully, only because of it failed on resetting the device. And on the other hand, failed on resetting is not the end of world, one could resolve the problem of resetting outside of libvirt, and do reattachment then. --- src/qemu/qemu_hotplug.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 4b60839..c3167ea 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2033,11 +2033,11 @@ qemuDomainDetachHostPciDevice(struct qemud_driver *driver, driver->inactivePciHostdevs) == 0) qemuReattachPciDevice(activePci, driver); else - ret = -1; + VIR_WARN("Device is detached, but failed to reset PCI device"); pciFreeDevice(pci); pciFreeDevice(activePci); } else { - ret = -1; + VIR_WARN("Device is detached, but failed to reset PCI device"); } if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) && -- 1.7.7.3
On 01/17/2012 01:46 PM, Osier Yang wrote:
It makes one curious to see error "Failed to detach device" while the device is detached successfully, only because of it failed on resetting the device. And on the other hand, failed on resetting is not the end of world, one could resolve the problem of resetting outside of libvirt, and do reattachment then. --- src/qemu/qemu_hotplug.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 4b60839..c3167ea 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2033,11 +2033,11 @@ qemuDomainDetachHostPciDevice(struct qemud_driver *driver, driver->inactivePciHostdevs) == 0) qemuReattachPciDevice(activePci, driver); else - ret = -1; + VIR_WARN("Device is detached, but failed to reset PCI device"); pciFreeDevice(pci); pciFreeDevice(activePci); } else { - ret = -1; + VIR_WARN("Device is detached, but failed to reset PCI device");
I'm not sure about this one. Failure to reset a PCI device could mean that a DMA transaction could still be in flight, and be exploited to expose memory that should not otherwise be visible. I'd like a second opinion before pushing this. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 01/17/2012 01:46 PM, Osier Yang wrote:
It makes one curious to see error "Failed to detach device" while the device is detached successfully, only because of it failed on resetting the device. And on the other hand, failed on resetting is not the end of world, one could resolve the problem of resetting outside of libvirt, and do reattachment then. --- src/qemu/qemu_hotplug.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 4b60839..c3167ea 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2033,11 +2033,11 @@ qemuDomainDetachHostPciDevice(struct qemud_driver *driver, driver->inactivePciHostdevs) == 0) qemuReattachPciDevice(activePci, driver); else - ret = -1; + VIR_WARN("Device is detached, but failed to reset PCI device"); In here, it's just a warning for user, do we expect user to do some cleanup work by themselves? for example, to return the pci device to host although
On 01/18/2012 08:18 AM, Eric Blake wrote: the pci device is attached with managed mode. if so, user must shutdown the guest firstly then manually reattach the pci device to host. BTW. I saw original codes do cleanup work on common branch(if ... else ...).
pciFreeDevice(pci); pciFreeDevice(activePci); } else { - ret = -1; + VIR_WARN("Device is detached, but failed to reset PCI device");
I'm not sure about this one. Failure to reset a PCI device could mean that a DMA transaction could still be in flight, and be exploited to expose memory that should not otherwise be visible.
I'd like a second opinion before pushing this.
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (3)
-
Alex Jia -
Eric Blake -
Osier Yang