[libvirt] [PATCHv5 6/9] spicevmc: support new qemu chardev

From: Daniel P. Berrange <berrange@redhat.com> Inspired by https://bugzilla.redhat.com/show_bug.cgi?id=615757 Add a new character device backend for virtio serial channels that activates the QEMU spice agent on the main channel using the vdagent spicevmc connection. The <target> must be type='virtio', and supports an optional name that specifies how the guest will see the channel (for now, name must be com.redhat.spice.0). <channel type='spicevmc'> <target type='virtio'/> <address type='virtio-serial' controller='1' bus='0' port='3'/> </channel> * docs/schemas/domain.rng: Support new XML. * docs/formatdomain.html.in: Document it. * src/conf/domain_conf.h (virDomainChrType): New enum value. * src/conf/domain_conf.c (virDomainChr): Add spicevmc. (virDomainChrDefParseXML, virDomainChrSourceDefParseXML) (virDomainChrDefParseTargetXML): Parse and enforce proper use. (virDomainChrSourceDefFormat, virDomainChrDefFormat): Format. * src/qemu/qemu_command.c (qemuBuildChrChardevStr) (qemuBuildCommandLine): Add qemu support. * tests/qemuxml2argvtest.c (domain): New test. * tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml: New file. * tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args: Likewise. Signed-off-by: Eric Blake <eblake@redhat.com> --- Notes: v5: new patch, but heavily ported from RHEL 6.0 docs/formatdomain.html.in | 17 +++++++ docs/schemas/domain.rng | 3 +- src/conf/domain_conf.c | 18 +++++++- src/conf/domain_conf.h | 2 + src/qemu/qemu_command.c | 47 ++++++++++++++++--- .../qemuxml2argv-channel-spicevmc.args | 9 ++++ .../qemuxml2argv-channel-spicevmc.xml | 34 ++++++++++++++ tests/qemuxml2argvtest.c | 3 + 8 files changed, 123 insertions(+), 10 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 9f1bb5f..d91fdb9 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1734,6 +1734,9 @@ qemu-kvm -net nic,model=? /dev/null <channel type='pty'> <target type='virtio' name='arbitrary.virtio.serial.port.name'/> </channel> + <channel type='spicevmc'> + <target type='virtio' name='com.redhat.spice.0'/> + </channel> </devices> ...</pre> @@ -1759,6 +1762,20 @@ qemu-kvm -net nic,model=? /dev/null optional element <code>address</code> can tie the channel to a particular <code>type='virtio-serial'</code> controller. <span class="since">Since 0.7.7</span></dd> + + <dt><code>spicevmc</code></dt> + <dd>Paravirtualized SPICE channel. The domain must also have a + SPICE server as a <a href="#elementsGraphics">graphics + device</a>, at which point the host piggy-backs messages + across the <code>main</code> channel. The <code>target</code> + element must be present, with + attribute <code>type='virtio'</code>; an optional + attribute <code>name</code> controls how the guest will have + access to the channel, and defaults + to <code>name='com.redhat.spice.0'</code>. The + optional <code>address</code> element can tie the channel to a + particular <code>type='virtio-serial'</code> controller. + <span class="since">Since 0.8.8</span></dd> </dl> <h5><a name="elementsCharHostInterface">Host interface</a></h5> diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 69fb432..9ffcf21 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -1475,6 +1475,7 @@ <value>stdio</value> <value>vc</value> <value>pty</value> + <value>spicevmc</value> </choice> </attribute> </define> @@ -1611,7 +1612,7 @@ <define name="virtioTarget"> <element name="target"> <attribute name="type"> - <value>virtio</value> + <value>virtio</value> </attribute> <optional> <attribute name="name"/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b1c775b..9b4ef8d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -226,7 +226,8 @@ VIR_ENUM_IMPL(virDomainChr, VIR_DOMAIN_CHR_TYPE_LAST, "stdio", "udp", "tcp", - "unix") + "unix", + "spicevmc") VIR_ENUM_IMPL(virDomainChrTcpProtocol, VIR_DOMAIN_CHR_TCP_PROTOCOL_LAST, "raw", @@ -3065,6 +3066,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, break; case VIR_DOMAIN_CHR_TYPE_STDIO: + case VIR_DOMAIN_CHR_TYPE_SPICEVMC: /* Nada */ break; @@ -3254,6 +3256,13 @@ virDomainChrDefParseXML(virCapsPtr caps, } } + if (def->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC && + def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO) { + virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spicevmc device type only supports virtio")); + goto error; + } + if (virDomainDeviceInfoParseXML(node, &def->info, flags) < 0) goto error; @@ -3361,6 +3370,12 @@ virDomainSmartcardDefParseXML(xmlNodePtr node, cur = node->children; if (virDomainChrSourceDefParseXML(&def->data.passthru, cur) < 0) goto error; + + if (def->data.passthru.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) { + virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("smartcard spicevmc device not supported")); + goto error; + } break; default: @@ -6843,6 +6858,7 @@ virDomainChrSourceDefFormat(virBufferPtr buf, case VIR_DOMAIN_CHR_TYPE_NULL: case VIR_DOMAIN_CHR_TYPE_VC: case VIR_DOMAIN_CHR_TYPE_STDIO: + case VIR_DOMAIN_CHR_TYPE_SPICEVMC: /* nada */ break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 81c3f38..9dff580 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -413,6 +413,7 @@ enum virDomainChrType { VIR_DOMAIN_CHR_TYPE_UDP, VIR_DOMAIN_CHR_TYPE_TCP, VIR_DOMAIN_CHR_TYPE_UNIX, + VIR_DOMAIN_CHR_TYPE_SPICEVMC, VIR_DOMAIN_CHR_TYPE_LAST, }; @@ -432,6 +433,7 @@ typedef virDomainChrSourceDef *virDomainChrSourceDefPtr; struct _virDomainChrSourceDef { int type; /* virDomainChrType */ union { + /* no <source> for null, vc, stdio, spicevmc */ struct { char *path; } file; /* pty, file, pipe, or device */ diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 40d92f4..1903c70 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2006,7 +2006,8 @@ qemuBuildUSBHostdevUsbDevStr(virDomainHostdevDefPtr dev) /* This function outputs a -chardev command line option which describes only the * host side of the character device */ static char * -qemuBuildChrChardevStr(virDomainChrSourceDefPtr dev, const char *alias) +qemuBuildChrChardevStr(virDomainChrSourceDefPtr dev, const char *alias, + unsigned long long qemuCmdFlags) { virBuffer buf = VIR_BUFFER_INITIALIZER; bool telnet; @@ -2072,6 +2073,21 @@ qemuBuildChrChardevStr(virDomainChrSourceDefPtr dev, const char *alias) dev->data.nix.path, dev->data.nix.listen ? ",server,nowait" : ""); break; + + case VIR_DOMAIN_CHR_TYPE_SPICEVMC: + if (!(qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV_SPICEVMC)) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("spicevmc not supported in this QEMU binary")); + goto error; + } + virBufferVSprintf(&buf, "spicevmc,id=char%s,name=vdagent", alias); + break; + + default: + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported chardev '%s'"), + virDomainChrTypeToString(dev->type)); + goto error; } if (virBufferError(&buf)) { @@ -2196,6 +2212,14 @@ qemuBuildVirtioSerialPortDevStr(virDomainChrDefPtr dev) virBufferVSprintf(&buf, ",chardev=char%s,id=%s", dev->info.alias, dev->info.alias); + if (dev->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC && + dev->target.name && + STRNEQ(dev->target.name, "com.redhat.spice.0")) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unsupported spicevmc target name '%s'"), + dev->target.name); + goto error; + } if (dev->target.name) { virBufferVSprintf(&buf, ",name=%s", dev->target.name); } @@ -2825,7 +2849,8 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuCmdFlags & QEMUD_CMD_FLAG_CHARDEV) { virCommandAddArg(cmd, "-chardev"); - if (!(chrdev = qemuBuildChrChardevStr(monitor_chr, "monitor"))) + if (!(chrdev = qemuBuildChrChardevStr(monitor_chr, "monitor", + qemuCmdFlags))) goto error; virCommandAddArg(cmd, chrdev); VIR_FREE(chrdev); @@ -3523,7 +3548,8 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(&smartcard->data.passthru, - smartcard->info.alias))) { + smartcard->info.alias, + qemuCmdFlags))) { virBufferFreeAndReset(&opt); goto error; } @@ -3560,7 +3586,8 @@ qemuBuildCommandLine(virConnectPtr conn, (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(&serial->source, - serial->info.alias))) + serial->info.alias, + qemuCmdFlags))) goto error; virCommandAddArg(cmd, devstr); VIR_FREE(devstr); @@ -3592,7 +3619,8 @@ qemuBuildCommandLine(virConnectPtr conn, (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(¶llel->source, - parallel->info.alias))) + parallel->info.alias, + qemuCmdFlags))) goto error; virCommandAddArg(cmd, devstr); VIR_FREE(devstr); @@ -3626,7 +3654,8 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(&channel->source, - channel->info.alias))) + channel->info.alias, + qemuCmdFlags))) goto error; virCommandAddArg(cmd, devstr); VIR_FREE(devstr); @@ -3653,7 +3682,8 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(&channel->source, - channel->info.alias))) + channel->info.alias, + qemuCmdFlags))) goto error; virCommandAddArg(cmd, devstr); VIR_FREE(devstr); @@ -3682,7 +3712,8 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, "-chardev"); if (!(devstr = qemuBuildChrChardevStr(&console->source, - console->info.alias))) + console->info.alias, + qemuCmdFlags))) goto error; virCommandAddArg(cmd, devstr); VIR_FREE(devstr); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args new file mode 100644 index 0000000..681f7c2 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args @@ -0,0 +1,9 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=spice \ +/usr/bin/qemu -S -M pc -m 214 -smp 1 -nodefconfig -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -device \ +virtio-serial-pci,id=virtio-serial1,bus=pci.0,addr=0xa -hda \ +/dev/HostVG/QEMUGuest1 -chardev spicevmc,id=charchannel0,name=vdagent -device \ +virtserialport,bus=virtio-serial1.0,nr=3,chardev=charchannel0,id=channel0\ +,name=com.redhat.spice.0 -usb -spice port=5903,tls-port=5904,addr=127.0.0.1,\ +x509-dir=/etc/pki/libvirt-spice,tls-channel=main -device \ +virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml new file mode 100644 index 0000000..0e82394 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml @@ -0,0 +1,34 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219136</memory> + <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='1'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/> + </controller> + <graphics type='spice' port='5903' tlsPort='5904' autoport='no' listen='127.0.0.1'> + <channel name='main' mode='secure'/> + </graphics> + <channel type='spicevmc'> + <target type='virtio' name='com.redhat.spice.0'/> + <address type='virtio-serial' controller='1' bus='0' port='3'/> + </channel> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 0b4bfeb..0726130 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -407,6 +407,9 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_NODEFCONFIG, false); DO_TEST("console-virtio", QEMUD_CMD_FLAG_DEVICE | QEMUD_CMD_FLAG_NODEFCONFIG, false); + DO_TEST("channel-spicevmc", QEMUD_CMD_FLAG_DEVICE | + QEMUD_CMD_FLAG_NODEFCONFIG | QEMUD_CMD_FLAG_SPICE | + QEMUD_CMD_FLAG_CHARDEV_SPICEVMC, false); DO_TEST("smartcard-host", QEMUD_CMD_FLAG_CHARDEV | QEMUD_CMD_FLAG_DEVICE | -- 1.7.4

