[libvirt] [PATCH 04/13] Add new domain device: "controller"

From: Wolfgang Mauerer <wolfgang.mauerer(a)siemens.com&gt; This augments virDomainDevice with a <controller> element that is used to represent disk controllers (e.g., scsi controllers). The XML format is given by <controller type="scsi" name="<my_id>"> <address type="pci" domain="0xNUM" bus="0xNUM" slot="0xNUM"/> </controller> where type denotes the disk interface (scsi, ide,...), name is an arbitrary string that identifies the controller for disk hotadd/remove, and the <address> element specifies the controller address on the PCI bus. Domain, bus and slot are specified as hexadecimal numbers. The address element can be omitted; in this case, an address will be assigned automatically. Currently, only hotplugging scsi controllers on a PCI bus is supported, and this only for qemu guests Routines for parsing this definition and the associated data structures are included in this commit. Signed-off-by: Wolfgang Mauerer <wolfgang.mauerer(a)siemens.com&gt; Signed-off-by: Jan Kiszka <jan.kiszka(a)siemens.com&gt; --- docs/schemas/domain.rng | 158 ++++++++++++++++++------ src/conf/domain_conf.c | 279 +++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 48 +++++++- src/libvirt_private.syms | 1 - src/qemu/qemu_driver.c | 1 - src/qemu/qemu_monitor_text.c | 47 +++++++ 6 files changed, 488 insertions(+), 46 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index b75f17e..0d5fb47 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -393,49 +393,129 @@ <define name="disk"> <element name="disk"> <optional> - <attribute name="device"> + <attribute name="device"> + <choice> + <value>floppy</value> + <value>disk</value> + <value>cdrom</value> + </choice> + </attribute> + </optional> + <optional> + <element name="controller"> + <optional> + <attribute name="bus"> + <ref name="unsignedInt"/> + </attribute> + </optional> + <optional> + <attribute name="unit"> + <ref name="unsignedInt"/> + </attribute> + </optional> + <choice> + <attribute name="name"> + <ref name="genericName"/> + </attribute> + <attribute name="pciaddr"> + <!-- Just for testing --> + <ref name="deviceName"/> + </attribute> + </choice> + </element> + </optional> + <choice> + <group> + <attribute name="type"> + <value>file</value> + </attribute> + <interleave> + <optional> + <element name="source"> + <attribute name="file"> + <ref name="absFilePath"/> + </attribute> + <empty/> + </element> + </optional> + <ref name="diskspec"/> + </interleave> + </group> + <group> + <attribute name="type"> + <value>block</value> + </attribute> + <interleave> + <optional> + <element name="source"> + <attribute name="dev"> + <ref name="deviceName"/> + </attribute> + <empty/> + </element> + </optional> + <ref name="diskspec"/> + </interleave> + </group> + <ref name="diskspec"/> + </choice> + </element> + </define> + <define name="target"> + <element name="target"> + <attribute name="dev"> + <ref name="deviceName"/> + </attribute> + <optional> + <attribute name="bus"> <choice> - <value>floppy</value> - <value>disk</value> - <value>cdrom</value> + <value>ide</value> + <value>fdc</value> + <value>scsi</value> + <value>virtio</value> + <value>xen</value> + <value>usb</value> + <value>uml</value> </choice> </attribute> </optional> - <choice> - <group> - <attribute name="type"> - <value>file</value> - </attribute> - <interleave> - <optional> - <element name="source"> - <attribute name="file"> - <ref name="absFilePath"/> - </attribute> - <empty/> - </element> - </optional> - <ref name="diskspec"/> - </interleave> - </group> - <group> - <attribute name="type"> - <value>block</value> - </attribute> - <interleave> - <optional> - <element name="source"> - <attribute name="dev"> - <ref name="deviceName"/> - </attribute> - <empty/> - </element> - </optional> - <ref name="diskspec"/> - </interleave> - </group> - <ref name="diskspec"/> - </choice> + </element> + </define> + + <define name="controller"> + <element name="controller"> + <interleave> + <attribute name="type"> + <choice> + <!-- For now, only SCSI is supported --> + <value>scsi</value> + </choice> + </attribute> + <attribute name="id"> + <ref name="genericName"/> + </attribute> + </interleave> + <optional> + <element name="address"> + <attribute name="type"> + <ref name="adressType"/> + </attribute> + <optional> + <attribute name="domain"> + <ref name="pciDomain"/> + </attribute> + </optional> + <attribute name="bus"> + <ref name="pciBus"/> + </attribute> + <attribute name="slot"> + <ref name="pciSlot"/> + </attribute> + <attribute name="function"> + <ref name="pciFunc"/> + </attribute> + </element> + </optional> </element> </define> <define name="target"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ecddb68..4e2b84e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -80,6 +80,7 @@ VIR_ENUM_IMPL(virDomainLifecycle, VIR_DOMAIN_LIFECYCLE_LAST, VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST, "disk", + "controller", "filesystem", "interface", "input", @@ -88,6 +89,9 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST, "hostdev", "watchdog") +VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, + "pci") + VIR_ENUM_IMPL(virDomainDisk, VIR_DOMAIN_DISK_TYPE_LAST, "block", "file") @@ -342,6 +346,16 @@ void virDomainInputDefFree(virDomainInputDefPtr def) VIR_FREE(def); } +void virDomainDeviceAddressFree(virDomainDeviceAddressPtr def) +{ + if (!def) + return; + + VIR_FREE(def->data); + + VIR_FREE(def); +} + void virDomainDiskDefFree(virDomainDiskDefPtr def) { if (!def) @@ -357,6 +371,18 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def) VIR_FREE(def); } +void virDomainControllerDefFree(virDomainControllerDefPtr def) +{ + if (!def) + return; + + VIR_FREE(def->type); + VIR_FREE(def->name); + VIR_FREE(def->address); + + VIR_FREE(def); +} + void virDomainFSDefFree(virDomainFSDefPtr def) { if (!def) @@ -708,6 +734,152 @@ void virDomainRemoveInactive(virDomainObjListPtr doms, virHashRemoveEntry(doms->objs, uuidstr, virDomainObjListDeallocator); } +/* Generate a string representation of a device address + * @param address Device address to stringify + */ +const char *virDomainFormatDeviceAddress(virDomainDeviceAddressPtr address) { + int ret; + char *tmp; + + if (!address) { + return "(undefined)"; + } + + switch (address->type) { + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI: + ret = virAsprintf(&tmp, "%x:%x:%x", address->data.pci.domain, + address->data.pci.bus, address->data.pci.slot); + if (!ret) + return tmp; + break; + default: + return "(unknown)"; + } + + return "(error)"; +} + +/* Compare two device addresses. Returns true if addresses are + * identical and false if the they differ (or are of different types) + * @param a, @para b Pointers to the addresses + */ +int virDomainCompareDeviceAddresses(virDomainDeviceAddressPtr a, + virDomainDeviceAddressPtr b) { + if (!a || !b || a->type != b-> type) { + return 0; + } + + switch (a->type) { + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI: + if (a->data.pci.domain == b->data.pci.domain && + a->data.pci.bus == b->data.pci.bus && + a->data.pci.slot == b->data.pci.slot) { + return 1; + } + + return 0; + } + + return 0; +} + +/* Parse the XML definition for a device address + * @param node XML nodeset to parse for device address definition + */ +static virDomainDeviceAddressPtr +virDomainDeviceAddressParseXML(virConnectPtr conn, + xmlNodePtr node) { + virDomainDeviceAddressPtr def; + char *type = NULL; + char *domain, *slot, *bus, *function; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(conn); + return NULL; + } + + type = virXMLPropString(node, "type"); + + if (type) { + if ((def->type = virDomainDeviceAddressTypeFromString(type)) < 0) { + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("unknown address type '%s'"), type); + goto error; + } + } else { + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("No type specified for device address")); + goto error; + } + + switch (def->type) { + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI: + domain = virXMLPropString(node, "domain"); + bus = virXMLPropString(node, "bus"); + slot = virXMLPropString(node, "slot"); + function = virXMLPropString(node, "function"); + + def->data.pci.domain = -1; + if (domain && + virStrToLong_i(domain, NULL, 16, &def->data.pci.domain) < 0) { + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse <address> 'domain' attribute")); + goto error; + } + + def->data.pci.bus = -1; + if (bus && + virStrToLong_i(bus, NULL, 16, &def->data.pci.bus) < 0) { + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse <address> 'bus' attribute")); + goto error; + } + + def->data.pci.slot = -1; + if (slot && + virStrToLong_i(slot, NULL, 16, &def->data.pci.slot) < 0) { + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse <address> 'slot' attribute")); + goto error; + } + + def->data.pci.function = -1; + if (function && + virStrToLong_i(function, NULL, 16, &def->data.pci.function) < 0) { + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse <address> 'function' attribute")); + goto error; + } + + if (def->data.pci.domain == -1 || def->data.pci.bus == -1 + || def->data.pci.slot == -1) { + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("Unsufficient specification for PCI address")); + goto error; + } + + break; + default: + /* Should not happen */ + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Unknown device address type")); + goto error; + } + +cleanup: + VIR_FREE(domain); + VIR_FREE(bus); + VIR_FREE(slot); + VIR_FREE(function); + + return def; + +error: + virDomainDeviceAddressFree(def); + def = NULL; + goto cleanup; +} + /* Parse the XML definition for a disk * @param node XML nodeset to parse for disk definition @@ -725,9 +897,10 @@ virDomainDiskDefParseXML(virConnectPtr conn, char *source = NULL; char *target = NULL; char *controller = NULL; + char *controller_pci_addr = NULL; char *bus_id = NULL; char *unit_id = NULL; - char *controller_id = NULL; + char *controller_name = NULL; char *bus = NULL; char *unit = NULL; char *cachetag = NULL; @@ -782,8 +955,9 @@ virDomainDiskDefParseXML(virConnectPtr conn, memmove(target, target+6, strlen(target)-5); } else if ((controller == NULL) && (xmlStrEqual(cur->name, BAD_CAST "controller"))) { - controller_id = virXMLPropString(cur, "id"); - bus_id = virXMLPropString(cur, "bus"); + controller_name = virXMLPropString(cur, "name"); + controller_pci_addr = virXMLPropString(cur, "pci_addr"); + bus_id = virXMLPropString(cur, "bus"); unit_id = virXMLPropString(cur, "unit"); } else if ((driverName == NULL) && (xmlStrEqual(cur->name, BAD_CAST "driver"))) { @@ -886,7 +1060,7 @@ virDomainDiskDefParseXML(virConnectPtr conn, } if (controller) { - def->controller_id = controller_id; + def->controller_name = controller_name; def->bus_id = -1; if (bus_id && virStrToLong_i(bus_id, NULL, 10, &def->bus_id) < 0) { @@ -901,8 +1075,26 @@ virDomainDiskDefParseXML(virConnectPtr conn, _("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); + 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, + 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, @@ -969,6 +1161,59 @@ cleanup: } +/* Parse the XML definition for a controller + * @param node XML nodeset to parse for controller definition + */ +static virDomainControllerDefPtr +virDomainControllerDefParseXML(virConnectPtr conn, + xmlNodePtr node, + int flags ATTRIBUTE_UNUSED) { + virDomainControllerDefPtr def; + xmlNodePtr cur; + char *type = NULL; + + if (VIR_ALLOC(def) < 0) { + virReportOOMError(conn); + return NULL; + } + + def->address = NULL; + type = virXMLPropString(node, "type"); + def->type = VIR_DOMAIN_DISK_BUS_SCSI; + if (type) { + if ((def->type = virDomainDiskBusTypeFromString(type)) < 0) { + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("unknown disk bus type '%s'"), type); + goto error; + } + } + + def->name = virXMLPropString(node, "name"); + + cur = node->children; + while (cur != NULL) { + if (cur->type == XML_ELEMENT_NODE && + def->address == NULL && + xmlStrEqual(cur->name, BAD_CAST "address")) { + + def->address = virDomainDeviceAddressParseXML(conn, cur); + } + + cur = cur->next; + } + + +cleanup: + VIR_FREE(type); + + return def; + + error: + virDomainControllerDefFree(def); + def = NULL; + goto cleanup; +} + /* Parse the XML definition for a disk * @param node XML nodeset to parse for disk definition */ @@ -2616,6 +2861,11 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virConnectPtr conn, dev->type = VIR_DOMAIN_DEVICE_DISK; if (!(dev->data.disk = virDomainDiskDefParseXML(conn, node, flags))) goto error; + } else if (xmlStrEqual(node->name, BAD_CAST "controller")) { + dev->type = VIR_DOMAIN_DEVICE_CONTROLLER; + if (!(dev->data.controller = + virDomainControllerDefParseXML(conn, node, flags))) + goto error; } else if (xmlStrEqual(node->name, BAD_CAST "filesystem")) { dev->type = VIR_DOMAIN_DEVICE_FS; if (!(dev->data.fs = virDomainFSDefParseXML(conn, node, flags))) @@ -3033,6 +3283,27 @@ static virDomainDefPtr virDomainDefParseXML(virConnectPtr conn, } VIR_FREE(nodes); + /* analysis of the disk controllers */ + if ((n = virXPathNodeSet(conn, "./devices/controller", ctxt, &nodes)) < 0) { + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot extract controller devices")); + goto error; + } + if (n && VIR_ALLOC_N(def->controllers, n) < 0) + goto no_memory; + for (i = 0 ; i < n ; i++) { + virDomainControllerDefPtr controller = + virDomainControllerDefParseXML(conn, nodes[i], flags); + if (!controller) + goto error; + + def->controllers[def->ncontrollers++] = controller; + } + /* qsort(def->controllers, def->ncontrollers, sizeof(*def->controllers), + virDomainControllerQSort); */ + VIR_FREE(nodes); + + /* analysis of the filesystems */ if ((n = virXPathNodeSet(conn, "./devices/filesystem", ctxt, &nodes)) < 0) { virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c034ae4..591143f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -63,6 +63,30 @@ enum virDomainVirtType { VIR_DOMAIN_VIRT_LAST, }; +enum virDomainDeviceAddressType { + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI, + + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST +}; + +typedef struct _virDomainDevicePCIAddress virDomainDevicePCIAddress; +struct _virDomainDevicePCIAddress { + int domain; + int bus; + int slot; + int function; +}; + +typedef struct _virDomainDeviceAddress virDomainDeviceAddress; +typedef virDomainDeviceAddress *virDomainDeviceAddressPtr; +struct _virDomainDeviceAddress { + int type; + union { + virDomainDevicePCIAddress pci; + } data; +}; + + /* Two types of disk backends */ enum virDomainDiskType { VIR_DOMAIN_DISK_TYPE_BLOCK, @@ -112,7 +136,12 @@ struct _virDomainDiskDef { int unit_id; /* Unit on the controller */ char *src; char *dst; - char *controller_id; + char *controller_name; + struct { + unsigned domain; + unsigned bus; + unsigned slot; + } controller_pci_addr; char *driverName; char *driverType; char *serial; @@ -127,6 +156,15 @@ struct _virDomainDiskDef { virStorageEncryptionPtr encryption; }; +/* Stores the virtual disk controller configuration */ +typedef struct _virDomainControllerDef virDomainControllerDef; +typedef virDomainControllerDef *virDomainControllerDefPtr; +struct _virDomainControllerDef { + int type; + char *name; + virDomainDeviceAddressPtr address; +}; + static inline int virDiskHasValidPciAddr(virDomainDiskDefPtr def) { @@ -483,6 +521,7 @@ enum virDomainDeviceType { VIR_DOMAIN_DEVICE_VIDEO, VIR_DOMAIN_DEVICE_HOSTDEV, VIR_DOMAIN_DEVICE_WATCHDOG, + VIR_DOMAIN_DEVICE_CONTROLLER, VIR_DOMAIN_DEVICE_LAST, }; @@ -493,6 +532,7 @@ struct _virDomainDeviceDef { int type; union { virDomainDiskDefPtr disk; + virDomainControllerDefPtr controller; virDomainFSDefPtr fs; virDomainNetDefPtr net; virDomainInputDefPtr input; @@ -605,6 +645,9 @@ struct _virDomainDef { int ndisks; virDomainDiskDefPtr *disks; + int ncontrollers; + virDomainControllerDefPtr *controllers; + int nfss; virDomainFSDefPtr *fss; @@ -691,6 +734,7 @@ virDomainObjPtr virDomainFindByName(const virDomainObjListPtr doms, void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def); void virDomainInputDefFree(virDomainInputDefPtr def); void virDomainDiskDefFree(virDomainDiskDefPtr def); +void virDomainControllerDefFree(virDomainControllerDefPtr def); void virDomainFSDefFree(virDomainFSDefPtr def); void virDomainNetDefFree(virDomainNetDefPtr def); void virDomainChrDefFree(virDomainChrDefPtr def); @@ -699,6 +743,7 @@ void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def); void virDomainVideoDefFree(virDomainVideoDefPtr def); void virDomainHostdevDefFree(virDomainHostdevDefPtr def); void virDomainDeviceDefFree(virDomainDeviceDefPtr def); +void virDomainDeviceAddressFree(virDomainDeviceAddressPtr def); void virDomainDefFree(virDomainDefPtr vm); void virDomainObjFree(virDomainObjPtr vm); @@ -832,6 +877,7 @@ VIR_ENUM_DECL(virDomainBoot) VIR_ENUM_DECL(virDomainFeature) VIR_ENUM_DECL(virDomainLifecycle) VIR_ENUM_DECL(virDomainDevice) +VIR_ENUM_DECL(virDomainDeviceAddress) VIR_ENUM_DECL(virDomainDisk) VIR_ENUM_DECL(virDomainDiskDevice) VIR_ENUM_DECL(virDomainDiskBus) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fb81ace..92cf86c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -68,7 +68,6 @@ virCgroupGetCpuacctUsage; virCgroupGetFreezerState; virCgroupSetFreezerState; - # datatypes.h virGetDomain; virGetInterface; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f022f89..af7b59c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4356,7 +4356,6 @@ static int qemudDomainChangeEjectableMedia(virConnectPtr conn, return ret; } - static int qemudDomainAttachPciDiskDevice(virConnectPtr conn, virDomainObjPtr vm, virDomainDeviceDefPtr dev) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 35cd330..f6f3e58 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -1481,6 +1481,53 @@ qemuMonitorParsePciAddReply(virDomainObjPtr vm, return 0; } +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; +} int qemuMonitorAddPCIHostDevice(const virDomainObjPtr vm, unsigned hostDomain ATTRIBUTE_UNUSED, -- 1.6.4