[libvirt] [PATCH 0/2]add usb-serial virtual device support for qemu
This two patches are trying to add support for qemu virtual device usb-serial. Add an optional 'type' attribute to <target> element of serial port device. There are two choices for its value, 'isa-serial' and 'usb-serial'. For backward compatibility, when attribute 'type' is missing the 'isa-serial' will be chose as before. Libvirt XML sample <serial type='pty'> <target type='usb-serial' port='0'/> <address type='usb' bus='0' port='1'/> </serial> qemu commandline: qemu ${other_vm_args} \ -chardev pty,id=charserial0 \ -device usb-serial,chardev=charserial0,id=serial0,bus=usb.0,port=1 Guannan Ren(2) [PATCH 1/2] qemu: add usb-serial caps flag [PATCH 2/2] qemu: add usb-serial support docs/formatdomain.html.in | 9 ++++++++- docs/schemas/domaincommon.rng | 20 +++++++++++++++++--- src/conf/domain_conf.c | 42 +++++++++++++++++++++++++++++++++++++++--- src/conf/domain_conf.h | 10 ++++++++++ src/libvirt_private.syms | 2 ++ src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 24 ++++++++++++++++++++++-- tests/qemuhelptest.c | 21 ++++++++++++++------- 9 files changed, 115 insertions(+), 16 deletions(-)
QEMU_CAPS_DEVICE_USB_SERIAL /* -device usb-serial */ --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemuhelptest.c | 21 ++++++++++++++------- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 17e6679..1fe9d07 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -199,6 +199,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, "cirrus-vga", "vmware-svga", "device-video-primary", + "usb-serial", ); struct _qemuCaps { @@ -1343,6 +1344,7 @@ struct qemuCapsStringFlags qemuCapsObjectTypes[] = { { "VGA", QEMU_CAPS_DEVICE_VGA }, { "cirrus-vga", QEMU_CAPS_DEVICE_CIRRUS_VGA }, { "vmware-svga", QEMU_CAPS_DEVICE_VMWARE_SVGA }, + { "usb-serial", QEMU_CAPS_DEVICE_USB_SERIAL}, }; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 3457852..d939821 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -162,6 +162,7 @@ enum qemuCapsFlags { QEMU_CAPS_DEVICE_VMWARE_SVGA = 122, /* -device vmware-svga */ QEMU_CAPS_DEVICE_VIDEO_PRIMARY = 123, /* safe to use -device XXX for primary video device */ + QEMU_CAPS_DEVICE_USB_SERIAL = 124, /* -device usb-serial */ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index 252ad3a..b71c292 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -395,7 +395,8 @@ mymain(void) QEMU_CAPS_DEVICE_QXL, QEMU_CAPS_DEVICE_VGA, QEMU_CAPS_DEVICE_CIRRUS_VGA, - QEMU_CAPS_DEVICE_VMWARE_SVGA); + QEMU_CAPS_DEVICE_VMWARE_SVGA, + QEMU_CAPS_DEVICE_USB_SERIAL); DO_TEST("qemu-kvm-0.12.3", 12003, 1, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -502,7 +503,8 @@ mymain(void) QEMU_CAPS_DEVICE_QXL, QEMU_CAPS_DEVICE_VGA, QEMU_CAPS_DEVICE_CIRRUS_VGA, - QEMU_CAPS_DEVICE_VMWARE_SVGA); + QEMU_CAPS_DEVICE_VMWARE_SVGA, + QEMU_CAPS_DEVICE_USB_SERIAL); DO_TEST("qemu-kvm-0.12.1.2-rhel61", 12001, 1, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -565,7 +567,8 @@ mymain(void) QEMU_CAPS_DEVICE_QXL, QEMU_CAPS_DEVICE_VGA, QEMU_CAPS_DEVICE_CIRRUS_VGA, - QEMU_CAPS_DEVICE_VMWARE_SVGA); + QEMU_CAPS_DEVICE_VMWARE_SVGA, + QEMU_CAPS_DEVICE_USB_SERIAL); DO_TEST("qemu-kvm-0.12.1.2-rhel62-beta", 12001, 1, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -715,7 +718,8 @@ mymain(void) QEMU_CAPS_DEVICE_QXL, QEMU_CAPS_DEVICE_VGA, QEMU_CAPS_DEVICE_CIRRUS_VGA, - QEMU_CAPS_DEVICE_VMWARE_SVGA); + QEMU_CAPS_DEVICE_VMWARE_SVGA, + QEMU_CAPS_DEVICE_USB_SERIAL); DO_TEST("qemu-1.1.0", 1001000, 0, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -801,7 +805,8 @@ mymain(void) QEMU_CAPS_DEVICE_QXL, QEMU_CAPS_DEVICE_VGA, QEMU_CAPS_DEVICE_CIRRUS_VGA, - QEMU_CAPS_DEVICE_VMWARE_SVGA); + QEMU_CAPS_DEVICE_VMWARE_SVGA, + QEMU_CAPS_DEVICE_USB_SERIAL); DO_TEST("qemu-1.2.0", 1002000, 0, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -898,7 +903,8 @@ mymain(void) QEMU_CAPS_DEVICE_VGA, QEMU_CAPS_DEVICE_CIRRUS_VGA, QEMU_CAPS_DEVICE_VMWARE_SVGA, - QEMU_CAPS_DEVICE_VIDEO_PRIMARY); + QEMU_CAPS_DEVICE_VIDEO_PRIMARY, + QEMU_CAPS_DEVICE_USB_SERIAL); DO_TEST("qemu-kvm-1.2.0", 1002000, 1, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -1000,7 +1006,8 @@ mymain(void) QEMU_CAPS_DEVICE_VGA, QEMU_CAPS_DEVICE_CIRRUS_VGA, QEMU_CAPS_DEVICE_VMWARE_SVGA, - QEMU_CAPS_DEVICE_VIDEO_PRIMARY); + QEMU_CAPS_DEVICE_VIDEO_PRIMARY, + QEMU_CAPS_DEVICE_USB_SERIAL); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 1.7.11.4
On 01/02/2013 11:57 PM, Guannan Ren wrote:
QEMU_CAPS_DEVICE_USB_SERIAL /* -device usb-serial */ --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemuhelptest.c | 21 ++++++++++++++------- 3 files changed, 17 insertions(+), 7 deletions(-)
ACK.
DO_TEST("qemu-kvm-1.2.0", 1002000, 1, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -1000,7 +1006,8 @@ mymain(void) QEMU_CAPS_DEVICE_VGA, QEMU_CAPS_DEVICE_CIRRUS_VGA, QEMU_CAPS_DEVICE_VMWARE_SVGA, - QEMU_CAPS_DEVICE_VIDEO_PRIMARY); + QEMU_CAPS_DEVICE_VIDEO_PRIMARY, + QEMU_CAPS_DEVICE_USB_SERIAL);
Side question - why are we still parsing -help output of 1.2.0 in our help test, since that should be using QMP instead? And when are we going to improve our testsuite to make sure our QMP capability parsing is doing what we want? But doesn't affect this patch. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Add an optional 'type' attribute to <target> element of serial port device. There are two choices for its value, 'isa-serial' and 'usb-serial'. For backward compatibility, when attribute 'type' is missing the 'isa-serial' will be chose as before. Libvirt XML sample <serial type='pty'> <target type='usb-serial' port='0'/> <address type='usb' bus='0' port='1'/> </serial> qemu commandline: qemu ${other_vm_args} \ -chardev pty,id=charserial0 \ -device usb-serial,chardev=charserial0,id=serial0,bus=usb.0,port=1 --- docs/formatdomain.html.in | 9 ++++++++- docs/schemas/domaincommon.rng | 20 +++++++++++++++++--- src/conf/domain_conf.c | 42 +++++++++++++++++++++++++++++++++++++++--- src/conf/domain_conf.h | 10 ++++++++++ src/libvirt_private.syms | 2 ++ src/qemu/qemu_command.c | 24 ++++++++++++++++++++++-- 6 files changed, 98 insertions(+), 9 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 94df6f8..7241bb9 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3677,7 +3677,14 @@ qemu-kvm -net nic,model=? /dev/null <p> <code>target</code> can have a <code>port</code> attribute, which specifies the port number. Ports are numbered starting from 0. There are - usually 0, 1 or 2 serial ports. + usually 0, 1 or 2 serial ports. There is also an optional + <code>type</code> attribute <span class="since">Since 1.0.2</span> + which has two choices for its value, one is< code>isa-serial</code>, + the other is <code>usb-serial</code>. If <code>type</code> is missing, + <code>isa-serial</code> will be used by default. For <code>usb-serial</code> + an optional sub-element <code><address></code> with + <code>type='usb'</code>which can tie the device to a particular controller, + <a href="#elementsAddress">documentedabove</a>. </p> <h6><a name="elementCharConsole">Console</a></h6> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 0529d62..7b7a2f9 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2453,12 +2453,26 @@ </attribute> </define> + <define name='qemucdevSerialTgtType'> + <attribute name='type'> + <choice> + <value>isa-serial</value> + <value>usb-serial</value> + </choice> + </attribute> + </define> + <define name="qemucdevTgtDef"> <element name="target"> <interleave> - <optional> - <ref name="qemucdevConsoleTgtType"/> - </optional> + <choice> + <optional> + <ref name="qemucdevConsoleTgtType"/> + </optional> + <optional> + <ref name="qemucdevSerialTgtType"/> + </optional> + </choice> <optional> <attribute name="port"/> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 79af087..9ece160 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -336,6 +336,11 @@ VIR_ENUM_IMPL(virDomainNetInterfaceLinkState, VIR_DOMAIN_NET_INTERFACE_LINK_STAT "up", "down") +VIR_ENUM_IMPL(virDomainChrSerialTarget, + VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST, + "isa-serial", + "usb-serial") + VIR_ENUM_IMPL(virDomainChrChannelTarget, VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST, "none", @@ -5445,6 +5450,9 @@ virDomainChrDefaultTargetType(virCapsPtr caps, break; case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL: + target = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA; + break; + case VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL: default: /* No target type yet*/ @@ -5457,7 +5465,8 @@ virDomainChrDefaultTargetType(virCapsPtr caps, static int virDomainChrTargetTypeFromString(virCapsPtr caps, - virDomainDefPtr def, + virDomainDefPtr vmdef, + virDomainChrDefPtr def, int devtype, const char *targetType) { @@ -5465,7 +5474,7 @@ virDomainChrTargetTypeFromString(virCapsPtr caps, int target = 0; if (!targetType) { - target = virDomainChrDefaultTargetType(caps, def, devtype); + target = virDomainChrDefaultTargetType(caps, vmdef, devtype); goto out; } @@ -5479,12 +5488,17 @@ virDomainChrTargetTypeFromString(virCapsPtr caps, break; case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL: + target = virDomainChrSerialTargetTypeFromString(targetType); + break; + case VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL: default: /* No target type yet*/ break; } + def->targetType_attr = true; + out: ret = target; return ret; @@ -5503,7 +5517,7 @@ virDomainChrDefParseTargetXML(virCapsPtr caps, const char *portStr = NULL; if ((def->targetType = - virDomainChrTargetTypeFromString(caps, vmdef, + virDomainChrTargetTypeFromString(caps, vmdef, def, def->deviceType, targetType)) < 0) { virReportError(VIR_ERR_XML_ERROR, _("unknown target type '%s' specified for character device"), @@ -5936,6 +5950,15 @@ virDomainChrDefParseXML(virCapsPtr caps, if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) goto error; + if (def->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && + def->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB && + def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("usb-serial requires address of usb type")); + goto error; + } + cleanup: VIR_FREE(type); @@ -7939,6 +7962,9 @@ virDomainChrTargetTypeToString(int deviceType, case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE: type = virDomainChrConsoleTargetTypeToString(targetType); break; + case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL: + type = virDomainChrSerialTargetTypeToString(targetType); + break; default: break; } @@ -13088,6 +13114,15 @@ virDomainChrDefFormat(virBufferPtr buf, def->target.port); break; + case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL: + if (def->targetType_attr) { + virBufferAsprintf(buf, + " <target type='%s' port='%d'/>\n", + virDomainChrTargetTypeToString(def->deviceType, + def->targetType), + def->target.port); + break; + } default: virBufferAsprintf(buf, " <target port='%d'/>\n", def->target.port); @@ -14415,6 +14450,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, (n < def->nserials)) { memcpy(&console, def->serials[n], sizeof(console)); console.deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE; + console.targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; } else { memcpy(&console, def->consoles[n], sizeof(console)); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a975a63..0e2746b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -909,6 +909,13 @@ enum virDomainChrDeviceType { VIR_DOMAIN_CHR_DEVICE_TYPE_LAST }; +enum virDomainChrSerialTargetType { + VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA = 0, + VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB, + + VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST +}; + enum virDomainChrChannelTargetType { VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_NONE = 0, VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD, @@ -994,6 +1001,8 @@ struct _virDomainChrSourceDef { /* A complete character device, both host and domain views. */ struct _virDomainChrDef { int deviceType; + + bool targetType_attr; int targetType; union { int port; /* parallel, serial, console */ @@ -2250,6 +2259,7 @@ VIR_ENUM_DECL(virDomainNetInterfaceLinkState) VIR_ENUM_DECL(virDomainChrDevice) VIR_ENUM_DECL(virDomainChrChannelTarget) VIR_ENUM_DECL(virDomainChrConsoleTarget) +VIR_ENUM_DECL(virDomainChrSerialTarget) VIR_ENUM_DECL(virDomainSmartcard) VIR_ENUM_DECL(virDomainChr) VIR_ENUM_DECL(virDomainChrTcpProtocol) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 497d5d3..aefdf9c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -293,6 +293,8 @@ virDomainChrConsoleTargetTypeToString; virDomainChrDefForeach; virDomainChrDefFree; virDomainChrDefNew; +virDomainChrSerialTargetTypeFromString; +virDomainChrSerialTargetTypeToString; virDomainChrSourceDefCopy; virDomainChrSourceDefFree; virDomainChrSpicevmcTypeFromString; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8a3de09..879b943 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6975,10 +6975,30 @@ qemuBuildChrDeviceStr(virDomainChrDefPtr serial, if (qemuBuildDeviceAddressStr(&cmd, &serial->info, caps) < 0) goto error; } - } else - virBufferAsprintf(&cmd, "isa-serial,chardev=char%s,id=%s", + } else { + virBufferAsprintf(&cmd, "%s,chardev=char%s,id=%s", + virDomainChrSerialTargetTypeToString(serial->targetType), serial->info.alias, serial->info.alias); + if (serial->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB) { + if (!qemuCapsGet(caps, QEMU_CAPS_DEVICE_USB_SERIAL)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("usb-serial is not supported in this QEMU binary")); + goto error; + } + + if (serial->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + serial->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("usb-serial requires address of usb type")); + goto error; + } + + if (qemuBuildDeviceAddressStr(&cmd, &serial->info, caps) < 0) + goto error; + } + } + if (virBufferError(&cmd)) { virReportOOMError(); goto error; -- 1.7.11.4
On 01/02/2013 11:57 PM, Guannan Ren wrote:
Add an optional 'type' attribute to <target> element of serial port device. There are two choices for its value, 'isa-serial' and 'usb-serial'. For backward compatibility, when attribute 'type' is missing the 'isa-serial' will be chose as before.
s/chose/chosen/
Libvirt XML sample
<serial type='pty'> <target type='usb-serial' port='0'/> <address type='usb' bus='0' port='1'/> </serial>
qemu commandline:
qemu ${other_vm_args} \ -chardev pty,id=charserial0 \ -device usb-serial,chardev=charserial0,id=serial0,bus=usb.0,port=1
+++ b/docs/formatdomain.html.in @@ -3677,7 +3677,14 @@ qemu-kvm -net nic,model=? /dev/null <p> <code>target</code> can have a <code>port</code> attribute, which specifies the port number. Ports are numbered starting from 0. There are - usually 0, 1 or 2 serial ports. + usually 0, 1 or 2 serial ports. There is also an optional + <code>type</code> attribute <span class="since">Since 1.0.2</span>
This is mid-sentence, so s/Since/since/
+ which has two choices for its value, one is< code>isa-serial</code>, + the other is <code>usb-serial</code>. If <code>type</code> is missing, + <code>isa-serial</code> will be used by default. For <code>usb-serial</code> + an optional sub-element <code><address></code> with + <code>type='usb'</code>which can tie the device to a particular controller, + <a href="#elementsAddress">documentedabove</a>.
s/documentedabove/documented above/
@@ -994,6 +1001,8 @@ struct _virDomainChrSourceDef { /* A complete character device, both host and domain views. */ struct _virDomainChrDef { int deviceType; + + bool targetType_attr;
The naming looks funny, when you are mixing camelCase and underscore_split in the same name. Also, do we really need this boolean? How many test files would have to change if we change all existing outputs to always include type='isa-serial', where the field is optional only on input; vs. your approach of using this bool to omit the field on output if the user did not specify it on input? Overall it looks reasonable, but depending on the answer to whether we should just always output type='isa-serial', you may have a v2 to submit. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 01/05/2013 04:24 AM, Eric Blake wrote:
Overall it looks reasonable, but depending on the answer to whether we should just always output type='isa-serial', you may have a v2 to submit.
As attribute 'type' is optional, my initial thoughts is that If type='value' is given explicitly on input, then we print it out on output too, if no type is given on input, we don't print it out on output either. Backward compatibility and being more flexible. There are indeed a number of testcases we need to modify if we always output type='isa-serial', but not a big workload. Guannan
On 01/04/2013 10:25 PM, Guannan Ren wrote:
On 01/05/2013 04:24 AM, Eric Blake wrote:
Overall it looks reasonable, but depending on the answer to whether we should just always output type='isa-serial', you may have a v2 to submit.
As attribute 'type' is optional, my initial thoughts is that If type='value' is given explicitly on input, then we print it out on output too, if no type is given on input, we don't print it out on output either. Backward compatibility and being more flexible.
Backward compatibility says that we must never fail to parse an older XML that used to be valid (therefore, the new attribute must be optional on input, and have a sane default when omitted). But it does not require that we cannot output more information than was provided on input (for example, we compute and output <address> elements for many devices, even when not present on input). Older code already has to be prepared to ignore attributes and elements that get output by newer libvirt. If having that information makes the output more legible, then I think it is worth always outputting the information by default.
There are indeed a number of testcases we need to modify if we always output type='isa-serial', but not a big workload.
Then I think it's better to always output type='isa-serial' even when the input omitted it. Anyone else with an opinion? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Add an optional 'type' attribute to <target> element of serial port device. There are two choices for its value, 'isa-serial' and 'usb-serial'. For backward compatibility, when attribute 'type' is missing the 'isa-serial' will be chosen as before. Libvirt XML sample <serial type='pty'> <target type='usb-serial' port='0'/> <address type='usb' bus='0' port='1'/> </serial> qemu commandline: qemu ${other_vm_args} \ -chardev pty,id=charserial0 \ -device usb-serial,chardev=charserial0,id=serial0,bus=usb.0,port=1 --- docs/formatdomain.html.in | 9 ++++++++- docs/schemas/domaincommon.rng | 20 +++++++++++++++++--- src/conf/domain_conf.c | 42 +++++++++++++++++++++++++++++++++++++++--- src/conf/domain_conf.h | 10 ++++++++++ src/libvirt_private.syms | 2 ++ src/qemu/qemu_command.c | 24 ++++++++++++++++++++++-- 6 files changed, 98 insertions(+), 9 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 94df6f8..7811056 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3677,7 +3677,14 @@ qemu-kvm -net nic,model=? /dev/null <p> <code>target</code> can have a <code>port</code> attribute, which specifies the port number. Ports are numbered starting from 0. There are - usually 0, 1 or 2 serial ports. + usually 0, 1 or 2 serial ports. There is also an optional + <code>type</code> attribute <span class="since">since 1.0.2</span> + which has two choices for its value, one is< code>isa-serial</code>, + the other is <code>usb-serial</code>. If <code>type</code> is missing, + <code>isa-serial</code> will be used by default. For <code>usb-serial</code> + an optional sub-element <code><address></code> with + <code>type='usb'</code>which can tie the device to a particular controller, + <a href="#elementsAddress">documented above</a>. </p> <h6><a name="elementCharConsole">Console</a></h6> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 0529d62..7b7a2f9 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2453,12 +2453,26 @@ </attribute> </define> + <define name='qemucdevSerialTgtType'> + <attribute name='type'> + <choice> + <value>isa-serial</value> + <value>usb-serial</value> + </choice> + </attribute> + </define> + <define name="qemucdevTgtDef"> <element name="target"> <interleave> - <optional> - <ref name="qemucdevConsoleTgtType"/> - </optional> + <choice> + <optional> + <ref name="qemucdevConsoleTgtType"/> + </optional> + <optional> + <ref name="qemucdevSerialTgtType"/> + </optional> + </choice> <optional> <attribute name="port"/> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 79af087..ccc7931 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -336,6 +336,11 @@ VIR_ENUM_IMPL(virDomainNetInterfaceLinkState, VIR_DOMAIN_NET_INTERFACE_LINK_STAT "up", "down") +VIR_ENUM_IMPL(virDomainChrSerialTarget, + VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST, + "isa-serial", + "usb-serial") + VIR_ENUM_IMPL(virDomainChrChannelTarget, VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST, "none", @@ -5445,6 +5450,9 @@ virDomainChrDefaultTargetType(virCapsPtr caps, break; case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL: + target = VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA; + break; + case VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL: default: /* No target type yet*/ @@ -5457,7 +5465,8 @@ virDomainChrDefaultTargetType(virCapsPtr caps, static int virDomainChrTargetTypeFromString(virCapsPtr caps, - virDomainDefPtr def, + virDomainDefPtr vmdef, + virDomainChrDefPtr def, int devtype, const char *targetType) { @@ -5465,7 +5474,7 @@ virDomainChrTargetTypeFromString(virCapsPtr caps, int target = 0; if (!targetType) { - target = virDomainChrDefaultTargetType(caps, def, devtype); + target = virDomainChrDefaultTargetType(caps, vmdef, devtype); goto out; } @@ -5479,12 +5488,17 @@ virDomainChrTargetTypeFromString(virCapsPtr caps, break; case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL: + target = virDomainChrSerialTargetTypeFromString(targetType); + break; + case VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL: default: /* No target type yet*/ break; } + def->targetTypeAttr = true; + out: ret = target; return ret; @@ -5503,7 +5517,7 @@ virDomainChrDefParseTargetXML(virCapsPtr caps, const char *portStr = NULL; if ((def->targetType = - virDomainChrTargetTypeFromString(caps, vmdef, + virDomainChrTargetTypeFromString(caps, vmdef, def, def->deviceType, targetType)) < 0) { virReportError(VIR_ERR_XML_ERROR, _("unknown target type '%s' specified for character device"), @@ -5936,6 +5950,15 @@ virDomainChrDefParseXML(virCapsPtr caps, if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) goto error; + if (def->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && + def->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB && + def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("usb-serial requires address of usb type")); + goto error; + } + cleanup: VIR_FREE(type); @@ -7939,6 +7962,9 @@ virDomainChrTargetTypeToString(int deviceType, case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE: type = virDomainChrConsoleTargetTypeToString(targetType); break; + case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL: + type = virDomainChrSerialTargetTypeToString(targetType); + break; default: break; } @@ -13088,6 +13114,15 @@ virDomainChrDefFormat(virBufferPtr buf, def->target.port); break; + case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL: + if (def->targetTypeAttr) { + virBufferAsprintf(buf, + " <target type='%s' port='%d'/>\n", + virDomainChrTargetTypeToString(def->deviceType, + def->targetType), + def->target.port); + break; + } default: virBufferAsprintf(buf, " <target port='%d'/>\n", def->target.port); @@ -14415,6 +14450,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, (n < def->nserials)) { memcpy(&console, def->serials[n], sizeof(console)); console.deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE; + console.targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; } else { memcpy(&console, def->consoles[n], sizeof(console)); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a975a63..93d2265 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -909,6 +909,13 @@ enum virDomainChrDeviceType { VIR_DOMAIN_CHR_DEVICE_TYPE_LAST }; +enum virDomainChrSerialTargetType { + VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_ISA = 0, + VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB, + + VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_LAST +}; + enum virDomainChrChannelTargetType { VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_NONE = 0, VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD, @@ -994,6 +1001,8 @@ struct _virDomainChrSourceDef { /* A complete character device, both host and domain views. */ struct _virDomainChrDef { int deviceType; + + bool targetTypeAttr; int targetType; union { int port; /* parallel, serial, console */ @@ -2250,6 +2259,7 @@ VIR_ENUM_DECL(virDomainNetInterfaceLinkState) VIR_ENUM_DECL(virDomainChrDevice) VIR_ENUM_DECL(virDomainChrChannelTarget) VIR_ENUM_DECL(virDomainChrConsoleTarget) +VIR_ENUM_DECL(virDomainChrSerialTarget) VIR_ENUM_DECL(virDomainSmartcard) VIR_ENUM_DECL(virDomainChr) VIR_ENUM_DECL(virDomainChrTcpProtocol) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5636661..6cd91d0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -293,6 +293,8 @@ virDomainChrConsoleTargetTypeToString; virDomainChrDefForeach; virDomainChrDefFree; virDomainChrDefNew; +virDomainChrSerialTargetTypeFromString; +virDomainChrSerialTargetTypeToString; virDomainChrSourceDefCopy; virDomainChrSourceDefFree; virDomainChrSpicevmcTypeFromString; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8a3de09..879b943 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6975,10 +6975,30 @@ qemuBuildChrDeviceStr(virDomainChrDefPtr serial, if (qemuBuildDeviceAddressStr(&cmd, &serial->info, caps) < 0) goto error; } - } else - virBufferAsprintf(&cmd, "isa-serial,chardev=char%s,id=%s", + } else { + virBufferAsprintf(&cmd, "%s,chardev=char%s,id=%s", + virDomainChrSerialTargetTypeToString(serial->targetType), serial->info.alias, serial->info.alias); + if (serial->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB) { + if (!qemuCapsGet(caps, QEMU_CAPS_DEVICE_USB_SERIAL)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("usb-serial is not supported in this QEMU binary")); + goto error; + } + + if (serial->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + serial->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("usb-serial requires address of usb type")); + goto error; + } + + if (qemuBuildDeviceAddressStr(&cmd, &serial->info, caps) < 0) + goto error; + } + } + if (virBufferError(&cmd)) { virReportOOMError(); goto error; -- 1.7.11.4
On 01/04/2013 10:25 PM, Guannan Ren wrote:
Add an optional 'type' attribute to <target> element of serial port device. There are two choices for its value, 'isa-serial' and 'usb-serial'. For backward compatibility, when attribute 'type' is missing the 'isa-serial' will be chosen as before.
Libvirt XML sample
<serial type='pty'> <target type='usb-serial' port='0'/> <address type='usb' bus='0' port='1'/> </serial>
qemu commandline:
qemu ${other_vm_args} \ -chardev pty,id=charserial0 \ -device usb-serial,chardev=charserial0,id=serial0,bus=usb.0,port=1
+++ b/docs/formatdomain.html.in @@ -3677,7 +3677,14 @@ qemu-kvm -net nic,model=? /dev/null <p> <code>target</code> can have a <code>port</code> attribute, which specifies the port number. Ports are numbered starting from 0. There are - usually 0, 1 or 2 serial ports. + usually 0, 1 or 2 serial ports. There is also an optional + <code>type</code> attribute <span class="since">since 1.0.2</span> + which has two choices for its value, one is< code>isa-serial</code>, + the other is <code>usb-serial</code>. If <code>type</code> is missing, + <code>isa-serial</code> will be used by default. For <code>usb-serial</code> + an optional sub-element <code><address></code> with + <code>type='usb'</code>which can tie the device to a particular controller,
This renders as: sub-element <address> with type='usb'which can tie You are missing a space, and it also sounds funny, so I suggest: s/which / /
@@ -994,6 +1001,8 @@ struct _virDomainChrSourceDef { /* A complete character device, both host and domain views. */ struct _virDomainChrDef { int deviceType; + + bool targetTypeAttr;
We still haven't answered the question of whether it is better to update all tests to add an output of explicit type='isa-serial' even when omitted on input (and this bool is not needed), or whether your approach of doing output only when present on input makes more sense. If the former, then we need a v3 that touches all affected tests; if the latter, then ACK to this patch with the doc fix. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 01/08/2013 07:12 AM, Eric Blake wrote:
This renders as:
sub-element <address> with type='usb'which can tie
You are missing a space, and it also sounds funny, so I suggest:
s/which / /
fixed.
@@ -994,6 +1001,8 @@ struct _virDomainChrSourceDef { /* A complete character device, both host and domain views. */ struct _virDomainChrDef { int deviceType; + + bool targetTypeAttr; We still haven't answered the question of whether it is better to update all tests to add an output of explicit type='isa-serial' even when omitted on input (and this bool is not needed), or whether your approach of doing output only when present on input makes more sense. If the former, then we need a v3 that touches all affected tests; if the latter, then ACK to this patch with the doc fix.
I prefer the latter without no third opinion. Thanks for the review. Pushed.
participants (2)
-
Eric Blake -
Guannan Ren