On 08/14/2012 06:27 AM, Laine Stump wrote:
> 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 <ss...@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?
This is a test to test the inactive flag that can be used with net-dumpxml.
An active network will have a pf along with the interface pool but when the
inactive flag is used only the inactive part of the network xml should be
formatted. Hence the address part of the xml is thrown out.
>
>
>> + <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;
>> }
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
