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

[libvirt] [PATCH 4/9] Add disk-based hotplugging for the qemu backend



When disks are added to a qemu backend with attach-device,
not just the disk, but a complete new PCI controller with
the disk attached is brought into the system. This patch
implements a proper disk hotplugging scheme for qemu.

Signed-off-by: Wolfgang Mauerer <wolfgang mauerer siemens com>
Signed-off-by: Jan Kiszka <jan kiszka siemens com>
---
 src/domain_conf.c |   46 ++++++++--
 src/domain_conf.h |    2 +-
 src/qemu_driver.c |  257 ++++++++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 277 insertions(+), 28 deletions(-)

diff --git a/src/domain_conf.c b/src/domain_conf.c
index bbaf944..d0fda64 100644
--- a/src/domain_conf.c
+++ b/src/domain_conf.c
@@ -663,7 +663,6 @@ virDomainDiskDefParseXML(virConnectPtr conn,
     char *driverType = NULL;
     char *source = NULL;
     char *target = NULL;
-    char *controller = NULL;
     char *controller_pci_addr = NULL;
     char *bus_id = NULL;
     char *unit_id = NULL;
@@ -720,12 +719,13 @@ virDomainDiskDefParseXML(virConnectPtr conn,
                 if (target &&
                     STRPREFIX(target, "ioemu:"))
                     memmove(target, target+6, strlen(target)-5);
-            } else if ((controller == NULL) &&
+            } else if ((controller_id == NULL && 
+                        controller_pci_addr == NULL) &&
                        (xmlStrEqual(cur->name, BAD_CAST "controller"))) {
-	      controller_id = virXMLPropString(cur, "id");
-	      controller_pci_addr =  virXMLPropString(cur, "pci_addr");
-	      bus_id = virXMLPropString(cur, "bus");
-	      unit_id = virXMLPropString(cur, "unit");
+                controller_id = virXMLPropString(cur, "id");
+                controller_pci_addr = virXMLPropString(cur, "pciaddr");
+                bus_id = virXMLPropString(cur, "bus");
+                unit_id = virXMLPropString(cur, "unit");
             } else if ((driverName == NULL) &&
                        (xmlStrEqual(cur->name, BAD_CAST "driver"))) {
                 driverName = virXMLPropString(cur, "name");
@@ -826,7 +826,13 @@ virDomainDiskDefParseXML(virConnectPtr conn,
         }
     }
 
-    if (controller) {
+    if (controller_id && controller_pci_addr) {
+        virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
+                             _("Both controller ID and address specified!"));
+        goto error;
+    }
+
+    if (controller_id) {
         def->controller_id = controller_id;
 
 	def->bus_id = -1;
@@ -857,12 +863,35 @@ virDomainDiskDefParseXML(virConnectPtr conn,
 	}
     }
 
+    /* TODO: Should we re-use devaddr for this purpose,
+       or is an extra field justified? */
+    def->controller_pci_addr.domain = -1;
+    def->controller_pci_addr.bus = -1;
+    def->controller_pci_addr.slot = -1;
+
+    if (controller_pci_addr &&
+        sscanf(controller_pci_addr, "%x:%x:%x",
+               &def->controller_pci_addr.domain,
+               &def->controller_pci_addr.bus,
+               &def->controller_pci_addr.slot) < 3) {
+        virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                             _("Unable to parse <controller> 'pciaddr' parameter '%s'"),
+                             controller_pci_addr);
+        goto error;
+    } 
+    
+    VIR_DEBUG("Controller PCI addr for disk parsed as %d:%d:%d\n",
+              def->controller_pci_addr.domain,
+              def->controller_pci_addr.bus,
+              def->controller_pci_addr.slot);
+
     if (def->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY &&
         def->bus != VIR_DOMAIN_DISK_BUS_FDC) {
         virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
                              _("Invalid bus type '%s' for floppy disk"), bus);
         goto error;
     }
