[libvirt] [PATCH] Spice: support auid, images and stream compression

[Daniel P. Berrange]

On Wed, Apr 13, 2011 at 04:37:00PM +0200, Michal Privoznik wrote:
> This extends the SPICE XML to allow variable compression settings for audio,
> images and streaming:
>     <graphics type='spice' port='5901' tlsPort='-1' autoport='yes'>
>         <image compression='auto_glz'/>
>         <jpeg compression='auto'/>
>         <zlib compression='auto'/>
>         <playback compression='on'/>
>     </graphics>


> +VIR_ENUM_IMPL(virDomainGraphicsSpiceImageCompression,
> +              VIR_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION_LAST,
> +              "auto_glz",
> +              "auto_lz",
> +              "quic",
> +              "glz",
> +              "lz",
> +              "off");
> +
> +VIR_ENUM_IMPL(virDomainGraphicsSpiceJpegCompression,
> +              VIR_DOMAIN_GRAPHICS_SPICE_JPEG_COMPRESSION_LAST,
> +              "auto",
> +              "never",
> +              "always");
> +
> +VIR_ENUM_IMPL(virDomainGraphicsSpiceZlibCompression,
> +              VIR_DOMAIN_GRAPHICS_SPICE_ZLIB_COMPRESSION_LAST,
> +              "auto",
> +              "never",
> +              "always");
> +
> +VIR_ENUM_IMPL(virDomainGraphicsSpicePlaybackCompression,
> +              VIR_DOMAIN_GRAPHICS_SPICE_PLAYBACK_COMPRESSION_LAST,
> +              "on",
> +              "off");
> +


> @@ -3917,6 +3943,11 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, int flags) {
>          if (virDomainGraphicsAuthDefParseXML(node, &def->data.spice.auth) < 0)
>              goto error;
>  
> +        def->data.spice.image = VIR_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION_LAST;
> +        def->data.spice.jpeg = VIR_DOMAIN_GRAPHICS_SPICE_JPEG_COMPRESSION_LAST;
> +        def->data.spice.zlib = VIR_DOMAIN_GRAPHICS_SPICE_ZLIB_COMPRESSION_LAST;
> +        def->data.spice.playback = VIR_DOMAIN_GRAPHICS_SPICE_PLAYBACK_COMPRESSION_LAST;
> +

Our normal coding practice is for the default setting to take the
value 0. Nothing should ever be assigned the value _LAST - that
only exists as a sentinal for the VIR_ENUM_IMPL() usage.

> @@ -3954,6 +3985,89 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, int flags) {
>                      VIR_FREE(mode);
>  
>                      def->data.spice.channels[nameval] = modeval;
> +                } else if (xmlStrEqual(cur->name, BAD_CAST "image")) {
> +                    const char *compression = virXMLPropString(cur, "compression");
> +                    int compressionVal;
> +
> +                    if (!compression) {
> +                        virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                                             _("spice image missing compression"));
> +                        goto error;
> +                    }
> +
> +                    if ((compressionVal =
> +                         virDomainGraphicsSpiceImageCompressionTypeFromString(compression)) < 0) {
> +                        virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> +                                             _("unknown spice image compression %s"),
> +                                             compression);
> +                        VIR_FREE(compression);
> +                        goto error;
> +                    }
> +                    VIR_FREE(compression);
> +
> +                    def->data.spice.image = compressionVal;
> +                } else if (xmlStrEqual(cur->name, BAD_CAST "jpeg")) {
> +                    const char *compression = virXMLPropString(cur, "compression");
> +                    int compressionVal;
> +
> +                    if (!compression) {
> +                        virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                                             _("spice jpeg missing compression"));
> +                        goto error;
> +                    }
> +
> +                    if ((compressionVal =
> +                         virDomainGraphicsSpiceJpegCompressionTypeFromString(compression)) < 0) {
> +                        virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> +                                             _("unknown spice jpeg compression %s"),
> +                                             compression);
> +                        VIR_FREE(compression);
> +                        goto error;
> +                    }
> +                    VIR_FREE(compression);
> +
> +                    def->data.spice.jpeg = compressionVal;
> +                } else if (xmlStrEqual(cur->name, BAD_CAST "zlib")) {
> +                    const char *compression = virXMLPropString(cur, "compression");
> +                    int compressionVal;
> +
> +                    if (!compression) {
> +                        virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                                             _("spice zlib missing compression"));
> +                        goto error;
> +                    }
> +
> +                    if ((compressionVal =
> +                         virDomainGraphicsSpiceZlibCompressionTypeFromString(compression)) < 0) {
> +                        virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> +                                             _("unknown spice zlib compression %s"),
> +                                             compression);
> +                        VIR_FREE(compression);
> +                        goto error;
> +                    }
> +
> +                    def->data.spice.zlib = compressionVal;
> +                } else if (xmlStrEqual(cur->name, BAD_CAST "playback")) {
> +                    const char *compression = virXMLPropString(cur, "compression");
> +                    int compressionVal;
> +
> +                    if (!compression) {
> +                        virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                                             _("spice playback missing compression"));
> +                        goto error;
> +                    }
> +
> +                    if ((compressionVal =
> +                         virDomainGraphicsSpicePlaybackCompressionTypeFromString(compression)) < 0) {
> +                        virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> +                                             _("unknown spice playback compression"));
> +                        VIR_FREE(compression);
> +                        goto error;
> +
> +                    }
> +                    VIR_FREE(compression);
> +
> +                    def->data.spice.playback = compressionVal;

All the error cases where TypeFromString failed, ought to be using
VIR_ERR_CONFIG_UNSUPPORTED as the error code.

> @@ -7817,6 +7931,22 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
>                                virDomainGraphicsSpiceChannelNameTypeToString(i),
>                                virDomainGraphicsSpiceChannelModeTypeToString(mode));
>          }
> +        if (def->data.spice.image !=
> +            VIR_DOMAIN_GRAPHICS_SPICE_IMAGE_COMPRESSION_LAST)
> +            virBufferVSprintf(buf, "      <image compression='%s'/>\n",
> +                              virDomainGraphicsSpiceImageCompressionTypeToString(def->data.spice.image));
> +        if (def->data.spice.jpeg !=
> +            VIR_DOMAIN_GRAPHICS_SPICE_JPEG_COMPRESSION_LAST)
> +            virBufferVSprintf(buf, "      <jpeg compression='%s'/>\n",
> +                              virDomainGraphicsSpiceJpegCompressionTypeToString(def->data.spice.jpeg));
> +        if (def->data.spice.zlib !=
> +            VIR_DOMAIN_GRAPHICS_SPICE_ZLIB_COMPRESSION_LAST)
> +            virBufferVSprintf(buf, "      <zlib compression='%s'/>\n",
> +                              virDomainGraphicsSpiceZlibCompressionTypeToString(def->data.spice.zlib));
> +        if (def->data.spice.playback !=
> +            VIR_DOMAIN_GRAPHICS_SPICE_PLAYBACK_COMPRESSION_LAST)
> +            virBufferVSprintf(buf, "      <playback compression='%s'/>\n",
> +                              virDomainGraphicsSpicePlaybackCompressionTypeToString(def->data.spice.playback));

This is related to the earlier note about default values being
zero. If we want to have these sub-elements skipped, then the
enums should likely get an additional entry '_DEFAULT' with
value zero, and then you can just do

   if (def->data.spice.playback)
      virBufferVSprintf(....)


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 :|