[libvirt] [PATCH 03/15] RNG updates, new xml parser/formatter code to support forward mode=hostdev
Laine Stump
laine at laine.org
Tue Aug 14 05:27:59 UTC 2012
On 08/10/2012 12:23 PM, Shradha Shah wrote:
> This patch introduces the new forward mode='hostdev' along with attribute managed
> Includes updates to the network RNG and new xml parser/formatter code.
This one still needs some tweaks. See below...
>
> Signed-off-by: Shradha Shah <sshah at solarflare.com>
> ---
> docs/schemas/network.rng | 82 +++++++++++++++++++--
> src/conf/network_conf.c | 127 ++++++++++++++++++++++++++++----
> src/conf/network_conf.h | 29 +++++++-
> src/network/bridge_driver.c | 18 ++--
> tests/networkxml2xmlin/hostdev-pf.xml | 11 +++
> tests/networkxml2xmlin/hostdev.xml | 10 +++
> tests/networkxml2xmlout/hostdev-pf.xml | 7 ++
> tests/networkxml2xmlout/hostdev.xml | 10 +++
> tests/networkxml2xmltest.c | 2 +
> 9 files changed, 263 insertions(+), 33 deletions(-)
>
> diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
> index 2ae879e..d1297cd 100644
> --- a/docs/schemas/network.rng
> +++ b/docs/schemas/network.rng
> @@ -82,17 +82,41 @@
> <value>passthrough</value>
> <value>private</value>
> <value>vepa</value>
> + <value>hostdev</value>
> + </choice>
> + </attribute>
> + </optional>
> +
> + <optional>
> + <attribute name="managed">
> + <choice>
> + <value>yes</value>
> + <value>no</value>
> </choice>
> </attribute>
> </optional>
> <interleave>
> - <zeroOrMore>
> - <element name='interface'>
> - <attribute name='dev'>
> - <ref name='deviceName'/>
> - </attribute>
> - </element>
> - </zeroOrMore>
> + <choice>
> + <group>
> + <zeroOrMore>
> + <element name='interface'>
> + <attribute name='dev'>
> + <ref name='deviceName'/>
> + </attribute>
> + </element>
> + </zeroOrMore>
> + </group>
> + <group>
> + <zeroOrMore>
> + <element name='address'>
> + <attribute name='type'>
> + <value>pci</value>
> + </attribute>
> + <ref name="pciaddress"/>
> + </element>
> + </zeroOrMore>
> + </group>
> + </choice>
> <optional>
> <element name='pf'>
> <attribute name='dev'>
> @@ -238,4 +262,48 @@
> </interleave>
> </element>
> </define>
> + <define name="pciaddress">
I'm kind of surprised there isn't already a pci address type in the rng...
Oh, *now* I see - there is already a pciaddress type, but it's defined
in domaincommon.rng, and you need to use it in network.rng.
Rather than defining the whole thing twice, you should either move the
definition to basictypes.rng or (if anyone objects to something that
complex going into a file that has "basic" in the name) to a new file -
devicetypes.rng or something like that. (This is analogous to the split
of the pci device data and functions into device_conf.[ch]).
> + <optional>
> + <attribute name="domain">
> + <ref name="pciDomain"/>
> + </attribute>
> + </optional>
> + <attribute name="bus">
> + <ref name="pciBus"/>
> + </attribute>
> + <attribute name="slot">
> + <ref name="pciSlot"/>
> + </attribute>
> + <attribute name="function">
> + <ref name="pciFunc"/>
> + </attribute>
> + <optional>
> + <attribute name="multifunction">
> + <choice>
> + <value>on</value>
> + <value>off</value>
> + </choice>
> + </attribute>
> + </optional>
> + </define>
> + <define name="pciDomain">
> + <data type="string">
> + <param name="pattern">(0x)?[0-9a-fA-F]{1,4}</param>
> + </data>
> + </define>
> + <define name="pciBus">
> + <data type="string">
> + <param name="pattern">(0x)?[0-9a-fA-F]{1,2}</param>
> + </data>
> + </define>
> + <define name="pciSlot">
> + <data type="string">
> + <param name="pattern">(0x)?[0-1]?[0-9a-fA-F]</param>
> + </data>
> + </define>
> + <define name="pciFunc">
> + <data type="string">
> + <param name="pattern">(0x)?[0-7]</param>
> + </data>
> + </define>
Yeah, basically everything from here up to my previous comment is the
same here and in domaincommon.rng - definitely move it. (put it it
basictypes.rng until/unless someone objects :-)
> </grammar>
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index a3714d9..294939d 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -49,7 +49,12 @@
>
> VIR_ENUM_IMPL(virNetworkForward,
> VIR_NETWORK_FORWARD_LAST,
> - "none", "nat", "route", "bridge", "private", "vepa", "passthrough" )
> + "none", "nat", "route", "bridge", "private", "vepa", "passthrough", "hostdev")
> +
> +VIR_ENUM_DECL(virNetworkForwardHostdevDevice)
> +VIR_ENUM_IMPL(virNetworkForwardHostdevDevice,
> + VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_LAST,
> + "none", "pci")
>
> virNetworkObjPtr virNetworkFindByUUID(const virNetworkObjListPtr nets,
> const unsigned char *uuid)
> @@ -94,6 +99,12 @@ virPortGroupDefClear(virPortGroupDefPtr def)
> static void
> virNetworkForwardIfDefClear(virNetworkForwardIfDefPtr def)
> {
> + VIR_FREE(def->device.dev);
> +}
> +
> +static void
> +virNetworkForwardPfDefClear(virNetworkForwardPfDefPtr def)
> +{
> VIR_FREE(def->dev);
> }
Ah, I see you encountered a practical reason for something that I
recently did just because the old way looked wrong - creating a separate
type for for ForwardPf. Depending on whose patches get pushed first,
there will be a conflict. We should probably push yours first, since
they have a concrete reason for the change.
>
> @@ -157,12 +168,13 @@ void virNetworkDefFree(virNetworkDefPtr def)
> VIR_FREE(def->domain);
>
> for (ii = 0 ; ii < def->nForwardPfs && def->forwardPfs ; ii++) {
> - virNetworkForwardIfDefClear(&def->forwardPfs[ii]);
> + virNetworkForwardPfDefClear(&def->forwardPfs[ii]);
> }
> VIR_FREE(def->forwardPfs);
>
> for (ii = 0 ; ii < def->nForwardIfs && def->forwardIfs ; ii++) {
> - virNetworkForwardIfDefClear(&def->forwardIfs[ii]);
> + if (def->forwardType != VIR_NETWORK_FORWARD_HOSTDEV)
> + virNetworkForwardIfDefClear(&def->forwardIfs[ii]);
Don't put the "if (def->forwardType != VIR_NETWORK_FORWARD_HOSTDEV)
here. It looks like there is a "type" field in the forwardIf now, so let
virNetworkForwardIfDefClear decide for itself if it needs to clear
anything.,
> }
> VIR_FREE(def->forwardIfs);
>
> @@ -929,11 +941,14 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
> xmlNodePtr *portGroupNodes = NULL;
> xmlNodePtr *forwardIfNodes = NULL;
> xmlNodePtr *forwardPfNodes = NULL;
> + xmlNodePtr *forwardAddrNodes = NULL;
> xmlNodePtr dnsNode = NULL;
> xmlNodePtr virtPortNode = NULL;
> xmlNodePtr forwardNode = NULL;
> - int nIps, nPortGroups, nForwardIfs, nForwardPfs;
> + int nIps, nPortGroups, nForwardIfs, nForwardPfs, nForwardAddrs;
> char *forwardDev = NULL;
> + char *forwardManaged = NULL;
> + char *type = NULL;
> xmlNodePtr save = ctxt->node;
> xmlNodePtr bandwidthNode = NULL;
>
> @@ -1079,17 +1094,30 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
> }
>
> forwardDev = virXPathString("string(./@dev)", ctxt);
> + forwardManaged = virXPathString("string(./@managed)", ctxt);
> + if(forwardManaged != NULL) {
> + if (STREQ(forwardManaged, "yes"))
Use STRCASEEQ just to allow for the unlikely case that someone will
enter "Yes" or "YES".
> + def->managed = 1;
> + else
> + def->managed = 0;
Everything allocated with VIR_ALLOC is initialized to 0 - you don't need
the else part of this.
> + }
>
> /* all of these modes can use a pool of physical interfaces */
> nForwardIfs = virXPathNodeSet("./interface", ctxt, &forwardIfNodes);
> nForwardPfs = virXPathNodeSet("./pf", ctxt, &forwardPfNodes);
> + nForwardAddrs = virXPathNodeSet("./address", ctxt, &forwardAddrNodes);
>
> - if (nForwardIfs < 0 || nForwardPfs < 0) {
> + if (nForwardIfs < 0 || nForwardPfs < 0 || nForwardAddrs < 0) {
> virReportError(VIR_ERR_XML_ERROR, "%s",
> _("No interface pool or SRIOV physical device given"));
> goto error;
> }
>
> + if ((nForwardIfs > 0) && (nForwardAddrs > 0)) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("Address and interface attributes are mutually exclusive"));
> + }
> +
Well, we *could* allow that, but I guess that would mean we would then
need to check for two entries pointing at the same hardware (and I doubt
anyone will ever want to do it anyway :-)
> if (nForwardPfs == 1) {
> if (VIR_ALLOC_N(def->forwardPfs, nForwardPfs) < 0) {
> virReportOOMError();
> @@ -1119,7 +1147,55 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
> _("Use of more than one physical interface is not allowed"));
> goto error;
> }
> - if (nForwardIfs > 0 || forwardDev) {
> + if (nForwardAddrs > 0) {
> + int ii;
> +
> + if (VIR_ALLOC_N(def->forwardIfs, nForwardAddrs) < 0) {
> + virReportOOMError();
> + goto error;
> + }
> +
> + if (forwardDev) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("A forward Dev should not be used when using address attribute"));
> + goto error;
> + }
Okay. That makes sense.
> +
> + for (ii = 0; ii < nForwardAddrs; ii++) {
> + type = virXMLPropString(*forwardAddrNodes, "type");
You should be using "forwardAddrNodes[ii]" rather than
*forwardAddrNodes, otherwise you're going to be getting all
nForwardAddrs of device data from the first address element, rather than
each being from a different one. (I think you did a cut-paste from the
PF parsing bit, which only allows a single element).
> +
> + if (type) {
> + if ((def->forwardIfs[ii].type = virNetworkForwardHostdevDeviceTypeFromString(type)) < 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("unknown address type '%s'"), type);
> + goto error;
> + }
> + } else {
> + virReportError(VIR_ERR_XML_ERROR,
> + "%s", _("No type specified for device address"));
> + goto error;
> + }
> +
> + switch (def->forwardIfs[ii].type) {
> + case VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI:
> + if (virDevicePCIAddressParseXML(*forwardAddrNodes, &(def->forwardIfs[ii].device.pci)) < 0)
Here also you need to use forwardAddrNodes[ii] instead of *forwardAddrNodes.
> + goto error;
> + break;
> +
> + /* Add USB case here */
> +
> + default:
> + virReportError(VIR_ERR_XML_ERROR,
> + _("unknown address type '%s'"), type);
> + goto error;
> + }
> +
> + def->forwardIfs[ii].usageCount = 0;
> + type = NULL;
This leaks the string pointed to by type. Instead you need to
VIR_FREE(type);
> + def->nForwardIfs++;
> + }
> + }
> + else if (nForwardIfs > 0 || forwardDev) {
> int ii;
>
> /* allocate array to hold all the portgroups */
> @@ -1130,7 +1206,8 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
>
> if (forwardDev) {
> def->forwardIfs[0].usageCount = 0;
> - def->forwardIfs[0].dev = forwardDev;
> + def->forwardIfs[0].device.dev = forwardDev;
> + def->forwardIfs[0].type = 0;
> forwardDev = NULL;
> def->nForwardIfs++;
> }
> @@ -1148,10 +1225,10 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
> if ((ii == 0) && (def->nForwardIfs == 1)) {
> /* both forwardDev and an interface element are present.
> * If they don't match, it's an error. */
> - if (STRNEQ(forwardDev, def->forwardIfs[0].dev)) {
> - virReportError(VIR_ERR_XML_ERROR,
> + if (STRNEQ(forwardDev, def->forwardIfs[0].device.dev)) {
> + virReportError(VIR_ERR_XML_ERROR,
> _("forward dev '%s' must match first interface element dev '%s' in network '%s'"),
> - def->forwardIfs[0].dev,
> + def->forwardIfs[0].device.dev,
> forwardDev, def->name);
> goto error;
> }
> @@ -1159,16 +1236,19 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
> continue;
> }
>
> - def->forwardIfs[ii].dev = forwardDev;
> + def->forwardIfs[ii].device.dev = forwardDev;
> forwardDev = NULL;
> def->forwardIfs[ii].usageCount = 0;
> + def->forwardIfs[ii].type = 0;
Instead of setting this to 0, add a new value
VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV to the enum.
> def->nForwardIfs++;
> }
> }
> + VIR_FREE(type);
> VIR_FREE(forwardDev);
> + VIR_FREE(forwardManaged);
> VIR_FREE(forwardPfNodes);
> VIR_FREE(forwardIfNodes);
> -
> + VIR_FREE(forwardAddrNodes);
> switch (def->forwardType) {
> case VIR_NETWORK_FORWARD_ROUTE:
> case VIR_NETWORK_FORWARD_NAT:
> @@ -1193,6 +1273,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
> case VIR_NETWORK_FORWARD_PRIVATE:
> case VIR_NETWORK_FORWARD_VEPA:
> case VIR_NETWORK_FORWARD_PASSTHROUGH:
> + case VIR_NETWORK_FORWARD_HOSTDEV:
> if (def->bridge) {
> virReportError(VIR_ERR_XML_ERROR,
> _("bridge name not allowed in %s mode (network '%s')"),
> @@ -1478,6 +1559,12 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int flags)
> }
> virBufferAddLit(&buf, " <forward");
> virBufferEscapeString(&buf, " dev='%s'", dev);
> + if (def->forwardType == VIR_NETWORK_FORWARD_HOSTDEV) {
> + if (def->managed == 1)
> + virBufferAddLit(&buf, " managed='yes'");
> + else
> + virBufferAddLit(&buf, " managed='no'");
> + }
> virBufferAsprintf(&buf, " mode='%s'%s>\n", mode,
> (def->nForwardIfs || def->nForwardPfs) ? "" : "/");
>
> @@ -1489,8 +1576,20 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int flags)
> if (def->nForwardIfs &&
> (!def->nForwardPfs || !(flags & VIR_NETWORK_XML_INACTIVE))) {
> for (ii = 0; ii < def->nForwardIfs; ii++) {
> - virBufferEscapeString(&buf, " <interface dev='%s'/>\n",
> - def->forwardIfs[ii].dev);
> + if (def->forwardType != VIR_NETWORK_FORWARD_HOSTDEV)
> + virBufferEscapeString(&buf, " <interface dev='%s'/>\n",
> + def->forwardIfs[ii].device.dev);
> + else {
> + if (def->forwardIfs[ii].type == VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI) {
> + if (virDevicePCIAddressFormat(&buf,
> + def->forwardIfs[ii].device.pci,
> + true) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("PCI address format failed"));
Although virDevicePCIAddressFormat() currently always succeeds, other
formatting functions that fail will log their own error message before
returning. So, in this case you should just goto error, without logging
any extra message.
> + goto error;
> + }
> + }
> + }
> }
> }
> if (def->nForwardPfs || def->nForwardIfs)
> diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
> index a95b382..a57db36 100644
> --- a/src/conf/network_conf.h
> +++ b/src/conf/network_conf.h
> @@ -36,6 +36,7 @@
> # include "virnetdevbandwidth.h"
> # include "virnetdevvportprofile.h"
> # include "virmacaddr.h"
> +# include "device_conf.h"
>
> enum virNetworkForwardType {
> VIR_NETWORK_FORWARD_NONE = 0,
> @@ -45,10 +46,19 @@ enum virNetworkForwardType {
> VIR_NETWORK_FORWARD_PRIVATE,
> VIR_NETWORK_FORWARD_VEPA,
> VIR_NETWORK_FORWARD_PASSTHROUGH,
> + VIR_NETWORK_FORWARD_HOSTDEV,
>
> VIR_NETWORK_FORWARD_LAST,
> };
>
> +enum virNetworkForwardHostdevDeviceType {
> + VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NONE = 0,
> + VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI,
As I mentioned above, add a VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV here.
> + /* USB Device to be added here when supported */
> +
> + VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_LAST,
> +};
> +
> typedef struct _virNetworkDHCPRangeDef virNetworkDHCPRangeDef;
> typedef virNetworkDHCPRangeDef *virNetworkDHCPRangeDefPtr;
> struct _virNetworkDHCPRangeDef {
> @@ -131,7 +141,19 @@ struct _virNetworkIpDef {
> typedef struct _virNetworkForwardIfDef virNetworkForwardIfDef;
> typedef virNetworkForwardIfDef *virNetworkForwardIfDefPtr;
> struct _virNetworkForwardIfDef {
> - char *dev; /* name of device */
> + int type;
> + union {
> + virDevicePCIAddress pci; /*PCI Address of device */
> + /* when USB devices are supported a new variable to be added here */
> + char *dev; /* name of device */
> + }device;
> + int usageCount; /* how many guest interfaces are bound to this device? */
> +};
> +
> +typedef struct _virNetworkForwardPfDef virNetworkForwardPfDef;
> +typedef virNetworkForwardPfDef *virNetworkForwardPfDefPtr;
> +struct _virNetworkForwardPfDef {
> + char *dev; /* name of device */
> int usageCount; /* how many guest interfaces are bound to this device? */
> };
>
> @@ -159,12 +181,13 @@ struct _virNetworkDef {
> bool mac_specified;
>
> int forwardType; /* One of virNetworkForwardType constants */
> + int managed; /* managed attribute for hostdev mode */
>
> /* If there are multiple forward devices (i.e. a pool of
> * interfaces), they will be listed here.
> */
> size_t nForwardPfs;
> - virNetworkForwardIfDefPtr forwardPfs;
> + virNetworkForwardPfDefPtr forwardPfs;
>
> size_t nForwardIfs;
> virNetworkForwardIfDefPtr forwardIfs;
> @@ -234,7 +257,7 @@ static inline const char *
> virNetworkDefForwardIf(const virNetworkDefPtr def, size_t n)
> {
> return ((def->forwardIfs && (def->nForwardIfs > n))
> - ? def->forwardIfs[n].dev : NULL);
> + ? def->forwardIfs[n].device.dev : NULL);
> }
>
> virPortGroupDefPtr virPortGroupFindByName(virNetworkDefPtr net,
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index f128bd0..df3cc25 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -2762,8 +2762,8 @@ networkCreateInterfacePool(virNetworkDefPtr netdef) {
> netdef->nForwardIfs = num_virt_fns;
>
> for (ii = 0; ii < netdef->nForwardIfs; ii++) {
> - netdef->forwardIfs[ii].dev = strdup(vfname[ii]);
> - if (!netdef->forwardIfs[ii].dev) {
> + netdef->forwardIfs[ii].device.dev = strdup(vfname[ii]);
> + if (!netdef->forwardIfs[ii].device.dev) {
> virReportOOMError();
> goto finish;
> }
> @@ -2985,7 +2985,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
> netdef->name);
> goto cleanup;
> }
> - iface->data.network.actual->data.direct.linkdev = strdup(dev->dev);
> + iface->data.network.actual->data.direct.linkdev = strdup(dev->device.dev);
> if (!iface->data.network.actual->data.direct.linkdev) {
> virReportOOMError();
> goto cleanup;
> @@ -2993,7 +2993,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
> /* we are now assured of success, so mark the allocation */
> dev->usageCount++;
> VIR_DEBUG("Using physical device %s, usageCount %d",
> - dev->dev, dev->usageCount);
> + dev->device.dev, dev->usageCount);
> }
> }
>
> @@ -3068,7 +3068,7 @@ networkNotifyActualDevice(virDomainNetDefPtr iface)
> /* find the matching interface in the pool and increment its usageCount */
>
> for (ii = 0; ii < netdef->nForwardIfs; ii++) {
> - if (STREQ(actualDev, netdef->forwardIfs[ii].dev)) {
> + if (STREQ(actualDev, netdef->forwardIfs[ii].device.dev)) {
> dev = &netdef->forwardIfs[ii];
> break;
> }
> @@ -3099,7 +3099,7 @@ networkNotifyActualDevice(virDomainNetDefPtr iface)
> /* we are now assured of success, so mark the allocation */
> dev->usageCount++;
> VIR_DEBUG("Using physical device %s, usageCount %d",
> - dev->dev, dev->usageCount);
> + dev->device.dev, dev->usageCount);
> }
>
> ret = 0;
> @@ -3169,7 +3169,7 @@ networkReleaseActualDevice(virDomainNetDefPtr iface)
> virNetworkForwardIfDefPtr dev = NULL;
>
> for (ii = 0; ii < netdef->nForwardIfs; ii++) {
> - if (STREQ(actualDev, netdef->forwardIfs[ii].dev)) {
> + if (STREQ(actualDev, netdef->forwardIfs[ii].device.dev)) {
> dev = &netdef->forwardIfs[ii];
> break;
> }
> @@ -3184,7 +3184,7 @@ networkReleaseActualDevice(virDomainNetDefPtr iface)
>
> dev->usageCount--;
> VIR_DEBUG("Releasing physical device %s, usageCount %d",
> - dev->dev, dev->usageCount);
> + dev->device.dev, dev->usageCount);
> }
>
> ret = 0;
> @@ -3265,7 +3265,7 @@ networkGetNetworkAddress(const char *netname, char **netaddr)
> case VIR_NETWORK_FORWARD_VEPA:
> case VIR_NETWORK_FORWARD_PASSTHROUGH:
> if ((netdef->nForwardIfs > 0) && netdef->forwardIfs)
> - dev_name = netdef->forwardIfs[0].dev;
> + dev_name = netdef->forwardIfs[0].device.dev;
>
> if (!dev_name) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> diff --git a/tests/networkxml2xmlin/hostdev-pf.xml b/tests/networkxml2xmlin/hostdev-pf.xml
> new file mode 100644
> index 0000000..e07db69
> --- /dev/null
> +++ b/tests/networkxml2xmlin/hostdev-pf.xml
> @@ -0,0 +1,11 @@
> +<network>
> + <name>hostdev</name>
> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
> + <forward mode="hostdev" managed="yes">
> + <pf dev='eth2'/>
> + <address type='pci' domain='0' bus='3' slot='0' function='1'/>
Is it legal to have a pf using a netdev name along with a list of pci
devices? I thought it was either/or?
> + <address type='pci' domain='0' bus='3' slot='0' function='2'/>
> + <address type='pci' domain='0' bus='3' slot='0' function='3'/>
> + <address type='pci' domain='0' bus='3' slot='0' function='4'/>
> + </forward>
> +</network>
> diff --git a/tests/networkxml2xmlin/hostdev.xml b/tests/networkxml2xmlin/hostdev.xml
> new file mode 100644
> index 0000000..0ec52d2
> --- /dev/null
> +++ b/tests/networkxml2xmlin/hostdev.xml
> @@ -0,0 +1,10 @@
> +<network>
> + <name>hostdev</name>
> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
> + <forward mode="hostdev" managed="yes">
> + <address type='pci' domain='0' bus='3' slot='0' function='1'/>
> + <address type='pci' domain='0' bus='3' slot='0' function='2'/>
> + <address type='pci' domain='0' bus='3' slot='0' function='3'/>
> + <address type='pci' domain='0' bus='3' slot='0' function='4'/>
> + </forward>
> +</network>
> diff --git a/tests/networkxml2xmlout/hostdev-pf.xml b/tests/networkxml2xmlout/hostdev-pf.xml
> new file mode 100644
> index 0000000..e955312
> --- /dev/null
> +++ b/tests/networkxml2xmlout/hostdev-pf.xml
> @@ -0,0 +1,7 @@
> +<network>
> + <name>hostdev</name>
> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
> + <forward mode="hostdev" managed="yes">
> + <pf dev='eth2'/>
Wait, so it's legal to do it, but the <address> elements are all tossed?
That doesn't seem good.
> + </forward>
> +</network>
> diff --git a/tests/networkxml2xmlout/hostdev.xml b/tests/networkxml2xmlout/hostdev.xml
> new file mode 100644
> index 0000000..0ec52d2
> --- /dev/null
> +++ b/tests/networkxml2xmlout/hostdev.xml
> @@ -0,0 +1,10 @@
> +<network>
> + <name>hostdev</name>
> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
> + <forward mode="hostdev" managed="yes">
> + <address type='pci' domain='0' bus='3' slot='0' function='1'/>
> + <address type='pci' domain='0' bus='3' slot='0' function='2'/>
> + <address type='pci' domain='0' bus='3' slot='0' function='3'/>
> + <address type='pci' domain='0' bus='3' slot='0' function='4'/>
> + </forward>
> +</network>
> diff --git a/tests/networkxml2xmltest.c b/tests/networkxml2xmltest.c
> index 8641c41..c9c8311 100644
> --- a/tests/networkxml2xmltest.c
> +++ b/tests/networkxml2xmltest.c
> @@ -105,6 +105,8 @@ mymain(void)
> DO_TEST("vepa-net");
> DO_TEST("bandwidth-network");
> DO_TEST_FULL("passthrough-pf", VIR_NETWORK_XML_INACTIVE);
> + DO_TEST("hostdev");
> + DO_TEST_FULL("hostdev-pf", VIR_NETWORK_XML_INACTIVE);
>
> return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE;
> }
More information about the libvir-list
mailing list