[libvirt] [PATCH] Allow custom metadata in network configuration XML
From: Brandon Bennett <bbennett@fb.com> This replicates the metadata field found in the domain configuration and adds it to the network configuration XML. --- docs/formatnetwork.html.in | 13 +++++++++++++ docs/schemas/basictypes.rng | 23 +++++++++++++++++++++++ docs/schemas/domaincommon.rng | 23 ----------------------- docs/schemas/network.rng | 5 +++++ src/conf/network_conf.c | 35 ++++++++++++++++++++++++++++++++++- src/conf/network_conf.h | 3 +++ tests/networkxml2xmlin/metadata.xml | 10 ++++++++++ tests/networkxml2xmlout/metadata.xml | 10 ++++++++++ tests/networkxml2xmltest.c | 1 + 9 files changed, 99 insertions(+), 24 deletions(-) create mode 100644 tests/networkxml2xmlin/metadata.xml create mode 100644 tests/networkxml2xmlout/metadata.xml diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 1cea931..15ebf0c 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -38,6 +38,10 @@ <network ipv6='yes' trustGuestRxFilters='no'> <name>default</name> <uuid>3e3fce45-4f53-4fa7-bb32-11f34168b82b</uuid> + <metadata> + <app1:foo xmlns:app1="http://app1.org/app1/">..</app1:foo>; + <app2:bar xmlns:app2="http://app1.org/app2/">..</app2:bar>; + </metadata> ...</pre> <dl> @@ -54,6 +58,12 @@ The format must be RFC 4122 compliant, eg <code>3e3fce45-4f53-4fa7-bb32-11f34168b82b</code>. If omitted when defining/creating a new network, a random UUID is generated. <span class="since">Since 0.3.0</span></dd> + <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 1.3.6</span></dd> <dt><code>ipv6</code></dt> <dd>When set to <code>yes</code>, the optional parameter <code>ipv6</code> enables @@ -73,6 +83,9 @@ override the setting in the network.</dd> </dl> ++ + + <h3><a name="elementsConnect">Connectivity</a></h3> <p> diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 83fd4ec..474ad77 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -495,4 +495,27 @@ </choice> </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> + </grammar> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 162c2e0..78eb3f5 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -5338,29 +5338,6 @@ </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/docs/schemas/network.rng b/docs/schemas/network.rng index 4edb6eb..b67a5ea 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -37,6 +37,11 @@ <text/> </element> + <!-- <metadata> element --> + <optional> + <ref name="metadata"/> + </optional> + <!-- <uuid> element --> <optional> <element name="uuid"><ref name="UUID"/></element> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 02b8cd7..4239c32 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -419,6 +419,9 @@ virNetworkDefFree(virNetworkDefPtr def) virNetDevBandwidthFree(def->bandwidth); virNetDevVlanClear(&def->vlan); + + xmlFreeNode(def->metadata); + VIR_FREE(def); } @@ -2059,6 +2062,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) xmlNodePtr save = ctxt->node; xmlNodePtr bandwidthNode = NULL; xmlNodePtr vlanNode; + xmlNodePtr metadataNode = NULL; if (VIR_ALLOC(def) < 0) return NULL; @@ -2388,8 +2392,12 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) } break; } - VIR_FREE(stp); + + /* Extract custom metadata */ + if ((metadataNode = virXPathNode("./metadata[1]", ctxt)) != NULL) + def->metadata = xmlCopyNode(metadataNode, 1); + ctxt->node = save; return def; @@ -2412,12 +2420,14 @@ virNetworkDefParse(const char *xmlStr, { xmlDocPtr xml; virNetworkDefPtr def = NULL; + int keepBlanksDefault = xmlKeepBlanksDefault(0); if ((xml = virXMLParse(filename, xmlStr, _("(network_definition)")))) { def = virNetworkDefParseNode(xml, xmlDocGetRootElement(xml)); xmlFreeDoc(xml); } + xmlKeepBlanksDefault(keepBlanksDefault); return def; } @@ -2736,6 +2746,29 @@ virNetworkDefFormatBuf(virBufferPtr buf, virUUIDFormat(uuid, uuidstr); virBufferAsprintf(buf, "<uuid>%s</uuid>\n", uuidstr); + 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) < 0) { + xmlBufferFree(xmlbuf); + xmlIndentTreeOutput = oldIndentTreeOutput; + goto error; + } + virBufferAsprintf(buf, "%s\n", (char *) xmlBufferContent(xmlbuf)); + xmlBufferFree(xmlbuf); + xmlIndentTreeOutput = oldIndentTreeOutput; + } + if (def->forward.type != VIR_NETWORK_FORWARD_NONE) { const char *dev = NULL; if (!def->forward.npfs) diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 0d34dfe..4481f60 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -253,6 +253,9 @@ struct _virNetworkDef { virNetDevBandwidthPtr bandwidth; virNetDevVlan vlan; int trustGuestRxFilters; /* enum virTristateBool */ + + /* Application-specific custom metadata */ + xmlNodePtr metadata; }; typedef struct _virNetworkObj virNetworkObj; diff --git a/tests/networkxml2xmlin/metadata.xml b/tests/networkxml2xmlin/metadata.xml new file mode 100644 index 0000000..c075f93 --- /dev/null +++ b/tests/networkxml2xmlin/metadata.xml @@ -0,0 +1,10 @@ +<network> + <name>host-bridge-net</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a8e</uuid> + <forward mode='bridge'/> + <bridge name='br0'/> + <metadata> + <app1:foo xmlns:app1="http://foo.org/">fooish</app1:foo> + <app2:bar xmlns:app2="http://bar.com/" maman="baz">barish</app2:bar> + </metadata> +</network> diff --git a/tests/networkxml2xmlout/metadata.xml b/tests/networkxml2xmlout/metadata.xml new file mode 100644 index 0000000..a9364ab --- /dev/null +++ b/tests/networkxml2xmlout/metadata.xml @@ -0,0 +1,10 @@ +<network> + <name>host-bridge-net</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a8e</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> + <forward mode='bridge'/> + <bridge name='br0'/> +</network> diff --git a/tests/networkxml2xmltest.c b/tests/networkxml2xmltest.c index d65f6aa..2a2c348 100644 --- a/tests/networkxml2xmltest.c +++ b/tests/networkxml2xmltest.c @@ -153,6 +153,7 @@ mymain(void) DO_TEST("host-bridge-no-flood"); DO_TEST_PARSE_ERROR("hostdev-duplicate"); DO_TEST_PARSE_ERROR("passthrough-duplicate"); + DO_TEST("metadata"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.7.4
2016-06-23 1:05 GMT+03:00 Brnadon Bennett <bbennett@fb.com>:
From: Brandon Bennett <bbennett@fb.com>
This replicates the metadata field found in the domain configuration and adds it to the network configuration XML.
Why not use domain metadata? WHy you create metadata for network and not want to use domain metadata? -- Vasiliy Tolstov, e-mail: v.tolstov@yoctocloud.net
On 06/23/2016 02:31 PM, Vasiliy Tolstov wrote:
2016-06-23 1:05 GMT+03:00 Brnadon Bennett <bbennett@fb.com>:
From: Brandon Bennett <bbennett@fb.com>
This replicates the metadata field found in the domain configuration and adds it to the network configuration XML.
Why not use domain metadata? WHy you create metadata for network and not want to use domain metadata?
I talked with him about it on IRC yesterday. He wants to do [something] in a network hook script, and presumably wants some bit of local configuration that is the same for all domains connected to a network, but can change depending on the network. It makes sense to me that any toplevel persistent piece of libvirt config should allow a <metadata> element with user-defined contents (so pools and volumes might have it too, if somebody had the need).
2016-06-23 22:37 GMT+03:00 Laine Stump <laine@laine.org>:
I talked with him about it on IRC yesterday. He wants to do [something] in a network hook script, and presumably wants some bit of local configuration that is the same for all domains connected to a network, but can change depending on the network. It makes sense to me that any toplevel persistent piece of libvirt config should allow a <metadata> element with user-defined contents (so pools and volumes might have it too, if somebody had the need).
Hm very interesting idea. I think this may be useful. Thanks for patch. -- Vasiliy Tolstov, e-mail: v.tolstov@yoctocloud.net
On 06/22/2016 06:05 PM, Brnadon Bennett wrote:
From: Brandon Bennett <bbennett@fb.com>
This replicates the metadata field found in the domain configuration and adds it to the network configuration XML. --- docs/formatnetwork.html.in | 13 +++++++++++++ docs/schemas/basictypes.rng | 23 +++++++++++++++++++++++ docs/schemas/domaincommon.rng | 23 ----------------------- docs/schemas/network.rng | 5 +++++ src/conf/network_conf.c | 35 ++++++++++++++++++++++++++++++++++- src/conf/network_conf.h | 3 +++ tests/networkxml2xmlin/metadata.xml | 10 ++++++++++ tests/networkxml2xmlout/metadata.xml | 10 ++++++++++ tests/networkxml2xmltest.c | 1 + 9 files changed, 99 insertions(+), 24 deletions(-) create mode 100644 tests/networkxml2xmlin/metadata.xml create mode 100644 tests/networkxml2xmlout/metadata.xml
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 1cea931..15ebf0c 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -38,6 +38,10 @@ <network ipv6='yes' trustGuestRxFilters='no'> <name>default</name> <uuid>3e3fce45-4f53-4fa7-bb32-11f34168b82b</uuid> + <metadata>
You used a space rather than a tab in the line just above here. If you run "make syntax-check", style errors like that will be pointed out. Other than that, this is a very faithful reproduction of commit fa981fc94, which added metada support to the domain and passes make syntax-check and make check. ACK to the patch. Unless someone has a specific reason why networks shouldn't have the same ability to store metadata as domains, I'll push this tomorrow (so that it makes it into the release). Thanks for putting your money where your mouth is and coming up with this patch in such a short time (we discussed it briefly on IRC yesterday)., and congratulations on your first libvirt patch!
On Wed, Jun 22, 2016 at 16:05:50 -0600, Brnadon Bennett wrote:
From: Brandon Bennett <bbennett@fb.com>
This replicates the metadata field found in the domain configuration and adds it to the network configuration XML.
Just a few notes before Laine pushes the patch:
--- docs/formatnetwork.html.in | 13 +++++++++++++ docs/schemas/basictypes.rng | 23 +++++++++++++++++++++++ docs/schemas/domaincommon.rng | 23 ----------------------- docs/schemas/network.rng | 5 +++++ src/conf/network_conf.c | 35 ++++++++++++++++++++++++++++++++++- src/conf/network_conf.h | 3 +++ tests/networkxml2xmlin/metadata.xml | 10 ++++++++++ tests/networkxml2xmlout/metadata.xml | 10 ++++++++++ tests/networkxml2xmltest.c | 1 + 9 files changed, 99 insertions(+), 24 deletions(-) create mode 100644 tests/networkxml2xmlin/metadata.xml create mode 100644 tests/networkxml2xmlout/metadata.xml
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 1cea931..15ebf0c 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in
[...]
@@ -73,6 +83,9 @@ override the setting in the network.</dd> </dl>
++ + +
Spurious whitespace.
<h3><a name="elementsConnect">Connectivity</a></h3>
<p> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 02b8cd7..4239c32 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c
[...]
@@ -2388,8 +2392,12 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) } break; } -
Again, this isn't necessary.
VIR_FREE(stp); + + /* Extract custom metadata */ + if ((metadataNode = virXPathNode("./metadata[1]", ctxt)) != NULL) + def->metadata = xmlCopyNode(metadataNode, 1);
Domain metadata code explicitly rejects duplicate entries. I think this should be used here too.
+ ctxt->node = save; return def;
Rest looks good to me. Peter
On 06/24/2016 07:01 AM, Peter Krempa wrote:
On Wed, Jun 22, 2016 at 16:05:50 -0600, Brnadon Bennett wrote:
From: Brandon Bennett <bbennett@fb.com>
This replicates the metadata field found in the domain configuration and adds it to the network configuration XML. Just a few notes before Laine pushes the patch:
--- docs/formatnetwork.html.in | 13 +++++++++++++ docs/schemas/basictypes.rng | 23 +++++++++++++++++++++++ docs/schemas/domaincommon.rng | 23 ----------------------- docs/schemas/network.rng | 5 +++++ src/conf/network_conf.c | 35 ++++++++++++++++++++++++++++++++++- src/conf/network_conf.h | 3 +++ tests/networkxml2xmlin/metadata.xml | 10 ++++++++++ tests/networkxml2xmlout/metadata.xml | 10 ++++++++++ tests/networkxml2xmltest.c | 1 + 9 files changed, 99 insertions(+), 24 deletions(-) create mode 100644 tests/networkxml2xmlin/metadata.xml create mode 100644 tests/networkxml2xmlout/metadata.xml
diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 1cea931..15ebf0c 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in [...]
@@ -73,6 +83,9 @@ override the setting in the network.</dd> </dl>
++ + + Spurious whitespace.
Right. I noticed that and meant to mention it, but forgot by the time I got to the end. I'll fix that too.
<h3><a name="elementsConnect">Connectivity</a></h3>
<p> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 02b8cd7..4239c32 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c
[...]
@@ -2388,8 +2392,12 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) } break; } - Again, this isn't necessary.
And that.
VIR_FREE(stp); + + /* Extract custom metadata */ + if ((metadataNode = virXPathNode("./metadata[1]", ctxt)) != NULL) + def->metadata = xmlCopyNode(metadataNode, 1);
Domain metadata code explicitly rejects duplicate entries. I think this should be used here too.
Ah, I was just comparing to the original patch for domain metadata, so I didn't notice that bit (or the separate patch to add an API to set/retrieve the metadata). Here are relevant patches for domain metadata: fa981fc94 - original c471e55e1 - public API (virDomain(Get|Set)Metadata()) 21d13ddc5 - hook up API to qemu driver 79093004 - remove duplicates 51a4178f2 - remove elements with no namespace (there are a bunch of other bugfixes too) The last two add a function called virDomainMetadataSanitize() that looked like it could easily be made a into a generic function called by both domain and network, and Peter suggested in IRC that it could go into util/virxml.[ch], so I just made a patch doing that and posted it. Once that's ACKed, I'll add in a call for the network metadata to this patch (along with the other minor tweaks), and push all three. As Brandon commented elsewhere, there is still the virNetwork(Get|Set)Metadata() APIs to add, but even the ability to have it in the XML is useful (for example, in the case of a network hook script).
+ ctxt->node = save; return def; Rest looks good to me.
Peter
On Fri, Jun 24, 2016 at 12:26:22 -0400, Laine Stump wrote:
On 06/24/2016 07:01 AM, Peter Krempa wrote:
On Wed, Jun 22, 2016 at 16:05:50 -0600, Brnadon Bennett wrote:
From: Brandon Bennett <bbennett@fb.com>
This replicates the metadata field found in the domain configuration and adds it to the network configuration XML. Just a few notes before Laine pushes the patch:
[...]
As Brandon commented elsewhere, there is still the virNetwork(Get|Set)Metadata() APIs to add, but even the ability to have it in the XML is useful (for example, in the case of a network hook script).
The setter API is necessary if you want to tweak the metadata for a running network, so it's necessary for any kind of dynamic metadata. It's okay to add it later though similarly as it was done with domain metadata. Peter
On 06/22/2016 06:05 PM, Brnadon Bennett wrote:
From: Brandon Bennett <bbennett@fb.com>
This replicates the metadata field found in the domain configuration and adds it to the network configuration XML.
Now that the freeze is over and libvirt-2.0.0 is released, I'm finally able to push this patch (with tweaks as I indicated in the review, including Peter's request to sanitize the metadata so there is at most one occurence of any given namespace). Thanks again for your contribution! (and feel free to whip up the patches for metadata get/set API :-)
participants (4)
-
Brnadon Bennett -
Laine Stump -
Peter Krempa -
Vasiliy Tolstov