[libvirt] [PATCH 1/2] domain_conf: add "usbredir" to list of valid spice channels
Add "usbredir" channel to list of recognized spice channels. RHBZ: 819498 Signed-off-by: Alon Levy <alevy@redhat.com> --- src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3fce7e5..10b023e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -428,7 +428,8 @@ VIR_ENUM_IMPL(virDomainGraphicsSpiceChannelName, "cursor", "playback", "record", - "smartcard"); + "smartcard", + "usbredir"); VIR_ENUM_IMPL(virDomainGraphicsSpiceChannelMode, VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_LAST, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 5aa8fc1..6581fea 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1097,6 +1097,7 @@ enum virDomainGraphicsSpiceChannelName { VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_PLAYBACK, VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_RECORD, VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_SMARTCARD, + VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_USBREDIR, VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST }; -- 1.7.10.1
qemu's behavior in this case is to change the spice server behavior to require secure connection to any channel not otherwise specified as being in plaintext mode. libvirt doesn't currently allow requesting this (via plaintext-channel=<channel name>). RHBZ: 819499 Signed-off-by: Alon Levy <alevy@redhat.com> --- src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 10b023e..140bc63 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -429,7 +429,8 @@ VIR_ENUM_IMPL(virDomainGraphicsSpiceChannelName, "playback", "record", "smartcard", - "usbredir"); + "usbredir", + "default"); VIR_ENUM_IMPL(virDomainGraphicsSpiceChannelMode, VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_LAST, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 6581fea..6189efa 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1098,6 +1098,7 @@ enum virDomainGraphicsSpiceChannelName { VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_RECORD, VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_SMARTCARD, VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_USBREDIR, + VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_DEFAULT, VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST }; -- 1.7.10.1
On 05/07/2012 06:33 AM, Alon Levy wrote:
qemu's behavior in this case is to change the spice server behavior to require secure connection to any channel not otherwise specified as being in plaintext mode. libvirt doesn't currently allow requesting this (via plaintext-channel=<channel name>).
RHBZ: 819499
Signed-off-by: Alon Levy <alevy@redhat.com> --- src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-)
Same complaints as for 1/2 (docs, RNG schema, tests). Also, is it ever valid to mark the default channel for plaintext (meaning all channels not marked secure are plaintext), or must it only be permitted for secure channels? -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 05/07/2012 09:53 AM, Eric Blake wrote:
On 05/07/2012 06:33 AM, Alon Levy wrote:
qemu's behavior in this case is to change the spice server behavior to require secure connection to any channel not otherwise specified as being in plaintext mode. libvirt doesn't currently allow requesting this (via plaintext-channel=<channel name>).
RHBZ: 819499
Signed-off-by: Alon Levy <alevy@redhat.com> --- src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-)
Same complaints as for 1/2 (docs, RNG schema, tests). Also, is it ever valid to mark the default channel for plaintext (meaning all channels not marked secure are plaintext), or must it only be permitted for secure channels?
Here's one of the existing tests: tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml <graphics type='spice' port='5903' tlsPort='5904' autoport='no' listen='127.0.0.1'> <listen type='address' address='127.0.0.1'/> <channel name='main' mode='secure'/> <channel name='inputs' mode='insecure'/> I'm wondering if rather than adding a new <channel name='default' mode='.../'>, it might make more sense to hoist the default channel security mode up one element. Something like: <graphics type='spice' default_mode='secure' ...> <channel name='main' mode='secure'/> <!-- redundant --> <channel name='inputs' mode='insecure'/> <!-- override default --> <channel name='usbredir'/> <!-- defaults to secure due to <graphics> --> While it is obvious that usbredir must be a valid channel name, it's not as obvious about 'default' being a channel name (since it is really more of the catchall for all other channels not explicitly listed). -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Mon, May 07, 2012 at 09:53:05AM -0600, Eric Blake wrote:
On 05/07/2012 06:33 AM, Alon Levy wrote:
qemu's behavior in this case is to change the spice server behavior to require secure connection to any channel not otherwise specified as being in plaintext mode. libvirt doesn't currently allow requesting this (via plaintext-channel=<channel name>).
RHBZ: 819499
Signed-off-by: Alon Levy <alevy@redhat.com> --- src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-)
Same complaints as for 1/2 (docs, RNG schema, tests). Also, is it ever valid to mark the default channel for plaintext (meaning all channels not marked secure are plaintext), or must it only be permitted for secure channels?
It's valid.
-- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 05/07/2012 06:33 AM, Alon Levy wrote:
Add "usbredir" channel to list of recognized spice channels.
RHBZ: 819498
Signed-off-by: Alon Levy <alevy@redhat.com> --- src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-)
Missing a change to the documentation in docs/formatdomain.html.in, and to the RelaxNG grammar in docs/schemas/domaincommon.rng. It would also be nice to see if there is an existing test in tests/qemuxml2argvdata/* that can be enhanced to cover the new XML, and/or add a new test. I'm not sure whether this counts as a simple enough addition to include in 0.9.12, or whether we should wait until after the release to take it, but the code patch itself looks reasonable. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Mon, May 07, 2012 at 09:50:22AM -0600, Eric Blake wrote:
On 05/07/2012 06:33 AM, Alon Levy wrote:
Add "usbredir" channel to list of recognized spice channels.
RHBZ: 819498
Signed-off-by: Alon Levy <alevy@redhat.com> --- src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-)
Missing a change to the documentation in docs/formatdomain.html.in, and to the RelaxNG grammar in docs/schemas/domaincommon.rng. It would also be nice to see if there is an existing test in tests/qemuxml2argvdata/* that can be enhanced to cover the new XML, and/or add a new test.
I'm not sure whether this counts as a simple enough addition to include in 0.9.12, or whether we should wait until after the release to take it, but the code patch itself looks reasonable.
I'll use 0.9.12 in the doc change.
-- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Add "usbredir" channel to list of recognized spice channels. RHBZ: 819498 Signed-off-by: Alon Levy <alevy@redhat.com> --- docs/formatdomain.html.in | 5 +- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 3 +- src/conf/domain_conf.h | 1 + .../qemuxml2argv-graphics-spice-usb-redir.args | 16 ++++++ .../qemuxml2argv-graphics-spice-usb-redir.xml | 53 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 6 +++ 7 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e1fe0c4..0525577 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2939,8 +2939,9 @@ qemu-kvm -net nic,model=? /dev/null include <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>. + <span class="since">since + 0.8.8</span>: <code>smartcard</code> and <span class="since"> + since 0.9.12</span> <code>usbredir</code>. </p> <pre> <graphics type='spice' port='-1' tlsPort='-1' autoport='yes'> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 8419ccc..77f2f6a 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1787,6 +1787,7 @@ <value>playback</value> <value>record</value> <value>smartcard</value> + <value>usbredir</value> </choice> </attribute> <attribute name="mode"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3fce7e5..10b023e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -428,7 +428,8 @@ VIR_ENUM_IMPL(virDomainGraphicsSpiceChannelName, "cursor", "playback", "record", - "smartcard"); + "smartcard", + "usbredir"); VIR_ENUM_IMPL(virDomainGraphicsSpiceChannelMode, VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_LAST, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 5aa8fc1..6581fea 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1097,6 +1097,7 @@ enum virDomainGraphicsSpiceChannelName { VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_PLAYBACK, VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_RECORD, VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_SMARTCARD, + VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_USBREDIR, VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST }; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.args new file mode 100644 index 0000000..35e51a7 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.args @@ -0,0 +1,16 @@ +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 -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c \ +-device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x4.0x7 \ +-device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x4 \ +-device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x4.0x1 \ +-device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x4.0x2 \ +-spice port=5903,tls-port=5904,addr=127.0.0.1,\ +x509-dir=/etc/pki/libvirt-spice,tls-channel=main,plaintext-channel=inputs,\ +tls-channel=usbredir,\ +image-compression=auto_glz,jpeg-wan-compression=auto,zlib-glz-wan-compression=auto,\ +playback-compression=on,streaming-video=filter,disable-copy-paste \ +-vga cirrus \ +-chardev socket,id=charredir0,host=localhost,port=4000 \ +-device usb-redir,chardev=charredir0,id=redir0 \ +-chardev spicevmc,id=charredir1,name=usbredir \ +-device usb-redir,chardev=charredir1,id=redir1,bus=usb.0,port=4 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.xml new file mode 100644 index 0000000..1dc23bd --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.xml @@ -0,0 +1,53 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu>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> + <graphics type='spice' port='5903' tlsPort='5904' autoport='no' listen='127.0.0.1'> + <listen type='address' address='127.0.0.1'/> + <channel name='main' mode='secure'/> + <channel name='inputs' mode='insecure'/> + <channel name='usbredir' mode='secure'/> + <image compression='auto_glz'/> + <jpeg compression='auto'/> + <zlib compression='auto'/> + <playback compression='on'/> + <streaming mode='filter'/> + <clipboard copypaste='no'/> + </graphics> + <controller type='usb' index='0' model='ich9-ehci1'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x7'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci1'> + <master startport='0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0' multifunction='on'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci2'> + <master startport='2'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x1'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci3'> + <master startport='4'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x2'/> + </controller> + <redirdev bus='usb' type='tcp'> + <source mode='connect' host='localhost' service='4000'/> + <protocol type='raw'/> + </redirdev> + <redirdev bus='usb' type='spicevmc'> + <address type='usb' bus='0' port='4'/> + </redirdev> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index e47a385..4d7cf69 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -543,6 +543,12 @@ mymain(void) QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE, QEMU_CAPS_SPICE, QEMU_CAPS_DEVICE_QXL_VGA); + DO_TEST("graphics-spice-usb-redir", false, + QEMU_CAPS_VGA, QEMU_CAPS_SPICE, + QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_USB_HUB, + QEMU_CAPS_ICH9_USB_EHCI1, QEMU_CAPS_USB_REDIR, + QEMU_CAPS_CHARDEV_SPICEVMC); DO_TEST("input-usbmouse", false, NONE); DO_TEST("input-usbtablet", false, NONE); -- 1.7.10.1
qemu's behavior in this case is to change the spice server behavior to require secure connection to any channel not otherwise specified as being in plaintext mode. libvirt doesn't currently allow requesting this (via plaintext-channel=<channel name>). RHBZ: 819499 Signed-off-by: Alon Levy <alevy@redhat.com> --- docs/formatdomain.html.in | 5 +++-- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml | 1 + 6 files changed, 8 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0525577..9758f73 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2940,8 +2940,9 @@ qemu-kvm -net nic,model=? /dev/null <code>inputs</code>, <code>cursor</code>, <code>playback</code>, <code>record</code>; <span class="since">since - 0.8.8</span>: <code>smartcard</code> and <span class="since"> - since 0.9.12</span> <code>usbredir</code>. + 0.8.8</span>: <code>smartcard</code>, <span class="since"> + since 0.9.12</span> <code>usbredir</code> and + <code>default</code>. </p> <pre> <graphics type='spice' port='-1' tlsPort='-1' autoport='yes'> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 77f2f6a..dec958d 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1788,6 +1788,7 @@ <value>record</value> <value>smartcard</value> <value>usbredir</value> + <value>default</value> </choice> </attribute> <attribute name="mode"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 10b023e..1c9a95d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -422,6 +422,7 @@ VIR_ENUM_IMPL(virDomainGraphicsAuthConnected, VIR_ENUM_IMPL(virDomainGraphicsSpiceChannelName, VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST, + "default", "main", "display", "inputs", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 6581fea..609fe45 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1090,6 +1090,7 @@ struct _virDomainGraphicsAuthDef { }; enum virDomainGraphicsSpiceChannelName { + VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_DEFAULT, VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MAIN, VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_DISPLAY, VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_INPUT, diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args index c9fdb99..698e39c 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args @@ -2,7 +2,7 @@ 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 -nodefaults -monitor \ unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda \ /dev/HostVG/QEMUGuest1 -usb -spice port=5903,tls-port=5904,addr=127.0.0.1,\ -x509-dir=/etc/pki/libvirt-spice,tls-channel=main,plaintext-channel=inputs,\ +x509-dir=/etc/pki/libvirt-spice,tls-channel=default,tls-channel=main,plaintext-channel=inputs,\ image-compression=auto_glz,jpeg-wan-compression=auto,zlib-glz-wan-compression=auto,\ playback-compression=on,streaming-video=filter,disable-copy-paste -vga \ qxl -global qxl.vram_size=18874368 -device qxl,id=video1,vram_size=33554432,bus=pci.0,addr=0x4 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml index 8930b60..f995317 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml @@ -24,6 +24,7 @@ <input type='mouse' bus='ps2'/> <graphics type='spice' port='5903' tlsPort='5904' autoport='no' listen='127.0.0.1'> <listen type='address' address='127.0.0.1'/> + <channel name='default' mode='secure'/> <channel name='main' mode='secure'/> <channel name='inputs' mode='insecure'/> <image compression='auto_glz'/> -- 1.7.10.1
On 05/08/2012 07:00 AM, Alon Levy wrote:
qemu's behavior in this case is to change the spice server behavior to require secure connection to any channel not otherwise specified as being in plaintext mode. libvirt doesn't currently allow requesting this (via plaintext-channel=<channel name>).
RHBZ: 819499
Signed-off-by: Alon Levy <alevy@redhat.com> --- docs/formatdomain.html.in | 5 +++-- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 1 + src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml | 1 + 6 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0525577..9758f73 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2940,8 +2940,9 @@ qemu-kvm -net nic,model=? /dev/null <code>inputs</code>, <code>cursor</code>, <code>playback</code>, <code>record</code>; <span class="since">since - 0.8.8</span>: <code>smartcard</code> and <span class="since"> - since 0.9.12</span> <code>usbredir</code>. + 0.8.8</span>: <code>smartcard</code>, <span class="since"> + since 0.9.12</span> <code>usbredir</code> and + <code>default</code>.
I decided to keep your approach of having default appear as a channel rather than a meta-annotation on the parent element (if only so that native-to-xml parsing is easier), but also thought that additional wording in the documentatino would help: diff --git c/docs/formatdomain.html.in i/docs/formatdomain.html.in index 1ccf7a3..4defcf1 100644 --- c/docs/formatdomain.html.in +++ i/docs/formatdomain.html.in @@ -2942,7 +2942,10 @@ qemu-kvm -net nic,model=? /dev/null (all <span class="since"> since 0.8.6</span>); <code>smartcard</code> (<span class="since">since 0.8.8</span>); and <code>usbredir</code> - (<span class="since">since 0.9.12</span>). + and <code>default</code> (<span class="since">since + 0.9.12</span>). The <code>default</code> channel is not + an actual channel, but specifies the settings for all + other channels not explicitly listed. </p> <pre> <graphics type='spice' port='-1' tlsPort='-1' autoport='yes'> ACK with that tweak. However, since I spoke with you on IRC about the possibility of an alternate implementation (https://www.redhat.com/archives/libvir-list/2012-May/msg00393.html), I'll wait to push until we can compare the two ideas. I'm sure that treating 'default' as a special case will be more lines of code, but might make the end-result XML more legible. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Add "usbredir" channel to list of recognized spice channels. RHBZ: 819498 Signed-off-by: Alon Levy <alevy@redhat.com> --- docs/formatdomain.html.in | 5 +- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 3 +- src/conf/domain_conf.h | 1 + .../qemuxml2argv-graphics-spice-usb-redir.args | 16 ++++++ .../qemuxml2argv-graphics-spice-usb-redir.xml | 53 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 6 +++ 7 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e1fe0c4..0525577 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2939,8 +2939,9 @@ qemu-kvm -net nic,model=? /dev/null include <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>. + <span class="since">since + 0.8.8</span>: <code>smartcard</code> and <span class="since"> + since 0.9.12</span> <code>usbredir</code>. </p> <pre> <graphics type='spice' port='-1' tlsPort='-1' autoport='yes'> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 8419ccc..77f2f6a 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1787,6 +1787,7 @@ <value>playback</value> <value>record</value> <value>smartcard</value> + <value>usbredir</value> </choice> </attribute> <attribute name="mode"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3fce7e5..10b023e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -428,7 +428,8 @@ VIR_ENUM_IMPL(virDomainGraphicsSpiceChannelName, "cursor", "playback", "record", - "smartcard"); + "smartcard", + "usbredir"); VIR_ENUM_IMPL(virDomainGraphicsSpiceChannelMode, VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_LAST, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 5aa8fc1..6581fea 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1097,6 +1097,7 @@ enum virDomainGraphicsSpiceChannelName { VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_PLAYBACK, VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_RECORD, VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_SMARTCARD, + VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_USBREDIR, VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST }; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.args new file mode 100644 index 0000000..35e51a7 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.args @@ -0,0 +1,16 @@ +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 -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c \ +-device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x4.0x7 \ +-device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x4 \ +-device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x4.0x1 \ +-device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x4.0x2 \ +-spice port=5903,tls-port=5904,addr=127.0.0.1,\ +x509-dir=/etc/pki/libvirt-spice,tls-channel=main,plaintext-channel=inputs,\ +tls-channel=usbredir,\ +image-compression=auto_glz,jpeg-wan-compression=auto,zlib-glz-wan-compression=auto,\ +playback-compression=on,streaming-video=filter,disable-copy-paste \ +-vga cirrus \ +-chardev socket,id=charredir0,host=localhost,port=4000 \ +-device usb-redir,chardev=charredir0,id=redir0 \ +-chardev spicevmc,id=charredir1,name=usbredir \ +-device usb-redir,chardev=charredir1,id=redir1,bus=usb.0,port=4 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.xml new file mode 100644 index 0000000..1dc23bd --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.xml @@ -0,0 +1,53 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu>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> + <graphics type='spice' port='5903' tlsPort='5904' autoport='no' listen='127.0.0.1'> + <listen type='address' address='127.0.0.1'/> + <channel name='main' mode='secure'/> + <channel name='inputs' mode='insecure'/> + <channel name='usbredir' mode='secure'/> + <image compression='auto_glz'/> + <jpeg compression='auto'/> + <zlib compression='auto'/> + <playback compression='on'/> + <streaming mode='filter'/> + <clipboard copypaste='no'/> + </graphics> + <controller type='usb' index='0' model='ich9-ehci1'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x7'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci1'> + <master startport='0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0' multifunction='on'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci2'> + <master startport='2'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x1'/> + </controller> + <controller type='usb' index='0' model='ich9-uhci3'> + <master startport='4'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x2'/> + </controller> + <redirdev bus='usb' type='tcp'> + <source mode='connect' host='localhost' service='4000'/> + <protocol type='raw'/> + </redirdev> + <redirdev bus='usb' type='spicevmc'> + <address type='usb' bus='0' port='4'/> + </redirdev> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index e47a385..4d7cf69 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -543,6 +543,12 @@ mymain(void) QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE, QEMU_CAPS_SPICE, QEMU_CAPS_DEVICE_QXL_VGA); + DO_TEST("graphics-spice-usb-redir", false, + QEMU_CAPS_VGA, QEMU_CAPS_SPICE, + QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, + QEMU_CAPS_PCI_MULTIFUNCTION, QEMU_CAPS_USB_HUB, + QEMU_CAPS_ICH9_USB_EHCI1, QEMU_CAPS_USB_REDIR, + QEMU_CAPS_CHARDEV_SPICEVMC); DO_TEST("input-usbmouse", false, NONE); DO_TEST("input-usbtablet", false, NONE); -- 1.7.10.1
qemu's behavior in this case is to change the spice server behavior to require secure connection to any channel not otherwise specified as being in plaintext mode. libvirt doesn't currently allow requesting this (via plaintext-channel=<channel name>). RHBZ: 819499 Signed-off-by: Alon Levy <alevy@redhat.com> --- docs/formatdomain.html.in | 3 +++ docs/schemas/domaincommon.rng | 9 +++++++++ src/conf/domain_conf.c | 20 ++++++++++++++++++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 13 +++++++++++++ .../qemuxml2argv-graphics-spice.args | 2 +- .../qemuxml2argv-graphics-spice.xml | 2 +- 7 files changed, 48 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0525577..c0268b2 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2929,6 +2929,9 @@ qemu-kvm -net nic,model=? /dev/null <span class="since">Since 0.9.3</span> NB, this may not be supported by all hypervisors. <span class="since">"spice" since 0.8.6</span>. + The <code>defaultMode</code> attribute sets the default channel + security policy, valid values are <code>secure</code>, + <code>insecure</code> and the default <code>any</code>. </p> <p> When SPICE has both a normal and TLS secured TCP port diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 77f2f6a..84369c7 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1774,6 +1774,15 @@ </choice> </attribute> </optional> + <optional> + <attribute name="defaultMode"> + <choice> + <value>any</value> + <value>secure</value> + <value>insecure</value> + </choice> + </attribute> + </optional> <interleave> <ref name="listenElements"/> <zeroOrMore> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 10b023e..a60ef5a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6071,6 +6071,8 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, char *port = virXMLPropString(node, "port"); char *tlsPort; char *autoport; + char *defaultMode; + int defaultModeVal; if (port) { if (virStrToLong_i(port, NULL, 10, &def->data.spice.port) < 0) { @@ -6103,6 +6105,20 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, VIR_FREE(autoport); } + def->data.spice.defaultMode = VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY; + + if ((defaultMode = virXMLPropString(node, "defaultMode")) != NULL) { + if ((defaultModeVal = virDomainGraphicsSpiceChannelModeTypeFromString(defaultMode)) < 0) { + virDomainReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown default spice channel mode %s"), + defaultMode); + VIR_FREE(defaultMode); + goto error; + } + def->data.spice.defaultMode = defaultModeVal; + VIR_FREE(defaultMode); + } + if (def->data.spice.port == -1 && def->data.spice.tlsPort == -1) { /* Legacy compat syntax, used -1 for auto-port */ def->data.spice.autoport = 1; @@ -12124,6 +12140,10 @@ virDomainGraphicsDefFormat(virBufferPtr buf, virBufferEscapeString(buf, " keymap='%s'", def->data.spice.keymap); + if (def->data.spice.defaultMode != VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY) + virBufferAsprintf(buf, " defaultMode='%s'", + virDomainGraphicsSpiceChannelModeTypeToString(def->data.spice.defaultMode)); + virDomainGraphicsAuthDefFormatAttr(buf, &def->data.spice.auth, flags); break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 6581fea..895ddc4 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1233,6 +1233,7 @@ struct _virDomainGraphicsDef { virDomainGraphicsAuthDef auth; unsigned int autoport :1; int channels[VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST]; + int defaultMode; int image; int jpeg; int zlib; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 070d13e..117542f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5463,6 +5463,7 @@ qemuBuildCommandLine(virConnectPtr conn, const char *listenAddr = NULL; char *netAddr = NULL; int ret; + int defaultMode = def->graphics[0]->data.spice.defaultMode; if (!qemuCapsGet(qemuCaps, QEMU_CAPS_SPICE)) { qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -5546,6 +5547,18 @@ qemuBuildCommandLine(virConnectPtr conn, virBufferAsprintf(&opt, ",x509-dir=%s", driver->spiceTLSx509certdir); + switch (defaultMode) { + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE: + virBufferAsprintf(&opt, ",tls-channel=default"); + break; + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_INSECURE: + virBufferAsprintf(&opt, ",plaintext-channel=default"); + break; + case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY: + /* nothing */ + break; + } + for (i = 0 ; i < VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST ; i++) { int mode = def->graphics[0]->data.spice.channels[i]; switch (mode) { diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args index c9fdb99..698e39c 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args @@ -2,7 +2,7 @@ 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 -nodefaults -monitor \ unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda \ /dev/HostVG/QEMUGuest1 -usb -spice port=5903,tls-port=5904,addr=127.0.0.1,\ -x509-dir=/etc/pki/libvirt-spice,tls-channel=main,plaintext-channel=inputs,\ +x509-dir=/etc/pki/libvirt-spice,tls-channel=default,tls-channel=main,plaintext-channel=inputs,\ image-compression=auto_glz,jpeg-wan-compression=auto,zlib-glz-wan-compression=auto,\ playback-compression=on,streaming-video=filter,disable-copy-paste -vga \ qxl -global qxl.vram_size=18874368 -device qxl,id=video1,vram_size=33554432,bus=pci.0,addr=0x4 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml index 8930b60..a3789f2 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml @@ -22,7 +22,7 @@ <controller type='usb' index='0'/> <controller type='ide' index='0'/> <input type='mouse' bus='ps2'/> - <graphics type='spice' port='5903' tlsPort='5904' autoport='no' listen='127.0.0.1'> + <graphics type='spice' port='5903' tlsPort='5904' autoport='no' listen='127.0.0.1' defaultMode='secure'> <listen type='address' address='127.0.0.1'/> <channel name='main' mode='secure'/> <channel name='inputs' mode='insecure'/> -- 1.7.10.1
On 05/08/2012 11:42 AM, Alon Levy wrote:
qemu's behavior in this case is to change the spice server behavior to require secure connection to any channel not otherwise specified as being in plaintext mode. libvirt doesn't currently allow requesting this (via plaintext-channel=<channel name>).
RHBZ: 819499
Signed-off-by: Alon Levy <alevy@redhat.com> --- docs/formatdomain.html.in | 3 +++ docs/schemas/domaincommon.rng | 9 +++++++++ src/conf/domain_conf.c | 20 ++++++++++++++++++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 13 +++++++++++++ .../qemuxml2argv-graphics-spice.args | 2 +- .../qemuxml2argv-graphics-spice.xml | 2 +- 7 files changed, 48 insertions(+), 2 deletions(-)
Definitely more code, but I think I like it better. In particular,
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0525577..c0268b2 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2929,6 +2929,9 @@ qemu-kvm -net nic,model=? /dev/null <span class="since">Since 0.9.3</span> NB, this may not be supported by all hypervisors. <span class="since">"spice" since 0.8.6</span>. + The <code>defaultMode</code> attribute sets the default channel + security policy, valid values are <code>secure</code>, + <code>insecure</code> and the default <code>any</code>.
I didn't realize that 'any' was different! Having a defaultMode call it out makes sense now, given this matrix: insecure any secure tls available plaintext tls tls tls disabled plaintext plaintext error And it also makes sense to the qemu command line - you can only specify plaintext or tls to lock a channel to a particular mode; if you omit the specification, then the mode defaults to the best available (according to whether tls is available). Missing a <span>since</span> notation; I've added that.
@@ -12124,6 +12140,10 @@ virDomainGraphicsDefFormat(virBufferPtr buf, virBufferEscapeString(buf, " keymap='%s'", def->data.spice.keymap);
+ if (def->data.spice.defaultMode != VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY) + virBufferAsprintf(buf, " defaultMode='%s'", + virDomainGraphicsSpiceChannelModeTypeToString(def->data.spice.defaultMode));
This means we don't output the user's explicit defaultMode='any'. It's easy enough to fix by making the enum have four values instead of three (the fourth value, 'default', is value 0 and not valid in XML, as the placeholder for no attribute present, but otherwise behaves like 'any'). Then again, you matched the style of the individual channels, so I won't bother changing it.
+++ b/src/conf/domain_conf.h @@ -1233,6 +1233,7 @@ struct _virDomainGraphicsDef { virDomainGraphicsAuthDef auth; unsigned int autoport :1; int channels[VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST]; + int defaultMode;
I like to add a comment to the enum values that are valid in this field. ACK; I squashed this in, then pushed: diff --git i/docs/formatdomain.html.in w/docs/formatdomain.html.in index 816af41..1478832 100644 --- i/docs/formatdomain.html.in +++ w/docs/formatdomain.html.in @@ -2931,7 +2931,11 @@ qemu-kvm -net nic,model=? /dev/null <span class="since">"spice" since 0.8.6</span>. The <code>defaultMode</code> attribute sets the default channel security policy, valid values are <code>secure</code>, - <code>insecure</code> and the default <code>any</code>. + <code>insecure</code> and the default <code>any</code> + (which is secure if possible, but falls back to insecure + rather than erroring out if no secure path is + available). <span class="since">"defaultMode" since + 0.9.12</span>. </p> <p> When SPICE has both a normal and TLS secured TCP port diff --git i/src/conf/domain_conf.h w/src/conf/domain_conf.h index 895ddc4..00178e1 100644 --- i/src/conf/domain_conf.h +++ w/src/conf/domain_conf.h @@ -1233,7 +1233,7 @@ struct _virDomainGraphicsDef { virDomainGraphicsAuthDef auth; unsigned int autoport :1; int channels[VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST]; - int defaultMode; + int defaultMode; /* enum virDomainGraphicsSpiceChannelMode */ int image; int jpeg; int zlib; -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Tue, May 08, 2012 at 12:19:45PM -0600, Eric Blake wrote:
On 05/08/2012 11:42 AM, Alon Levy wrote:
qemu's behavior in this case is to change the spice server behavior to require secure connection to any channel not otherwise specified as being in plaintext mode. libvirt doesn't currently allow requesting this (via plaintext-channel=<channel name>).
RHBZ: 819499
Signed-off-by: Alon Levy <alevy@redhat.com> --- docs/formatdomain.html.in | 3 +++ docs/schemas/domaincommon.rng | 9 +++++++++ src/conf/domain_conf.c | 20 ++++++++++++++++++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 13 +++++++++++++ .../qemuxml2argv-graphics-spice.args | 2 +- .../qemuxml2argv-graphics-spice.xml | 2 +- 7 files changed, 48 insertions(+), 2 deletions(-)
Definitely more code, but I think I like it better. In particular,
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0525577..c0268b2 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2929,6 +2929,9 @@ qemu-kvm -net nic,model=? /dev/null <span class="since">Since 0.9.3</span> NB, this may not be supported by all hypervisors. <span class="since">"spice" since 0.8.6</span>. + The <code>defaultMode</code> attribute sets the default channel + security policy, valid values are <code>secure</code>, + <code>insecure</code> and the default <code>any</code>.
I didn't realize that 'any' was different! Having a defaultMode call it out makes sense now, given this matrix:
insecure any secure tls available plaintext tls tls tls disabled plaintext plaintext error
And it also makes sense to the qemu command line - you can only specify plaintext or tls to lock a channel to a particular mode; if you omit the specification, then the mode defaults to the best available (according to whether tls is available).
Your table is correct and a good summary. But to be clear the "any" value is not a valid value to put in qemu's command line, it's the internal default state (actually it's oring of the two valid channel modes, insecure and secure). But it behaves exactly like your table explains, i.e. not setting tls-channel=default or plaintext-channel=default results in the middle column, setting tls-channel=default in the right, and plaintext-channel=default in the left. You could have expanded the table because it's also possible to start spice with only tls-port set, and no port, then you would get the reverse of the right column: insecure any secure both plaintext tls tls only clear plaintext plaintext error only tls error plaintext tls
Missing a <span>since</span> notation; I've added that.
@@ -12124,6 +12140,10 @@ virDomainGraphicsDefFormat(virBufferPtr buf, virBufferEscapeString(buf, " keymap='%s'", def->data.spice.keymap);
+ if (def->data.spice.defaultMode != VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY) + virBufferAsprintf(buf, " defaultMode='%s'", + virDomainGraphicsSpiceChannelModeTypeToString(def->data.spice.defaultMode));
This means we don't output the user's explicit defaultMode='any'. It's easy enough to fix by making the enum have four values instead of three (the fourth value, 'default', is value 0 and not valid in XML, as the placeholder for no attribute present, but otherwise behaves like 'any'). Then again, you matched the style of the individual channels, so I won't bother changing it.
+++ b/src/conf/domain_conf.h @@ -1233,6 +1233,7 @@ struct _virDomainGraphicsDef { virDomainGraphicsAuthDef auth; unsigned int autoport :1; int channels[VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST]; + int defaultMode;
I like to add a comment to the enum values that are valid in this field.
ACK; I squashed this in, then pushed:
Looks good.
diff --git i/docs/formatdomain.html.in w/docs/formatdomain.html.in index 816af41..1478832 100644 --- i/docs/formatdomain.html.in +++ w/docs/formatdomain.html.in @@ -2931,7 +2931,11 @@ qemu-kvm -net nic,model=? /dev/null <span class="since">"spice" since 0.8.6</span>. The <code>defaultMode</code> attribute sets the default channel security policy, valid values are <code>secure</code>, - <code>insecure</code> and the default <code>any</code>. + <code>insecure</code> and the default <code>any</code> + (which is secure if possible, but falls back to insecure + rather than erroring out if no secure path is + available). <span class="since">"defaultMode" since + 0.9.12</span>. </p> <p> When SPICE has both a normal and TLS secured TCP port diff --git i/src/conf/domain_conf.h w/src/conf/domain_conf.h index 895ddc4..00178e1 100644 --- i/src/conf/domain_conf.h +++ w/src/conf/domain_conf.h @@ -1233,7 +1233,7 @@ struct _virDomainGraphicsDef { virDomainGraphicsAuthDef auth; unsigned int autoport :1; int channels[VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST]; - int defaultMode; + int defaultMode; /* enum virDomainGraphicsSpiceChannelMode */ int image; int jpeg; int zlib;
-- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Tue, May 08, 2012 at 10:10:34PM +0300, Alon Levy wrote:
On Tue, May 08, 2012 at 12:19:45PM -0600, Eric Blake wrote:
On 05/08/2012 11:42 AM, Alon Levy wrote:
qemu's behavior in this case is to change the spice server behavior to require secure connection to any channel not otherwise specified as being in plaintext mode. libvirt doesn't currently allow requesting this (via plaintext-channel=<channel name>).
RHBZ: 819499
Signed-off-by: Alon Levy <alevy@redhat.com> --- docs/formatdomain.html.in | 3 +++ docs/schemas/domaincommon.rng | 9 +++++++++ src/conf/domain_conf.c | 20 ++++++++++++++++++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 13 +++++++++++++ .../qemuxml2argv-graphics-spice.args | 2 +- .../qemuxml2argv-graphics-spice.xml | 2 +- 7 files changed, 48 insertions(+), 2 deletions(-)
Definitely more code, but I think I like it better. In particular,
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0525577..c0268b2 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2929,6 +2929,9 @@ qemu-kvm -net nic,model=? /dev/null <span class="since">Since 0.9.3</span> NB, this may not be supported by all hypervisors. <span class="since">"spice" since 0.8.6</span>. + The <code>defaultMode</code> attribute sets the default channel + security policy, valid values are <code>secure</code>, + <code>insecure</code> and the default <code>any</code>.
I didn't realize that 'any' was different! Having a defaultMode call it out makes sense now, given this matrix:
insecure any secure tls available plaintext tls tls tls disabled plaintext plaintext error
And it also makes sense to the qemu command line - you can only specify plaintext or tls to lock a channel to a particular mode; if you omit the specification, then the mode defaults to the best available (according to whether tls is available).
Your table is correct and a good summary. But to be clear the "any" value is not a valid value to put in qemu's command line, it's the internal default state (actually it's oring of the two valid channel modes, insecure and secure). But it behaves exactly like your table explains, i.e. not setting tls-channel=default or plaintext-channel=default results in the middle column, setting tls-channel=default in the right, and plaintext-channel=default in the left. You could have expanded the table because it's also possible to start spice with only tls-port set, and no port, then you would get the reverse of the right column:
insecure any secure both plaintext tls tls This would actually be: both plaintext whichever-the-client-first-connects-to tls
only clear plaintext plaintext error only tls error plaintext tls Oops, should read: only tls error tls tls
Missing a <span>since</span> notation; I've added that.
@@ -12124,6 +12140,10 @@ virDomainGraphicsDefFormat(virBufferPtr buf, virBufferEscapeString(buf, " keymap='%s'", def->data.spice.keymap);
+ if (def->data.spice.defaultMode != VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY) + virBufferAsprintf(buf, " defaultMode='%s'", + virDomainGraphicsSpiceChannelModeTypeToString(def->data.spice.defaultMode));
This means we don't output the user's explicit defaultMode='any'. It's easy enough to fix by making the enum have four values instead of three (the fourth value, 'default', is value 0 and not valid in XML, as the placeholder for no attribute present, but otherwise behaves like 'any'). Then again, you matched the style of the individual channels, so I won't bother changing it.
+++ b/src/conf/domain_conf.h @@ -1233,6 +1233,7 @@ struct _virDomainGraphicsDef { virDomainGraphicsAuthDef auth; unsigned int autoport :1; int channels[VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST]; + int defaultMode;
I like to add a comment to the enum values that are valid in this field.
ACK; I squashed this in, then pushed:
Looks good.
diff --git i/docs/formatdomain.html.in w/docs/formatdomain.html.in index 816af41..1478832 100644 --- i/docs/formatdomain.html.in +++ w/docs/formatdomain.html.in @@ -2931,7 +2931,11 @@ qemu-kvm -net nic,model=? /dev/null <span class="since">"spice" since 0.8.6</span>. The <code>defaultMode</code> attribute sets the default channel security policy, valid values are <code>secure</code>, - <code>insecure</code> and the default <code>any</code>. + <code>insecure</code> and the default <code>any</code> + (which is secure if possible, but falls back to insecure + rather than erroring out if no secure path is + available). <span class="since">"defaultMode" since + 0.9.12</span>. </p> <p> When SPICE has both a normal and TLS secured TCP port diff --git i/src/conf/domain_conf.h w/src/conf/domain_conf.h index 895ddc4..00178e1 100644 --- i/src/conf/domain_conf.h +++ w/src/conf/domain_conf.h @@ -1233,7 +1233,7 @@ struct _virDomainGraphicsDef { virDomainGraphicsAuthDef auth; unsigned int autoport :1; int channels[VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST]; - int defaultMode; + int defaultMode; /* enum virDomainGraphicsSpiceChannelMode */ int image; int jpeg; int zlib;
-- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 05/08/2012 07:00 AM, Alon Levy wrote:
Add "usbredir" channel to list of recognized spice channels.
RHBZ: 819498
Signed-off-by: Alon Levy <alevy@redhat.com> --- docs/formatdomain.html.in | 5 +- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 3 +- src/conf/domain_conf.h | 1 + .../qemuxml2argv-graphics-spice-usb-redir.args | 16 ++++++ .../qemuxml2argv-graphics-spice-usb-redir.xml | 53 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 6 +++ 7 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-usb-redir.xml
I've been convinced that this is a bug worth fixing for 0.9.12 (without properly listing all channels, you can inadvertently end up with insecure channels, and security fixes outweigh the risk of new XML after rc1 freeze).
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e1fe0c4..0525577 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2939,8 +2939,9 @@ qemu-kvm -net nic,model=? /dev/null include <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>. + <span class="since">since + 0.8.8</span>: <code>smartcard</code> and <span class="since"> + since 0.9.12</span> <code>usbredir</code>.
Missing a colon; I tweaked this slightly. ACK and pushed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Alon Levy -
Eric Blake