[libvirt] [PATCH 02/14] Retain disk PCI address across libvirtd restarts

Mark McLoughlin markmc at redhat.com
Mon Jul 20 11:51:12 UTC 2009


When we hot-plug a disk device into a qemu guest, we need to retain its
PCI address so that it can be removed again later. Currently, we do
retain the slot number, but not across libvirtd restarts.

Add <state devaddr="xxxx:xx:xx"/> to the disk device XML config when the
VIR_DOMAIN_XML_INTERNAL_STATUS flag is used. We still don't parse the
domain and bus number, but the format allows us to do that in future.

* src/domain_conf.h: replace slotnum with pci_addr

* src/domain_conf.c: handle formatting and parsing the address

* src/qemu_driver.c: store the parsed slot number as a full PCI address,
  and use this address with the pci_del monitor command

* src/vbox/vbox_tmpl.c: we're debug printing slotnum here even though
  it can never be set, just delete it
---
 src/domain_conf.c    |   22 +++++++++++++++++++---
 src/domain_conf.h    |    2 +-
 src/qemu_driver.c    |   42 +++++++++++++++++++++++++++++-------------
 src/vbox/vbox_tmpl.c |    1 -
 4 files changed, 49 insertions(+), 18 deletions(-)

diff --git a/src/domain_conf.c b/src/domain_conf.c
index 10e6ac6..16f7d73 100644
--- a/src/domain_conf.c
+++ b/src/domain_conf.c
@@ -287,6 +287,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
     VIR_FREE(def->dst);
     VIR_FREE(def->driverName);
     VIR_FREE(def->driverType);
+    VIR_FREE(def->pci_addr);
 
     VIR_FREE(def);
 }