Not sure this makes sense to do; posting it for comments, but I'm okay with ditching the idea (especially since existing RHEL 6.0 guests have already been using XML that targets virtio, which means changing to vdagent would cause the need for a naming transition). * docs/schemas/domain.rng: Tweak the spicevmc XML. * docs/formatdomain.html.in: Document it. * src/conf/domain_conf.h (virDomainChrChannelTargetType): New enum value. * src/conf/domain_conf.c (virDomainChrChannelTarget): Add vdagent. (virDomainChrDefParseXML, virDomainChrSourceDefParseXML) (virDomainChrDefParseTargetXML): Parse and enforce proper use. (virDomainChrSourceDefFormat, virDomainChrDefFormat): Format. * src/qemu/qemu_command.c (qemuBuildChrChardevStr) (qemuBuildCommandLine): Add qemu support. * tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml: Tweak. --- docs/formatdomain.html.in | 12 +++++----- docs/schemas/domain.rng | 13 ++++++++++++ src/conf/domain_conf.c | 20 ++++++++++++++++-- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 21 ++++++++++++++++++++ .../qemuxml2argv-channel-spicevmc.args | 4 +- .../qemuxml2argv-channel-spicevmc.xml | 2 +- 7 files changed, 61 insertions(+), 12 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index d91fdb9..a9d2f84 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1735,7 +1735,7 @@ qemu-kvm -net nic,model=? /dev/null <target type='virtio' name='arbitrary.virtio.serial.port.name'/> </channel> <channel type='spicevmc'> - <target type='virtio' name='com.redhat.spice.0'/> + <target type='vdagent' name='com.redhat.spice.0'/> </channel> </devices> ...</pre> @@ -1767,11 +1767,11 @@ qemu-kvm -net nic,model=? /dev/null <dd>Paravirtualized SPICE channel. The domain must also have a SPICE server as a <a href="#elementsGraphics">graphics device</a>, at which point the host piggy-backs messages - across the <code>main</code> channel. The <code>target</code> - element must be present, with - attribute <code>type='virtio'</code>; an optional - attribute <code>name</code> controls how the guest will have - access to the channel, and defaults + across the <code>main</code> channel. For now, the only + supported channel is vdagent, so the <code>target</code> + element must have attribute <code>type='vdagent'</code>; an + optional attribute <code>name</code> controls how the guest + will have access to the channel, and defaults to <code>name='com.redhat.spice.0'</code>. The optional <code>address</code> element can tie the channel to a particular <code>type='virtio-serial'</code> controller. diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 9ffcf21..40f33e8 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -1619,6 +1619,18 @@ </optional> </element> </define> + <define name="vdagentTarget"> + <element name="target"> + <attribute name="type"> + <value>vdagent</value> + </attribute> + <optional> + <attribute name="name"> + <value>com.redhat.spice.0</value> + </attribute> + </optional> + </element> + </define> <define name="channel"> <element name="channel"> <ref name="qemucdevSrcType"/> @@ -1627,6 +1639,7 @@ <choice> <ref name="guestfwdTarget"/> <ref name="virtioTarget"/> + <ref name="vdagentTarget"/> </choice> <optional> <ref name="address"/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9b4ef8d..3d7d63e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -201,7 +201,8 @@ VIR_ENUM_IMPL(virDomainNetBackend, VIR_DOMAIN_NET_BACKEND_TYPE_LAST, VIR_ENUM_IMPL(virDomainChrChannelTarget, VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST, "guestfwd", - "virtio") + "virtio", + "vdagent") VIR_ENUM_IMPL(virDomainChrConsoleTarget, VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST, @@ -2944,6 +2945,16 @@ virDomainChrDefParseTargetXML(virCapsPtr caps, case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: def->target.name = virXMLPropString(cur, "name"); break; + + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VDAGENT: + if (def->source.type != VIR_DOMAIN_CHR_TYPE_SPICEVMC) { + virDomainReportError(VIR_ERR_XML_ERROR, "%s", + _("vdagent only valid with spicevmc " + "channel")); + goto error; + } + + break; } break; @@ -3257,9 +3268,9 @@ virDomainChrDefParseXML(virCapsPtr caps, } if (def->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC && - def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO) { + def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VDAGENT) { virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spicevmc device type only supports virtio")); + _("spicevmc device type only supports vdagent")); goto error; } @@ -6993,6 +7004,9 @@ virDomainChrDefFormat(virBufferPtr buf, break; } + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VDAGENT: + break; + } virBufferAddLit(buf, "/>\n"); break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 9dff580..0728dc3 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -389,6 +389,7 @@ enum virDomainChrDeviceType { enum virDomainChrChannelTargetType { VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD = 0, VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO, + VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VDAGENT, VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST, }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1903c70..ba60d0a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3694,6 +3694,27 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, devstr); VIR_FREE(devstr); break; + + case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VDAGENT: + if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("virtio channel requires QEMU to support -device")); + goto error; + } + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(&channel->source, + channel->info.alias, + qemuCmdFlags))) + goto error; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + + virCommandAddArg(cmd, "-device"); + if (!(devstr = qemuBuildVirtioSerialPortDevStr(channel))) + goto error; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + break; } } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args index 681f7c2..3e1eaed 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args @@ -3,7 +3,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=spice \ -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -device \ virtio-serial-pci,id=virtio-serial1,bus=pci.0,addr=0xa -hda \ /dev/HostVG/QEMUGuest1 -chardev spicevmc,id=charchannel0,name=vdagent -device \ -virtserialport,bus=virtio-serial1.0,nr=3,chardev=charchannel0,id=channel0\ -,name=com.redhat.spice.0 -usb -spice port=5903,tls-port=5904,addr=127.0.0.1,\ +virtserialport,bus=virtio-serial1.0,nr=3,chardev=charchannel0,id=channel0 \ +-usb -spice port=5903,tls-port=5904,addr=127.0.0.1,\ x509-dir=/etc/pki/libvirt-spice,tls-channel=main -device \ virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml index 0e82394..b99f3ce 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml @@ -26,7 +26,7 @@ <channel name='main' mode='secure'/> </graphics> <channel type='spicevmc'> - <target type='virtio' name='com.redhat.spice.0'/> + <target type='vdagent'/> <address type='virtio-serial' controller='1' bus='0' port='3'/> </channel> <memballoon model='virtio'/> -- 1.7.4

