[libvirt] [PATCH v3 2/2] domain_conf: add "default" to list of valid spice channels

[Eric Blake]

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 at 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 at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120508/fac5a761/attachment-0001.sig>