Eric,
Many Thanks for your comments and for pushing the patches through.
I understand the review comments you and Daniel posted on my patch series and was going to work on them today.
I saw your modified patches and wanted to thank you for your contribution towards the patch series and for pushing them upstream.
Regards,
Shradha Shah
On 01/11/2012 07:31 PM, Eric Blake wrote:
> 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>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list