On Thu, Feb 03, 2011 at 09:16:09PM -0700, Eric Blake wrote:
Not sure this makes sense to do; posting it for comments, but I'm okay with ditching the idea (especially since existing RHEL 6.0 guests have already been using XML that targets virtio, which means changing to vdagent would cause the need for a naming transition).
This patch doesn't fit the modelling.
index 0e82394..b99f3ce 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml @@ -26,7 +26,7 @@ <channel name='main' mode='secure'/> </graphics> <channel type='spicevmc'> - <target type='virtio' name='com.redhat.spice.0'/> + <target type='vdagent'/> <address type='virtio-serial' controller='1' bus='0' port='3'/> </channel>
In <target> 'type' is refering to the guest hardware type, which is always 'virtio'. 'vdgent' refers to a service running over the device, not the device type. The '@name' attribute is used to identify services, in this particular case 'com.redhat.com.spice.0' refers to the vdagent service type. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

This reverts commit e73f3dfd7ec15b635c2c97f8d57957f5aaff3e74. --- Nothing to see here... docs/formatdomain.html.in | 12 +++++----- docs/schemas/domain.rng | 13 ------------ src/conf/domain_conf.c | 20 ++---------------- src/conf/domain_conf.h | 1 - src/qemu/qemu_command.c | 21 -------------------- .../qemuxml2argv-channel-spicevmc.args | 4 +- .../qemuxml2argv-channel-spicevmc.xml | 2 +- 7 files changed, 12 insertions(+), 61 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index a9d2f84..d91fdb9 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1735,7 +1735,7 @@ qemu-kvm -net nic,model=? /dev/null <target type='virtio' name='arbitrary.virtio.serial.port.name'/> </channel> <channel type='spicevmc'> - <target type='vdagent' name='com.redhat.spice.0'/> + <target type='virtio' name='com.redhat.spice.0'/> </channel> </devices> ...</pre> @@ -1767,11 +1767,11 @@ qemu-kvm -net nic,model=? /dev/null <dd>Paravirtualized SPICE channel. The domain must also have a SPICE server as a <a href="#elementsGraphics">graphics device</a>, at which point the host piggy-backs messages - across the <code>main</code> channel. For now, the only - supported channel is vdagent, so the <code>target</code> - element must have attribute <code>type='vdagent'</code>; an - optional attribute <code>name</code> controls how the guest - will have access to the channel, and defaults + across the <code>main</code> channel. The <code>target</code> + element must be present, with + attribute <code>type='virtio'</code>; an optional + attribute <code>name</code> controls how the guest will have + access to the channel, and defaults to <code>name='com.redhat.spice.0'</code>. The optional <code>address</code> element can tie the channel to a particular <code>type='virtio-serial'</code> controller. diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 40f33e8..9ffcf21 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -1619,18 +1619,6 @@ </optional> </element> </define> - <define name="vdagentTarget"> - <element name="target"> - <attribute name="type"> - <value>vdagent</value> - </attribute> - <optional> - <attribute name="name"> - <value>com.redhat.spice.0</value> - </attribute> - </optional> - </element> - </define> <define name="channel"> <element name="channel"> <ref name="qemucdevSrcType"/> @@ -1639,7 +1627,6 @@ <choice> <ref name="guestfwdTarget"/> <ref name="virtioTarget"/> - <ref name="vdagentTarget"/> </choice> <optional> <ref name="address"/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3d7d63e..9b4ef8d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -201,8 +201,7 @@ VIR_ENUM_IMPL(virDomainNetBackend, VIR_DOMAIN_NET_BACKEND_TYPE_LAST, VIR_ENUM_IMPL(virDomainChrChannelTarget, VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST, "guestfwd", - "virtio", - "vdagent") + "virtio") VIR_ENUM_IMPL(virDomainChrConsoleTarget, VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_LAST, @@ -2945,16 +2944,6 @@ virDomainChrDefParseTargetXML(virCapsPtr caps, case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: def->target.name = virXMLPropString(cur, "name"); break; - - case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VDAGENT: - if (def->source.type != VIR_DOMAIN_CHR_TYPE_SPICEVMC) { - virDomainReportError(VIR_ERR_XML_ERROR, "%s", - _("vdagent only valid with spicevmc " - "channel")); - goto error; - } - - break; } break; @@ -3268,9 +3257,9 @@ virDomainChrDefParseXML(virCapsPtr caps, } if (def->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC && - def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VDAGENT) { + def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO) { virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spicevmc device type only supports vdagent")); + _("spicevmc device type only supports virtio")); goto error; } @@ -7004,9 +6993,6 @@ virDomainChrDefFormat(virBufferPtr buf, break; } - case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VDAGENT: - break; - } virBufferAddLit(buf, "/>\n"); break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0728dc3..9dff580 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -389,7 +389,6 @@ enum virDomainChrDeviceType { enum virDomainChrChannelTargetType { VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD = 0, VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO, - VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VDAGENT, VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_LAST, }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ba60d0a..1903c70 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3694,27 +3694,6 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, devstr); VIR_FREE(devstr); break; - - case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VDAGENT: - if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { - qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("virtio channel requires QEMU to support -device")); - goto error; - } - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&channel->source, - channel->info.alias, - qemuCmdFlags))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - - virCommandAddArg(cmd, "-device"); - if (!(devstr = qemuBuildVirtioSerialPortDevStr(channel))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); - break; } } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args index 3e1eaed..681f7c2 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args @@ -3,7 +3,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=spice \ -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -device \ virtio-serial-pci,id=virtio-serial1,bus=pci.0,addr=0xa -hda \ /dev/HostVG/QEMUGuest1 -chardev spicevmc,id=charchannel0,name=vdagent -device \ -virtserialport,bus=virtio-serial1.0,nr=3,chardev=charchannel0,id=channel0 \ --usb -spice port=5903,tls-port=5904,addr=127.0.0.1,\ +virtserialport,bus=virtio-serial1.0,nr=3,chardev=charchannel0,id=channel0\ +,name=com.redhat.spice.0 -usb -spice port=5903,tls-port=5904,addr=127.0.0.1,\ x509-dir=/etc/pki/libvirt-spice,tls-channel=main -device \ virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml index b99f3ce..0e82394 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml @@ -26,7 +26,7 @@ <channel name='main' mode='secure'/> </graphics> <channel type='spicevmc'> - <target type='vdagent'/> + <target type='virtio' name='com.redhat.spice.0'/> <address type='virtio-serial' controller='1' bus='0' port='3'/> </channel> <memballoon model='virtio'/> -- 1.7.4

Adds <smartcard mode='passthrough' type='spicevmc'/>, which uses the new <channel name='smartcard'/> of <graphics type='spice'>. * docs/schemas/domain.rng: Support new XML. * docs/formatdomain.html.in: Document it. * src/conf/domain_conf.h (virDomainGraphicsSpiceChannelName): New enum value. (virDomainChrSpicevmcName): New enum. (virDomainChrSourceDef): Distinguish spicevmc types. * src/conf/domain_conf.c (virDomainGraphicsSpiceChannelName): Add smartcard. (virDomainSmartcardDefParseXML): Parse it. (virDomainChrDefParseXML, virDomainSmartcardDefParseXML): Set spicevmc name. (virDomainChrSpicevmc): New enum conversion functions. * src/libvirt_private.syms: Export new functions. * src/qemu/qemu_command.c (qemuBuildChrChardevStr): Conditionalize name. * tests/qemuxml2argvtest.c (domain): New test. * tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.args: New file. * tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.xml: Likewise. --- Notes: v5: new patch docs/formatdomain.html.in | 14 ++++++--- docs/schemas/domain.rng | 1 + src/conf/domain_conf.c | 27 +++++++++++++------ src/conf/domain_conf.h | 12 ++++++++- src/libvirt_private.syms | 2 + src/qemu/qemu_command.c | 3 +- ...emuxml2argv-smartcard-passthrough-spicevmc.args | 7 +++++ ...qemuxml2argv-smartcard-passthrough-spicevmc.xml | 16 +++++++++++ tests/qemuxml2argvtest.c | 4 +++ 9 files changed, 70 insertions(+), 16 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index d91fdb9..43c78fc 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1021,6 +1021,7 @@ <protocol type='raw'/> <address type='ccid' controller='0' slot='0'/> </smartcard> + <smartcard mode='passthrough' type='spicevmc'/> </devices> ... </pre> @@ -1063,9 +1064,11 @@ files). In this mode of operation, an additional attribute <code>type</code> is required, matching one of the supported <a href="#elementsConsole">serial device</a> types, to - describe the host side of the tunnel; <code>type='tcp'</code> is - typical. Further sub-elements, such - as <code><source></code>, are required according to the + describe the host side of the tunnel; <code>type='tcp'</code> + or <code>type='spicevmc'</code> (which uses the smartcard + channel of a <a href="#elementsGraphics">SPICE graphics + device</a>) are typical. Further sub-elements, such + as <code><source></code>, may be required according to the given type, although a <code><target></code> sub-element is not required (since the consumer of the character device is the hypervisor itself, rather than a device visible in the @@ -1505,8 +1508,9 @@ qemu-kvm -net nic,model=? /dev/null can be desirable to restrict what channels can be run on each port. This is achieved by adding one or more <channel> elements inside the main <graphics> element. Valid channel names include - <code>main</code>,<code>display</code>,<code>inputs</code>,<code>cursor</code>, - <code>playback</code>,<code>record</code>. + <code>main</code>, <code>display</code>, <code>inputs</code>, + <code>cursor</code>, <code>playback</code>, <code>record</code>; + and <span class="since">since 0.8.8</span>: <code>smartcard</code>. </p> <pre> <graphics type='spice' port='-1' tlsPort='-1' autoport='yes'> diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 9ffcf21..53c07fd 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -1226,6 +1226,7 @@ <value>cursor</value> <value>playback</value> <value>record</value> + <value>smartcard</value> </choice> </attribute> <attribute name="mode"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9b4ef8d..9369ed4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -235,6 +235,10 @@ VIR_ENUM_IMPL(virDomainChrTcpProtocol, VIR_DOMAIN_CHR_TCP_PROTOCOL_LAST, "telnets", "tls") +VIR_ENUM_IMPL(virDomainChrSpicevmc, VIR_DOMAIN_CHR_SPICEVMC_LAST, + "vdagent", + "smartcard") + VIR_ENUM_IMPL(virDomainSmartcard, VIR_DOMAIN_SMARTCARD_TYPE_LAST, "host", "host-certificates", @@ -304,7 +308,8 @@ VIR_ENUM_IMPL(virDomainGraphicsSpiceChannelName, "inputs", "cursor", "playback", - "record"); + "record", + "smartcard"); VIR_ENUM_IMPL(virDomainGraphicsSpiceChannelMode, VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_LAST, @@ -3256,11 +3261,15 @@ virDomainChrDefParseXML(virCapsPtr caps, } } - if (def->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC && - def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO) { - virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("spicevmc device type only supports virtio")); - goto error; + if (def->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) { + if (def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO) { + virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("spicevmc device type only supports " + "virtio")); + goto error; + } else { + def->source.data.spicevmc = VIR_DOMAIN_CHR_SPICEVMC_VDAGENT; + } } if (virDomainDeviceInfoParseXML(node, &def->info, flags) < 0) @@ -3372,10 +3381,10 @@ virDomainSmartcardDefParseXML(xmlNodePtr node, goto error; if (def->data.passthru.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) { - virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("smartcard spicevmc device not supported")); - goto error; + def->data.passthru.data.spicevmc + = VIR_DOMAIN_CHR_SPICEVMC_SMARTCARD; } + break; default: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 9dff580..5d35e43 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -427,13 +427,20 @@ enum virDomainChrTcpProtocol { VIR_DOMAIN_CHR_TCP_PROTOCOL_LAST, }; +enum virDomainChrSpicevmcName { + VIR_DOMAIN_CHR_SPICEVMC_VDAGENT, + VIR_DOMAIN_CHR_SPICEVMC_SMARTCARD, + + VIR_DOMAIN_CHR_SPICEVMC_LAST, +}; + /* The host side information for a character device. */ typedef struct _virDomainChrSourceDef virDomainChrSourceDef; typedef virDomainChrSourceDef *virDomainChrSourceDefPtr; struct _virDomainChrSourceDef { int type; /* virDomainChrType */ union { - /* no <source> for null, vc, stdio, spicevmc */ + /* no <source> for null, vc, stdio */ struct { char *path; } file; /* pty, file, pipe, or device */ @@ -453,6 +460,7 @@ struct _virDomainChrSourceDef { char *path; bool listen; } nix; + int spicevmc; } data; }; @@ -623,6 +631,7 @@ enum virDomainGraphicsSpiceChannelName { VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_CURSOR, VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_PLAYBACK, VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_RECORD, + VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_SMARTCARD, VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST }; @@ -1360,6 +1369,7 @@ VIR_ENUM_DECL(virDomainChrConsoleTarget) VIR_ENUM_DECL(virDomainSmartcard) VIR_ENUM_DECL(virDomainChr) VIR_ENUM_DECL(virDomainChrTcpProtocol) +VIR_ENUM_DECL(virDomainChrSpicevmc) VIR_ENUM_DECL(virDomainSoundModel) VIR_ENUM_DECL(virDomainMemballoonModel) VIR_ENUM_DECL(virDomainSysinfo) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 52e44fa..88e270c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -193,6 +193,8 @@ virDomainChrConsoleTargetTypeToString; virDomainChrDefForeach; virDomainChrDefFree; virDomainChrSourceDefFree; +virDomainChrSpicevmcTypeFromString; +virDomainChrSpicevmcTypeToString; virDomainChrTcpProtocolTypeFromString; virDomainChrTcpProtocolTypeToString; virDomainChrTypeFromString; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1903c70..043ffad 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2080,7 +2080,8 @@ qemuBuildChrChardevStr(virDomainChrSourceDefPtr dev, const char *alias, _("spicevmc not supported in this QEMU binary")); goto error; } - virBufferVSprintf(&buf, "spicevmc,id=char%s,name=vdagent", alias); + virBufferVSprintf(&buf, "spicevmc,id=char%s,name=%s", alias, + virDomainChrSpicevmcTypeToString(dev->data.spicevmc)); break; default: diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.args b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.args new file mode 100644 index 0000000..8408a3e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.args @@ -0,0 +1,7 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev \ +socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon \ +chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device \ +usb-ccid,id=ccid0 -chardev spicevmc,id=charsmartcard0,name=smartcard \ +-device ccid-card-passthru,chardev=charsmartcard0,id=smartcard0,bus=ccid0.0 \ +-usb -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.xml b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.xml new file mode 100644 index 0000000..19512eb --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-smartcard-passthrough-spicevmc.xml @@ -0,0 +1,16 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219136</memory> + <currentMemory>219200</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <devices> + <emulator>/usr/bin/qemu</emulator> + <smartcard mode='passthrough' type='spicevmc'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 0726130..9032528 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -420,6 +420,10 @@ mymain(int argc, char **argv) DO_TEST("smartcard-passthrough-tcp", QEMUD_CMD_FLAG_CHARDEV | QEMUD_CMD_FLAG_DEVICE | QEMUD_CMD_FLAG_NODEFCONFIG | QEMUD_CMD_FLAG_CCID_PASSTHRU, false); + DO_TEST("smartcard-passthrough-spicevmc", + QEMUD_CMD_FLAG_CHARDEV | QEMUD_CMD_FLAG_DEVICE | + QEMUD_CMD_FLAG_NODEFCONFIG | QEMUD_CMD_FLAG_CCID_PASSTHRU | + QEMUD_CMD_FLAG_CHARDEV_SPICEVMC, false); DO_TEST("smartcard-controller", QEMUD_CMD_FLAG_CHARDEV | QEMUD_CMD_FLAG_DEVICE | QEMUD_CMD_FLAG_NODEFCONFIG | QEMUD_CMD_FLAG_CCID_EMULATED, false); -- 1.7.4

