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

[libvirt] [PATCH 05/13] Add disk-based hotplugging for the qemu backend



From: Wolfgang Mauerer <wolfgang mauerer siemens com>

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/conf/domain_conf.c   |   33 ++++++-
 src/conf/domain_conf.h   |    5 +-
 src/libvirt_private.syms |    3 +-
 src/qemu/qemu_driver.c   |  243 +++++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 278 insertions(+), 6 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4e2b84e..91115b6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -896,7 +896,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;
@@ -953,7 +952,8 @@ virDomainDiskDefParseXML(virConnectPtr conn,
                 if (target &&
                     STRPREFIX(target, "ioemu:"))
                     memmove(target, target+6, strlen(target)-5);
-            } else if ((controller == NULL) &&
+            } else if ((controller_name == NULL &&
+                        controller_pci_addr == NULL) &&
                        (xmlStrEqual(cur->name, BAD_CAST "controller"))) {
                 controller_name = virXMLPropString(cur, "name");
                 controller_pci_addr =  virXMLPropString(cur, "pci_addr");
@@ -1059,7 +1059,13 @@ virDomainDiskDefParseXML(virConnectPtr conn,
         }
     }
 
-    if (controller) {
+    if (controller_name && controller_pci_addr) {
+        virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
+                             _("Both controller ID and address specified!"));
+        goto error;
+    }
+
+    if (controller_name) {
         def->controller_name = controller_name;
 
         def->bus_id = -1;
@@ -1090,6 +1096,23 @@ 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 %x:%x:%x\n",
               def->controller_pci_addr.domain,
               def->controller_pci_addr.bus,
@@ -1101,6 +1124,7 @@ virDomainDiskDefParseXML(virConnectPtr conn,
                              _("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,
@@ -1126,6 +1150,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/conf/domain_conf.h b/src/conf/domain_conf.h
index 591143f..b30d08c 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -133,7 +133,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_name;
@@ -744,6 +744,9 @@ void virDomainVideoDefFree(virDomainVideoDefPtr def);
 void virDomainHostdevDefFree(virDomainHostdevDefPtr def);
 void virDomainDeviceDefFree(virDomainDeviceDefPtr def);
 void virDomainDeviceAddressFree(virDomainDeviceAddressPtr def);
+int virDomainCompareDeviceAddresses(virDomainDeviceAddressPtr a,
+                                    virDomainDeviceAddressPtr b);
+const char *virDomainFormatDeviceAddress(virDomainDeviceAddressPtr address);
 void virDomainDefFree(virDomainDefPtr vm);
 void virDomainObjFree(virDomainObjPtr vm);
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 92cf86c..e73ed94 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -152,7 +152,8 @@ virDomainObjListGetActiveIDs;
 virDomainObjListNumOfDomains;
 virDomainObjListInit;
 virDomainObjListDeinit;
-
+virDomainCompareDeviceAddresses;
+virDomainFormatDeviceAddress;
 
 # domain_event.h
 virDomainEventCallbackListAdd;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index af7b59c..9f71562 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4356,6 +4356,247 @@ static int qemudDomainChangeEjectableMedia(virConnectPtr conn,
     return ret;
 }
 
+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)) {
+            qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+                           _("target %s already exists"), dev->data.disk->dst);
+            return -1;
+        }
+    }
+
+    if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0) {
+        virReportOOMError(conn);
+        return -1;
+    }
+
+try_command:
+    safe_path = qemudEscapeMonitorArg(dev->data.disk->src);
+    if (!safe_path) {
+        virReportOOMError(conn);
+        return -1;
+    }
+
+    controller_specified = 0;
+    /* Partially specified PCI addresses (should not happen, anyway)
+       don't count as specification */
+    if (dev->data.disk->controller_name != 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);
+        return ret;
+    }
+
+    if (qemudMonitorCommand(vm, cmd, &reply) < 0) {
+        qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+                         _("cannot attach %s disk"), type);
+        VIR_FREE(cmd);
+        return -1;
+    }
+
+    VIR_FREE(cmd);
+
+    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;
+    unsigned domain, bus, slot;
+
+    /* Only SCSI controllers are supported at the moment */
+    if (dev->data.controller->address &&
+        dev->data.controller->address->type !=
+                               VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+        qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+                         _("Only SCSI controller hotplug is supported!"));
+        return -1;
+    }
+
+    /* 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->name &&
+             STREQ(vm->def->controllers[i]->name,
+                   dev->data.controller->name)) ||
+            virDomainCompareDeviceAddresses(vm->def->controllers[i]->address,
+                                            dev->data.controller->address)) {
+            qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+                           _("Controller (id:%s, address: %s) already exists"),
+                           dev->data.controller->name ?
+                           dev->data.controller->name : "(none)",
+                           virDomainFormatDeviceAddress(dev->data.controller->address));
+            return -1;
+        }
+    }
+
+    if (VIR_REALLOC_N(vm->def->controllers, vm->def->ncontrollers+1) < 0) {
+        virReportOOMError(conn);
+        return -1;
+    }
+
+try_command:
+    if (dev->data.controller->address != NULL) {
+        domain = dev->data.controller->address->data.pci.domain;
+        bus    = dev->data.controller->address->data.pci.bus;
+        slot   = dev->data.controller->address->data.pci.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);
+    }
+
+    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->address->data.pci.domain = domain;
+    dev->data.controller->address->data.pci.bus    = bus;
+    dev->data.controller->address->data.pci.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;
+}
+
 static int qemudDomainAttachPciDiskDevice(virConnectPtr conn,
                                           virDomainObjPtr vm,
                                           virDomainDeviceDefPtr dev)
@@ -4720,7 +4961,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]