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 <bbenn...@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


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to