[libvirt] [PATCH 05/13] Add disk-based hotplugging for the qemu backend
wolfgang.mauerer at siemens.com
wolfgang.mauerer at siemens.com
Mon Nov 16 23:53:36 UTC 2009
From: Wolfgang Mauerer <wolfgang.mauerer at 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 at siemens.com>
Signed-off-by: Jan Kiszka <jan.kiszka at 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
More information about the libvir-list
mailing list