[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 5/5 v3] Added new option to virsh net-dumpxml called --inactive



On 12/14/2011 03:50 AM, Shradha Shah wrote:
> The above option helps to differentiate between implicit and explicit interface pools.
> ---
>  include/libvirt/libvirt.h.in |    4 ++++
>  src/conf/network_conf.c      |   10 +++++++---
>  src/conf/network_conf.h      |    2 +-
>  src/network/bridge_driver.c  |    4 ++--
>  src/test/test_driver.c       |    2 +-
>  src/vbox/vbox_tmpl.c         |    2 +-
>  tests/networkxml2xmltest.c   |    2 +-
>  tools/virsh.c                |   13 +++++++++++--
>  8 files changed, 28 insertions(+), 11 deletions(-)

In addition to Daniel's comments, you are missing documentation for the
new flag; src/libvirt.c needs to mention the valid 'flags' value, and
tools/virsh.pod needs to document the new code.

I'm squashing this to your last patch, then rebasing things slightly
(I'm moving portions of network_conf.c that format pf on output to patch
3, to parallel parsing it on input, so that this patch is only left with
adding flags support to network_conf.c), then pushing the series.

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 21b2d0a..009479c 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -1080,14 +1080,12 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)

         /* all of these modes can use a pool of physical interfaces */
         nForwardIfs = virXPathNodeSet("./interface", ctxt,
&forwardIfNodes);
-        if (nForwardIfs <= 0) {
-            nForwardPfs = virXPathNodeSet("./pf", ctxt, &forwardPfNodes);
+        nForwardPfs = virXPathNodeSet("./pf", ctxt, &forwardPfNodes);

-            if (nForwardPfs < 0) {
-                virNetworkReportError(VIR_ERR_XML_ERROR,
-                                      _("No interface pool or SRIOV
physical device given"));
-                goto error;
-            }
+        if (nForwardIfs < 0 || nForwardPfs < 0) {
+            virNetworkReportError(VIR_ERR_XML_ERROR,
+                                  _("No interface pool or SRIOV
physical device given"));
+            goto error;
         }

         if (nForwardPfs == 1) {
@@ -1118,7 +1116,8 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
             virNetworkReportError(VIR_ERR_XML_ERROR,
                                   _("Use of more than one physical
interface is not allowed"));
             goto error;
-        } else if ((nForwardIfs > 0) || forwardDev) {
+        }
+        if (nForwardIfs > 0 || forwardDev) {
             int ii;

             /* allocate array to hold all the portgroups */
@@ -1465,7 +1464,7 @@ char *virNetworkDefFormat(const virNetworkDefPtr
def, unsigned int flags)

     if (def->forwardType != VIR_NETWORK_FORWARD_NONE) {
         const char *dev = NULL;
-        if (def->nForwardPfs < 1)
+        if (!def->nForwardPfs)
             dev = virNetworkDefForwardIf(def, 0);
         const char *mode = virNetworkForwardTypeToString(def->forwardType);

@@ -1476,31 +1475,24 @@ char *virNetworkDefFormat(const virNetworkDefPtr
def, unsigned int flags)
             goto error;
         }
         virBufferAddLit(&buf, "  <forward");
-        if (dev)
-            virBufferEscapeString(&buf, " dev='%s'", dev);
+        virBufferEscapeString(&buf, " dev='%s'", dev);
         virBufferAsprintf(&buf, " mode='%s'%s>\n", mode,
-                          def->nForwardIfs ? "" : "/");
+                          (def->nForwardIfs || def->nForwardPfs) ? "" :
"/");

-        if (def->nForwardPfs) {
-            if (def->forwardPfs->dev) {
-                virBufferEscapeString(&buf, "    <pf dev='%s'/>\n",
-                                      def->forwardPfs[0].dev);
-            }
-        }
+        /* For now, hard-coded to at most 1 forwardPfs */
+        if (def->forwardPfs)
+            virBufferEscapeString(&buf, "    <pf dev='%s'/>\n",
+                                  def->forwardPfs[0].dev);

-        if (flags && def->nForwardPfs)
-            goto escape;
-
-        if (def->nForwardIfs) {
+        if (def->nForwardIfs &&
+            (!def->nForwardPfs || !(flags & VIR_NETWORK_XML_INACTIVE))) {
             for (ii = 0; ii < def->nForwardIfs; ii++) {
-                if (def->forwardIfs[ii].dev) {
-                    virBufferEscapeString(&buf, "    <interface
dev='%s'/>\n",
-                                          def->forwardIfs[ii].dev);
-                }
+                virBufferEscapeString(&buf, "    <interface dev='%s'/>\n",
+                                      def->forwardIfs[ii].dev);
             }
         }
-    escape:
-        virBufferAddLit(&buf, "  </forward>\n");
+        if (def->nForwardPfs || def->nForwardIfs)
+            virBufferAddLit(&buf, "  </forward>\n");
     }

     if (def->forwardType == VIR_NETWORK_FORWARD_NONE ||
diff --git a/src/libvirt.c b/src/libvirt.c
index dbf0296..a540424 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -9835,11 +9835,16 @@ error:
 /**
  * virNetworkGetXMLDesc:
  * @network: a network object
- * @flags: extra flags; not used yet, so callers should always pass 0
+ * @flags: bitwise-OR of virNetworkXMLFlags
  *
  * Provide an XML description of the network. The description may be reused
  * later to relaunch the network with virNetworkCreateXML().
  *
+ * Normally, if a network included a physical function, the output includes
+ * all virtual functions tied to that physical interface.  If @flags
includes
+ * VIR_NETWORK_XML_INACTIVE, then the expansion of virtual interfaces is
+ * not performed.
+ *
  * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case
of error.
  *         the caller must free() the returned value.
  */
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 1b5c0b8..2a98e7e 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1,7 +1,7 @@
 /*
  * test.c: A "mock" hypervisor for use by application unit tests
  *
- * Copyright (C) 2006-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2012 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
diff --git a/tests/networkxml2xmltest.c b/tests/networkxml2xmltest.c
index 4249caa..6cb9d90 100644
--- a/tests/networkxml2xmltest.c
+++ b/tests/networkxml2xmltest.c
@@ -14,7 +14,8 @@
 #include "testutilsqemu.h"

 static int
-testCompareXMLToXMLFiles(const char *inxml, const char *outxml)
+testCompareXMLToXMLFiles(const char *inxml, const char *outxml,
+                         unsigned int flags)
 {
     char *inXmlData = NULL;
     char *outXmlData = NULL;
@@ -30,7 +31,7 @@ testCompareXMLToXMLFiles(const char *inxml, const char
*outxml)
     if (!(dev = virNetworkDefParseString(inXmlData)))
         goto fail;

-    if (!(actual = virNetworkDefFormat(dev, 0)))
+    if (!(actual = virNetworkDefFormat(dev, flags)))
         goto fail;

     if (STRNEQ(outXmlData, actual)) {
@@ -48,21 +49,27 @@ testCompareXMLToXMLFiles(const char *inxml, const
char *outxml)
     return ret;
 }

+struct testInfo {
+    const char *name;
+    unsigned int flags;
+};
+
 static int
 testCompareXMLToXMLHelper(const void *data)
 {
+    const struct testInfo *info = data;
     int result = -1;
     char *inxml = NULL;
     char *outxml = NULL;

     if (virAsprintf(&inxml, "%s/networkxml2xmlin/%s.xml",
-                    abs_srcdir, (const char*)data) < 0 ||
+                    abs_srcdir, info->name) < 0 ||
         virAsprintf(&outxml, "%s/networkxml2xmlout/%s.xml",
-                    abs_srcdir, (const char*)data) < 0) {
+                    abs_srcdir, info->name) < 0) {
         goto cleanup;
     }

-    result = testCompareXMLToXMLFiles(inxml, outxml);
+    result = testCompareXMLToXMLFiles(inxml, outxml, info->flags);

 cleanup:
     free(inxml);
@@ -76,10 +83,14 @@ mymain(void)
 {
     int ret = 0;

-#define DO_TEST(name) \
-    if (virtTestRun("Network XML-2-XML " name, \
-                    1, testCompareXMLToXMLHelper, (name)) < 0) \
-        ret = -1
+#define DO_TEST_FULL(name, flags)                                       \
+    do {                                                                \
+        const struct testInfo info = {name, flags};                     \
+        if (virtTestRun("Network XML-2-XML " name,                      \
+                        1, testCompareXMLToXMLHelper, &info) < 0)       \
+            ret = -1;                                                   \
+    } while (0)
+#define DO_TEST(name) DO_TEST_FULL(name, 0)

     DO_TEST("isolated-network");
     DO_TEST("routed-network");
@@ -93,6 +104,7 @@ mymain(void)
     DO_TEST("host-bridge-net");
     DO_TEST("vepa-net");
     DO_TEST("bandwidth-network");
+    DO_TEST_FULL("passthrough-pf", VIR_NETWORK_XML_INACTIVE);

     return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
 }
diff --git a/tools/virsh.c b/tools/virsh.c
index ba07e0f..3869d9d 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -7298,15 +7298,14 @@ cmdNetworkDumpXML(vshControl *ctl, const vshCmd
*cmd)
     char *dump;
     unsigned int flags = 0;
     int inactive;
-
+
     if (!vshConnectionUsability(ctl, ctl->conn))
         return false;

     if (!(network = vshCommandOptNetwork(ctl, cmd, NULL)))
         return false;
-
+
     inactive = vshCommandOptBool (cmd, "inactive");
-
     if (inactive)
         flags |= VIR_NETWORK_XML_INACTIVE;

diff --git a/tools/virsh.pod b/tools/virsh.pod
index 1abf448..f2793cd 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1523,9 +1523,11 @@ not instantiated.
 Destroy (stop) a given virtual network specified by its name or UUID. This
 takes effect immediately.

-=item B<net-dumpxml> I<network>
+=item B<net-dumpxml> I<network> [I<--inactive>]

 Output the virtual network information as an XML dump to stdout.
+If I<--inactive> is specified, then physical functions are not
+expanded into their associated virtual functions.

 =item B<net-edit> I<network>

-- 
1.7.7.5



-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]