From: Wolfgang Mauerer <wolfgang.maue...@siemens.com> We need this multiple times later on.
Signed-off-by: Wolfgang Mauerer <wolfgang.maue...@siemens.com> Signed-off-by: Jan Kiszka <jan.kis...@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 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list