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>