The function that parses the <forward> subelement of a network used to fail/log an error if the network definition contained both a <pf> element as well as at least one <interface> or <address> element. That check was present because the configuration of a network should have either one <pf>, one or more <interface>, or one or more <address>, but never combinations of multiple kinds. This caused a problem when libvirtd was restarted with a network already active - when a network with a <pf> element is started, the referenced PF (Physical Function of an SRIOV-capable network card) is checked for VFs (Virtual Functions), and the <forward> is filled in with a list of all VFs for that PF either in the form of their PCI addresses (a list of <address>) or their netdev names (a list of <interface>); the <pf> element is not removed though. When libvirtd is restarted, it parses the network status and finds both the original <pf> from the config, as well as the list of either <address> or <interface>, fails the parse, and the network is not added to the active list. This failure is often obscured because the network is marked as autostart so libvirt immediately restarts it. It seems odd to me that <interface> and <address> are stored in the same array rather than keeping two separate arrays, and having separate arrays would have made the check much simpler. However, changing to use two separate arrays would have required changes in more places, potentially creating more conflicts and (more importantly) more possible regressions in the event of a backport, so I chose to keep the existing data structure in order to localize the change. It appears that this problem has been in the code ever since support for <pf> was added (0.9.10), but until commit 34cc3b2f106e296df5e64309620c79d16fd76c85 (first in libvirt 1.2.4) networks with interface pools were not properly marked as active on restart anyway, so there is no point in backporting this patch any further than that. --- src/conf/network_conf.c | 10 +--------- src/network/bridge_driver.c | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index f947d89..dce3360 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-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -1638,14 +1638,6 @@ virNetworkForwardDefParseXML(const char *networkName, goto cleanup; } - if (((nForwardIfs > 0) + (nForwardAddrs > 0) + (nForwardPfs > 0)) > 1) { - virReportError(VIR_ERR_XML_ERROR, - _("<address>, <interface>, and <pf> elements in <forward> " - "of network %s are mutually exclusive"), - networkName); - goto cleanup; - } - forwardDev = virXPathString("string(./@dev)", ctxt); if (forwardDev && (nForwardAddrs > 0 || nForwardPfs > 0)) { virReportError(VIR_ERR_XML_ERROR, "%s", diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 2798010..af16c25 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2734,6 +2734,7 @@ networkValidate(virNetworkDefPtr def, virNetworkIpDefPtr ipdef; bool ipv4def = false, ipv6def = false; bool bandwidthAllowed = true; + bool usesInterface = false, usesAddress = false; /* check for duplicate networks */ if (virNetworkObjIsDuplicate(&driver->networks, def, check_active) < 0) @@ -2798,6 +2799,40 @@ networkValidate(virNetworkDefPtr def, bandwidthAllowed = false; } + /* we support configs with a single PF defined: + * <pf dev='eth0'/> + * or with a list of netdev names: + * <interface dev='eth9'/> + * OR a list of PCI addresses + * <address type='pci' domain='0' bus='4' slot='0' function='1'/> + * but not any combination of those. + * + * Since <interface> and <address> are for some strange reason + * stored in the same array, we need to cycle through it and check + * the type of each. + */ + for (i = 0; i < def->forward.nifs; i++) { + switch ((virNetworkForwardHostdevDeviceType) + def->forward.ifs[i].type) { + case VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV: + usesInterface = true; + break; + case VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI: + usesAddress = true; + break; + case VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NONE: + case VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_LAST: + break; + } + } + if ((def->forward.npfs > 0) + usesInterface + usesAddress > 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("<address>, <interface>, and <pf> elements of " + "<forward> in network %s are mutually exclusive"), + def->name); + return -1; + } + /* We only support dhcp on one IPv4 address and * on one IPv6 address per defined network */ -- 2.1.0