You still forgot to amend the subject line, even though you noticed it
in v1 :)
On 04/14/2011 02:44 AM, 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>
>
> All new element are optional.
s/element/elements/
> ---
> Diff to v1:
> - _DEFAULT in all enums
> - VIR_ERR_CONFIG_UNSUPPORTED if TypeFromString fails
>
> docs/formatdomain.html.in | 12 ++
> docs/schemas/domain.rng | 50 ++++++++
> src/conf/domain_conf.c | 125 ++++++++++++++++++++
> src/conf/domain_conf.h | 46 +++++++
> src/libvirt_private.syms | 8 ++
> src/qemu/qemu_command.c | 12 ++
> .../qemuxml2argv-graphics-spice.args | 4 +-
> .../qemuxml2argv-graphics-spice.xml | 4 +
> 8 files changed, 260 insertions(+), 1 deletions(-)
Nice - docs and tests to go along with the new feature.
> +++ b/docs/formatdomain.html.in
> @@ -1664,6 +1664,18 @@ qemu-kvm -net nic,model=? /dev/null
> <channel name='main' mode='secure'/>
> <channel name='record' mode='insecure'/>
> </graphics></pre>
> + <p>
> +Spice supports variable compression settings for audio, images and streaming.
The example should show at least one of these. Also, pre-existing, but
other paragraphs are indented rather than flush left (xhtml doesn't care
about indentation when rendering, and our conversions to other formats
manage to deal with arbitrary indentation even though whitespace is
normally significant in xml).
> +This setting are accessible via <code>compression</code> attribute in all
s/This setting/These settings/
s/via /via the /
> +following elements: <code>image</code> to set image compression (accept
s/accept/accepts/g
> +<code>auto_glz</code>, <code>auto_lz</code>, <code>quic</code>,
> +<code>glz</code>, <code>lz</code>, <code>off</code>), <code>jpeg</code> for
> +JPEG compression for images over wan (accept <code>auto</code>,
> +<code>never</code>, <code>always</code>), <code>zlib</code> for configuring
> +wan image compression (accept <code>auto</code>, <code>never</code>,
> +<code>always</code>) and <code>playback</code> for enabling audio stream
> +compression (accept <code>on</code> or <code>off</code>)
s/$/./
Needs a 'since 0.9.1' designation.
> +++ b/docs/schemas/domain.rng
> @@ -1283,6 +1283,56 @@
> <empty/>
> </element>
> </zeroOrMore>
> + <optional>
> + <element name="image">
> + <attribute name="compression">
> + <choice>
Needs an <interleave>; otherwise, the RelaxNG validator will reject
this, even though the code will parse it:
<graphics type='spice' port='5901' tlsPort='-1' autoport='yes'>
<image compression='auto_glz'/>
<channel name='record' mode='insecure'/>
</graphics>
> @@ -3954,6 +3984,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) {
s/</<=/ - we want to reject compression='default' on input parsing,
since we never generate it on output dumping. Applies to all four
TypeFromString uses.
> +++ b/src/qemu/qemu_command.c
> @@ -4032,6 +4032,18 @@ qemuBuildCommandLine(virConnectPtr conn,
> break;
> }
> }
> + if (def->graphics[0]->data.spice.image)
> + virBufferVSprintf(&opt, ",image-compression=%s",
> + virDomainGraphicsSpiceImageCompressionTypeToString(def->graphics[0]->data.spice.image));
Yuck - these lines are way long, but I don't know of any way to shrink them.
Is there any worry that there are qemu versions that support spice but
not compression, where we would need to add a capability check in
qemu_caps.c? But if so, we can do that as a separate patch.
ACK with those nits. So here's what I squashed in (modulo reindentation
of domain.rng), before pushing.
diff --git i/docs/formatdomain.html.in w/docs/formatdomain.html.in
index 2ca3f76..928fa9b 100644
--- i/docs/formatdomain.html.in
+++ w/docs/formatdomain.html.in
@@ -1663,18 +1663,24 @@ qemu-kvm -net nic,model=? /dev/null
<graphics type='spice' port='-1' tlsPort='-1' autoport='yes'>
<channel name='main' mode='secure'/>
<channel name='record' mode='insecure'/>
+ <image compression='auto_glz'/>
</graphics></pre>
<p>
-Spice supports variable compression settings for audio, images and
streaming.
-This setting are accessible via <code>compression</code> attribute in all
-following elements: <code>image</code> to set image compression (accept
-<code>auto_glz</code>, <code>auto_lz</code>, <code>quic</code>,
-<code>glz</code>, <code>lz</code>, <code>off</code>), <code>jpeg</code> for
-JPEG compression for images over wan (accept <code>auto</code>,
-<code>never</code>, <code>always</code>), <code>zlib</code> for configuring
-wan image compression (accept <code>auto</code>, <code>never</code>,
-<code>always</code>) and <code>playback</code> for enabling audio stream
-compression (accept <code>on</code> or <code>off</code>)
+ Spice supports variable compression settings for audio,
+ images and streaming, <span class="since">since
+ 0.9.1</span>. These settings are accessible via
+ the <code>compression</code> attribute in all following
+ elements: <code>image</code> to set image compression
+ (accepts <code>auto_glz</code>, <code>auto_lz</code>,
+ <code>quic</code>, <code>glz</code>, <code>lz</code>,
+ <code>off</code>), <code>jpeg</code> for JPEG
+ compression for images over wan
+ (accepts <code>auto</code>, <code>never</code>,
+ <code>always</code>), <code>zlib</code> for configuring
+ wan image compression (accepts <code>auto</code>,
+ <code>never</code>, <code>always</code>)
+ and <code>playback</code> for enabling audio stream
+ compression (accepts <code>on</code> or <code>off</code>).
</p>
</dd>
<dt><code>"rdp"</code></dt>
diff --git i/docs/schemas/domain.rng w/docs/schemas/domain.rng
index f0578f8..7163c6e 100644
--- i/docs/schemas/domain.rng
+++ w/docs/schemas/domain.rng
@@ -1260,6 +1260,7 @@
<text/>
</attribute>
</optional>
+ <interleave>
<zeroOrMore>
<element name="channel">
<attribute name="name">
@@ -1333,6 +1334,7 @@
<empty/>
</element>
</optional>
+ </interleave>
</group>
<group>
<attribute name="type">
diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c
index cadaa0b..4afc489 100644
--- i/src/conf/domain_conf.c
+++ w/src/conf/domain_conf.c
@@ -3996,7 +3996,7 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, int
flags) {
}
if ((compressionVal =
-
virDomainGraphicsSpiceImageCompressionTypeFromString(compression)) < 0) {
+
virDomainGraphicsSpiceImageCompressionTypeFromString(compression)) <= 0) {
virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unknown spice image
compression %s"),
compression);
@@ -4017,7 +4017,7 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, int
flags) {
}
if ((compressionVal =
-
virDomainGraphicsSpiceJpegCompressionTypeFromString(compression)) < 0) {
+
virDomainGraphicsSpiceJpegCompressionTypeFromString(compression)) <= 0) {
virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unknown spice jpeg
compression %s"),
compression);
@@ -4038,7 +4038,7 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, int
flags) {
}
if ((compressionVal =
-
virDomainGraphicsSpiceZlibCompressionTypeFromString(compression)) < 0) {
+
virDomainGraphicsSpiceZlibCompressionTypeFromString(compression)) <= 0) {
virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unknown spice zlib
compression %s"),
compression);
@@ -4058,7 +4058,7 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, int
flags) {
}
if ((compressionVal =
-
virDomainGraphicsSpicePlaybackCompressionTypeFromString(compression)) < 0) {
+
virDomainGraphicsSpicePlaybackCompressionTypeFromString(compression)) <=
0) {
virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unknown spice playback
compression"));
VIR_FREE(compression);
--
Eric Blake eblake@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list