[libvirt] [PATCH] qemu_hotplug: Don't free the PCI device structure after hot-unplug
The pciDevice structure corresponding to the device being hot-unplugged was freed after it was "stolen" from activeList. The pointer was still used for eg-inactive list. This patch removes the free of the structure and frees it only if reset fails on the device. --- src/qemu/qemu_hotplug.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index dfca7e2..ee5a9ba 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2005,12 +2005,14 @@ qemuDomainDetachHostPciDevice(struct qemud_driver *driver, if (pci) { activePci = pciDeviceListSteal(driver->activePciHostdevs, pci); if (pciResetDevice(activePci, driver->activePciHostdevs, - driver->inactivePciHostdevs) == 0) + driver->inactivePciHostdevs) == 0) { qemuReattachPciDevice(activePci, driver); - else + } else { + /* reset of the device failed, treat it as if it was returned */ + pciFreeDevice(activePci); ret = -1; + } pciFreeDevice(pci); - pciFreeDevice(activePci); } else { ret = -1; } -- 1.7.3.4
On 05/21/2012 09:53 AM, Peter Krempa wrote:
The pciDevice structure corresponding to the device being hot-unplugged was freed after it was "stolen" from activeList. The pointer was still used for eg-inactive list. This patch removes the free of the structure and frees it only if reset fails on the device. --- src/qemu/qemu_hotplug.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-)
ACK. This whole area of code is rather nasty. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
The pciDevice structure corresponding to the device being hot-unplugged was freed after it was "stolen" from activeList. The pointer was still used for eg-inactive list. This patch removes the free of the structure and frees it only if reset fails on the device. --- I've added a check for activePci to be non-null. This should not happen now that the activePciHostdevs list does not get corrupted, but if the lookup for some strange reason fails, don't enter pciResetDevice with NULL activePci that would cause a segfault. --- src/qemu/qemu_hotplug.c | 11 +++++++---- 1 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index dfca7e2..51b8915 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2004,13 +2004,16 @@ qemuDomainDetachHostPciDevice(struct qemud_driver *driver, subsys->u.pci.slot, subsys->u.pci.function); if (pci) { activePci = pciDeviceListSteal(driver->activePciHostdevs, pci); - if (pciResetDevice(activePci, driver->activePciHostdevs, - driver->inactivePciHostdevs) == 0) + if (activePci && + pciResetDevice(activePci, driver->activePciHostdevs, + driver->inactivePciHostdevs) == 0) { qemuReattachPciDevice(activePci, driver); - else + } else { + /* reset of the device failed, treat it as if it was returned */ + pciFreeDevice(activePci); ret = -1; + } pciFreeDevice(pci); - pciFreeDevice(activePci); } else { ret = -1; } -- 1.7.3.4
On 05/22/2012 03:00 AM, Peter Krempa wrote:
The pciDevice structure corresponding to the device being hot-unplugged was freed after it was "stolen" from activeList. The pointer was still used for eg-inactive list. This patch removes the free of the structure and frees it only if reset fails on the device. --- I've added a check for activePci to be non-null. This should not happen now that the activePciHostdevs list does not get corrupted, but if the lookup for some strange reason fails, don't enter pciResetDevice with NULL activePci that would cause a segfault. ---
src/qemu/qemu_hotplug.c | 11 +++++++---- 1 files changed, 7 insertions(+), 4 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 05/22/2012 02:42 PM, Eric Blake wrote:
On 05/22/2012 03:00 AM, Peter Krempa wrote:
The pciDevice structure corresponding to the device being hot-unplugged was freed after it was "stolen" from activeList. The pointer was still used for eg-inactive list. This patch removes the free of the structure and frees it only if reset fails on the device. --- I've added a check for activePci to be non-null. This should not happen now that the activePciHostdevs list does not get corrupted, but if the lookup for some strange reason fails, don't enter pciResetDevice with NULL activePci that would cause a segfault. ---
src/qemu/qemu_hotplug.c | 11 +++++++---- 1 files changed, 7 insertions(+), 4 deletions(-)
ACK.
Thanks, pushed. Peter
participants (2)
-
Eric Blake -
Peter Krempa