[libvirt] [PATCH 1/3] Clarify why some controllers don't generate -device string in QEMU driver
The QEMU driver contained code to generate a -device string for piix4-ide, but wasn't using it. This change removes this string generation. It also adds a comment explaining why IDE and FDC controllers don't generate -device strings. The change also generates an error if a sata controller is specified for a QEMU domain, as this isn't supported. * src/qemu/qemu_conf.c: Remove VIR_DOMAIN_CONTROLLER_TYPE_IDE handler in qemuBuildControllerDevStr(). Ignore IDE and FDC controllers. Error if SATA controller is discovered. Add comments. --- src/qemu/qemu_conf.c | 26 ++++++++++++++++++-------- 1 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index f4a6c08..3b7793f 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -2121,11 +2121,8 @@ qemuBuildControllerDevStr(virDomainControllerDefPtr def) virBufferVSprintf(&buf, ",id=scsi%d", def->idx); break; + /* We always get an IDE controller, whether we want it or not. */ case VIR_DOMAIN_CONTROLLER_TYPE_IDE: - virBufferAddLit(&buf, "piix4-ide"); - virBufferVSprintf(&buf, ",id=ide%d", def->idx); - break; - default: goto error; } @@ -3141,16 +3138,29 @@ int qemudBuildCommandLine(virConnectPtr conn, if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { for (i = 0 ; i < def->ncontrollers ; i++) { - char *scsi; - if (def->controllers[i]->type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI) + virDomainControllerDefPtr cont = def->controllers[i]; + + /* We don't add an explicit IDE or FD controller because the + * provided PIIX4 device already includes one. It isn't possible to + * remove the PIIX4. */ + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE || + cont->type == VIR_DOMAIN_CONTROLLER_TYPE_FDC) continue; + /* QEMU doesn't implement a SATA driver */ + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA) { + qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, + "%s", _("SATA is not supported with this QEMU binary")); + goto error; + } + ADD_ARG_LIT("-device"); - if (!(scsi = qemuBuildControllerDevStr(def->controllers[i]))) + char *devstr; + if (!(devstr = qemuBuildControllerDevStr(def->controllers[i]))) goto no_memory; - ADD_ARG(scsi); + ADD_ARG(devstr); } } -- 1.6.6
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'/> <channel type='pty'> <target type='virtio' name='org.linux-kvm.port.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 | 25 ++++- src/conf/domain_conf.c | 124 +++++++++++++++++--- src/conf/domain_conf.h | 10 ++- src/libvirt_private.syms | 2 +- src/qemu/qemu_conf.c | 2 +- .../qemuxml2argv-channel-virtio.xml | 28 +++++ tests/qemuxml2xmltest.c | 1 + 7 files changed, 173 insertions(+), 19 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-virtio.xml diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 827ff6f..a3247b9 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -530,6 +530,7 @@ <value>ide</value> <value>scsi</value> <value>sata</value> + <value>virtio-serial</value> </choice> </attribute> </optional> @@ -1120,12 +1121,34 @@ <attribute name="port"/> </element> </define> + <define name="virtioTarget"> + <element name="target"> + <attribute name="type"> + <value>virtio</value> + </attribute> + <optional> + <attribute name="name"/> + </optional> + <optional> + <attribute name="bytelimit"/> + </optional> + <optional> + <attribute name="guestbytelimit"/> + </optional> + <optional> + <attribute name="cachebuffers"/> + </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> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e548d1d..3b96086 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -124,7 +124,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", @@ -148,7 +149,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", @@ -449,6 +451,13 @@ 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.virtio.name); + VIR_FREE(def->target.virtio.byteLimit); + VIR_FREE(def->target.virtio.guestByteLimit); + VIR_FREE(def->target.virtio.cacheBuffers); + break; } switch (def->type) { @@ -1460,7 +1469,7 @@ virDomainControllerDefParseXML(virConnectPtr conn, type = virXMLPropString(node, "type"); if (type) { - if ((def->type = virDomainDiskBusTypeFromString(type)) < 0) { + if ((def->type = virDomainControllerTypeFromString(type)) < 0) { virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, _("unknown disk controller type '%s'"), type); goto error; @@ -2081,6 +2090,48 @@ virDomainChrDefParseXML(virConnectPtr conn, virSocketSetPort(def->target.addr, port); break; + case VIR_DOMAIN_CHR_TARGET_TYPE_VIRTIO: + def->target.virtio.name + = virXMLPropString(cur, "name"); + def->target.virtio.byteLimit + = virXMLPropString(cur, "bytelimit"); + def->target.virtio.guestByteLimit + = virXMLPropString(cur, "guestbytelimit"); + def->target.virtio.cacheBuffers + = virXMLPropString(cur, "cachebuffers"); + + /* Ensure bytelimit and guestbytelimit are positive integers + * if they are defined */ + unsigned int testUI; + if (def->target.virtio.byteLimit && + virStrToLong_ui(def->target.virtio.byteLimit, + NULL, 10, &testUI) < 0) { + virDomainReportError(conn, VIR_ERR_XML_ERROR, + _("Invalid byte_limit: %s"), + def->target.virtio.byteLimit); + goto error; + } + if (def->target.virtio.guestByteLimit && + virStrToLong_ui(def->target.virtio.guestByteLimit, + NULL, 10, &testUI) < 0) { + virDomainReportError(conn, VIR_ERR_XML_ERROR, + _("Invalid guest_byte_limit: %s"), + def->target.virtio.guestByteLimit); + goto error; + } + + /* Ensure that cacheBuffers is either 0 or 1 if it defined + */ + if (def->target.virtio.cacheBuffers && + STRNEQ(def->target.virtio.cacheBuffers, "0") && + STRNEQ(def->target.virtio.cacheBuffers, "1")) { + virDomainReportError(conn, VIR_ERR_XML_ERROR, + _("Invalid cache_buffers: %s"), + def->target.virtio.cacheBuffers); + goto error; + } + break; + default: virDomainReportError(conn, VIR_ERR_XML_ERROR, _("unexpected target type type %u"), @@ -3640,12 +3691,6 @@ static virDomainDefPtr virDomainDefParseXML(virConnectPtr conn, } 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(conn, "./devices/filesystem", ctxt, &nodes)) < 0) { virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, @@ -3970,6 +4015,11 @@ static virDomainDefPtr virDomainDefParseXML(virConnectPtr conn, goto error; } + /* Auto-add any implied controllers which aren't present + */ + if (virDomainDefAddImplicitControllers(def) < 0) + goto error; + return def; no_memory: @@ -4247,9 +4297,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; @@ -4302,7 +4352,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; } @@ -4310,13 +4360,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, @@ -4333,6 +4403,9 @@ int virDomainDefAddDiskControllers(virDomainDefPtr def) VIR_DOMAIN_DISK_BUS_IDE) < 0) return -1; + if (virDomainDefMaybeAddVirtioSerialController(def) < 0) + return -1; + return 0; } @@ -4837,6 +4910,7 @@ virDomainChrDefFormat(virConnectPtr conn, 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; @@ -4950,6 +5024,26 @@ virDomainChrDefFormat(virConnectPtr conn, break; } + case VIR_DOMAIN_CHR_TARGET_TYPE_VIRTIO: + virBufferVSprintf(buf, " <target type='virtio'"); + if (def->target.virtio.name) { + virBufferVSprintf(buf, " name='%s'", def->target.virtio.name); + } + if (def->target.virtio.byteLimit) { + virBufferVSprintf(buf, " bytelimit='%s'", + def->target.virtio.byteLimit); + } + if (def->target.virtio.guestByteLimit) { + virBufferVSprintf(buf, " guestbytelimit='%s'", + def->target.virtio.guestByteLimit); + } + if (def->target.virtio.cacheBuffers) { + virBufferVSprintf(buf, " cachebuffers='%s'", + def->target.virtio.cacheBuffers); + } + virBufferVSprintf(buf, "/>\n"); + break; + case VIR_DOMAIN_CHR_TARGET_TYPE_PARALLEL: case VIR_DOMAIN_CHR_TARGET_TYPE_SERIAL: case VIR_DOMAIN_CHR_TARGET_TYPE_CONSOLE: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 7be090d..da2115e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -166,6 +166,7 @@ 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 }; @@ -260,6 +261,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 }; @@ -293,6 +295,12 @@ struct _virDomainChrDef { union { int port; /* parallel, serial, console */ virSocketAddrPtr addr; /* guestfwd */ + struct { + char *name; + char *byteLimit; + char *guestByteLimit; + char *cacheBuffers; + } virtio; /* virtio */ } target; int type; @@ -782,7 +790,7 @@ virDomainObjPtr virDomainObjParseNode(virConnectPtr conn, xmlDocPtr xml, xmlNodePtr root); -int virDomainDefAddDiskControllers(virDomainDefPtr def); +int virDomainDefAddImplicitControllers(virDomainDefPtr def); #endif char *virDomainDefFormat(virConnectPtr conn, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d56fb7d..9dd1866 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -184,7 +184,7 @@ virDomainDeviceInfoIsSet; virDomainControllerTypeToString; virDomainControllerDefFree; virDomainDeviceAddressTypeToString; -virDomainDefAddDiskControllers; +virDomainDefAddImplicitControllers; virDomainDefClearPCIAddresses; virDomainDefClearDeviceAliases; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 3b7793f..0c67334 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -5410,7 +5410,7 @@ virDomainDefPtr qemuParseCommandLine(virConnectPtr conn, 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..da5fa6c --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio.xml @@ -0,0 +1,28 @@ +<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'/> + <channel type='pty'> + <target type='virtio' name='org.linux-kvm.port.0' bytelimit='1048576' guestbytelimit='1048576' cachebuffers='1'/> + </channel> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 0302696..7ea8e79 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-product"); DO_TEST("hostdev-usb-address"); -- 1.6.6
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 and the following for a virtio channel: -chardev pty,id=channel0 \ -device virtserialport,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 | 51 +++++++++++++++++++- .../qemuxml2argv-channel-virtio.args | 1 + tests/qemuxml2argvtest.c | 1 + 3 files changed, 52 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 0c67334..73c6e28 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1681,7 +1681,6 @@ qemuAssignDevicePCISlots(virDomainDefPtr def) } for (i = 0; i < def->nchannels ; i++) { /* Nada - none are PCI based (yet) */ - /* XXX virtio-serial will need one */ } if (def->watchdog) { qemuAssignDevicePCISlot(&def->watchdog->info, nextslot++); @@ -2121,6 +2120,15 @@ 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); + break; + /* We always get an IDE controller, whether we want it or not. */ case VIR_DOMAIN_CONTROLLER_TYPE_IDE: default: @@ -3508,6 +3516,47 @@ int qemudBuildCommandLine(virConnectPtr conn, } ADD_ARG(virBufferContentAndReset(&buf)); + break; + + case VIR_DOMAIN_CHR_TARGET_TYPE_VIRTIO: + if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { + qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT, "%s", + _("virtio channel requires QEMU to support -device")); + goto error; + } + + qemudBuildCommandLineChrDevChardevStr(channel, &buf); + if (virBufferError(&buf)) + goto error; + + ADD_ARG_LIT("-chardev"); + ADD_ARG(virBufferContentAndReset(&buf)); + + virBufferVSprintf(&buf, "virtserialport,chardev=%s", + channel->info.alias); + if (channel->target.virtio.name) { + virBufferVSprintf(&buf, ",name=%s", + channel->target.virtio.name); + } + if (channel->target.virtio.byteLimit) { + virBufferVSprintf(&buf, ",byte_limit=%s", + channel->target.virtio.byteLimit); + } + if (channel->target.virtio.guestByteLimit) { + virBufferVSprintf(&buf, ",guest_byte_limit=%s", + channel->target.virtio.guestByteLimit); + } + if (channel->target.virtio.cacheBuffers) { + virBufferVSprintf(&buf, ",cache_buffers=%s", + channel->target.virtio.cacheBuffers); + } + if (virBufferError(&buf)) + goto error; + + ADD_ARG_LIT("-device"); + ADD_ARG(virBufferContentAndReset(&buf)); + + break; } } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio.args b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio.args new file mode 100644 index 0000000..ded97d1 --- /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,bus=pci.0,addr=0x4 -hda /dev/HostVG/QEMUGuest1 -chardev pty,id=channel0 -device virtserialport,chardev=channel0,name=org.linux-kvm.port.0,byte_limit=1048576,guest_byte_limit=1048576,cache_buffers=1 -usb -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index fc237c2..f6b33c2 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -297,6 +297,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
participants (1)
-
Matthew Booth