[libvirt] [PATCH 10/13] Factor out the method to get the PCI address of a controller for a given disk
wolfgang.mauerer at siemens.com
wolfgang.mauerer at siemens.com
Mon Nov 16 23:53:41 UTC 2009
From: Wolfgang Mauerer <wolfgang.mauerer at siemens.com>
We need this multiple times later on.
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 | 26 ++----
src/conf/domain_conf.h | 8 ++
src/qemu/qemu_driver.c | 242 +++++++++++++++++++++++++++---------------------
3 files changed, 153 insertions(+), 123 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e90975f..48015a2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1077,37 +1077,20 @@ virDomainDiskDefParseXML(virConnectPtr conn,
def->unit_id = -1;
if (unit_id && virStrToLong_i(unit_id, NULL, 10, &def->unit_id) < 0) {
virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
- _("Cannot parse <controller> 'unit' attribute"));
- goto error;
- }
-
- /* TODO: Should we re-use devaddr for this purpose,
- or is an extra field justified? */
- 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> 'pci_addr' parameter '%s'"),
- controller_pci_addr);
+ _("Cannot parse <controller> 'unit' attribute"));
goto error;
}
}
/* 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'"),
+ _("Unable to parse <controller> 'pci_addr' parameter '%s'"),
controller_pci_addr);
goto error;
}
@@ -1215,6 +1198,11 @@ virDomainControllerDefParseXML(virConnectPtr conn,
}
def->name = virXMLPropString(node, "name");
+ if (!def->name) {
+ virDomainReportError(conn, VIR_ERR_INVALID_ARG,
+ _("cannot use controller without name"));
+ goto error;
+ }
cur = node->children;
while (cur != NULL) {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index b30d08c..e058c56 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -171,6 +171,14 @@ virDiskHasValidPciAddr(virDomainDiskDefPtr def)
return def->pci_addr.domain || def->pci_addr.bus || def->pci_addr.slot;
}
+static inline int
+virDiskHasValidController(virDomainDiskDefPtr def)
+{
+ return def->controller_name != NULL ||
+ def->controller_pci_addr.domain || def->controller_pci_addr.domain
+ || def->controller_pci_addr.slot;
+}
+
/* Two types of disk backends */
enum virDomainFSType {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4f647c4..9f44685 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4356,44 +4356,20 @@ 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;
- }
+enum {
+ CONTROLLER_FOUND = 0,
+ NO_CONTROLLER_FOUND = -1, /* None specified, and none found */
+ CONTROLLER_INEXISTENT = -2, /* Controller specified, but not found */
+};
+static virDomainControllerDefPtr
+qemudGetDiskController(virDomainObjPtr vm, virDomainDeviceDefPtr dev,
+ int* ret) {
+ int controller_specified = 0;
+ int domain, bus, slot;
+ virDomainControllerDefPtr conPtr = NULL;
+ int i;
+ *ret = CONTROLLER_FOUND;
- controller_specified = 0;
/* Partially specified PCI addresses (should not happen, anyway)
don't count as specification */
if (dev->data.disk->controller_name != NULL ||
@@ -4411,16 +4387,17 @@ try_command:
the admissible controller types (SCSI, virtio) have already
been checked in qemudDomainAttachDevice. */
for (i = 0 ; i < vm->def->ncontrollers ; i++) {
- if (vm->def->controllers[i]->type == dev->data.disk->type)
+ if (vm->def->controllers[i]->type == dev->data.disk->type) {
+ conPtr = vm->def->controllers[i];
break;
+ }
}
- if (i == vm->def->ncontrollers) {
- qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- _("Cannot find appropriate controller for hot-add"));
- return -1;
+ if (!conPtr) {
+ *ret = NO_CONTROLLER_FOUND;
}
+
domain = vm->def->controllers[i]->address->data.pci.domain;
bus = vm->def->controllers[i]->address->data.pci.bus;
slot = vm->def->controllers[i]->address->data.pci.slot;
@@ -4428,21 +4405,20 @@ try_command:
if (controller_specified && dev->data.disk->controller_name) {
for (i = 0 ; i < vm->def->ncontrollers ; i++) {
- if (STREQ(dev->data.disk->controller_name,
+ if (vm->def->controllers[i]->name &&
+ STREQ(dev->data.disk->controller_name,
vm->def->controllers[i]->name)) {
+ conPtr = vm->def->controllers[i];
+ domain = vm->def->controllers[i]->address->data.pci.domain;
+ bus = vm->def->controllers[i]->address->data.pci.bus;
+ slot = vm->def->controllers[i]->address->data.pci.slot;
break;
- }
+ }
}
- if (i == vm->def->ncontrollers) {
- qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- _("Controller does not exist"));
- return -1;
- }
-
- domain = vm->def->controllers[i]->address->data.pci.domain;
- bus = vm->def->controllers[i]->address->data.pci.bus;
- slot = vm->def->controllers[i]->address->data.pci.slot;
+ if (!conPtr) {
+ *ret = CONTROLLER_INEXISTENT;
+ }
} else if (controller_specified) {
domain = dev->data.disk->controller_pci_addr.domain;
bus = dev->data.disk->controller_pci_addr.bus;
@@ -4452,35 +4428,109 @@ try_command:
if (domain == vm->def->controllers[i]->address->data.pci.domain &&
bus == vm->def->controllers[i]->address->data.pci.bus &&
slot == vm->def->controllers[i]->address->data.pci.slot)
+ conPtr = vm->def->controllers[i];
break;
}
- if (i == vm->def->ncontrollers) {
+ if (!conPtr) {
+ *ret = CONTROLLER_INEXISTENT;
+ }
+ }
+
+ return conPtr;
+}
+
+static int qemudDomainAttachDiskDevice(virConnectPtr conn,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev)
+{
+ int ret, i;
+ char *cmd, *reply;
+ char *safe_path;
+ const char* type = NULL;
+ int tryOldSyntax = 0;
+ int bus_id, unit_id;
+ int domain, bus, slot;
+ virDomainControllerDefPtr conPtr;
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ char* bus_unit_string;
+
+ /* Both controller and disk can specify a type like SCSI. While
+ a virtual disk as such is not bound to a specific bus type,
+ the specification is here for historical reasons. When controller
+ and disk type specification disagree, the former takes precedence
+ and we print a warning message, but still add the disk. */
+ if (virDiskHasValidController(dev->data.disk)) {
+ conPtr = qemudGetDiskController(vm, dev, &ret);
+ if (conPtr && dev->data.disk->bus != conPtr->type) {
+ VIR_WARN0(_("Controller and disk bus type disagree, controller " \
+ "takes precedence\n"));
+ type = virDomainDiskBusTypeToString(conPtr->type);
+ }
+ }
+ if (!type) {
+ type = virDomainDiskBusTypeToString(dev->data.disk->bus);
+ }
+
+ 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;
+ }
+
+ conPtr = qemudGetDiskController(vm, dev, &ret);
+ if (!conPtr) {
+ switch (ret) {
+ case CONTROLLER_INEXISTENT:
qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
_("Controller does not exist"));
return -1;
- }
- }
- /* At this point, we have a valid (domain, bus, slot) triple
- that identifies the controller, regardless if an explicit
- controller has been given or not. */
- if (dev->data.disk->bus_id != -1) {
+ break; /* A bit pointless */
+ case NO_CONTROLLER_FOUND:
+ qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ _("Cannot find appropriate controller for hot-add"));
+ return -1;
+ break;
+ }
+ }
+
+ /* At this point, we have a valid controller, regardless if an explicit
+ controller has been specified or not. */
+ domain = conPtr->address->data.pci.domain;
+ bus = conPtr->address->data.pci.bus;
+ slot = conPtr->address->data.pci.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);
+ if (dev->data.disk->unit_id != -1) {
+ virBufferVSprintf(&buf, ",unit=%d", dev->data.disk->unit_id);
}
- bus_unit_string = virBufferContentAndReset(&buf);
+ 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 ? 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 ? bus_unit_string : "");
+ VIR_DEBUG ("%s: drive_add %.2x:%.2x:%.2x file=%s,if=%s%s", vm->def->name,
+ domain, bus, slot, safe_path, type,
+ bus_unit_string ? bus_unit_string : "");
+ ret = virAsprintf(&cmd, "drive_add %s%.2x:%.2x:%.2x file=%s,if=%s%s",
+ (tryOldSyntax ? "" : "pci_addr="), domain, bus,
+ slot, safe_path, type,
+ bus_unit_string ? bus_unit_string : "");
VIR_FREE(safe_path);
if (ret == -1) {
@@ -4497,39 +4547,24 @@ try_command:
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 (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->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;
- }
+ if (dev->data.disk->unit_id == -1) {
+ dev->data.disk->unit_id = unit_id;
}
dev->data.disk->pci_addr.domain = domain;
@@ -4538,7 +4573,6 @@ try_command:
virDomainDiskInsertPreAlloced(vm->def, dev->data.disk);
-
return 0;
}
@@ -4590,7 +4624,7 @@ try_command:
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(&tmp, "%.2x:%.2x:%.2x", domain, bus, slot);
ret |= virAsprintf(&cmd, "pci_add %s%s storage if=%s",
(tryOldSyntax ? "" : "pci_addr="), tmp, type);
} else {
--
1.6.4
More information about the libvir-list
mailing list