On Thu, Feb 03, 2011 at 21:16:11 -0700, Eric Blake wrote:
Adds <smartcard mode='passthrough' type='spicevmc'/>, which uses the new <channel name='smartcard'/> of <graphics type='spice'>. ... diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index d91fdb9..43c78fc 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1505,8 +1508,9 @@ qemu-kvm -net nic,model=? /dev/null can be desirable to restrict what channels can be run on each port. This is achieved by adding one or more <channel> elements inside the main <graphics> element. Valid channel names include - <code>main</code>,<code>display</code>,<code>inputs</code>,<code>cursor</code>, - <code>playback</code>,<code>record</code>. + <code>main</code>, <code>display</code>, <code>inputs</code>, + <code>cursor</code>, <code>playback</code>, <code>record</code>; + and <span class="since">since 0.8.8</span>: <code>smartcard</code>. </p> <pre> ^^ tabs before spaces, but that's in a context, not your fault. It could be worth fixing while touching the lines above. But it's not a big deal. <graphics type='spice' port='-1' tlsPort='-1' autoport='yes'>
ACK Jirka

On 02/04/2011 09:25 AM, Jiri Denemark wrote:
On Thu, Feb 03, 2011 at 21:16:11 -0700, Eric Blake wrote:
Adds <smartcard mode='passthrough' type='spicevmc'/>, which uses the new <channel name='smartcard'/> of <graphics type='spice'>. ... diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index d91fdb9..43c78fc 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1505,8 +1508,9 @@ qemu-kvm -net nic,model=? /dev/null can be desirable to restrict what channels can be run on each port. This is achieved by adding one or more <channel> elements inside the main <graphics> element. Valid channel names include - <code>main</code>,<code>display</code>,<code>inputs</code>,<code>cursor</code>, - <code>playback</code>,<code>record</code>. + <code>main</code>, <code>display</code>, <code>inputs</code>, + <code>cursor</code>, <code>playback</code>, <code>record</code>; + and <span class="since">since 0.8.8</span>: <code>smartcard</code>. </p> <pre> ^^ tabs before spaces, but that's in a context, not your fault. It could be worth fixing while touching the lines above. But it's not a big deal. <graphics type='spice' port='-1' tlsPort='-1' autoport='yes'>
ACK
Thanks for the series review. I've nuked the bogus 7 and 8 (the experiment and its reversion), and pushed the remaining 8 patches. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On 02/04/2011 09:38 AM, Eric Blake wrote:
+ and <span class="since">since 0.8.8</span>: <code>smartcard</code>. </p> <pre> ^^ tabs before spaces, but that's in a context, not your fault. It could be worth fixing while touching the lines above. But it's not a big deal. <graphics type='spice' port='-1' tlsPort='-1' autoport='yes'>
ACK
Thanks for the series review. I've nuked the bogus 7 and 8 (the experiment and its reversion), and pushed the remaining 8 patches.
Oh, and I meant to say that I'd rather defer the TAB cleanup to a separate patch (as well as tweak cfg.mk to catch it during 'make syntax-check' as well as tweak .dir-locals.el to make emacs honor it). -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On Thu, Feb 03, 2011 at 21:16:08 -0700, Eric Blake wrote:
From: Daniel P. Berrange <berrange@redhat.com>
Inspired by https://bugzilla.redhat.com/show_bug.cgi?id=615757
Add a new character device backend for virtio serial channels that activates the QEMU spice agent on the main channel using the vdagent spicevmc connection. The <target> must be type='virtio', and supports an optional name that specifies how the guest will see the channel (for now, name must be com.redhat.spice.0).
<channel type='spicevmc'> <target type='virtio'/> <address type='virtio-serial' controller='1' bus='0' port='3'/> </channel>
* docs/schemas/domain.rng: Support new XML. * docs/formatdomain.html.in: Document it. * src/conf/domain_conf.h (virDomainChrType): New enum value. * src/conf/domain_conf.c (virDomainChr): Add spicevmc. (virDomainChrDefParseXML, virDomainChrSourceDefParseXML) (virDomainChrDefParseTargetXML): Parse and enforce proper use. (virDomainChrSourceDefFormat, virDomainChrDefFormat): Format. * src/qemu/qemu_command.c (qemuBuildChrChardevStr) (qemuBuildCommandLine): Add qemu support. * tests/qemuxml2argvtest.c (domain): New test. * tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml: New file. * tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args: Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
I didn't check if the qemu command line generated by this patch follows what qemu requires for spicemvc channel but the patch looks good and the XML modeling for this is the same as what Dan used in a RHEL-specific patch before. Thus, ACK Jirka

