[libvirt] [PATCH 0/2] prevent duplicate entries in network device pools
This is a simple bugfix, but proper testing required the ability to test for parse failures in networkxml2xmltest, which wasn't yet an option, so I added a separate prerequisite patch to do that (in case someone wants to backport the new testing option without the bugfix). Laine Stump (2): test: enable testing for expected parse errors in network XML network: prevent duplicate entries in network device pools src/conf/network_conf.c | 33 ++++++++-- tests/networkxml2xmlin/hostdev-duplicate.xml | 11 ++++ tests/networkxml2xmlin/passthrough-duplicate.xml | 10 +++ tests/networkxml2xmltest.c | 77 ++++++++++++++++++------ 4 files changed, 108 insertions(+), 23 deletions(-) create mode 100644 tests/networkxml2xmlin/hostdev-duplicate.xml create mode 100644 tests/networkxml2xmlin/passthrough-duplicate.xml -- 2.5.5
This is patterned after similar functionality for domain XML tests, but tries harder to avoid reading non-existent networkxml2xmlout data file when parse fails. --- tests/networkxml2xmltest.c | 75 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 57 insertions(+), 18 deletions(-) diff --git a/tests/networkxml2xmltest.c b/tests/networkxml2xmltest.c index 8d60aa8..b83396b 100644 --- a/tests/networkxml2xmltest.c +++ b/tests/networkxml2xmltest.c @@ -16,26 +16,58 @@ #define VIR_FROM_THIS VIR_FROM_NONE +typedef enum { + TEST_COMPARE_NET_XML2XML_RESULT_SUCCESS, + TEST_COMPARE_NET_XML2XML_RESULT_FAIL_PARSE, + TEST_COMPARE_NET_XML2XML_RESULT_FAIL_FORMAT, + TEST_COMPARE_NET_XML2XML_RESULT_FAIL_COMPARE, +} testCompareNetXML2XMLResult; + static int testCompareXMLToXMLFiles(const char *inxml, const char *outxml, - unsigned int flags) + unsigned int flags, + testCompareNetXML2XMLResult expectResult) { char *actual = NULL; - int ret = -1; + int ret; + testCompareNetXML2XMLResult result = TEST_COMPARE_DOM_XML2XML_RESULT_SUCCESS; virNetworkDefPtr dev = NULL; - if (!(dev = virNetworkDefParseFile(inxml))) - goto fail; - - if (!(actual = virNetworkDefFormat(dev, flags))) - goto fail; + if (!(dev = virNetworkDefParseFile(inxml))) { + result = TEST_COMPARE_NET_XML2XML_RESULT_FAIL_PARSE; + goto cleanup; + } + if (expectResult == TEST_COMPARE_NET_XML2XML_RESULT_FAIL_PARSE) + goto cleanup; - if (virtTestCompareToFile(actual, outxml) < 0) - goto fail; + if (!(actual = virNetworkDefFormat(dev, flags))) { + result = TEST_COMPARE_NET_XML2XML_RESULT_FAIL_FORMAT; + goto cleanup; + } + if (expectResult == TEST_COMPARE_NET_XML2XML_RESULT_FAIL_FORMAT) + goto cleanup; + + if (virtTestCompareToFile(actual, outxml) < 0) { + result = TEST_COMPARE_NET_XML2XML_RESULT_FAIL_COMPARE; + goto cleanup; + } + if (expectResult == TEST_COMPARE_NET_XML2XML_RESULT_FAIL_COMPARE) + goto cleanup; - ret = 0; + cleanup: + if (result == expectResult) { + ret = 0; + if (expectResult != TEST_COMPARE_NET_XML2XML_RESULT_SUCCESS) { + VIR_TEST_DEBUG("Got expected failure code=%d msg=%s", + result, virGetLastErrorMessage()); + } + } else { + ret = -1; + VIR_TEST_DEBUG("Expected result code=%d but received code=%d", + expectResult, result); + } + virResetLastError(); - fail: VIR_FREE(actual); virNetworkDefFree(dev); return ret; @@ -44,6 +76,7 @@ testCompareXMLToXMLFiles(const char *inxml, const char *outxml, struct testInfo { const char *name; unsigned int flags; + testCompareNetXML2XMLResult expectResult; }; static int @@ -61,7 +94,8 @@ testCompareXMLToXMLHelper(const void *data) goto cleanup; } - result = testCompareXMLToXMLFiles(inxml, outxml, info->flags); + result = testCompareXMLToXMLFiles(inxml, outxml, info->flags, + info->expectResult); cleanup: VIR_FREE(inxml); @@ -75,14 +109,19 @@ mymain(void) { int ret = 0; -#define DO_TEST_FULL(name, flags) \ +#define DO_TEST_FULL(name, flags, expectResult) \ do { \ - const struct testInfo info = {name, flags}; \ + const struct testInfo info = {name, flags, expectResult}; \ if (virtTestRun("Network XML-2-XML " name, \ - testCompareXMLToXMLHelper, &info) < 0) \ + testCompareXMLToXMLHelper, &info) < 0) \ ret = -1; \ } while (0) -#define DO_TEST(name) DO_TEST_FULL(name, 0) +#define DO_TEST(name) \ + DO_TEST_FULL(name, 0, TEST_COMPARE_NET_XML2XML_RESULT_SUCCESS) +#define DO_TEST_FLAGS(name, flags) \ + DO_TEST_FULL(name, flags, TEST_COMPARE_NET_XML2XML_RESULT_SUCCESS) +#define DO_TEST_PARSE_ERROR(name) \ + DO_TEST_FULL(name, 0, TEST_COMPARE_NET_XML2XML_RESULT_FAIL_PARSE) DO_TEST("dhcp6host-routed-network"); DO_TEST("empty-allow-ipv6"); @@ -106,9 +145,9 @@ mymain(void) DO_TEST("vepa-net"); DO_TEST("bandwidth-network"); DO_TEST("openvswitch-net"); - DO_TEST_FULL("passthrough-pf", VIR_NETWORK_XML_INACTIVE); + DO_TEST_FLAGS("passthrough-pf", VIR_NETWORK_XML_INACTIVE); DO_TEST("hostdev"); - DO_TEST_FULL("hostdev-pf", VIR_NETWORK_XML_INACTIVE); + DO_TEST_FLAGS("hostdev-pf", VIR_NETWORK_XML_INACTIVE); DO_TEST("passthrough-address-crash"); DO_TEST("nat-network-explicit-flood"); DO_TEST("host-bridge-no-flood"); -- 2.5.5
Prior to this patch we didn't make any attempt to prevent two entries in the array of interfaces/PCI devices from pointing to the same device. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1002423 --- src/conf/network_conf.c | 33 ++++++++++++++++++++---- tests/networkxml2xmlin/hostdev-duplicate.xml | 11 ++++++++ tests/networkxml2xmlin/passthrough-duplicate.xml | 10 +++++++ tests/networkxml2xmltest.c | 2 ++ 4 files changed, 51 insertions(+), 5 deletions(-) create mode 100644 tests/networkxml2xmlin/hostdev-duplicate.xml create mode 100644 tests/networkxml2xmlin/passthrough-duplicate.xml diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 4fb2e2a..043c79b 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1,7 +1,7 @@ /* * network_conf.c: network XML handling * - * Copyright (C) 2006-2015 Red Hat, Inc. + * Copyright (C) 2006-2016 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -1799,7 +1799,7 @@ virNetworkForwardDefParseXML(const char *networkName, xmlXPathContextPtr ctxt, virNetworkForwardDefPtr def) { - size_t i; + size_t i, j; int ret = -1; xmlNodePtr *forwardIfNodes = NULL; xmlNodePtr *forwardPfNodes = NULL; @@ -1936,6 +1936,16 @@ virNetworkForwardDefParseXML(const char *networkName, continue; } + for (j = 0; j < i; j++) { + if (STREQ_NULLABLE(def->ifs[j].device.dev, forwardDev)) { + virReportError(VIR_ERR_XML_ERROR, + _("interface '%s' can only be " + "listed once in network %s"), + forwardDev, networkName); + goto cleanup; + } + } + def->ifs[i].device.dev = forwardDev; forwardDev = NULL; def->ifs[i].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV; @@ -1963,12 +1973,25 @@ virNetworkForwardDefParseXML(const char *networkName, switch (def->ifs[i].type) { case VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI: - if (virDevicePCIAddressParseXML(forwardAddrNodes[i], - &def->ifs[i].device.pci) < 0) { + { + virDevicePCIAddressPtr addr = &def->ifs[i].device.pci; + + if (virDevicePCIAddressParseXML(forwardAddrNodes[i], addr) < 0) { goto cleanup; } + for (j = 0; j < i; j++) { + if (virDevicePCIAddressEqual(addr, &def->ifs[j].device.pci)) { + virReportError(VIR_ERR_XML_ERROR, + _("PCI device '%04x:%02x:%02x.%x' can " + "only be listed once in network %s"), + addr->domain, addr->bus, + addr->slot, addr->function, + networkName); + goto cleanup; + } + } break; - + } /* Add USB case here if we ever find a reason to support it */ default: diff --git a/tests/networkxml2xmlin/hostdev-duplicate.xml b/tests/networkxml2xmlin/hostdev-duplicate.xml new file mode 100644 index 0000000..79e55aa --- /dev/null +++ b/tests/networkxml2xmlin/hostdev-duplicate.xml @@ -0,0 +1,11 @@ +<network> + <name>hostdev</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <forward mode='hostdev' managed='yes'> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x1'/> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x2'/> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x3'/> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x3'/> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x4'/> + </forward> +</network> diff --git a/tests/networkxml2xmlin/passthrough-duplicate.xml b/tests/networkxml2xmlin/passthrough-duplicate.xml new file mode 100644 index 0000000..8f645f7 --- /dev/null +++ b/tests/networkxml2xmlin/passthrough-duplicate.xml @@ -0,0 +1,10 @@ +<network> + <name>passthrough-duplicate</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a8b</uuid> + <forward mode="passthrough"> + <interface dev="eth1"/> + <interface dev="eth2"/> + <interface dev="eth3"/> + <interface dev="eth3"/> + </forward> +</network> diff --git a/tests/networkxml2xmltest.c b/tests/networkxml2xmltest.c index b83396b..eafd473 100644 --- a/tests/networkxml2xmltest.c +++ b/tests/networkxml2xmltest.c @@ -151,6 +151,8 @@ mymain(void) DO_TEST("passthrough-address-crash"); DO_TEST("nat-network-explicit-flood"); DO_TEST("host-bridge-no-flood"); + DO_TEST_PARSE_ERROR("hostdev-duplicate"); + DO_TEST_PARSE_ERROR("passthrough-duplicate"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.5.5
On 04/18/2016 03:12 PM, Laine Stump wrote:
This is a simple bugfix, but proper testing required the ability to test for parse failures in networkxml2xmltest, which wasn't yet an option, so I added a separate prerequisite patch to do that (in case someone wants to backport the new testing option without the bugfix).
Laine Stump (2): test: enable testing for expected parse errors in network XML network: prevent duplicate entries in network device pools
src/conf/network_conf.c | 33 ++++++++-- tests/networkxml2xmlin/hostdev-duplicate.xml | 11 ++++ tests/networkxml2xmlin/passthrough-duplicate.xml | 10 +++ tests/networkxml2xmltest.c | 77 ++++++++++++++++++------ 4 files changed, 108 insertions(+), 23 deletions(-) create mode 100644 tests/networkxml2xmlin/hostdev-duplicate.xml create mode 100644 tests/networkxml2xmlin/passthrough-duplicate.xml
ACK - Cole
participants (2)
-
Cole Robinson -
Laine Stump