On Tue, Apr 26, 2011 at 11:40:01AM +0800, Wen Congyang wrote: > Steps to reproduce this bug: > 1. # cat net.xml # 00:03.0 has been used > <interface type='network'> > <mac address='52:54:00:04:72:f3'/> > <source network='default'/> > <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> > </interface> > > 2. # virsh attach-device vm1 net.xml > error: Failed to attach device from net.xml > error: internal error unable to reserve PCI address 0:0:3 > > 3. # virsh attach-device vm1 net.xml > error: Failed to attach device from net.xml > error: internal error unable to execute QEMU command 'device_add': Device 'rtl8139' could not be initialized > > The reason of this bug is that: we can not reserve PCI address 0:0:3 because it has > been used, but we release PCI address when we reserve it failed. > > --- > src/qemu/qemu_hotplug.c | 13 +++++++++++++ > 1 files changed, 13 insertions(+), 0 deletions(-) > > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c > index b03f774..5fdb013 100644 > --- a/src/qemu/qemu_hotplug.c > +++ b/src/qemu/qemu_hotplug.c > @@ -147,6 +147,7 @@ int qemuDomainAttachPciDiskDevice(struct qemud_driver *driver, > qemuDomainObjPrivatePtr priv = vm->privateData; > char *devstr = NULL; > char *drivestr = NULL; > + bool releaseaddr = false; > > for (i = 0 ; i< vm->def->ndisks ; i++) { > if (STREQ(vm->def->disks[i]->dst, disk->dst)) { > @@ -163,6 +164,7 @@ int qemuDomainAttachPciDiskDevice(struct qemud_driver *driver, > if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { > if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs,&disk->info)< 0) > goto error; > + releaseaddr = true; > if (qemuAssignDeviceDiskAlias(disk, qemuCaps)< 0) > goto error; > > @@ -221,6 +223,7 @@ error: > > if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)&& > (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)&& > + releaseaddr&& > qemuDomainPCIAddressReleaseAddr(priv->pciaddrs,&disk->info)< 0) > VIR_WARN("Unable to release PCI address on %s", disk->src); > > @@ -242,6 +245,7 @@ int qemuDomainAttachPciControllerDevice(struct qemud_driver *driver, > const char* type = virDomainControllerTypeToString(controller->type); > char *devstr = NULL; > qemuDomainObjPrivatePtr priv = vm->privateData; > + bool releaseaddr = false; > > for (i = 0 ; i< vm->def->ncontrollers ; i++) { > if ((vm->def->controllers[i]->type == controller->type)&& > @@ -256,6 +260,7 @@ int qemuDomainAttachPciControllerDevice(struct qemud_driver *driver, > if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { > if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs,&controller->info)< 0) > goto cleanup; > + releaseaddr = true; > if (qemuAssignDeviceControllerAlias(controller)< 0) > goto cleanup; > > @@ -288,6 +293,7 @@ cleanup: > if ((ret != 0)&& > qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)&& > (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)&& > + releaseaddr&& > qemuDomainPCIAddressReleaseAddr(priv->pciaddrs,&controller->info)< 0) > VIR_WARN0("Unable to release PCI address on controller"); > > @@ -559,6 +565,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, > int ret = -1; > virDomainDevicePCIAddress guestAddr; > int vlan; > + bool releaseaddr = false; > > if (!qemuCapsGet(qemuCaps, QEMU_CAPS_HOST_NET_ADD)) { > qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > @@ -595,6 +602,8 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, > qemuDomainPCIAddressEnsureAddr(priv->pciaddrs,&net->info)< 0) > goto cleanup; > > + releaseaddr = true; > + > if (qemuCapsGet(qemuCaps, QEMU_CAPS_NETDEV)&& > qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { > vlan = -1; > @@ -694,6 +703,7 @@ cleanup: > if ((ret != 0)&& > qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)&& > (net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)&& > + releaseaddr&& > qemuDomainPCIAddressReleaseAddr(priv->pciaddrs,&net->info)< 0) > VIR_WARN0("Unable to release PCI address on NIC"); > > @@ -757,6 +767,7 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver *driver, > char *devstr = NULL; > int configfd = -1; > char *configfd_name = NULL; > + bool releaseaddr = false; > > if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs+1)< 0) { > virReportOOMError(); > @@ -771,6 +782,7 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver *driver, > goto error; > if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs,&hostdev->info)< 0) > goto error; > + releaseaddr = true; > if (qemuCapsGet(qemuCaps, QEMU_CAPS_PCI_CONFIGFD)) { > configfd = qemuOpenPCIConfig(hostdev); > if (configfd>= 0) { > @@ -823,6 +835,7 @@ int qemuDomainAttachHostPciDevice(struct qemud_driver *driver, > error: > if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)&& > (hostdev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)&& > + releaseaddr&& > qemuDomainPCIAddressReleaseAddr(priv->pciaddrs,&hostdev->info)< 0) > VIR_WARN0("Unable to release PCI address on host device"); The logic fo the fix sounds right, and it looks complete, ACK,

Daniel