qemu 0.13.0 (at least as built for Fedora 14, and also backported to RHEL 6.0 qemu) supported an older syntax for a spicevmc channel; it's not as flexible (it has an implicit name and hides the chardev aspect), but now that we support spicevmc, we might as well target both variants. * src/qemu/qemu_capabilities.h (QEMUD_CMD_FLAG_DEVICE_SPICEVMC): New flag. * src/qemu/qemu_capabilities.c (qemuCapsParseDeviceStr): Set it correctly. * src/qemu/qemu_command.h (qemuBuildVirtioSerialPortDevStr): Drop declaration. * src/qemu/qemu_command.c (qemuBuildVirtioSerialPortDevStr): Alter signature, check flag. (qemuBuildCommandLine): Adjust caller and check flag. * tests/qemuhelptest.c (mymain): Update test. * tests/qemuxml2argvtest.c (mymain): New test. * tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc-old.xml: New file. * tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc-old.args: Likewise. --- v5: new patch, based in part on the same RHEL 6.0 patch that introduced spicevmc xml support src/qemu/qemu_capabilities.c | 4 ++ src/qemu/qemu_capabilities.h | 3 +- src/qemu/qemu_command.c | 49 ++++++++++++++------ src/qemu/qemu_command.h | 2 - tests/qemuhelptest.c | 6 ++- .../qemuxml2argv-channel-spicevmc-old.args | 8 +++ .../qemuxml2argv-channel-spicevmc-old.xml | 34 ++++++++++++++ tests/qemuxml2argvtest.c | 3 + 8 files changed, 89 insertions(+), 20 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc-old.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc-old.xml diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 10f9d62..0e1f79c 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1091,6 +1091,10 @@ qemuCapsParseDeviceStr(const char *str, unsigned long long *flags) *flags |= QEMUD_CMD_FLAG_CCID_EMULATED; if (strstr(str, "name \"ccid-card-passthru\"")) *flags |= QEMUD_CMD_FLAG_CCID_PASSTHRU; + /* Prefer -chardev spicevmc (detected earlier) over -device spicevmc */ + if (!(*flags & QEMUD_CMD_FLAG_CHARDEV_SPICEVMC) && + strstr(str, "name \"spicevmc\"")) + *flags |= QEMUD_CMD_FLAG_DEVICE_SPICEVMC; /* Features of given devices. */ if (strstr(str, "pci-assign.configfd")) diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 9ee7639..dd39b3b 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -90,7 +90,8 @@ enum qemuCapsFlags { QEMUD_CMD_FLAG_PCI_BOOTINDEX = (1LL << 53), /* pci-assign.bootindex */ QEMUD_CMD_FLAG_CCID_EMULATED = (1LL << 54), /* -device ccid-card-emulated */ QEMUD_CMD_FLAG_CCID_PASSTHRU = (1LL << 55), /* -device ccid-card-passthru */ - QEMUD_CMD_FLAG_CHARDEV_SPICEVMC = (1LL << 56), /* -chardev spicevmc */ + QEMUD_CMD_FLAG_CHARDEV_SPICEVMC = (1LL << 56), /* newer -chardev spicevmc */ + QEMUD_CMD_FLAG_DEVICE_SPICEVMC = (1LL << 57), /* older -device spicevmc*/ }; virCapsPtr qemuCapsInit(virCapsPtr old_caps); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 043ffad..e2ae514 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2183,12 +2183,16 @@ error: } -char * -qemuBuildVirtioSerialPortDevStr(virDomainChrDefPtr dev) +static char * +qemuBuildVirtioSerialPortDevStr(virDomainChrDefPtr dev, + unsigned long long qemuCmdFlags) { virBuffer buf = VIR_BUFFER_INITIALIZER; if (dev->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE) virBufferAddLit(&buf, "virtconsole"); + else if ((qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE_SPICEVMC) && + dev->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) + virBufferAddLit(&buf, "spicevmc"); else virBufferAddLit(&buf, "virtserialport"); @@ -2211,8 +2215,6 @@ qemuBuildVirtioSerialPortDevStr(virDomainChrDefPtr dev) dev->info.addr.vioserial.port); } - virBufferVSprintf(&buf, ",chardev=char%s,id=%s", - dev->info.alias, dev->info.alias); if (dev->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC && dev->target.name && STRNEQ(dev->target.name, "com.redhat.spice.0")) { @@ -2221,8 +2223,15 @@ qemuBuildVirtioSerialPortDevStr(virDomainChrDefPtr dev) dev->target.name); goto error; } - if (dev->target.name) { - virBufferVSprintf(&buf, ",name=%s", dev->target.name); + if ((qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE_SPICEVMC) && + dev->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) { + virBufferVSprintf(&buf, ",id=%s", dev->info.alias); + } else { + virBufferVSprintf(&buf, ",chardev=char%s,id=%s", + dev->info.alias, dev->info.alias); + if (dev->target.name) { + virBufferVSprintf(&buf, ",name=%s", dev->target.name); + } } if (virBufferError(&buf)) { virReportOOMError(); @@ -3681,16 +3690,25 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - virCommandAddArg(cmd, "-chardev"); - if (!(devstr = qemuBuildChrChardevStr(&channel->source, - channel->info.alias, - qemuCmdFlags))) - goto error; - virCommandAddArg(cmd, devstr); - VIR_FREE(devstr); + if ((qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE_SPICEVMC) && + channel->source.type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) { + /* spicevmc was originally introduced via a -device + * with a backend internal to qemu; although we prefer + * the newer -chardev interface. */ + ; + } else { + virCommandAddArg(cmd, "-chardev"); + if (!(devstr = qemuBuildChrChardevStr(&channel->source, + channel->info.alias, + qemuCmdFlags))) + goto error; + virCommandAddArg(cmd, devstr); + VIR_FREE(devstr); + } virCommandAddArg(cmd, "-device"); - if (!(devstr = qemuBuildVirtioSerialPortDevStr(channel))) + if (!(devstr = qemuBuildVirtioSerialPortDevStr(channel, + qemuCmdFlags))) goto error; virCommandAddArg(cmd, devstr); VIR_FREE(devstr); @@ -3720,7 +3738,8 @@ qemuBuildCommandLine(virConnectPtr conn, VIR_FREE(devstr); virCommandAddArg(cmd, "-device"); - if (!(devstr = qemuBuildVirtioSerialPortDevStr(console))) + if (!(devstr = qemuBuildVirtioSerialPortDevStr(console, + qemuCmdFlags))) goto error; virCommandAddArg(cmd, devstr); VIR_FREE(devstr); diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 6d57007..8135046 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -105,8 +105,6 @@ char * qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev, int qemuOpenPCIConfig(virDomainHostdevDefPtr dev); -char * qemuBuildVirtioSerialPortDevStr(virDomainChrDefPtr dev); - /* Legacy, pre device support */ char * qemuBuildUSBHostdevUsbDevStr(virDomainHostdevDefPtr dev); /* Current, best practice */ diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index 20ec08d..3a04b61 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -351,7 +351,8 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_SPICE | QEMUD_CMD_FLAG_VGA_NONE | QEMUD_CMD_FLAG_MIGRATE_QEMU_FD | - QEMUD_CMD_FLAG_DRIVE_AIO, + QEMUD_CMD_FLAG_DRIVE_AIO | + QEMUD_CMD_FLAG_DEVICE_SPICEVMC, 12001, 1, 0); DO_TEST("qemu-kvm-0.12.3", QEMUD_CMD_FLAG_VNC_COLON | @@ -435,7 +436,8 @@ mymain(int argc, char **argv) QEMUD_CMD_FLAG_SPICE | QEMUD_CMD_FLAG_VGA_NONE | QEMUD_CMD_FLAG_MIGRATE_QEMU_FD | - QEMUD_CMD_FLAG_DRIVE_AIO, + QEMUD_CMD_FLAG_DRIVE_AIO | + QEMUD_CMD_FLAG_DEVICE_SPICEVMC, 13000, 1, 0); DO_TEST("qemu-kvm-0.12.1.2-rhel61", QEMUD_CMD_FLAG_VNC_COLON | diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc-old.args b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc-old.args new file mode 100644 index 0000000..7f499c7 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc-old.args @@ -0,0 +1,8 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=spice \ +/usr/bin/qemu -S -M pc -m 214 -smp 1 -nodefconfig -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -device \ +virtio-serial-pci,id=virtio-serial1,bus=pci.0,addr=0xa -hda \ +/dev/HostVG/QEMUGuest1 -device spicevmc,bus=virtio-serial1.0,nr=3,id=channel0 \ +-usb -spice port=5903,tls-port=5904,addr=127.0.0.1,\ +x509-dir=/etc/pki/libvirt-spice,tls-channel=main -device \ +virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc-old.xml b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc-old.xml new file mode 100644 index 0000000..0e82394 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc-old.xml @@ -0,0 +1,34 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219136</memory> + <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='1'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/> + </controller> + <graphics type='spice' port='5903' tlsPort='5904' autoport='no' listen='127.0.0.1'> + <channel name='main' mode='secure'/> + </graphics> + <channel type='spicevmc'> + <target type='virtio' name='com.redhat.spice.0'/> + <address type='virtio-serial' controller='1' bus='0' port='3'/> + </channel> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 9032528..9512bdc 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -410,6 +410,9 @@ mymain(int argc, char **argv) DO_TEST("channel-spicevmc", QEMUD_CMD_FLAG_DEVICE | QEMUD_CMD_FLAG_NODEFCONFIG | QEMUD_CMD_FLAG_SPICE | QEMUD_CMD_FLAG_CHARDEV_SPICEVMC, false); + DO_TEST("channel-spicevmc-old", QEMUD_CMD_FLAG_DEVICE | + QEMUD_CMD_FLAG_NODEFCONFIG | QEMUD_CMD_FLAG_SPICE | + QEMUD_CMD_FLAG_DEVICE_SPICEVMC, false); DO_TEST("smartcard-host", QEMUD_CMD_FLAG_CHARDEV | QEMUD_CMD_FLAG_DEVICE | -- 1.7.4