+
     if (def->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY &&
         def->bus == VIR_DOMAIN_DISK_BUS_FDC) {
         virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
@@ -888,6 +917,9 @@ virDomainDiskDefParseXML(virConnectPtr conn,
         goto error;
     }
 
+    VIR_DEBUG("Disk PCI address parsed as %d:%d:%d\n", 
+              def->pci_addr.domain, def->pci_addr.bus, def->pci_addr.slot);
+
     def->src = source;
     source = NULL;
     def->dst = target;
diff --git a/src/domain_conf.h b/src/domain_conf.h
index 6b3cb09..41df8f6 100644
--- a/src/domain_conf.h
+++ b/src/domain_conf.h
@@ -107,7 +107,7 @@ struct _virDomainDiskDef {
     int device;
     int bus;          /* Bus type, e.g. scsi or ide */
     int bus_id;       /* Bus number on the controller */
-    int unit_id;      /* Unit on the controller */
+    int unit_id;      /* Unit on the controller (both -1 if unspecified) */
     char *src;
     char *dst;
     char *controller_id;
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index a65334f..4a30615 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -5264,7 +5264,7 @@ qemudParsePciAddReply(virDomainObjPtr vm,
 {
     char *s, *e;
 
-    DEBUG("%s: pci_add reply: %s", vm->def->name, reply);
+    VIR_DEBUG("%s: pci_add reply: %s", vm->def->name, reply);
 
     /* If the command succeeds qemu prints:
      * OK bus 0, slot XXX...
@@ -5322,16 +5322,70 @@ qemudParsePciAddReply(virDomainObjPtr vm,
     return 0;
 }
 
-static int qemudDomainAttachPciDiskDevice(virConnectPtr conn,
-                                          virDomainObjPtr vm,
-                                          virDomainDeviceDefPtr dev)
+static int
+qemudParseDriveAddReply(virDomainObjPtr vm,
+			const char *reply,
+			int *bus,
+			int *unit)
+{
+    char *s, *e;
+
+    DEBUG("%s: drive_add reply: %s", vm->def->name, reply);
+
+    /* If the command succeeds qemu prints:
+     * OK bus X, unit Y
+     */
+
+    if (!(s = strstr(reply, "OK ")))
+        return -1;
+
+    s += 3;
+
+    if (STRPREFIX(s, "bus ")) {
+        s += strlen("bus ");
+
+        if (virStrToLong_i(s, &e, 10, bus) == -1) {
+            VIR_WARN(_("Unable to parse bus '%s'\n"), s);
+            return -1;
+        }
+
+        if (!STRPREFIX(e, ", ")) {
+            VIR_WARN(_("Expected ', ' parsing drive_add reply '%s'\n"), s);
+            return -1;
+        }
+        s = e + 2;
+    }
+
+    if (!STRPREFIX(s, "unit ")) {
+        VIR_WARN(_("Expected 'unit ' parsing drive_add reply '%s'\n"), s);
+        return -1;
+    }
+    s += strlen("bus ");
+
+    if (virStrToLong_i(s, &e, 10, unit) == -1) {
+        VIR_WARN(_("Unable to parse unit number '%s'\n"), s);
+        return -1;
+    }
+
+    return 0;
+}
+
+static int qemudDomainAttachDiskDevice(virConnectPtr conn,
+				       virDomainObjPtr vm,
+				       virDomainDeviceDefPtr dev)
 {
     int ret, i;
     char *cmd, *reply;
     char *safe_path;
+    /* NOTE: Later patch will use type of the controller instead
+       of the disk */
     const char* type = virDomainDiskBusTypeToString(dev->data.disk->bus);
     int tryOldSyntax = 0;
     unsigned domain, bus, slot;
+    int bus_id, unit_id;
+    virBuffer buf = VIR_BUFFER_INITIALIZER;
+    char* bus_unit_string;
+    int controller_specified;
 
     for (i = 0 ; i < vm->def->ndisks ; i++) {
         if (STREQ(vm->def->disks[i]->dst, dev->data.disk->dst)) {
@@ -5353,8 +5407,48 @@ try_command:
         return -1;
     }
 
-    ret = virAsprintf(&cmd, "pci_add %s storage file=%s,if=%s",
-                      (tryOldSyntax ? "0": "pci_addr=auto"), safe_path, type);
+    controller_specified = 0;
+    /* Partially specified PCI addresses (should not happen, anyway)
+       don't count as specification */
+    if (dev->data.disk->controller_id != NULL || 
+	(dev->data.disk->controller_pci_addr.domain != -1 &&
+	 dev->data.disk->controller_pci_addr.bus    != -1 &&
+	 dev->data.disk->controller_pci_addr.slot   != -1)) {
+        controller_specified = 1;
+    }
+
+    if (controller_specified) {
+        /* NOTE: Proper check for the controller will be implemented
+           in a later commit */
+	domain = dev->data.disk->controller_pci_addr.domain;
+	bus    = dev->data.disk->controller_pci_addr.bus;
+	slot   = dev->data.disk->controller_pci_addr.slot;
+
+        if (dev->data.disk->bus_id != -1) {
+            virBufferVSprintf(&buf, ",bus=%d", dev->data.disk->bus_id);
+        }
+        
+        if (dev->data.disk->unit_id != -1) {
+            virBufferVSprintf(&buf, ",unit=%d", dev->data.disk->unit_id);
+        }
+
+        bus_unit_string = virBufferContentAndReset(&buf);
+
+        VIR_DEBUG ("%s: drive_add %d:%d:%d file=%s,if=%s%s", vm->def->name,
+                   domain, bus, slot, safe_path, type, bus_unit_string);
+        ret = virAsprintf(&cmd, "drive_add %s%d:%d:%d file=%s,if=%s%s",
+			  (tryOldSyntax ? "" : "pci_addr="), domain, bus, 
+                          slot, safe_path, type, bus_unit_string);
+    }
+    else {
+        /* Old-style behaviour: If no <controller> reference is given, then
+           hotplug a new controller with the disk attached. */
+        VIR_DEBUG ("%s: pci_add %s storage file=%s,if=%s", vm->def->name,
+                   (tryOldSyntax ? "0": "pci_addr=auto"), safe_path, type);
+        ret = virAsprintf(&cmd, "pci_add %s storage file=%s,if=%s",
+                   (tryOldSyntax ? "0": "pci_addr=auto"), safe_path, type);
+    }
+
     VIR_FREE(safe_path);
     if (ret == -1) {
         virReportOOMError(conn);
@@ -5370,26 +5464,149 @@ try_command:
 
     VIR_FREE(cmd);
 
-    if (qemudParsePciAddReply(vm, reply, &domain, &bus, &slot) < 0) {
-        if (!tryOldSyntax && strstr(reply, "invalid char in expression")) {
-            VIR_FREE(reply);
-            tryOldSyntax = 1;
-            goto try_command;
+    if (controller_specified) {
+        if (qemudParseDriveAddReply(vm, reply, &bus_id, &unit_id) < 0) {
+	    if (!tryOldSyntax && strstr(reply, "invalid char in expression")) {
+	        VIR_FREE(reply);
+		tryOldSyntax = 1;
+		goto try_command;
+	    }
+	    qemudReportError (conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+			      _("adding %s disk failed: %s"), type, reply);
+	    VIR_FREE(reply);
+	    return -1;
+	}
+
+	if (dev->data.disk->bus_id == -1) {
+	    dev->data.disk->bus_id = bus_id;
+	}
+
+	if (dev->data.disk->unit_id == -1) {
+	    dev->data.disk->unit_id = unit_id;
+	}
+    } else {
+        if (qemudParsePciAddReply(vm, reply, &domain, &bus, &slot) < 0) {
+	    if (!tryOldSyntax && strstr(reply, "invalid char in expression")) {
+	        VIR_FREE(reply);
+		tryOldSyntax = 1;
+		goto try_command;
+	    }
+	    
+	    qemudReportError (conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+			      _("adding %s disk failed: %s"), type, reply);
+	    VIR_FREE(reply);
+	    return -1;
+	}
+    }
+	
+    dev->data.disk->pci_addr.domain = domain;
+    dev->data.disk->pci_addr.bus    = bus;
+    dev->data.disk->pci_addr.slot   = slot;
+
+    virDomainDiskInsertPreAlloced(vm->def, dev->data.disk);
+
+
+    return 0;
+}
+
+static int qemudDomainAttachDiskController(virConnectPtr conn,
+					   virDomainObjPtr vm,
+					   virDomainDeviceDefPtr dev)
+{
+    int ret, i;
+    char *cmd, *reply;
+    char *tmp;
+    const char* type = virDomainDiskBusTypeToString(dev->data.controller->type);
+    int tryOldSyntax = 0;
+    int controllerAddressSpecified = 0;
+    unsigned domain, bus, slot;
+
+    /* Test if the controller already exists, both by ID and
+       by PCI address */
+    for (i = 0 ; i < vm->def->ncontrollers ; i++) {
+        if ((dev->data.controller->id && 
+             STREQ(vm->def->controllers[i]->id, dev->data.controller->id)) ||
+            (vm->def->controllers[i]->pci_addr.domain ==
+             dev->data.controller->pci_addr.domain &&
+             vm->def->controllers[i]->pci_addr.bus ==
+             dev->data.controller->pci_addr.bus &&
+             vm->def->controllers[i]->pci_addr.slot ==
+             dev->data.controller->pci_addr.slot)) {
+            qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+                             _("Controller (id:%s, %d:%d:%d) already exists"), 
+                             dev->data.controller->id ?
+                             dev->data.controller->id : "(none)",
+                             dev->data.controller->pci_addr.domain, 
+                             dev->data.controller->pci_addr.bus, 
+                             dev->data.controller->pci_addr.slot);
+            return -1;
         }
+    }
 
-        qemudReportError (conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
-                          _("adding %s disk failed: %s"), type, reply);
-        VIR_FREE(reply);
+    if (VIR_REALLOC_N(vm->def->controllers, vm->def->ncontrollers+1) < 0) {
+        virReportOOMError(conn);
         return -1;
     }
 
-    VIR_FREE(reply);
+    if (dev->data.controller->pci_addr.domain != -1 &&
+	dev->data.controller->pci_addr.bus    != -1 &&
+	dev->data.controller->pci_addr.slot   != -1) {
+        controllerAddressSpecified = 1;
+    }
 
-    dev->data.disk->pci_addr.domain = domain;
-    dev->data.disk->pci_addr.bus    = bus;
-    dev->data.disk->pci_addr.slot   = slot;
+try_command:
+    if (controllerAddressSpecified) {
+        domain = dev->data.controller->pci_addr.domain;
+	bus    = dev->data.controller->pci_addr.bus;
+	slot   = dev->data.controller->pci_addr.slot;
+
+	ret = virAsprintf(&tmp, "%d:%d:%d", domain, bus, slot);
+        ret |= virAsprintf(&cmd, "pci_add %s%s storage if=%s",
+			   (tryOldSyntax ? "" : "pci_addr="), tmp, type);
+    } else {
+        ret = virAsprintf(&cmd, "pci_add %s storage if=%s",
+			   (tryOldSyntax ? "0" : "pci_addr=auto"), type);
+    }
 
-    virDomainDiskInsertPreAlloced(vm->def, dev->data.disk);
+    if (ret == -1) {
+        virReportOOMError(conn);
+        return ret;
+    }
+
+    if (qemudMonitorCommand(vm, cmd, &reply) < 0) {
+        qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+                         _("cannot attach %s controller"), type);
+        VIR_FREE(cmd);
+        return -1;
+    }
+
+    VIR_FREE(cmd);
+
+    if (qemudParsePciAddReply(vm, reply, &domain, &bus, &slot) < 0) {
+        if (!tryOldSyntax && strstr(reply, "invalid char in expression")) {
+	    VIR_FREE(reply);
+	    tryOldSyntax = 1;
+	    goto try_command;
+	}
+	    
+	qemudReportError (conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+			  _("adding %s controller failed: %s"), type, reply);
+	VIR_FREE(reply);
+	return -1;
+    }
+	
+    /* Also fill in when the address was explicitely specified in
+       case qemu changed it (TODO: Can this really happen?) */
+    dev->data.controller->pci_addr.domain = domain;
+    dev->data.controller->pci_addr.bus    = bus;
+    dev->data.controller->pci_addr.slot   = slot;
+
+    VIR_FREE(reply);
+
+    /* NOTE: Sort function is added in a later commit */
+    vm->def->controllers[vm->def->ncontrollers++] = dev->data.controller;
+    /*    qsort(vm->def->disks, vm->def->ndisks, sizeof(*vm->def->disks),
+    virDomainDiskQSort);*/
 
     return 0;
 }
@@ -5868,7 +6085,7 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
                 ret = qemudDomainAttachUsbMassstorageDevice(dom->conn, vm, dev);
             } else if (dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_SCSI ||
                        dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) {
-                ret = qemudDomainAttachPciDiskDevice(dom->conn, vm, dev);
+                ret = qemudDomainAttachDiskDevice(dom->conn, vm, dev);
             } else {
                 qemudReportError(dom->conn, dom, NULL, VIR_ERR_NO_SUPPORT,
                                  _("disk bus '%s' cannot be hotplugged."),
-- 
1.6.4


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