[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]