On 01/23/2012 07:26 PM, Zeeshan Ali (Khattak) wrote: > From: "Zeeshan Ali (Khattak)" <zeeshanak@xxxxxxxxx> > > 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@xxxxxxxxxx +1-919-301-3266 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