[libvirt] [PATCH] release PCI address only when we have ensured it successfully

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"); -- 1.7.1

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 -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

At 2011-4-27 14:43, Daniel Veillard write:
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,
Thanks, pushed.
Daniel
participants (3)
-
Daniel Veillard
-
ghostwcy
-
Wen Congyang