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@xxxxxxxxxxx>
Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
---
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
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list