[libvirt] [PATCH] conf: Fix incorrect spice graphic XML format on compression options

If spice graphics has no <channel> elements, the output graphics XML is messed up. To prevent this, we need to end the <graphics> element just before adding any compression selecting elements. --- src/conf/domain_conf.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3f2fb11..2800db5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8082,6 +8082,12 @@ virDomainGraphicsDefFormat(virBufferPtr buf, virDomainGraphicsSpiceChannelNameTypeToString(i), virDomainGraphicsSpiceChannelModeTypeToString(mode)); } + if (!children && (def->data.spice.image || def->data.spice.jpeg || + def->data.spice.zlib || def->data.spice.playback || + def->data.spice.streaming)) { + virBufferAddLit(buf, ">\n"); + children = 1; + } if (def->data.spice.image) virBufferAsprintf(buf, " <image compression='%s'/>\n", virDomainGraphicsSpiceImageCompressionTypeToString(def->data.spice.image)); -- 1.7.5.rc3

On 24.05.2011 13:43, Michal Privoznik wrote:
If spice graphics has no <channel> elements, the output graphics XML is messed up. To prevent this, we need to end the <graphics> element By messing up I mean something like this: ...... <graphics type='spice' autoport='yes' <image compression='auto_glz'/> <jpeg compression='auto'/> <zlib compression='auto'/> <playback compression='on'/> /> ...... just before adding any compression selecting elements. --- src/conf/domain_conf.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3f2fb11..2800db5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8082,6 +8082,12 @@ virDomainGraphicsDefFormat(virBufferPtr buf, virDomainGraphicsSpiceChannelNameTypeToString(i), virDomainGraphicsSpiceChannelModeTypeToString(mode)); } + if (!children && (def->data.spice.image || def->data.spice.jpeg || + def->data.spice.zlib || def->data.spice.playback || + def->data.spice.streaming)) { + virBufferAddLit(buf, ">\n"); + children = 1; + } if (def->data.spice.image) virBufferAsprintf(buf, " <image compression='%s'/>\n", virDomainGraphicsSpiceImageCompressionTypeToString(def->data.spice.image));

2011/5/24 Michal Prívozník <mprivozn@redhat.com>:
On 24.05.2011 13:43, Michal Privoznik wrote:
If spice graphics has no <channel> elements, the output graphics XML is messed up. To prevent this, we need to end the <graphics> element By messing up I mean something like this: ...... <graphics type='spice' autoport='yes' <image compression='auto_glz'/> <jpeg compression='auto'/> <zlib compression='auto'/> <playback compression='on'/> /> ...... just before adding any compression selecting elements. --- src/conf/domain_conf.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3f2fb11..2800db5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8082,6 +8082,12 @@ virDomainGraphicsDefFormat(virBufferPtr buf, virDomainGraphicsSpiceChannelNameTypeToString(i), virDomainGraphicsSpiceChannelModeTypeToString(mode)); } + if (!children && (def->data.spice.image || def->data.spice.jpeg || + def->data.spice.zlib || def->data.spice.playback || + def->data.spice.streaming)) { + virBufferAddLit(buf, ">\n"); + children = 1; + } if (def->data.spice.image) virBufferAsprintf(buf, " <image compression='%s'/>\n", virDomainGraphicsSpiceImageCompressionTypeToString(def->data.spice.image));
Could you add test file to the qemu tests that shows the broken formatting and passes cleanly after your patch is applied? This way we can avoid future regressions in XML formatting in this area, because will complain. Matthias

On Tue, May 24, 2011 at 13:43:30 +0200, Michal Privoznik wrote:
If spice graphics has no <channel> elements, the output graphics XML is messed up. To prevent this, we need to end the <graphics> element just before adding any compression selecting elements. --- src/conf/domain_conf.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3f2fb11..2800db5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8082,6 +8082,12 @@ virDomainGraphicsDefFormat(virBufferPtr buf, virDomainGraphicsSpiceChannelNameTypeToString(i), virDomainGraphicsSpiceChannelModeTypeToString(mode)); } + if (!children && (def->data.spice.image || def->data.spice.jpeg || + def->data.spice.zlib || def->data.spice.playback || + def->data.spice.streaming)) { + virBufferAddLit(buf, ">\n"); + children = 1; + } if (def->data.spice.image) virBufferAsprintf(buf, " <image compression='%s'/>\n", virDomainGraphicsSpiceImageCompressionTypeToString(def->data.spice.image));
I feel like it's about time to rework all XML formating to just create a tree of XML nodes and use one universal function to transform the tree into it's string representation (aka xmlSaveTree() from libxml). Am I the only one? ACK Jirka

On 31.05.2011 14:24, Jiri Denemark wrote:
On Tue, May 24, 2011 at 13:43:30 +0200, Michal Privoznik wrote:
If spice graphics has no <channel> elements, the output graphics XML is messed up. To prevent this, we need to end the <graphics> element just before adding any compression selecting elements. --- src/conf/domain_conf.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3f2fb11..2800db5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8082,6 +8082,12 @@ virDomainGraphicsDefFormat(virBufferPtr buf, virDomainGraphicsSpiceChannelNameTypeToString(i), virDomainGraphicsSpiceChannelModeTypeToString(mode)); } + if (!children && (def->data.spice.image || def->data.spice.jpeg || + def->data.spice.zlib || def->data.spice.playback || + def->data.spice.streaming)) { + virBufferAddLit(buf, ">\n"); + children = 1; + } if (def->data.spice.image) virBufferAsprintf(buf, " <image compression='%s'/>\n", virDomainGraphicsSpiceImageCompressionTypeToString(def->data.spice.image));
I feel like it's about time to rework all XML formating to just create a tree of XML nodes and use one universal function to transform the tree into it's string representation (aka xmlSaveTree() from libxml). Am I the only one?
ACK
Jirka
Thanks, pushed. The test patch will follow. Michal
participants (4)
-
Jiri Denemark
-
Matthias Bolte
-
Michal Privoznik
-
Michal Prívozník