@@ -643,7 +644,7 @@ int virDomainDiskCompare(virDomainDiskDefPtr a,
 static virDomainDiskDefPtr
 virDomainDiskDefParseXML(virConnectPtr conn,
                          xmlNodePtr node,
-                         int flags ATTRIBUTE_UNUSED) {
+                         int flags) {
     virDomainDiskDefPtr def;
     xmlNodePtr cur;
     char *type = NULL;
@@ -654,6 +655,7 @@ virDomainDiskDefParseXML(virConnectPtr conn,
     char *target = NULL;
     char *bus = NULL;
     char *cachetag = NULL;
+    char *pci_addr = NULL;
 
     if (VIR_ALLOC(def) < 0) {
         virReportOOMError(conn);
@@ -708,6 +710,9 @@ virDomainDiskDefParseXML(virConnectPtr conn,
                 def->readonly = 1;
             } else if (xmlStrEqual(cur->name, BAD_CAST "shareable")) {
                 def->shared = 1;
+            } else if ((flags & VIR_DOMAIN_XML_INTERNAL_STATUS) &&
+                       xmlStrEqual(cur->name, BAD_CAST "state")) {
+                pci_addr = virXMLPropString(cur, "devaddr");
             }
         }
         cur = cur->next;
@@ -815,6 +820,8 @@ virDomainDiskDefParseXML(virConnectPtr conn,
     driverName = NULL;
     def->driverType = driverType;
     driverType = NULL;
+    def->pci_addr = pci_addr;
+    pci_addr = NULL;
 
 cleanup:
     VIR_FREE(bus);
@@ -825,6 +832,7 @@ cleanup:
     VIR_FREE(driverType);
     VIR_FREE(driverName);
     VIR_FREE(cachetag);
+    VIR_FREE(pci_addr);
 
     return def;
 
@@ -3388,7 +3396,8 @@ virDomainLifecycleDefFormat(virConnectPtr conn,
 static int
 virDomainDiskDefFormat(virConnectPtr conn,
                        virBufferPtr buf,
-                       virDomainDiskDefPtr def)
+                       virDomainDiskDefPtr def,
+                       int flags)
 {
     const char *type = virDomainDiskTypeToString(def->type);
     const char *device = virDomainDiskDeviceTypeToString(def->device);
@@ -3446,6 +3455,13 @@ virDomainDiskDefFormat(virConnectPtr conn,
     if (def->shared)
         virBufferAddLit(buf, "      <shareable/>\n");
 
+    if ((flags & VIR_DOMAIN_XML_INTERNAL_STATUS) && def->pci_addr) {
+        virBufferAddLit(buf, "      <state");
+        if (def->pci_addr)
+            virBufferEscapeString(buf, " devaddr='%s'", def->pci_addr);
+        virBufferAddLit(buf, "/>\n");
+    }
+
     virBufferAddLit(buf, "    </disk>\n");
 
     return 0;
@@ -4048,7 +4064,7 @@ char *virDomainDefFormat(virConnectPtr conn,
                               def->emulator);
 
     for (n = 0 ; n < def->ndisks ; n++)
-        if (virDomainDiskDefFormat(conn, &buf, def->disks[n]) < 0)
+        if (virDomainDiskDefFormat(conn, &buf, def->disks[n], flags) < 0)
             goto cleanup;
 
     for (n = 0 ; n < def->nfss ; n++)
diff --git a/src/domain_conf.h b/src/domain_conf.h
index 6e111fa..1766b61 100644
--- a/src/domain_conf.h
+++ b/src/domain_conf.h
@@ -106,7 +106,7 @@ struct _virDomainDiskDef {
     int cachemode;
     unsigned int readonly : 1;
     unsigned int shared : 1;
-    int slotnum; /* pci slot number for unattach */
+    char *pci_addr; /* for detach */
 };
 
 
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 00dc6e5..3dec630 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -4390,6 +4390,8 @@ try_command:
         return -1;
     }
 
+    VIR_FREE(cmd);
+
     DEBUG ("%s: pci_add reply: %s", vm->def->name, reply);
     /* If the command succeeds qemu prints:
      * OK bus 0, slot XXX...
@@ -4399,22 +4401,28 @@ try_command:
     if ((s = strstr(reply, "OK ")) &&
         (s = strstr(s, "slot "))) {
         char *dummy = s;
+        int domain, bus, slot;
+
         s += strlen("slot ");
 
-        if (virStrToLong_i ((const char*)s, &dummy, 10, &dev->data.disk->slotnum) == -1)
-            VIR_WARN("%s", _("Unable to parse slot number\n"));
         /* XXX not neccessarily always going to end up in domain 0 / bus 0 :-( */
-        /* XXX this slotnum is not persistant across restarts :-( */
+        domain = bus = 0;
+        if (virStrToLong_i ((const char*)s, &dummy, 10, &slot) == -1)
+            VIR_WARN("%s", _("Unable to parse slot number\n"));
+        else if (virAsprintf(&dev->data.disk->pci_addr, "%.4x:%.2x:%.2x",
+                             domain, bus, slot) < 0) {
+            virReportOOMError(conn);
+            VIR_FREE(reply);
+            return -1;
+        }
     } else if (!tryOldSyntax && strstr(reply, "invalid char in expression")) {
         VIR_FREE(reply);
-        VIR_FREE(cmd);
         tryOldSyntax = 1;
         goto try_command;
     } else {
         qemudReportError (conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
                           _("adding %s disk failed: %s"), type, reply);
         VIR_FREE(reply);
-        VIR_FREE(cmd);
         return -1;
     }
 
@@ -4423,7 +4431,7 @@ try_command:
           virDomainDiskQSort);
 
     VIR_FREE(reply);
-    VIR_FREE(cmd);
+
     return 0;
 }
 
@@ -4660,21 +4668,29 @@ static int qemudDomainDetachPciDiskDevice(virConnectPtr conn,
         goto cleanup;
     }
 
-    if (detach->slotnum < 1) {
+    if (!detach->pci_addr) {
         qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
-                         _("disk %s cannot be detached - invalid slot number %d"),
-                           detach->dst, detach->slotnum);
+                         _("disk %s cannot be detached - no PCI address for device"),
+                           detach->dst);
         goto cleanup;
     }
 
 try_command:
     if (tryOldSyntax) {
-        if (virAsprintf(&cmd, "pci_del 0 %d", detach->slotnum) < 0) {
+        const char *slot = strrchr(detach->pci_addr, ':');
+        if (!slot) {
+            qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
+                             _("Invalid PCI address for device '%s' : %s"),
+                             detach->dst, detach->pci_addr);
+            goto cleanup;
+        }
+        ++slot;
+        if (virAsprintf(&cmd, "pci_del 0 %s", slot) < 0) {
             virReportOOMError(conn);
             goto cleanup;
         }
     } else {
-        if (virAsprintf(&cmd, "pci_del pci_addr=0:0:%d", detach->slotnum) < 0) {
+        if (virAsprintf(&cmd, "pci_del pci_addr=%s", detach->pci_addr) < 0) {
             virReportOOMError(conn);
             goto cleanup;
         }
@@ -4698,8 +4714,8 @@ try_command:
     if (strstr(reply, "invalid slot") ||
         strstr(reply, "Invalid pci address")) {
         qemudReportError (conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
-                          _("failed to detach disk %s: invalid slot %d: %s"),
-                          detach->dst, detach->slotnum, reply);
+                          _("failed to detach disk %s: invalid PCI address %s: %s"),
+                          detach->dst, detach->pci_addr, reply);
         goto cleanup;
     }
 
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 3208f03..a2b958a 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -2712,7 +2712,6 @@ static virDomainPtr vboxDomainDefineXML(virConnectPtr conn, const char *xml) {
                     DEBUG("disk(%d) cachemode:  %d", i, def->disks[i]->cachemode);
                     DEBUG("disk(%d) readonly:   %s", i, def->disks[i]->readonly ? "True" : "False");
                     DEBUG("disk(%d) shared:     %s", i, def->disks[i]->shared ? "True" : "False");
-                    DEBUG("disk(%d) slotnum:    %d", i, def->disks[i]->slotnum);
 
                     if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
                         if (def->disks[i]->type == VIR_DOMAIN_DISK_TYPE_FILE) {
-- 
1.6.2.5




More information about the libvir-list mailing list