On Fri, Feb 04, 2011 at 09:16:25 -0700, Eric Blake wrote:
qemu 0.13.0 (at least as built for Fedora 14, and also backported to RHEL 6.0 qemu) supported an older syntax for a spicevmc channel; it's not as flexible (it has an implicit name and hides the chardev aspect), but now that we support spicevmc, we might as well target both variants.
--- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -90,7 +90,8 @@ enum qemuCapsFlags { QEMUD_CMD_FLAG_PCI_BOOTINDEX = (1LL << 53), /* pci-assign.bootindex */ QEMUD_CMD_FLAG_CCID_EMULATED = (1LL << 54), /* -device ccid-card-emulated */ QEMUD_CMD_FLAG_CCID_PASSTHRU = (1LL << 55), /* -device ccid-card-passthru */ - QEMUD_CMD_FLAG_CHARDEV_SPICEVMC = (1LL << 56), /* -chardev spicevmc */ + QEMUD_CMD_FLAG_CHARDEV_SPICEVMC = (1LL << 56), /* newer -chardev spicevmc */ + QEMUD_CMD_FLAG_DEVICE_SPICEVMC = (1LL << 57), /* older -device spicevmc*/ };
Oh, we are getting very close to 64. I'd better start working on the virBitmap rewrite.
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 6d57007..8135046 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -105,8 +105,6 @@ char * qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev,
int qemuOpenPCIConfig(virDomainHostdevDefPtr dev);
-char * qemuBuildVirtioSerialPortDevStr(virDomainChrDefPtr dev); - /* Legacy, pre device support */ char * qemuBuildUSBHostdevUsbDevStr(virDomainHostdevDefPtr dev); /* Current, best practice */
OK, apparently not used outside of qemu_command.c ACK Jirka
participants (3)
-
Daniel P. Berrange
-
Eric Blake
-
Jiri Denemark