Isaku Yamahata <yamah...@valinux.co.jp> writes:
> On Tue, Feb 22, 2011 at 06:36:20PM +0100, William Dauchy wrote:
>> `qdev_free` when unplug a pci device
>> It resolves a bug when detaching/attaching a network device
>>
>> # virsh detach-interface dom0 network --mac 52:54:00:f6:84:ba
>> Interface detached successfully
>>
>> # virsh attach-interface dom0 network default --mac 52:54:00:f6:84:ba
>> error: Failed to attach interface
>> error: internal error unable to execute QEMU command 'device_add':
>> Duplicate ID 'net0' for device
>>
>> A detached pci device was not freed of the list `qemu_device_opts`
>> ---
>> hw/pci.c | 5 ++++-
>> 1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/hw/pci.c b/hw/pci.c
>> index 8b76cea..1e9a8f0 100644
>> --- a/hw/pci.c
>> +++ b/hw/pci.c
>> @@ -1671,13 +1671,16 @@ static int pci_unplug_device(DeviceState *qdev)
>> {
>> PCIDevice *dev = DO_UPCAST(PCIDevice, qdev, qdev);
>> PCIDeviceInfo *info = container_of(qdev->info, PCIDeviceInfo, qdev);
>> + int unplug;
>>
>> if (info->no_hotplug) {
>> qerror_report(QERR_DEVICE_NO_HOTPLUG, info->qdev.name);
>> return -1;
>> }
>> - return dev->bus->hotplug(dev->bus->hotplug_qdev, dev,
>> + unplug = dev->bus->hotplug(dev->bus->hotplug_qdev, dev,
>> PCI_HOTPLUG_DISABLED);
>> + qdev_free(qdev);
>
> Good catch.
> qdev_free() should be called only when unplug had succeeded.
> Although piix4 unplug always successes, pcie unplug may fail.
>
>> + return unplug;
>> }
>>
>> void pci_qdev_register(PCIDeviceInfo *info)
I don't think this patch is correct. Let me explain.
Device hot unplug is *not* guaranteed to succeed.
For some buses, such as USB, it always succeeds immediately, i.e. when
the device_del monitor command finishes, the device is gone. Live is
good.
But for PCI, device_del merely initiates the ACPI unplug rain dance. It
doesn't wait for the dance to complete. Why? The dance can take an
unpredictable amount of time, including forever.
Problem: Subsequent device_add can fail if it reuses the qdev ID or PCI
slot, and the unplug has not yet completed (race condition), or it
failed. Yes, Virginia, PCI hotplug *can* fail.
When unplug succeeds, the qdev is automatically destroyed.
pciej_write() does that for PIIX4. Looks like pcie_cap_slot_event()
does it for PCIE.
Your patch adds a *second* qdev_free(). No good.
