Add support for virtio-serial by defining a new 'virtio' channel target type and a virtio-serial controller. Allows the following to be specified in a domain: <controller type='virtio-serial' index='0' max_ports='16' vectors='4'/> <channel type='pty'> <target type='virtio' name='org.linux-kvm.port.0'/> <address type='virtio-serial' controller='0' bus='0'/> </channel> * docs/schemas/domain.rng: Add virtio-serial controller and virtio channel type. * src/conf/domain_conf.[ch]: Domain parsing/serialization for virtio-serial controller and virtio channel. * tests/qemuxml2xmltest.c tests/qemuxml2argvdata/qemuxml2argv-channel-virtio.xml : add domain xml parsing test * src/libvirt_private.syms src/qemu/qemu_conf.c: virDomainDefAddDiskControllers() renamed to virDomainDefAddImplicitControllers() --- docs/schemas/domain.rng | 71 +++++- src/conf/domain_conf.c | 234 +++++++++++++++++--- src/conf/domain_conf.h | 25 ++- src/libvirt_private.syms | 2 +- src/qemu/qemu_conf.c | 2 +- .../qemuxml2argv-channel-virtio.xml | 35 +++ tests/qemuxml2xmltest.c | 1 + 7 files changed, 320 insertions(+), 50 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio.xml diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 53b82ce..85df8b8 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -523,16 +523,36 @@ </define> <define name="controller"> <element name="controller"> - <optional> - <attribute name="type"> - <choice> - <value>fdc</value> - <value>ide</value> - <value>scsi</value> - <value>sata</value> - </choice> - </attribute> - </optional> + <choice> + <group> + <optional> + <attribute name="type"> + <choice> + <value>fdc</value> + <value>ide</value> + <value>scsi</value> + <value>sata</value> + </choice> + </attribute> + </optional> + </group> + <!-- virtio-serial can have 2 additional attributes --> + <group> + <attribute name="type"> + <value>virtio-serial</value> + </attribute> + <optional> + <attribute name="max_ports"> + <ref name="unsignedInt"/> + </attribute> + </optional> + <optional> + <attribute name="vectors"> + <ref name="unsignedInt"/> + </attribute> + </optional> + </group> + </choice> <attribute name="index"> <ref name="unsignedInt"/> </attribute> @@ -1139,12 +1159,25 @@ <attribute name="port"/> </element> </define> + <define name="virtioTarget"> + <element name="target"> + <attribute name="type"> + <value>virtio</value> + </attribute> + <optional> + <attribute name="name"/> + </optional> + </element> + </define> <define name="channel"> <element name="channel"> <ref name="qemucdevSrcType"/> <interleave> <ref name="qemucdevSrcDef"/> - <ref name="guestfwdTarget"/> + <choice> + <ref name="guestfwdTarget"/> + <ref name="virtioTarget"/> + </choice> <optional> <ref name="address"/> </optional> @@ -1269,6 +1302,16 @@ <ref name="driveUnit"/> </attribute> </define> + <define name="virtioserialaddress"> + <attribute name="controller"> + <ref name="driveController"/> + </attribute> + <optional> + <attribute name="bus"> + <ref name="driveBus"/> + </attribute> + </optional> + </define> <!-- Devices attached to a domain. --> @@ -1413,6 +1456,12 @@ </attribute> <ref name="driveaddress"/> </group> + <group> + <attribute name="type"> + <value>virtio-serial</value> + </attribute> + <ref name="virtioserialaddress"/> + </group> </choice> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 59fd251..58f353b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -93,7 +93,8 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST, VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, "none", "pci", - "drive"); + "drive", + "virtio-serial"); VIR_ENUM_IMPL(virDomainDisk, VIR_DOMAIN_DISK_TYPE_LAST, "block", @@ -125,7 +126,8 @@ VIR_ENUM_IMPL(virDomainController, VIR_DOMAIN_CONTROLLER_TYPE_LAST, "ide", "fdc", "scsi", - "sata") + "sata", + "virtio-serial") VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST, "mount", @@ -150,7 +152,8 @@ VIR_ENUM_IMPL(virDomainChrTarget, VIR_DOMAIN_CHR_TARGET_TYPE_LAST, "parallel", "serial", "console", - "guestfwd") + "guestfwd", + "virtio") VIR_ENUM_IMPL(virDomainChr, VIR_DOMAIN_CHR_TYPE_LAST, "null", @@ -459,6 +462,10 @@ void virDomainChrDefFree(virDomainChrDefPtr def) case VIR_DOMAIN_CHR_TARGET_TYPE_GUESTFWD: VIR_FREE(def->target.addr); break; + + case VIR_DOMAIN_CHR_TARGET_TYPE_VIRTIO: + VIR_FREE(def->target.name); + break; } switch (def->type) { @@ -811,6 +818,13 @@ int virDomainDeviceDriveAddressIsValid(virDomainDeviceDriveAddressPtr addr ATTRI /*return addr->controller || addr->bus || addr->unit;*/ return 1; /* 0 is valid for all fields, so any successfully parsed addr is valid */ } + + +int virDomainDeviceVirtioSerialAddressIsValid( + virDomainDeviceVirtioSerialAddressPtr addr ATTRIBUTE_UNUSED) +{ + return 1; /* 0 is valid for all fields, so any successfully parsed addr is valid */ +} #endif /* !PROXY */ @@ -916,7 +930,8 @@ void virDomainDefClearDeviceAliases(virDomainDefPtr def) */ static int virDomainDeviceInfoFormat(virBufferPtr buf, virDomainDeviceInfoPtr info, - int flags) + int flags, + const char *indent) { if (!info) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -926,14 +941,15 @@ static int virDomainDeviceInfoFormat(virBufferPtr buf, if (info->alias && !(flags & VIR_DOMAIN_XML_INACTIVE)) { - virBufferVSprintf(buf, " <alias name='%s'/>\n", info->alias); + virBufferVSprintf(buf, "%s<alias name='%s'/>\n", indent, info->alias); } if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) return 0; /* We'll be in domain/devices/[device type]/ so 3 level indent */ - virBufferVSprintf(buf, " <address type='%s'", + virBufferVSprintf(buf, "%s<address type='%s'", + indent, virDomainDeviceAddressTypeToString(info->type)); switch (info->type) { @@ -952,6 +968,12 @@ static int virDomainDeviceInfoFormat(virBufferPtr buf, info->addr.drive.unit); break; + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL: + virBufferVSprintf(buf, " controller='%d' bus='%d'", + info->addr.vioserial.controller, + info->addr.vioserial.bus); + break; + default: virDomainReportError(VIR_ERR_INTERNAL_ERROR, _("unknown address type '%d'"), info->type); @@ -1075,6 +1097,50 @@ cleanup: return ret; } + +static int +virDomainDeviceVirtioSerialAddressParseXML( + xmlNodePtr node, + virDomainDeviceVirtioSerialAddressPtr addr +) +{ + char *controller, *bus; + int ret = -1; + + memset(addr, 0, sizeof(*addr)); + + controller = virXMLPropString(node, "controller"); + bus = virXMLPropString(node, "bus"); + + if (controller && + virStrToLong_ui(controller, NULL, 10, &addr->controller) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse <address> 'controller' attribute")); + goto cleanup; + } + + if (bus && + virStrToLong_ui(bus, NULL, 10, &addr->bus) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse <address> 'bus' attribute")); + goto cleanup; + } + + if (!virDomainDeviceVirtioSerialAddressIsValid(addr)) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Insufficient specification for " + "virtio serial address")); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(controller); + VIR_FREE(bus); + return ret; +} + /* Parse the XML definition for a device address * @param node XML nodeset to parse for device address definition */ @@ -1137,6 +1203,12 @@ virDomainDeviceInfoParseXML(xmlNodePtr node, goto cleanup; break; + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL: + if (virDomainDeviceVirtioSerialAddressParseXML + (address, &info->addr.vioserial) < 0) + goto cleanup; + break; + default: /* Should not happen */ virDomainReportError(VIR_ERR_INTERNAL_ERROR, @@ -1462,9 +1534,9 @@ virDomainControllerDefParseXML(xmlNodePtr node, type = virXMLPropString(node, "type"); if (type) { - if ((def->type = virDomainDiskBusTypeFromString(type)) < 0) { + if ((def->type = virDomainControllerTypeFromString(type)) < 0) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown disk controller type '%s'"), type); + _("Unknown controller type '%s'"), type); goto error; } } @@ -1473,7 +1545,7 @@ virDomainControllerDefParseXML(xmlNodePtr node, if (idx) { if (virStrToLong_i(idx, NULL, 10, &def->idx) < 0) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot parse disk controller index %s"), idx); + _("Cannot parse controller index %s"), idx); goto error; } } @@ -1481,10 +1553,49 @@ virDomainControllerDefParseXML(xmlNodePtr node, if (virDomainDeviceInfoParseXML(node, &def->info, flags) < 0) goto error; + switch (def->type) { + case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: + 0; /* C requires a statement immediately following a label */ + + char *max_ports = virXMLPropString(node, "max_ports"); + if (max_ports) { + int r = virStrToLong_i(max_ports, NULL, 10, + &def->opts.vioserial.max_ports); + if (r != 0 || def->opts.vioserial.max_ports < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid max_ports: %s"), max_ports); + VIR_FREE(max_ports); + goto error; + } + } else { + def->opts.vioserial.max_ports = -1; + } + VIR_FREE(max_ports); + + char *vectors = virXMLPropString(node, "vectors"); + if (vectors) { + int r = virStrToLong_i(vectors, NULL, 10, + &def->opts.vioserial.vectors); + if (r != 0 || def->opts.vioserial.vectors < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid vectors: %s"), vectors); + VIR_FREE(vectors); + goto error; + } + } else { + def->opts.vioserial.vectors = -1; + } + VIR_FREE(vectors); + break; + + default: + break; + } + if (def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Disk controllers must use the 'pci' address type")); + _("Controllers must use the 'pci' address type")); goto error; } @@ -2086,6 +2197,10 @@ virDomainChrDefParseXML(xmlNodePtr node, virSocketSetPort(def->target.addr, port); break; + case VIR_DOMAIN_CHR_TARGET_TYPE_VIRTIO: + def->target.name = virXMLPropString(cur, "name"); + break; + default: virDomainReportError(VIR_ERR_XML_ERROR, _("unexpected target type type %u"), @@ -2096,7 +2211,6 @@ virDomainChrDefParseXML(xmlNodePtr node, cur = cur->next; } - switch (def->type) { case VIR_DOMAIN_CHR_TYPE_NULL: /* Nada */ @@ -3629,12 +3743,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, } VIR_FREE(nodes); - /* Auto-add any further disk controllers implied by declared <disk> - * elements, but not present as <controller> elements - */ - if (virDomainDefAddDiskControllers(def) < 0) - goto error; - /* analysis of the filesystems */ if ((n = virXPathNodeSet("./devices/filesystem", ctxt, &nodes)) < 0) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, @@ -3948,6 +4056,11 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, goto error; } + /* Auto-add any implied controllers which aren't present + */ + if (virDomainDefAddImplicitControllers(def) < 0) + goto error; + return def; no_memory: @@ -4211,9 +4324,9 @@ cleanup: return obj; } -static int virDomainDefMaybeAddDiskController(virDomainDefPtr def, - int type, - int idx) +static int virDomainDefMaybeAddController(virDomainDefPtr def, + int type, + int idx) { int found = 0; int i; @@ -4266,7 +4379,7 @@ static int virDomainDefAddDiskControllersForType(virDomainDefPtr def, } for (i = 0 ; i <= maxController ; i++) { - if (virDomainDefMaybeAddDiskController(def, controllerType, i) < 0) + if (virDomainDefMaybeAddController(def, controllerType, i) < 0) return -1; } @@ -4274,13 +4387,33 @@ static int virDomainDefAddDiskControllersForType(virDomainDefPtr def, } +static int virDomainDefMaybeAddVirtioSerialController(virDomainDefPtr def) +{ + /* Look for any virtio serial device */ + int i; + for (i = 0 ; i < def->nchannels ; i++) { + virDomainChrDefPtr channel = def->channels[i]; + + if (channel->targetType == VIR_DOMAIN_CHR_TARGET_TYPE_VIRTIO) { + /* Try to add a virtio serial controller with index 0 */ + if (virDomainDefMaybeAddController(def, + VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL, 0) < 0) + return -1; + break; + } + } + + return 0; +} + + /* - * Based on the declared <address type=drive> info for any disks, + * Based on the declared <address/> info for any devices, * add neccessary drive controllers which are not already present * in the XML. This is for compat with existing apps which will * not know/care about <controller> info in the XML */ -int virDomainDefAddDiskControllers(virDomainDefPtr def) +int virDomainDefAddImplicitControllers(virDomainDefPtr def) { if (virDomainDefAddDiskControllersForType(def, VIR_DOMAIN_CONTROLLER_TYPE_SCSI, @@ -4297,6 +4430,9 @@ int virDomainDefAddDiskControllers(virDomainDefPtr def) VIR_DOMAIN_DISK_BUS_IDE) < 0) return -1; + if (virDomainDefMaybeAddVirtioSerialController(def) < 0) + return -1; + return 0; } @@ -4597,7 +4733,7 @@ virDomainDiskDefFormat(virBufferPtr buf, virStorageEncryptionFormat(buf, def->encryption) < 0) return -1; - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, flags, " ") < 0) return -1; virBufferAddLit(buf, " </disk>\n"); @@ -4622,9 +4758,25 @@ virDomainControllerDefFormat(virBufferPtr buf, " <controller type='%s' index='%d'", type, def->idx); + switch (def->type) { + case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: + if (def->opts.vioserial.max_ports != -1) { + virBufferVSprintf(buf, " max_ports='%d'", + def->opts.vioserial.max_ports); + } + if (def->opts.vioserial.vectors != -1) { + virBufferVSprintf(buf, " vectors='%d'", + def->opts.vioserial.vectors); + } + break; + + default: + break; + } + if (virDomainDeviceInfoIsSet(&def->info)) { virBufferAddLit(buf, ">\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, flags, " ") < 0) return -1; virBufferAddLit(buf, " </controller>\n"); } else { @@ -4680,7 +4832,7 @@ virDomainFSDefFormat(virBufferPtr buf, if (def->readonly) virBufferAddLit(buf, " <readonly/>\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, flags, " ") < 0) return -1; virBufferAddLit(buf, " </filesystem>\n"); @@ -4769,7 +4921,7 @@ virDomainNetDefFormat(virBufferPtr buf, virBufferEscapeString(buf, " <model type='%s'/>\n", def->model); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, flags, " ") < 0) return -1; virBufferAddLit(buf, " </interface>\n"); @@ -4792,6 +4944,7 @@ virDomainChrDefFormat(virBufferPtr buf, switch (def->targetType) { /* channel types are in a common channel element */ case VIR_DOMAIN_CHR_TARGET_TYPE_GUESTFWD: + case VIR_DOMAIN_CHR_TARGET_TYPE_VIRTIO: elementName = "channel"; break; @@ -4905,11 +5058,18 @@ virDomainChrDefFormat(virBufferPtr buf, break; } + case VIR_DOMAIN_CHR_TARGET_TYPE_VIRTIO: + virBufferAddLit(buf, " <target type='virtio'"); + if (def->target.name) { + virBufferVSprintf(buf, " name='%s'", def->target.name); + } + virBufferAddLit(buf, "/>\n"); + break; + case VIR_DOMAIN_CHR_TARGET_TYPE_PARALLEL: case VIR_DOMAIN_CHR_TARGET_TYPE_SERIAL: case VIR_DOMAIN_CHR_TARGET_TYPE_CONSOLE: - virBufferVSprintf(buf, " <target port='%d'/>\n", - def->target.port); + virBufferVSprintf(buf, " <target port='%d'/>\n", def->target.port); break; default: @@ -4919,8 +5079,10 @@ virDomainChrDefFormat(virBufferPtr buf, return -1; } - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) - return -1; + if (virDomainDeviceInfoIsSet(&def->info)) { + if (virDomainDeviceInfoFormat(buf, &def->info, flags, " ") < 0) + return -1; + } virBufferVSprintf(buf, " </%s>\n", elementName); @@ -4946,7 +5108,7 @@ virDomainSoundDefFormat(virBufferPtr buf, if (virDomainDeviceInfoIsSet(&def->info)) { virBufferAddLit(buf, ">\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, flags, " ") < 0) return -1; virBufferAddLit(buf, " </sound>\n"); } else { @@ -4982,7 +5144,7 @@ virDomainWatchdogDefFormat(virBufferPtr buf, if (virDomainDeviceInfoIsSet(&def->info)) { virBufferAddLit(buf, ">\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, flags, " ") < 0) return -1; virBufferAddLit(buf, " </watchdog>\n"); } else { @@ -5033,7 +5195,7 @@ virDomainVideoDefFormat(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, flags, " ") < 0) return -1; virBufferAddLit(buf, " </video>\n"); @@ -5065,7 +5227,7 @@ virDomainInputDefFormat(virBufferPtr buf, if (virDomainDeviceInfoIsSet(&def->info)) { virBufferAddLit(buf, ">\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, flags, " ") < 0) return -1; virBufferAddLit(buf, " </input>\n"); } else { @@ -5218,7 +5380,7 @@ virDomainHostdevDefFormat(virBufferPtr buf, virBufferAddLit(buf, " </source>\n"); - if (virDomainDeviceInfoFormat(buf, &def->info, flags) < 0) + if (virDomainDeviceInfoFormat(buf, &def->info, flags, " ") < 0) return -1; virBufferAddLit(buf, " </hostdev>\n"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 782af95..d4b0930 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -68,6 +68,7 @@ enum virDomainDeviceAddressType { VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE, + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST }; @@ -89,6 +90,13 @@ struct _virDomainDeviceDriveAddress { unsigned int unit; }; +typedef struct _virDomainDeviceVirtioSerialAddress virDomainDeviceVirtioSerialAddress; +typedef virDomainDeviceVirtioSerialAddress *virDomainDeviceVirtioSerialAddressPtr; +struct _virDomainDeviceVirtioSerialAddress { + unsigned int controller; + unsigned int bus; +}; + typedef struct _virDomainDeviceInfo virDomainDeviceInfo; typedef virDomainDeviceInfo *virDomainDeviceInfoPtr; struct _virDomainDeviceInfo { @@ -97,6 +105,7 @@ struct _virDomainDeviceInfo { union { virDomainDevicePCIAddress pci; virDomainDeviceDriveAddress drive; + virDomainDeviceVirtioSerialAddress vioserial; } addr; }; @@ -166,16 +175,27 @@ enum virDomainControllerType { VIR_DOMAIN_CONTROLLER_TYPE_FDC, VIR_DOMAIN_CONTROLLER_TYPE_SCSI, VIR_DOMAIN_CONTROLLER_TYPE_SATA, + VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL, VIR_DOMAIN_CONTROLLER_TYPE_LAST }; +typedef struct _virDomainVirtioSerialOpts virDomainVirtioSerialOpts; +typedef virDomainVirtioSerialOpts *virDomainVirtioSerialOptsPtr; +struct _virDomainVirtioSerialOpts { + int max_ports; /* -1 == undef */ + int vectors; /* -1 == undef */ +}; + /* Stores the virtual disk controller configuration */ typedef struct _virDomainControllerDef virDomainControllerDef; typedef virDomainControllerDef *virDomainControllerDefPtr; struct _virDomainControllerDef { int type; int idx; + union { + virDomainVirtioSerialOpts vioserial; + } opts; virDomainDeviceInfo info; }; @@ -271,6 +291,7 @@ enum virDomainChrTargetType { VIR_DOMAIN_CHR_TARGET_TYPE_SERIAL, VIR_DOMAIN_CHR_TARGET_TYPE_CONSOLE, VIR_DOMAIN_CHR_TARGET_TYPE_GUESTFWD, + VIR_DOMAIN_CHR_TARGET_TYPE_VIRTIO, VIR_DOMAIN_CHR_TARGET_TYPE_LAST }; @@ -304,6 +325,7 @@ struct _virDomainChrDef { union { int port; /* parallel, serial, console */ virSocketAddrPtr addr; /* guestfwd */ + char *name; /* virtio */ } target; int type; @@ -744,6 +766,7 @@ int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info, int type); int virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr addr); int virDomainDeviceDriveAddressIsValid(virDomainDeviceDriveAddressPtr addr); +int virDomainDeviceVirtioSerialAddressIsValid(virDomainDeviceVirtioSerialAddressPtr addr); int virDomainDeviceInfoIsSet(virDomainDeviceInfoPtr info); void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info); void virDomainDefClearPCIAddresses(virDomainDefPtr def); @@ -790,7 +813,7 @@ virDomainObjPtr virDomainObjParseNode(virCapsPtr caps, xmlDocPtr xml, xmlNodePtr root); -int virDomainDefAddDiskControllers(virDomainDefPtr def); +int virDomainDefAddImplicitControllers(virDomainDefPtr def); #endif char *virDomainDefFormat(virDomainDefPtr def, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index aa826d6..284db08 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -186,7 +186,7 @@ virDomainDeviceInfoIsSet; virDomainControllerTypeToString; virDomainControllerDefFree; virDomainDeviceAddressTypeToString; -virDomainDefAddDiskControllers; +virDomainDefAddImplicitControllers; virDomainDefClearPCIAddresses; virDomainDefClearDeviceAliases; virDomainDeviceInfoIterate; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index c9fe55b..456ee34 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -5697,7 +5697,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, goto no_memory; } - if (virDomainDefAddDiskControllers(def) < 0) + if (virDomainDefAddImplicitControllers(def) < 0) goto error; return def; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio.xml b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio.xml new file mode 100644 index 0000000..29af510 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio.xml @@ -0,0 +1,35 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219200</memory> + <currentMemory>219200</currentMemory> + <vcpu cpuset='1-4,8-20,525'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <controller type='virtio-serial' index='0' max_ports='16' vectors='4'/> + <controller type='virtio-serial' index='1'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/> + </controller> + <channel type='pty'> + <target type='virtio' name='org.linux-kvm.port.0'/> + </channel> + <channel type='pty'> + <target type='virtio' name='org.linux-kvm.port.1'/> + <address type='virtio-serial' controller='1' bus='0'/> + </channel> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index e807d7b..ace2be8 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -133,6 +133,7 @@ mymain(int argc, char **argv) DO_TEST("parallel-tcp"); DO_TEST("console-compat"); DO_TEST("channel-guestfwd"); + DO_TEST("channel-virtio"); DO_TEST("hostdev-usb-address"); DO_TEST("hostdev-pci-address"); -- 1.6.6

On 18/02/10 12:34, Daniel P. Berrange wrote: I'm not 100% sure what vectors does, however I believe this is a resource usage tuning knob and therefore can't be inferred. max_ports we could possibly default. However, virtio-serial also supports hot-plug, although I haven't added libvirt support for it. -device virtserialport,bus=channel0.0... I've called 'channel0' the controller, and '0' the bus. I could remove this. I was originally putting <address> elsewhere, which screwed up the indentation. Will do. 0 has a special meaning for vectors. From https://fedoraproject.org/wiki/Features/VirtioSerial#How_To_Test: If '0' is specified here, MSI will be disabled and a GSI interrupt will be used. I used a signed int for both values for consistency. Matt -- Matthew Booth, RHCA, RHCSS Red Hat Engineering, Virtualisation Team M: +44 (0)7977 267231 GPG ID: D33C3490 GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490

On 18/02/10 14:09, Daniel P. Berrange wrote: Updated patch attached. -- Matthew Booth, RHCA, RHCSS Red Hat Engineering, Virtualisation Team M: +44 (0)7977 267231 GPG ID: D33C3490 GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490

Support virtio-serial controller and virtio channel in QEMU backend. Will output the following for virtio-serial controller: -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x4,max_ports=16,vectors=4 and the following for a virtio channel: -chardev pty,id=channel0 \ -device virtserialport,bus=virtio-serial0.0,chardev=channel0,name=org.linux-kvm.port.0 * src/qemu/qemu_conf.c: Add argument output for virtio * tests/qemuxml2argvtest.c tests/qemuxml2argvdata/qemuxml2argv-channel-virtio.args : Add test for QEMU command line generation --- src/qemu/qemu_conf.c | 91 +++++++++++++++++++- .../qemuxml2argv-channel-virtio.args | 1 + tests/qemuxml2argvtest.c | 1 + 3 files changed, 92 insertions(+), 1 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio.args diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 456ee34..110409d 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -2168,7 +2168,6 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs) } for (i = 0; i < def->nchannels ; i++) { /* Nada - none are PCI based (yet) */ - /* XXX virtio-serial will need one */ } if (def->watchdog && def->watchdog->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { @@ -2465,6 +2464,23 @@ qemuBuildControllerDevStr(virDomainControllerDefPtr def) virBufferVSprintf(&buf, ",id=scsi%d", def->idx); break; + case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL: + if (def->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + virBufferAddLit(&buf, "virtio-serial-pci"); + } else { + virBufferAddLit(&buf, "virtio-serial"); + } + virBufferVSprintf(&buf, ",id=virtio-serial%d", def->idx); + if (def->opts.vioserial.max_ports != -1) { + virBufferVSprintf(&buf, ",max_ports=%d", + def->opts.vioserial.max_ports); + } + if (def->opts.vioserial.vectors != -1) { + virBufferVSprintf(&buf, ",vectors=%d", + def->opts.vioserial.vectors); + } + break; + /* We always get an IDE controller, whether we want it or not. */ case VIR_DOMAIN_CONTROLLER_TYPE_IDE: default: @@ -3830,6 +3846,79 @@ int qemudBuildCommandLine(virConnectPtr conn, } VIR_FREE(addr); ADD_ARG(devstr); + break; + + case VIR_DOMAIN_CHR_TARGET_TYPE_VIRTIO: + if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { + qemuReportError(VIR_ERR_NO_SUPPORT, "%s", + _("virtio channel requires QEMU to support -device")); + goto error; + } + + ADD_ARG_LIT("-chardev"); + if (!(devstr = qemuBuildChrChardevStr(channel))) + goto error; + ADD_ARG(devstr); + + virBuffer virtiodev = VIR_BUFFER_INITIALIZER; + virBufferAddLit(&virtiodev, "virtserialport"); + + if (channel->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + /* Check it's a virtio-serial address */ + if (channel->info.type != + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL) + { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("virtio serial device has invalid " + "address type")); + virBufferFreeAndReset(&virtiodev); + goto error; + } + + /* Look for the virtio-serial controller */ + const char *vsalias = NULL; + int vs = 0; + int c; + for (c = 0; c < def->ncontrollers; c++) { + if (def->controllers[c]->type != + VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL) + { + continue; + } + + if (vs == channel->info.addr.vioserial.controller) { + vsalias = def->controllers[c]->info.alias; + break; + } + + vs++; + } + + if (!vsalias) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("virtio serial device address controller " + "does not exist")); + virBufferFreeAndReset(&virtiodev); + goto error; + } + + virBufferVSprintf(&virtiodev, ",bus=%s.%d", + vsalias, channel->info.addr.vioserial.bus); + } + + virBufferVSprintf(&virtiodev, ",chardev=%s", channel->info.alias); + if (channel->target.name) { + virBufferVSprintf(&virtiodev, ",name=%s", channel->target.name); + } + if (virBufferError(&virtiodev)) { + virReportOOMError(); + goto error; + } + + ADD_ARG_LIT("-device"); + ADD_ARG(virBufferContentAndReset(&virtiodev)); + + break; } } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio.args b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio.args new file mode 100644 index 0000000..4097065 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio.args @@ -0,0 +1 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefaults -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -device virtio-serial-pci,id=virtio-serial0,max_ports=16,vectors=4,bus=pci.0,addr=0x4 -device virtio-serial-pci,id=virtio-serial1,bus=pci.0,addr=0xa -hda /dev/HostVG/QEMUGuest1 -chardev pty,id=channel0 -device virtserialport,chardev=channel0,name=org.linux-kvm.port.0 -chardev pty,id=channel1 -device virtserialport,bus=virtio-serial1.0,chardev=channel1,name=org.linux-kvm.port.1 -usb -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index aa42f99..a8c9ed3 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -318,6 +318,7 @@ mymain(int argc, char **argv) DO_TEST("console-compat-chardev", QEMUD_CMD_FLAG_CHARDEV|QEMUD_CMD_FLAG_DEVICE); DO_TEST("channel-guestfwd", QEMUD_CMD_FLAG_CHARDEV|QEMUD_CMD_FLAG_DEVICE); + DO_TEST("channel-virtio", QEMUD_CMD_FLAG_DEVICE); DO_TEST("watchdog", 0); DO_TEST("watchdog-device", QEMUD_CMD_FLAG_DEVICE); -- 1.6.6

On 18/02/10 12:39, Daniel P. Berrange wrote: Would this comment still stand if the code below was simplified to hard-code alias? Actually, this code is really to look for the controller's alias. Is it safe to hard-code it? -- Matthew Booth, RHCA, RHCSS Red Hat Engineering, Virtualisation Team M: +44 (0)7977 267231 GPG ID: D33C3490 GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490

On 18/02/10 14:11, Daniel P. Berrange wrote: Update patch attached. Matt -- Matthew Booth, RHCA, RHCSS Red Hat Engineering, Virtualisation Team M: +44 (0)7977 267231 GPG ID: D33C3490 GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490