[libvirt] [PATCH] XML: Escape double-hyphens in XML comment

To quote <http://www.w3.org/TR/REC-xml/#sec-comments>:
For compatibility, the string "--" (double-hyphen) must not occur within comments.
For example this breaks creating snapshots: $ virsh snapshot-create-as $VM "comment--bug" $ xmllint --noout /var/lib/libvirt/qemu/snapshot/$VM/comment--bug.xml /var/lib/libvirt/qemu/snapshot/$VM/comment--bug.xml:4: parser error : Comment not terminated <!-- WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES virsh snapshot-edit $VM comment--bug $ /etc/init.d/libvirt-bin restart error : qemuDomainSnapshotLoad:367 : Failed to parse snapshot XML from file '/var/lib/libvirt/qemu/snapshot/$VM/comment--bug.xml' $ virsh snapshot-list ucs32-33787-performance Name Creation Time State ------------------------------------------------------------ Also applies to QEMU domains, where the name contains double-hyphens. As a work-around break any sequence of consecutive hyphens by inserting (arbitrarily chosen) backslashes, as there is no escaping rule defined as far as I know. I've not yet checked, how libvirt/libxml handles a document with multiple root nodes, as this would create interesting possibilities if I can name my domain --><domain...<devices>...<disk...<source dev="/dev/sda"/>... It might break on the "/" as this is the only prohibited character for QEMU domains, as the name is directly used as a file name. To fix such broken files (by hand), remove the broken comment by doing something like this after making sure libvirtd is stopped: sed -i -ne '/^<domainsnapshot>$/,$p' /var/lib/libvirt/qemu/snapshot/*/*.xml sed -i -ne '/^<domain /,$p' /etc/libvirt/qemu/*.xml sed -i -ne '/^<pool /,$p' /etc/libvirt/storage/*.xml @Debian: The bug is also in libvirt-0.9.12. Signed-off-by: Philipp Hahn <hahn@univention.de> --- src/util/virxml.c | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/src/util/virxml.c b/src/util/virxml.c index dd530a6..ad45e68 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -798,6 +798,34 @@ const char *virXMLPickShellSafeComment(const char *str1, const char *str2) return NULL; } +/* + * Break sequence of hyphens by inserting (arbitrarily chosen) backslashes. + * <http://www.w3.org/TR/REC-xml/#sec-comments>: + * > For compatibility, the string "--" (double-hyphen) must not occur within + * > comments. + */ +static int virXMLEmitEscapedComment(int fd, + const char *str) +{ + size_t len; + + if (!strstr(str, "--")) { + len = strlen(str); + if (safewrite(fd, str, len) != len) + return -1; + return 0; + } + + for (;*str;str++) { + if (safewrite(fd, str, 1) != 1) + return -1; + if (str[0] == '-' && str[1] == '-') + if (safewrite(fd, "\\", 1) != 1) + return -1; + } + return 0; +} + static int virXMLEmitWarning(int fd, const char *name, const char *cmd) @@ -822,16 +850,14 @@ static int virXMLEmitWarning(int fd, if (safewrite(fd, prologue, len) != len) return -1; - len = strlen(cmd); - if (safewrite(fd, cmd, len) != len) + if (virXMLEmitEscapedComment(fd, cmd) < 0) return -1; if (name) { if (safewrite(fd, " ", 1) != 1) return -1; - len = strlen(name); - if (safewrite(fd, name, len) != len) + if (virXMLEmitEscapedComment(fd, name) < 0) return -1; } -- 1.8.5.3

On 02/21/2014 10:01 AM, Philipp Hahn wrote:
To quote <http://www.w3.org/TR/REC-xml/#sec-comments>:
For compatibility, the string "--" (double-hyphen) must not occur within comments.
For example this breaks creating snapshots: $ virsh snapshot-create-as $VM "comment--bug" $ xmllint --noout /var/lib/libvirt/qemu/snapshot/$VM/comment--bug.xml /var/lib/libvirt/qemu/snapshot/$VM/comment--bug.xml:4: parser error : Comment not terminated <!-- WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES virsh snapshot-edit $VM comment--bug $ /etc/init.d/libvirt-bin restart error : qemuDomainSnapshotLoad:367 : Failed to parse snapshot XML from file '/var/lib/libvirt/qemu/snapshot/$VM/comment--bug.xml' $ virsh snapshot-list ucs32-33787-performance Name Creation Time State ------------------------------------------------------------
Also applies to QEMU domains, where the name contains double-hyphens.
This no longer reproduces with current master, it has been fixed by either of: commit 0b121614a2086a8e66ae1f004fe912ba7c1d8a75 Author: Ján Tomko <jtomko@redhat.com> CommitDate: 2012-10-29 14:38:43 +0100 xml: print uuids in the warning commit 9b704ab8235af010b1fda4886201aab02098b969 Author: Ján Tomko <jtomko@redhat.com> CommitDate: 2012-10-23 14:24:31 +0200 xml: omit domain name from comment if it contains double hyphen Jan

On 21.02.2014 12:18, Ján Tomko wrote:
This no longer reproduces with current master, it has been fixed by either of: commit 0b121614a2086a8e66ae1f004fe912ba7c1d8a75 Author: Ján Tomko <jtomko@redhat.com> CommitDate: 2012-10-29 14:38:43 +0100
xml: print uuids in the warning
commit 9b704ab8235af010b1fda4886201aab02098b969 Author: Ján Tomko <jtomko@redhat.com> CommitDate: 2012-10-23 14:24:31 +0200
xml: omit domain name from comment if it contains double hyphen
Thanks and sorry for the noise - your solution looks much better :-) Philipp

On 02/21/2014 02:01 AM, Philipp Hahn wrote:
To quote <http://www.w3.org/TR/REC-xml/#sec-comments>:
For compatibility, the string "--" (double-hyphen) must not occur within comments.
+/* + * Break sequence of hyphens by inserting (arbitrarily chosen) backslashes. + * <http://www.w3.org/TR/REC-xml/#sec-comments>: + * > For compatibility, the string "--" (double-hyphen) must not occur within + * > comments. + */ +static int virXMLEmitEscapedComment(int fd, + const char *str) +{ + size_t len; + + if (!strstr(str, "--")) { + len = strlen(str); + if (safewrite(fd, str, len) != len) + return -1; + return 0; + } + + for (;*str;str++) {
Style: spaces after ';'. This would also be a perfect candidate for Dan's virStringReplace patch, rather than open-coding your own replacement loop. That, and Jan already pointed out better patches to be backported :) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Eric Blake
-
Ján Tomko
-
Philipp Hahn