[libvirt] Allow custom metadata in domain configuration XML
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org> Applications can now insert custom nodes and hierarchies into domain cofiguration XML. Although currently not enforced, application are required to use their own namespaces on every custom node they insert. --- docs/formatdomain.html.in | 18 +++++++++ docs/schemas/domaincommon.rng | 26 +++++++++++++ src/conf/domain_conf.c | 24 ++++++++++++ src/conf/domain_conf.h | 3 ++ tests/domainsnapshotxml2xmlout/metadata.xml | 38 ++++++++++++++++++++ tests/domainsnapshotxml2xmltest.c | 1 + tests/qemuxml2argvdata/qemuxml2argv-metadata.args | 4 ++ tests/qemuxml2argvdata/qemuxml2argv-metadata.xml | 29 +++++++++++++++ .../qemuxml2xmloutdata/qemuxml2xmlout-metadata.xml | 29 +++++++++++++++ tests/qemuxml2xmltest.c | 2 + 10 files changed, 174 insertions(+), 0 deletions(-) create mode 100644 tests/domainsnapshotxml2xmlout/metadata.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-metadata.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-metadata.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-metadata.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index de9b480..fa7dc7c 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3548,6 +3548,24 @@ qemu-kvm -net nic,model=? /dev/null sub-element <code>label</code> are supported. </p> + <h3><a name="customMetadata">Custom metadata</a></h3> + +<pre> + ... + <metadata> + <app1:foo xmlns:app1="http://app1.org/app1/">..</app1:foo>; + <app2:bar xmlns:app2="http://app1.org/app2/">..</app2:bar>; + </metadata> + ...</pre> + + <dl> + <dt><code>metadata</code></dt> + <dd><code>metadata</code> node could be used by applications to + store custom metadata in the form of XML nodes/trees. Applications + must use custom namespaces on their XML nodes/trees. + <span class="since">Since 0.9.9</span></dd> + </dl> + <h2><a name="examples">Example configs</a></h2> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 2041dfb..b42d758 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -43,6 +43,9 @@ <ref name="seclabel"/> </optional> <optional> + <ref name="metadata"/> + </optional> + <optional> <ref name='qemucmdline'/> </optional> </interleave> @@ -2942,6 +2945,29 @@ </element> </define> + <define name="metadata"> + <element name="metadata"> + <zeroOrMore> + <ref name="customElement"/> + </zeroOrMore> + </element> + </define> + + <define name="customElement"> + <element> + <anyName/> + <zeroOrMore> + <choice> + <attribute> + <anyName/> + </attribute> + <text/> + <ref name="customElement"/> + </choice> + </zeroOrMore> + </element> + </define> + <!-- Type library diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8eed85b..fb78d93 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1500,6 +1500,9 @@ void virDomainDefFree(virDomainDefPtr def) if (def->namespaceData && def->ns.free) (def->ns.free)(def->namespaceData); + if (def->metadata) + xmlFreeNode(def->metadata); + VIR_FREE(def); } @@ -8072,6 +8075,11 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, def->os.smbios_mode = VIR_DOMAIN_SMBIOS_NONE; /* not present */ } + /* Extract custom metadata */ + if ((node = virXPathNode("./metadata[1]", ctxt)) != NULL) { + def->metadata = xmlCopyNode (node, 1); + } + /* we have to make a copy of all of the callback pointers here since * we won't have the virCaps structure available during free */ @@ -11833,6 +11841,22 @@ virDomainDefFormatInternal(virDomainDefPtr def, goto cleanup; } + /* Custom metadata comes at the end */ + if (def->metadata) { + xmlBufferPtr xmlbuf; + + xmlbuf = xmlBufferCreate(); + if (xmlNodeDump(xmlbuf, def->metadata->doc, def->metadata, 4, 1) < 0) { + xmlBufferFree(xmlbuf); + goto cleanup; + } + virBufferAdjustIndent(buf, 2); + virBufferAdd(buf, (char *) xmlBufferContent(xmlbuf), xmlBufferLength(xmlbuf)); + virBufferAdjustIndent(buf, -2); + xmlBufferFree(xmlbuf); + virBufferAddLit(buf, "\n"); + } + virBufferAddLit(buf, "</domain>\n"); if (virBufferError(buf)) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a49795c..3b522a9 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1525,6 +1525,9 @@ struct _virDomainDef { void *namespaceData; virDomainXMLNamespace ns; + + /* Application-specific custom metadata */ + xmlNodePtr metadata; }; enum virDomainTaintFlags { diff --git a/tests/domainsnapshotxml2xmlout/metadata.xml b/tests/domainsnapshotxml2xmlout/metadata.xml new file mode 100644 index 0000000..45180c9 --- /dev/null +++ b/tests/domainsnapshotxml2xmlout/metadata.xml @@ -0,0 +1,38 @@ +<domainsnapshot> + <name>my snap name</name> + <description>!@#$%^</description> + <state>running</state> + <parent> + <name>earlier_snap</name> + </parent> + <creationTime>1272917631</creationTime> + <domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219100</memory> + <currentMemory>219100</currentMemory> + <vcpu cpuset='1-4,8-20,525'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> + <metadata> + <app1:foo xmlns:app1="http://foo.org/">fooish</app1:foo> + <app2:bar xmlns:app2="http://bar.com/" maman="baz">barish</app2:bar> + </metadata> + </domain> +</domainsnapshot> diff --git a/tests/domainsnapshotxml2xmltest.c b/tests/domainsnapshotxml2xmltest.c index 90ff9bb..5c2e670 100644 --- a/tests/domainsnapshotxml2xmltest.c +++ b/tests/domainsnapshotxml2xmltest.c @@ -109,6 +109,7 @@ mymain(void) DO_TEST("noparent_nodescription_noactive", NULL, 0); DO_TEST("noparent_nodescription", NULL, 1); DO_TEST("noparent", "9d37b878-a7cc-9f9a-b78f-49b3abad25a8", 0); + DO_TEST("metadata", "c7a5fdbd-edaf-9455-926a-d65c16db1809", 0); virCapabilitiesFree(driver.caps); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-metadata.args b/tests/qemuxml2argvdata/qemuxml2argv-metadata.args new file mode 100644 index 0000000..651793d --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-metadata.args @@ -0,0 +1,4 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ +pc -m 214 -smp 1 -name QEMUGuest1 -nographic -monitor unix:/tmp/test-monitor,\ +server,nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial \ +none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-metadata.xml b/tests/qemuxml2argvdata/qemuxml2argv-metadata.xml new file mode 100644 index 0000000..a6888ee --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-metadata.xml @@ -0,0 +1,29 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219100</memory> + <currentMemory>219100</currentMemory> + <vcpu cpuset='1-4,8-20,525'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> + <metadata> + <app1:foo xmlns:app1="http://foo.org/">fooish</app1:foo> + <app2:bar xmlns:app2="http://bar.com/" maman="baz">barish</app2:bar> + </metadata> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-metadata.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-metadata.xml new file mode 100644 index 0000000..a6888ee --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-metadata.xml @@ -0,0 +1,29 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219100</memory> + <currentMemory>219100</currentMemory> + <vcpu cpuset='1-4,8-20,525'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <memballoon model='virtio'/> + </devices> + <metadata> + <app1:foo xmlns:app1="http://foo.org/">fooish</app1:foo> + <app2:bar xmlns:app2="http://bar.com/" maman="baz">barish</app2:bar> + </metadata> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 293c2a7..df317fd 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -210,6 +210,8 @@ mymain(void) DO_TEST_DIFFERENT("graphics-listen-network2"); DO_TEST_DIFFERENT("graphics-spice-timeout"); + DO_TEST_DIFFERENT("metadata"); + virCapabilitiesFree(driver.caps); return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); -- 1.7.7.5
On 01/23/2012 07:26 PM, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
Applications can now insert custom nodes and hierarchies into domain cofiguration XML. Although currently not enforced, application are
s/cofiguration/configuration/ s/application are/applications are/
required to use their own namespaces on every custom node they insert.
I also mentioned that we request only one element per namespace, in light of Dan's proposal for a new API that can get and set metadata on a per-namespace basis, so that applications don't have to filter all elements for the one they are interested in: https://www.redhat.com/archives/libvir-list/2012-January/msg00902.html
--- docs/formatdomain.html.in | 18 +++++++++ docs/schemas/domaincommon.rng | 26 +++++++++++++ src/conf/domain_conf.c | 24 ++++++++++++ src/conf/domain_conf.h | 3 ++ tests/domainsnapshotxml2xmlout/metadata.xml | 38 ++++++++++++++++++++ tests/domainsnapshotxml2xmltest.c | 1 + tests/qemuxml2argvdata/qemuxml2argv-metadata.args | 4 ++ tests/qemuxml2argvdata/qemuxml2argv-metadata.xml | 29 +++++++++++++++ .../qemuxml2xmloutdata/qemuxml2xmlout-metadata.xml | 29 +++++++++++++++ tests/qemuxml2xmltest.c | 2 +
Thanks for taking in my suggestions. 'git send-email --subject-prefix=PATCHv2' makes it easier to state when you are sending a v2. I added you to AUTHORS; let me know if I need to update the spelling to match your preferred listing.
+<pre> + ... + <metadata> + <app1:foo xmlns:app1="http://app1.org/app1/">..</app1:foo>; + <app2:bar xmlns:app2="http://app1.org/app2/">..</app2:bar>; + </metadata> + ...</pre>
Someday, we might try to list actual metadata (like danpb's suggestion of <virtman:guest xmlns:virtman="http://virt-manager.org/guets/1.0";>); but I'm okay waiting for some implementation experience before listing something that actually appears in typical distro setups.
+ + <dl> + <dt><code>metadata</code></dt> + <dd><code>metadata</code> node could be used by applications to + store custom metadata in the form of XML nodes/trees. Applications + must use custom namespaces on their XML nodes/trees. + <span class="since">Since 0.9.9</span></dd>
since 0.9.10; and document the further restriction of only one element per namespace. I also think that this section belongs up next to <description>; but I'll move it as a followup patch.
+++ b/docs/schemas/domaincommon.rng @@ -43,6 +43,9 @@ <ref name="seclabel"/> </optional> <optional> + <ref name="metadata"/> + </optional> + <optional>
I'm moving this up next to description now. That is, both <description> and <metadata> are overall metadata about the domain, and thus belong next to one another.
<ref name='qemucmdline'/> </optional> </interleave> @@ -2942,6 +2945,29 @@ </element> </define>
+ <define name="metadata"> + <element name="metadata"> + <zeroOrMore> + <ref name="customElement"/>
Indentation.
+++ b/src/conf/domain_conf.c @@ -1500,6 +1500,9 @@ void virDomainDefFree(virDomainDefPtr def) if (def->namespaceData && def->ns.free) (def->ns.free)(def->namespaceData);
+ if (def->metadata) + xmlFreeNode(def->metadata);
Useless use of if before free, since xmlFreeNode is documented as a no-op on NULL. I'll update our list of function names to include xmlFreeNode(), so that 'make syntax-check' will prevent this in the future, as a followup patch.
+ VIR_FREE(def); }
@@ -8072,6 +8075,11 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, def->os.smbios_mode = VIR_DOMAIN_SMBIOS_NONE; /* not present */ }
+ /* Extract custom metadata */ + if ((node = virXPathNode("./metadata[1]", ctxt)) != NULL) { + def->metadata = xmlCopyNode (node, 1);
Style - libvirt doesn't use space before ( in function calls.
@@ -11833,6 +11841,22 @@ virDomainDefFormatInternal(virDomainDefPtr def, goto cleanup; }
+ /* Custom metadata comes at the end */ + if (def->metadata) { + xmlBufferPtr xmlbuf;
I'll move this earlier in a subsequent patch.
+ + xmlbuf = xmlBufferCreate(); + if (xmlNodeDump(xmlbuf, def->metadata->doc, def->metadata, 4, 1) < 0) {
I'm still not convinced you got this right. </me 2 hours later, after reading libxml2 docs> Nope. The problem is that for indentation on output to work, we have to explicitly request ignoring extra whitespace on input, via xmlKeepBlanksDefault() (it defaults to 1, but must be 0 during the parse). Furthermore, the indentation generated by libxml defaults to 2 spaces per 1 level, which means that we need virBufferGetIndent / 2, to properly adjust things.
+ xmlBufferFree(xmlbuf); + goto cleanup; + } + virBufferAdjustIndent(buf, 2); + virBufferAdd(buf, (char *) xmlBufferContent(xmlbuf), xmlBufferLength(xmlbuf));
It's nice to stay in 80 columns, when possible.
diff --git a/tests/domainsnapshotxml2xmlout/metadata.xml b/tests/domainsnapshotxml2xmlout/metadata.xml new file mode 100644 index 0000000..45180c9 --- /dev/null +++ b/tests/domainsnapshotxml2xmlout/metadata.xml @@ -0,0 +1,38 @@ +<domainsnapshot> + <name>my snap name</name> + <description>!@#$%^</description> + <state>running</state> + <parent> + <name>earlier_snap</name> + </parent> + <creationTime>1272917631</creationTime> + <domain type='qemu'> + <name>QEMUGuest1</name>
+ </devices> + <metadata> + <app1:foo xmlns:app1="http://foo.org/">fooish</app1:foo> + <app2:bar xmlns:app2="http://bar.com/" maman="baz">barish</app2:bar> + </metadata>
Looks like a good test; the reason that you couldn't trigger any indentation issues was that you weren't actually regenerating indentation, but reusing the spacing that was present on input. I'll have to tweak this as well when I reorder things. Here's what I'm squashing in to your patch, before I work on the followups to reorder things. diff --git i/docs/formatdomain.html.in w/docs/formatdomain.html.in index 69a85b6..6b025e8 100644 --- i/docs/formatdomain.html.in +++ w/docs/formatdomain.html.in @@ -3559,7 +3559,7 @@ qemu-kvm -net nic,model=? /dev/null <h3><a name="customMetadata">Custom metadata</a></h3> <pre> - ... + ... <metadata> <app1:foo xmlns:app1="http://app1.org/app1/">..</app1:foo>; <app2:bar xmlns:app2="http://app1.org/app2/">..</app2:bar>; @@ -3568,10 +3568,12 @@ qemu-kvm -net nic,model=? /dev/null <dl> <dt><code>metadata</code></dt> - <dd><code>metadata</code> node could be used by applications to + <dd>The <code>metadata</code> node can be used by applications to store custom metadata in the form of XML nodes/trees. Applications - must use custom namespaces on their XML nodes/trees. - <span class="since">Since 0.9.9</span></dd> + must use custom namespaces on their XML nodes/trees, with only + one top-level element per namespace (if the application needs + structure, they should have sub-elements to their namespace + element). <span class="since">Since 0.9.10</span></dd> </dl> <h2><a name="examples">Example configs</a></h2> diff --git i/docs/schemas/domaincommon.rng w/docs/schemas/domaincommon.rng index b42d758..4fa968d 100644 --- i/docs/schemas/domaincommon.rng +++ w/docs/schemas/domaincommon.rng @@ -26,6 +26,9 @@ <ref name="description"/> </optional> <optional> + <ref name="metadata"/> + </optional> + <optional> <ref name="cpu"/> </optional> <optional> @@ -43,9 +46,6 @@ <ref name="seclabel"/> </optional> <optional> - <ref name="metadata"/> - </optional> - <optional> <ref name='qemucmdline'/> </optional> </interleave> @@ -2948,7 +2948,7 @@ <define name="metadata"> <element name="metadata"> <zeroOrMore> - <ref name="customElement"/> + <ref name="customElement"/> </zeroOrMore> </element> </define> diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c index fb78d93..f2c8d02 100644 --- i/src/conf/domain_conf.c +++ w/src/conf/domain_conf.c @@ -1500,8 +1500,7 @@ void virDomainDefFree(virDomainDefPtr def) if (def->namespaceData && def->ns.free) (def->ns.free)(def->namespaceData); - if (def->metadata) - xmlFreeNode(def->metadata); + xmlFreeNode(def->metadata); VIR_FREE(def); } @@ -8077,7 +8076,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* Extract custom metadata */ if ((node = virXPathNode("./metadata[1]", ctxt)) != NULL) { - def->metadata = xmlCopyNode (node, 1); + def->metadata = xmlCopyNode(node, 1); } /* we have to make a copy of all of the callback pointers here since @@ -8219,6 +8218,7 @@ virDomainDefParse(const char *xmlStr, { xmlDocPtr xml; virDomainDefPtr def = NULL; + int keepBlanksDefault = xmlKeepBlanksDefault(0); if ((xml = virXMLParse(filename, xmlStr, _("(domain_definition)")))) { def = virDomainDefParseNode(caps, xml, xmlDocGetRootElement(xml), @@ -8226,6 +8226,7 @@ virDomainDefParse(const char *xmlStr, xmlFreeDoc(xml); } + xmlKeepBlanksDefault(keepBlanksDefault); return def; } @@ -8319,6 +8320,7 @@ virDomainObjParseFile(virCapsPtr caps, { xmlDocPtr xml; virDomainObjPtr obj = NULL; + int keepBlanksDefault = xmlKeepBlanksDefault(0); if ((xml = virXMLParseFile(filename))) { obj = virDomainObjParseNode(caps, xml, @@ -8327,6 +8329,7 @@ virDomainObjParseFile(virCapsPtr caps, xmlFreeDoc(xml); } + xmlKeepBlanksDefault(keepBlanksDefault); return obj; } @@ -11844,17 +11847,25 @@ virDomainDefFormatInternal(virDomainDefPtr def, /* Custom metadata comes at the end */ if (def->metadata) { xmlBufferPtr xmlbuf; - + int oldIndentTreeOutput = xmlIndentTreeOutput; + + /* Indentation on output requires that we previously set + * xmlKeepBlanksDefault to 0 when parsing; also, libxml does 2 + * spaces per level of indentation of intermediate elements, + * but no leading indentation before the starting element. + * Thankfully, libxml maps what looks like globals into + * thread-local uses, so we are thread-safe. */ + xmlIndentTreeOutput = 1; xmlbuf = xmlBufferCreate(); - if (xmlNodeDump(xmlbuf, def->metadata->doc, def->metadata, 4, 1) < 0) { + if (xmlNodeDump(xmlbuf, def->metadata->doc, def->metadata, + virBufferGetIndent(buf, false) / 2 + 1, 1) < 0) { xmlBufferFree(xmlbuf); + xmlIndentTreeOutput = oldIndentTreeOutput; goto cleanup; } - virBufferAdjustIndent(buf, 2); - virBufferAdd(buf, (char *) xmlBufferContent(xmlbuf), xmlBufferLength(xmlbuf)); - virBufferAdjustIndent(buf, -2); + virBufferAsprintf(buf, " %s\n", (char *) xmlBufferContent(xmlbuf)); xmlBufferFree(xmlbuf); - virBufferAddLit(buf, "\n"); + xmlIndentTreeOutput = oldIndentTreeOutput; } virBufferAddLit(buf, "</domain>\n"); @@ -12541,11 +12552,14 @@ virDomainSnapshotDefParseString(const char *xmlStr, struct timeval tv; int active; char *tmp; + int keepBlanksDefault = xmlKeepBlanksDefault(0); xml = virXMLParseCtxt(NULL, xmlStr, _("(domain_snapshot)"), &ctxt); if (!xml) { + xmlKeepBlanksDefault(keepBlanksDefault); return NULL; } + xmlKeepBlanksDefault(keepBlanksDefault); if (VIR_ALLOC(def) < 0) { virReportOOMError(); diff --git i/tests/qemuxml2argvdata/qemuxml2argv-metadata.xml w/tests/qemuxml2argvdata/qemuxml2argv-metadata.xml index a6888ee..aa9de07 100644 --- i/tests/qemuxml2argvdata/qemuxml2argv-metadata.xml +++ w/tests/qemuxml2argvdata/qemuxml2argv-metadata.xml @@ -23,7 +23,8 @@ <memballoon model='virtio'/> </devices> + <!-- intentional mis-indentation --> <metadata> - <app1:foo xmlns:app1="http://foo.org/">fooish</app1:foo> - <app2:bar xmlns:app2="http://bar.com/" maman="baz">barish</app2:bar> + <app1:foo xmlns:app1="http://foo.org/">fooish</app1:foo> + <app2:bar xmlns:app2="http://bar.com/" maman="baz">barish</app2:bar> </metadata> </domain> -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Noticed while reviewing the previous patch; thankfully, there are no violations. * cfg.mk (useless_free_options): Add xmlFreeNode. ---
+ if (def->metadata) + xmlFreeNode(def->metadata); Useless use of if before free, since xmlFreeNode is documented as a no-op on NULL. I'll update our list of function names to include xmlFreeNode(), so that 'make syntax-check' will prevent this in the future, as a followup patch.
Pushing under the trivial rule. cfg.mk | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/cfg.mk b/cfg.mk index 20a085e..d853caf 100644 --- a/cfg.mk +++ b/cfg.mk @@ -171,6 +171,7 @@ useless_free_options = \ --name=xmlBufferFree \ --name=xmlFree \ --name=xmlFreeDoc \ + --name=xmlFreeNode \ --name=xmlXPathFreeContext \ --name=xmlXPathFreeObject -- 1.7.7.5
It's better to group all the metadata together. This is a cosmetic output change; since the RNG allows interleave, it doesn't matter where the user stuck it on input, and an XPath query will find it in the same location when parsing the output. * src/conf/domain_conf.c (virDomainDefFormatInternal): Output metadata earlier. * docs/formatdomain.html.in: Update documentation. * tests/domainsnapshotxml2xmlout/metadata.xml: Update test. * tests/qemuxml2xmloutdata/qemuxml2xmlout-metadata.xml: Likewise. --- As threatened here:
I also think that this section belongs up next to <description>; but I'll move it as a followup patch.
docs/formatdomain.html.in | 39 +++++++---------- src/conf/domain_conf.c | 47 ++++++++++---------- tests/domainsnapshotxml2xmlout/metadata.xml | 8 ++-- .../qemuxml2xmloutdata/qemuxml2xmlout-metadata.xml | 8 ++-- 4 files changed, 47 insertions(+), 55 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6b025e8..dca9a81 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -33,6 +33,10 @@ <name>fv0</name> <uuid>4dea22b31d52d8f32516782e98ab3fa0</uuid> <description>Some human readable description</description> + <metadata> + <app1:foo xmlns:app1="http://app1.org/app1/">..</app1:foo>; + <app2:bar xmlns:app2="http://app1.org/app2/">..</app2:bar>; + </metadata> ...</pre> <dl> @@ -56,9 +60,18 @@ <dt><code>description</code></dt> <dd>The content of the <code>description</code> element provides a - human readable description of the virtual machine. This data is not - used by libvirt in any way, it can contain any information the user - wants. <span class="since">Since 0.7.2</span></dd> + human readable description of the virtual machine. This data is not + used by libvirt in any way, it can contain any information the user + wants. <span class="since">Since 0.7.2</span></dd> + + <dt><code>metadata</code></dt> + <dd>The <code>metadata</code> node can be used by applications + to store custom metadata in the form of XML + nodes/trees. Applications must use custom namespaces on their + XML nodes/trees, with only one top-level element per namespace + (if the application needs structure, they should have + sub-elements to their namespace + element). <span class="since">Since 0.9.10</span></dd> </dl> <h3><a name="elementsOS">Operating system booting</a></h3> @@ -3556,26 +3569,6 @@ qemu-kvm -net nic,model=? /dev/null sub-element <code>label</code> are supported. </p> - <h3><a name="customMetadata">Custom metadata</a></h3> - -<pre> - ... - <metadata> - <app1:foo xmlns:app1="http://app1.org/app1/">..</app1:foo>; - <app2:bar xmlns:app2="http://app1.org/app2/">..</app2:bar>; - </metadata> - ...</pre> - - <dl> - <dt><code>metadata</code></dt> - <dd>The <code>metadata</code> node can be used by applications to - store custom metadata in the form of XML nodes/trees. Applications - must use custom namespaces on their XML nodes/trees, with only - one top-level element per namespace (if the application needs - structure, they should have sub-elements to their namespace - element). <span class="since">Since 0.9.10</span></dd> - </dl> - <h2><a name="examples">Example configs</a></h2> <p> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f2c8d02..e872396 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11431,6 +11431,29 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferEscapeString(buf, " <description>%s</description>\n", def->description); + if (def->metadata) { + xmlBufferPtr xmlbuf; + int oldIndentTreeOutput = xmlIndentTreeOutput; + + /* Indentation on output requires that we previously set + * xmlKeepBlanksDefault to 0 when parsing; also, libxml does 2 + * spaces per level of indentation of intermediate elements, + * but no leading indentation before the starting element. + * Thankfully, libxml maps what looks like globals into + * thread-local uses, so we are thread-safe. */ + xmlIndentTreeOutput = 1; + xmlbuf = xmlBufferCreate(); + if (xmlNodeDump(xmlbuf, def->metadata->doc, def->metadata, + virBufferGetIndent(buf, false) / 2 + 1, 1) < 0) { + xmlBufferFree(xmlbuf); + xmlIndentTreeOutput = oldIndentTreeOutput; + goto cleanup; + } + virBufferAsprintf(buf, " %s\n", (char *) xmlBufferContent(xmlbuf)); + xmlBufferFree(xmlbuf); + xmlIndentTreeOutput = oldIndentTreeOutput; + } + virBufferAsprintf(buf, " <memory>%lu</memory>\n", def->mem.max_balloon); virBufferAsprintf(buf, " <currentMemory>%lu</currentMemory>\n", def->mem.cur_balloon); @@ -11844,30 +11867,6 @@ virDomainDefFormatInternal(virDomainDefPtr def, goto cleanup; } - /* Custom metadata comes at the end */ - if (def->metadata) { - xmlBufferPtr xmlbuf; - int oldIndentTreeOutput = xmlIndentTreeOutput; - - /* Indentation on output requires that we previously set - * xmlKeepBlanksDefault to 0 when parsing; also, libxml does 2 - * spaces per level of indentation of intermediate elements, - * but no leading indentation before the starting element. - * Thankfully, libxml maps what looks like globals into - * thread-local uses, so we are thread-safe. */ - xmlIndentTreeOutput = 1; - xmlbuf = xmlBufferCreate(); - if (xmlNodeDump(xmlbuf, def->metadata->doc, def->metadata, - virBufferGetIndent(buf, false) / 2 + 1, 1) < 0) { - xmlBufferFree(xmlbuf); - xmlIndentTreeOutput = oldIndentTreeOutput; - goto cleanup; - } - virBufferAsprintf(buf, " %s\n", (char *) xmlBufferContent(xmlbuf)); - xmlBufferFree(xmlbuf); - xmlIndentTreeOutput = oldIndentTreeOutput; - } - virBufferAddLit(buf, "</domain>\n"); if (virBufferError(buf)) diff --git a/tests/domainsnapshotxml2xmlout/metadata.xml b/tests/domainsnapshotxml2xmlout/metadata.xml index 45180c9..f0ad70b 100644 --- a/tests/domainsnapshotxml2xmlout/metadata.xml +++ b/tests/domainsnapshotxml2xmlout/metadata.xml @@ -9,6 +9,10 @@ <domain type='qemu'> <name>QEMUGuest1</name> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <metadata> + <app1:foo xmlns:app1="http://foo.org/">fooish</app1:foo> + <app2:bar xmlns:app2="http://bar.com/" maman="baz">barish</app2:bar> + </metadata> <memory>219100</memory> <currentMemory>219100</currentMemory> <vcpu cpuset='1-4,8-20,525'>1</vcpu> @@ -30,9 +34,5 @@ <controller type='ide' index='0'/> <memballoon model='virtio'/> </devices> - <metadata> - <app1:foo xmlns:app1="http://foo.org/">fooish</app1:foo> - <app2:bar xmlns:app2="http://bar.com/" maman="baz">barish</app2:bar> - </metadata> </domain> </domainsnapshot> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-metadata.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-metadata.xml index a6888ee..a029404 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-metadata.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-metadata.xml @@ -1,6 +1,10 @@ <domain type='qemu'> <name>QEMUGuest1</name> <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <metadata> + <app1:foo xmlns:app1="http://foo.org/">fooish</app1:foo> + <app2:bar xmlns:app2="http://bar.com/" maman="baz">barish</app2:bar> + </metadata> <memory>219100</memory> <currentMemory>219100</currentMemory> <vcpu cpuset='1-4,8-20,525'>1</vcpu> @@ -22,8 +26,4 @@ <controller type='ide' index='0'/> <memballoon model='virtio'/> </devices> - <metadata> - <app1:foo xmlns:app1="http://foo.org/">fooish</app1:foo> - <app2:bar xmlns:app2="http://bar.com/" maman="baz">barish</app2:bar> - </metadata> </domain> -- 1.7.7.5
On Wed, Jan 25, 2012 at 2:28 AM, Eric Blake <eblake@redhat.com> wrote:
It's better to group all the metadata together. This is a cosmetic output change; since the RNG allows interleave, it doesn't matter where the user stuck it on input, and an XPath query will find it in the same location when parsing the output.
Looks right. ACK! -- Regards, Zeeshan Ali (Khattak) FSF member#5124
On 01/24/2012 05:49 PM, Zeeshan Ali (Khattak) wrote:
On Wed, Jan 25, 2012 at 2:28 AM, Eric Blake <eblake@redhat.com> wrote:
It's better to group all the metadata together. This is a cosmetic output change; since the RNG allows interleave, it doesn't matter where the user stuck it on input, and an XPath query will find it in the same location when parsing the output.
Looks right. ACK!
Thanks; pushed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Zeeshan Ali (Khattak)