10:26 a.m.
This patch series supports the forward mode='hostdev'. The functionality
of this mode is the same as interface type='hostdev' but with the added
benefit of using interface pools.
The patch series also contains a patch to support use of interface names
and PCI device addresses interchangeably in a network xml, and return
the appropriate one in actualDevice when networkAllocateActualDevice is
called. This functionality is not supported for any other forward mode
except hostdev.
Currently this patch series also does not support USB hostdev
passthrough.
At the top level managed attribute can be specified for a pf dev or an
interface dev (with identical results as when it's specified for a
hostdev
Shradha Shah (5):
Code to return interface name or pci_addr of the VF in actualDevice
Moved the code to create implicit interface pool from PF to a new
function
Introduce forward mode='hostdev' for network XML in order to use the
functionality of interface pools.
Forward Mode Hostdev Implementation
Supporting managed option for forward devs when using HOSTDEV mode
docs/schemas/network.rng | 1 +
src/conf/network_conf.c | 119 +++++++++++++++-
src/conf/network_conf.h | 13 ++-
src/libvirt_private.syms | 3 +
src/network/bridge_driver.c | 325 +++++++++++++++++++++++++++++++++++--------
src/qemu/qemu_command.c | 14 ++
src/util/pci.c | 7 +-
src/util/pci.h | 3 +
src/util/virnetdev.c | 134 +++++++++++++++++-
src/util/virnetdev.h | 19 +++
10 files changed, 568 insertions(+), 70 deletions(-)
--
1.7.4.4
10:29 a.m.
New subject: [libvirt] [PATCH 1/5] Code to return interface name or pci_addr of the VF in actualDevice
The network pool should be able to keep track of both, network device names nad PCI
addresses, and return the appropriate one in the actualDevice when
networkAllocateActualDevice is called.
Signed-off-by: Shradha Shah <sshah(a)solarflare.com>
---
src/conf/network_conf.c | 55 ++++++++++++++++++++++++++++++++++++++++++-
src/conf/network_conf.h | 10 +++++++-
src/network/bridge_driver.c | 47 ++++++++++++++++++++++++++++++++++++
src/util/pci.c | 5 +++-
src/util/virnetdev.c | 6 +---
5 files changed, 116 insertions(+), 7 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 6515efe..8fcba16 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -98,6 +98,12 @@ virPortGroupDefClear(virPortGroupDefPtr def)
}
static void
+virNetworkForwardPfDefClear(virNetworkForwardPfDefPtr def)
+{
+ VIR_FREE(def->dev);
+}
+
+static void
virNetworkForwardIfDefClear(virNetworkForwardIfDefPtr def)
{
VIR_FREE(def->dev);
@@ -163,7 +169,7 @@ 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);
@@ -925,6 +931,44 @@ error:
return result;
}
+/* Function to compare strings with wildcard strings*/
+/* When editing this function also edit the one in src/network/bridge_driver.c*/
+static int
+wildcmp(const char *wild, const char *string)
+{
+/* Written by Jack Handy - <A
href="mailto:jakkhandy@hotmail.com">jakkhandy@hotmail.com</A>*/
+ const char *cp = NULL, *mp = NULL;
+
+ while ((*string) && (*wild != '*')) {
+ if ((*wild != *string) && (*wild != '?')) {
+ return 0;
+ }
+ wild++;
+ string++;
+ }
+
+ while (*string) {
+ if (*wild == '*') {
+ if (!*++wild) {
+ return 1;
+ }
+ mp = wild;
+ cp = string+1;
+ } else if ((*wild == *string) || (*wild == '?')) {
+ wild++;
+ string++;
+ } else {
+ wild = mp;
+ string = cp++;
+ }
+ }
+
+ while (*wild == '*') {
+ wild++;
+ }
+ return !*wild;
+}
+
static virNetworkDefPtr
virNetworkDefParseXML(xmlXPathContextPtr ctxt)
{
@@ -1170,6 +1214,15 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
def->nForwardIfs++;
}
}
+ int i;
+ for (i = 0; i < nForwardIfs; i++) {
+ if (wildcmp("????:??:??.?", def->forwardIfs[i].dev)) {
+ def->forwardIfs[i].isPciAddr = true;
+ } else {
+ def->forwardIfs[i].isPciAddr = false;
+ }
+ }
+
VIR_FREE(forwardDev);
VIR_FREE(forwardPfNodes);
VIR_FREE(forwardIfNodes);
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 4339a69..b205cb0 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -133,6 +133,14 @@ typedef virNetworkForwardIfDef *virNetworkForwardIfDefPtr;
struct _virNetworkForwardIfDef {
char *dev; /* name of device */
int usageCount; /* how many guest interfaces are bound to this device? */
+ bool isPciAddr; /* Differentiate a VF based on interface name or pci addr*/
+};
+
+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? */
};
typedef struct _virPortGroupDef virPortGroupDef;
@@ -163,7 +171,7 @@ struct _virNetworkDef {
* interfaces), they will be listed here.
*/
size_t nForwardPfs;
- virNetworkForwardIfDefPtr forwardPfs;
+ virNetworkForwardPfDefPtr forwardPfs;
size_t nForwardIfs;
virNetworkForwardIfDefPtr forwardIfs;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index d82212f..41e3a49 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -87,6 +87,43 @@ struct network_driver {
char *logDir;
};
+/* Function to compare strings with wildcard strings*/
+/* When editing this function also edit the one in src/conf/network_conf.c*/
+static int
+wildcmp(const char *wild, const char *string)
+{
+ // Written by Jack Handy - <A
href="mailto:jakkhandy@hotmail.com">jakkhandy@otmail.com</A>
+ const char *cp = NULL, *mp = NULL;
+
+ while ((*string) && (*wild != '*')) {
+ if ((*wild != *string) && (*wild != '?')) {
+ return 0;
+ }
+ wild++;
+ string++;
+ }
+
+ while (*string) {
+ if (*wild == '*') {
+ if (!*++wild) {
+ return 1;
+ }
+ mp = wild;
+ cp = string+1;
+ } else if ((*wild == *string) || (*wild == '?')) {
+ wild++;
+ string++;
+ } else {
+ wild = mp;
+ string = cp++;
+ }
+ }
+
+ while (*wild == '*') {
+ wild++;
+ }
+ return !*wild;
+}
static void networkDriverLock(struct network_driver *driver)
{
@@ -2918,6 +2955,16 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
goto cleanup;
}
netdef->forwardIfs[ii].usageCount = 0;
+ if
(wildcmp("????:??:??.?",netdef->forwardIfs[ii].dev))
+ netdef->forwardIfs[ii].isPciAddr = true;
+ else
+ netdef->forwardIfs[ii].isPciAddr = false;
+
+ if (netdef->forwardIfs[ii].isPciAddr == true) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Passthrough mode does not work
with PCI addresses and needs Interface names"));
+ goto cleanup;
+ }
}
}
diff --git a/src/util/pci.c b/src/util/pci.c
index b3be405..88e4ac5 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -2113,8 +2113,11 @@ pciDeviceNetName(char *device_link_sysfs_path, char **netname)
}
dir = opendir(pcidev_sysfs_net_path);
- if (dir == NULL)
+ if (dir == NULL) {
+ VIR_DEBUG("No interface name");
+ ret = 1;
goto out;
+ }
while ((entry = readdir(dir))) {
if (STREQ(entry->d_name, ".") ||
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index d53352f..a328294 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -989,7 +989,6 @@ virNetDevGetVirtualFunctions(const char *pfname,
char *pf_sysfs_device_link = NULL;
char *pci_sysfs_device_link = NULL;
struct pci_config_address **virt_fns;
- char *pciConfigAddr;
if (virNetDevSysfsFile(&pf_sysfs_device_link, pfname, "device") <
0)
return ret;
@@ -1009,12 +1008,12 @@ virNetDevGetVirtualFunctions(const char *pfname,
virt_fns[i]->bus,
virt_fns[i]->slot,
virt_fns[i]->function,
- &pciConfigAddr) < 0) {
+ &((*vfname)[i])) < 0) {
virReportSystemError(ENOSYS, "%s",
_("Failed to get PCI Config Address
String"));
goto cleanup;
}
- if (pciSysfsFile(pciConfigAddr, &pci_sysfs_device_link) < 0) {
+ if (pciSysfsFile((*vfname)[i], &pci_sysfs_device_link) < 0) {
virReportSystemError(ENOSYS, "%s",
_("Failed to get PCI SYSFS file"));
goto cleanup;
@@ -1037,7 +1036,6 @@ cleanup:
VIR_FREE(virt_fns);
VIR_FREE(pf_sysfs_device_link);
VIR_FREE(pci_sysfs_device_link);
- VIR_FREE(pciConfigAddr);
return ret;
}
--
1.7.4.4
10:50 a.m.
New subject: [libvirt] [PATCH 1/5] Code to return interface name or pci_addr of the VF in actualDevice
On Fri, Jun 08, 2012 at 04:29:14PM +0100, Shradha Shah wrote:
@@ -925,6 +931,44 @@ error:
return result;
}
+/* Function to compare strings with wildcard strings*/
+/* When editing this function also edit the one in src/network/bridge_driver.c*/
+static int
+wildcmp(const char *wild, const char *string)
+{
+/* Written by Jack Handy - <A
href="mailto:jakkhandy@hotmail.com">jakkhandy@hotmail.com</A>*/
Isn't this just reimplementing the Glibc 'fnmatch' function that
we already use ?
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
12:14 p.m.
New subject: [libvirt] [PATCH 1/5] Code to return interface name or pci_addr of the VF in actualDevice
On 06/11/2012 11:50 AM, Daniel P. Berrange wrote:
On Fri, Jun 08, 2012 at 04:29:14PM +0100, Shradha Shah wrote:
> @@ -925,6 +931,44 @@ error:
> return result;
> }
>
> +/* Function to compare strings with wildcard strings*/
> +/* When editing this function also edit the one in src/network/bridge_driver.c*/
> +static int
> +wildcmp(const char *wild, const char *string)
> +{
> +/* Written by Jack Handy - <A
href="mailto:jakkhandy@hotmail.com">jakkhandy@hotmail.com</A>*/
Isn't this just reimplementing the Glibc 'fnmatch' function that
we already use ?
Beyond that, this is being used to support another case of crowding
multiple data items into a single attribute, which we actively
discourage (see the current thread about dhcpsnoop :-).
Instead of re-purposing the singular char *dev, it would be better to
follow the example of, e.g., virDomainDeviceInfo, and have a union like
this:
int type;
union {
virDomainDevicePCIAddress pci;
char *dev;
} addr;
or just put both directly in the struct if you might want both to be
populated at the same time for some reason (maybe you want to derive all
the PCI addresses from the netdev name, or vice versa, and cache them
here. Or maybe you don't, I don't have an opinion).
The XML for this would then look something like this:
<network>
<name>hostdev-network</name>
<forward mode="hostdev">
<interface type='pci' domain='0x0000' bus='0x04'
slot='0x00'
function='0x01'/>
<interface type='pci' domain='0x0000' bus='0x04'
slot='0x00'
function='0x02'/>
<interface type='pci' domain='0x0000' bus='0x04'
slot='0x00'
function='0x03'/>
</forward>
</network>
Or maybe, for consistency with source addresses in domain device
definitions, it should be:
<network>
<name>hostdev-network</name>
<forward mode="hostdev">
<address type='pci' domain='0x0000' bus='0x04'
slot='0x00'
function='0x01'/>
<address type='pci' domain='0x0000' bus='0x04'
slot='0x00'
function='0x02'/>
<address type='pci' domain='0x0000' bus='0x04'
slot='0x00'
function='0x03'/>
</forward>
</network>
(anyone else have an opinion on this?)
As a preparation for this, you should probably have a prerequisite patch
that renames virDomainDevicePCIAddress to virDevicePCIAddress, moves it
to the new file device_conf.h, and also moves
virDomainDevicePCIAddressParseXML into device_conf.c (renaming it on the
way). Similarly, the part of virDomainHostdevSourceFormat that formats a
pci address could/should be moved into its own function in device_conf.c.
(I'm suggesting the name "device_conf.c" rather than
"pci_device_conf.c"
because I'm thinking it might be good to rename/move the other
virDomainDevice*Address structs and functions to the same place. Before
taking the time to make this reorganization, it might be good to get
feedback from some other people.)
11:03 a.m.
New subject: [libvirt] [PATCH 1/5] Code to return interface name or pci_addr of the VF in actualDevice
On 06/11/2012 01:14 PM, Laine Stump wrote:
On 06/11/2012 11:50 AM, Daniel P. Berrange wrote:
> On Fri, Jun 08, 2012 at 04:29:14PM +0100, Shradha Shah wrote:
>> @@ -925,6 +931,44 @@ error:
>> return result;
>> }
>>
>> +/* Function to compare strings with wildcard strings*/
>> +/* When editing this function also edit the one in
src/network/bridge_driver.c*/
>> +static int
>> +wildcmp(const char *wild, const char *string)
>> +{
>> +/* Written by Jack Handy - <A
href="mailto:jakkhandy@hotmail.com">jakkhandy@hotmail.com</A>*/
> Isn't this just reimplementing the Glibc 'fnmatch' function that
> we already use ?
Beyond that, this is being used to support another case of crowding
multiple data items into a single attribute, which we actively
discourage (see the current thread about dhcpsnoop :-).
Instead of re-purposing the singular char *dev, it would be better to
follow the example of, e.g., virDomainDeviceInfo, and have a union like
this:
int type;
union {
virDomainDevicePCIAddress pci;
char *dev;
} addr;
or just put both directly in the struct if you might want both to be
populated at the same time for some reason (maybe you want to derive all
the PCI addresses from the netdev name, or vice versa, and cache them
here. Or maybe you don't, I don't have an opinion).
The XML for this would then look something like this:
<network>
<name>hostdev-network</name>
<forward mode="hostdev">
<interface type='pci' domain='0x0000' bus='0x04'
slot='0x00'
function='0x01'/>
<interface type='pci' domain='0x0000' bus='0x04'
slot='0x00'
function='0x02'/>
<interface type='pci' domain='0x0000' bus='0x04'
slot='0x00'
function='0x03'/>
</forward>
</network>
Or maybe, for consistency with source addresses in domain device
definitions, it should be:
<network>
<name>hostdev-network</name>
<forward mode="hostdev">
<address type='pci' domain='0x0000' bus='0x04'
slot='0x00'
function='0x01'/>
<address type='pci' domain='0x0000' bus='0x04'
slot='0x00'
function='0x02'/>
<address type='pci' domain='0x0000' bus='0x04'
slot='0x00'
function='0x03'/>
</forward>
</network>
Also, about the "managed" attribute you introduced in PATCH 5/5 (which I
suggested you should add at the same time as the rest of the new XML):
Do you really think anyone will ever want some of the interfaces to be
managed='yes' and some managed='no'? If you rework the XML as I suggest
above, you would be able to reuse the same function to parse and format
pci addresses; if you add a "managed" attribute to each address element,
you would no longer be able to do that.
Alternately, if you just add a single managed attribute to <forward>,
the <address> sub-element would remain identical to the <address> in the
domain devices' <source> element, so the parse/format functions could be
shared (and the consistency would also make mixups less likely).
Something like this:
<network>
<name>hostdev-network</name>
<forward mode="hostdev" managed="yes">
<address type='pci' domain='0x0000' bus='0x04'
slot='0x00' function='0x01'/>
<address type='pci' domain='0x0000' bus='0x04'
slot='0x00' function='0x02'/>
<address type='pci' domain='0x0000' bus='0x04'
slot='0x00' function='0x03'/>
</forward>
</network
You lose some flexibility, but are able to re-use more code. If it never
makes sense to mix managed and un-managed (I don't know enough about the
topic to have the answer to that), then the lost flexibility is meaningless.
(anyone else have an opinion on this?)
The same question holds for this new addition :-)
As a preparation for this, you should probably have a prerequisite
patch
that renames virDomainDevicePCIAddress to virDevicePCIAddress, moves it
to the new file device_conf.h, and also moves
virDomainDevicePCIAddressParseXML into device_conf.c (renaming it on the
way). Similarly, the part of virDomainHostdevSourceFormat that formats a
pci address could/should be moved into its own function in device_conf.c.
(I'm suggesting the name "device_conf.c" rather than
"pci_device_conf.c"
because I'm thinking it might be good to rename/move the other
virDomainDevice*Address structs and functions to the same place. Before
taking the time to make this reorganization, it might be good to get
feedback from some other people.)
11:08 a.m.
New subject: [libvirt] [PATCH 1/5] Code to return interface name or pci_addr of the VF in actualDevice
On Tue, Jun 12, 2012 at 12:03:02PM -0400, Laine Stump wrote:
On 06/11/2012 01:14 PM, Laine Stump wrote:
> On 06/11/2012 11:50 AM, Daniel P. Berrange wrote:
>> On Fri, Jun 08, 2012 at 04:29:14PM +0100, Shradha Shah wrote:
>>> @@ -925,6 +931,44 @@ error:
>>> return result;
>>> }
>>>
>>> +/* Function to compare strings with wildcard strings*/
>>> +/* When editing this function also edit the one in
src/network/bridge_driver.c*/
>>> +static int
>>> +wildcmp(const char *wild, const char *string)
>>> +{
>>> +/* Written by Jack Handy - <A
href="mailto:jakkhandy@hotmail.com">jakkhandy@hotmail.com</A>*/
>> Isn't this just reimplementing the Glibc 'fnmatch' function that
>> we already use ?
> Beyond that, this is being used to support another case of crowding
> multiple data items into a single attribute, which we actively
> discourage (see the current thread about dhcpsnoop :-).
>
> Instead of re-purposing the singular char *dev, it would be better to
> follow the example of, e.g., virDomainDeviceInfo, and have a union like
> this:
>
> int type;
> union {
> virDomainDevicePCIAddress pci;
> char *dev;
> } addr;
>
> or just put both directly in the struct if you might want both to be
> populated at the same time for some reason (maybe you want to derive all
> the PCI addresses from the netdev name, or vice versa, and cache them
> here. Or maybe you don't, I don't have an opinion).
>
> The XML for this would then look something like this:
>
> <network>
> <name>hostdev-network</name>
> <forward mode="hostdev">
> <interface type='pci' domain='0x0000' bus='0x04'
slot='0x00'
> function='0x01'/>
> <interface type='pci' domain='0x0000' bus='0x04'
slot='0x00'
> function='0x02'/>
> <interface type='pci' domain='0x0000' bus='0x04'
slot='0x00'
> function='0x03'/>
> </forward>
> </network>
>
> Or maybe, for consistency with source addresses in domain device
> definitions, it should be:
>
> <network>
> <name>hostdev-network</name>
> <forward mode="hostdev">
> <address type='pci' domain='0x0000' bus='0x04'
slot='0x00'
> function='0x01'/>
> <address type='pci' domain='0x0000' bus='0x04'
slot='0x00'
> function='0x02'/>
> <address type='pci' domain='0x0000' bus='0x04'
slot='0x00'
> function='0x03'/>
> </forward>
> </network>
Also, about the "managed" attribute you introduced in PATCH 5/5 (which I
suggested you should add at the same time as the rest of the new XML):
Do you really think anyone will ever want some of the interfaces to be
managed='yes' and some managed='no'? If you rework the XML as I suggest
above, you would be able to reuse the same function to parse and format
pci addresses; if you add a "managed" attribute to each address element,
you would no longer be able to do that.
Alternately, if you just add a single managed attribute to <forward>,
the <address> sub-element would remain identical to the <address> in the
domain devices' <source> element, so the parse/format functions could be
shared (and the consistency would also make mixups less likely).
Something like this:
<network>
<name>hostdev-network</name>
<forward mode="hostdev" managed="yes">
<address type='pci' domain='0x0000' bus='0x04'
slot='0x00' function='0x01'/>
<address type='pci' domain='0x0000' bus='0x04'
slot='0x00' function='0x02'/>
<address type='pci' domain='0x0000' bus='0x04'
slot='0x00' function='0x03'/>
</forward>
</network
You lose some flexibility, but are able to re-use more code. If it never
makes sense to mix managed and un-managed (I don't know enough about the
topic to have the answer to that), then the lost flexibility is meaningless.
> (anyone else have an opinion on this?)
The same question holds for this new addition :-)
Yes, I tend to agree with your suggestions that we should model the
PCI addresses fully and not have their encoded in a string.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
10:29 a.m.
New subject: [libvirt] [PATCH 2/5] Moved the code to create implicit interface pool from PF to a new function
This makes the code reusable
Signed-off-by: Shradha Shah <sshah(a)solarflare.com>
---
src/network/bridge_driver.c | 109 ++++++++++++++++++++++++++-----------------
1 files changed, 66 insertions(+), 43 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 41e3a49..8540003 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2761,6 +2761,60 @@ int networkRegister(void) {
* "backend" function table.
*/
+/* networkCreateInterfacePool:
+ * @netdef: the original NetDef from the network
+ *
+ * Creates an implicit interface pool of VF's when a PF dev is given
+ */
+static int
+networkCreateInterfacePool(virNetworkDefPtr netdef) {
+ unsigned int num_virt_fns = 0;
+ char **vfname = NULL;
+ int ret = -1, ii = 0;
+
+ if ((virNetDevGetVirtualFunctions(netdef->forwardPfs->dev,
+ &vfname, &num_virt_fns)) < 0) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not get Virtual functions on %s"),
+ netdef->forwardPfs->dev);
+ goto finish;
+ }
+
+ if (num_virt_fns == 0) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ _("No Vf's present on SRIOV PF %s"),
+ netdef->forwardPfs->dev);
+ goto finish;
+ }
+
+ if ((VIR_ALLOC_N(netdef->forwardIfs, num_virt_fns)) < 0) {
+ virReportOOMError();
+ goto finish;
+ }
+
+ netdef->nForwardIfs = num_virt_fns;
+
+ for (ii = 0; ii < netdef->nForwardIfs; ii++) {
+ netdef->forwardIfs[ii].dev = strdup(vfname[ii]);
+ if (!netdef->forwardIfs[ii].dev) {
+ virReportOOMError();
+ goto finish;
+ }
+ netdef->forwardIfs[ii].usageCount = 0;
+ if (wildcmp("????:??:??.?", netdef->forwardIfs[ii].dev))
+ netdef->forwardIfs[ii].isPciAddr = true;
+ else
+ netdef->forwardIfs[ii].isPciAddr = false;
+ }
+
+ ret = 0;
+finish:
+ for (ii = 0; ii < num_virt_fns; ii++)
+ VIR_FREE(vfname[ii]);
+ VIR_FREE(vfname);
+ return ret;
+}
+
/* networkAllocateActualDevice:
* @iface: the original NetDef from the domain
*
@@ -2779,8 +2833,6 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
virNetworkObjPtr network;
virNetworkDefPtr netdef;
virPortGroupDefPtr portgroup;
- unsigned int num_virt_fns = 0;
- char **vfname = NULL;
int ii;
int ret = -1;
@@ -2858,7 +2910,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
(netdef->forwardType == VIR_NETWORK_FORWARD_VEPA) ||
(netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH)) {
virNetDevVPortProfilePtr virtport = NULL;
-
+ int rc = -1;
/* <forward type='bridge|private|vepa|passthrough'> are all
* VIR_DOMAIN_NET_TYPE_DIRECT.
*/
@@ -2926,57 +2978,31 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
*/
if (netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH) {
if ((netdef->nForwardPfs > 0) && (netdef->nForwardIfs
<= 0)) {
- if ((virNetDevGetVirtualFunctions(netdef->forwardPfs->dev,
- &vfname, &num_virt_fns))
< 0) {
+ if((rc = networkCreateInterfacePool(netdef)) < 0) {
networkReportError(VIR_ERR_INTERNAL_ERROR,
- _("Could not get Virtual functions on
%s"),
- netdef->forwardPfs->dev);
+ _("Could not create Interface Pool from
PF"));
goto cleanup;
}
-
- if (num_virt_fns == 0) {
+ }
+
+ /*if dev names are pci addrs in passthrough mode: error*/
+ for (ii = 0; ii <netdef->nForwardIfs; ii++) {
+ if (netdef->forwardIfs[ii].isPciAddr == true) {
networkReportError(VIR_ERR_INTERNAL_ERROR,
- _("No Vf's present on SRIOV PF
%s"),
- netdef->forwardPfs->dev);
+ _("Passthrough mode does not work with
PCI addresses and needs Interface names"));
goto cleanup;
}
-
- if ((VIR_ALLOC_N(netdef->forwardIfs, num_virt_fns)) < 0) {
- virReportOOMError();
- goto cleanup;
- }
-
- netdef->nForwardIfs = num_virt_fns;
-
- for (ii = 0; ii < netdef->nForwardIfs; ii++) {
- netdef->forwardIfs[ii].dev = strdup(vfname[ii]);
- if (!netdef->forwardIfs[ii].dev) {
- virReportOOMError();
- goto cleanup;
- }
- netdef->forwardIfs[ii].usageCount = 0;
- if
(wildcmp("????:??:??.?",netdef->forwardIfs[ii].dev))
- netdef->forwardIfs[ii].isPciAddr = true;
- else
- netdef->forwardIfs[ii].isPciAddr = false;
-
- if (netdef->forwardIfs[ii].isPciAddr == true) {
- networkReportError(VIR_ERR_INTERNAL_ERROR,
- _("Passthrough mode does not work
with PCI addresses and needs Interface names"));
- goto cleanup;
- }
- }
}
-
+
/* pick first dev with 0 usageCount */
for (ii = 0; ii < netdef->nForwardIfs; ii++) {
if (netdef->forwardIfs[ii].usageCount == 0) {
dev = &netdef->forwardIfs[ii];
break;
- }
+ }
}
- } else if ((netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE)
&&
+ }else if ((netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE) &&
iface->data.network.actual->data.direct.virtPortProfile
&&
(iface->data.network.actual->data.direct.virtPortProfile->virtPortType
== VIR_NETDEV_VPORT_PROFILE_8021QBH)) {
@@ -3017,9 +3043,6 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
ret = 0;
cleanup:
- for (ii = 0; ii < num_virt_fns; ii++)
- VIR_FREE(vfname[ii]);
- VIR_FREE(vfname);
if (network)
virNetworkObjUnlock(network);
if (ret < 0) {
--
1.7.4.4
11:12 a.m.
New subject: [libvirt] [PATCH 2/5] Moved the code to create implicit interface pool from PF to a new function
On 06/08/2012 11:29 AM, Shradha Shah wrote:
This makes the code reusable
Rather than adding new code, then moving that new code around along with
existing code that needs to be moved, it would be cleaner and easier to
follow if you first moved the existing code, then introduced the new
code (directly in the place that it will eventually live).
Signed-off-by: Shradha Shah <sshah(a)solarflare.com>
---
src/network/bridge_driver.c | 109 ++++++++++++++++++++++++++-----------------
1 files changed, 66 insertions(+), 43 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 41e3a49..8540003 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2761,6 +2761,60 @@ int networkRegister(void) {
* "backend" function table.
*/
+/* networkCreateInterfacePool:
+ * @netdef: the original NetDef from the network
+ *
+ * Creates an implicit interface pool of VF's when a PF dev is given
+ */
+static int
+networkCreateInterfacePool(virNetworkDefPtr netdef) {
+ unsigned int num_virt_fns = 0;
+ char **vfname = NULL;
+ int ret = -1, ii = 0;
+
+ if ((virNetDevGetVirtualFunctions(netdef->forwardPfs->dev,
+ &vfname, &num_virt_fns)) < 0) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not get Virtual functions on %s"),
+ netdef->forwardPfs->dev);
+ goto finish;
+ }
+
+ if (num_virt_fns == 0) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ _("No Vf's present on SRIOV PF %s"),
+ netdef->forwardPfs->dev);
+ goto finish;
+ }
+
+ if ((VIR_ALLOC_N(netdef->forwardIfs, num_virt_fns)) < 0) {
+ virReportOOMError();
+ goto finish;
+ }
+
+ netdef->nForwardIfs = num_virt_fns;
+
+ for (ii = 0; ii < netdef->nForwardIfs; ii++) {
+ netdef->forwardIfs[ii].dev = strdup(vfname[ii]);
+ if (!netdef->forwardIfs[ii].dev) {
+ virReportOOMError();
+ goto finish;
+ }
+ netdef->forwardIfs[ii].usageCount = 0;
+ if (wildcmp("????:??:??.?", netdef->forwardIfs[ii].dev))
+ netdef->forwardIfs[ii].isPciAddr = true;
+ else
+ netdef->forwardIfs[ii].isPciAddr = false;
+ }
+
+ ret = 0;
+finish:
+ for (ii = 0; ii < num_virt_fns; ii++)
+ VIR_FREE(vfname[ii]);
+ VIR_FREE(vfname);
+ return ret;
+}
+
/* networkAllocateActualDevice:
* @iface: the original NetDef from the domain
*
@@ -2779,8 +2833,6 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
virNetworkObjPtr network;
virNetworkDefPtr netdef;
virPortGroupDefPtr portgroup;
- unsigned int num_virt_fns = 0;
- char **vfname = NULL;
int ii;
int ret = -1;
@@ -2858,7 +2910,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
(netdef->forwardType == VIR_NETWORK_FORWARD_VEPA) ||
(netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH)) {
virNetDevVPortProfilePtr virtport = NULL;
-
+ int rc = -1;
/* <forward type='bridge|private|vepa|passthrough'> are all
* VIR_DOMAIN_NET_TYPE_DIRECT.
*/
@@ -2926,57 +2978,31 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
*/
if (netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH) {
if ((netdef->nForwardPfs > 0) && (netdef->nForwardIfs
<= 0)) {
- if ((virNetDevGetVirtualFunctions(netdef->forwardPfs->dev,
- &vfname, &num_virt_fns))
< 0) {
+ if((rc = networkCreateInterfacePool(netdef)) < 0) {
networkReportError(VIR_ERR_INTERNAL_ERROR,
- _("Could not get Virtual functions on
%s"),
- netdef->forwardPfs->dev);
+ _("Could not create Interface Pool from
PF"));
goto cleanup;
}
-
- if (num_virt_fns == 0) {
+ }
+
+ /*if dev names are pci addrs in passthrough mode: error*/
+ for (ii = 0; ii <netdef->nForwardIfs; ii++) {
+ if (netdef->forwardIfs[ii].isPciAddr == true) {
networkReportError(VIR_ERR_INTERNAL_ERROR,
- _("No Vf's present on SRIOV PF
%s"),
- netdef->forwardPfs->dev);
+ _("Passthrough mode does not work with
PCI addresses and needs Interface names"));
goto cleanup;
}
-
- if ((VIR_ALLOC_N(netdef->forwardIfs, num_virt_fns)) < 0) {
- virReportOOMError();
- goto cleanup;
- }
-
- netdef->nForwardIfs = num_virt_fns;
-
- for (ii = 0; ii < netdef->nForwardIfs; ii++) {
- netdef->forwardIfs[ii].dev = strdup(vfname[ii]);
- if (!netdef->forwardIfs[ii].dev) {
- virReportOOMError();
- goto cleanup;
- }
- netdef->forwardIfs[ii].usageCount = 0;
- if
(wildcmp("????:??:??.?",netdef->forwardIfs[ii].dev))
- netdef->forwardIfs[ii].isPciAddr = true;
- else
- netdef->forwardIfs[ii].isPciAddr = false;
-
- if (netdef->forwardIfs[ii].isPciAddr == true) {
- networkReportError(VIR_ERR_INTERNAL_ERROR,
- _("Passthrough mode does not work
with PCI addresses and needs Interface names"));
- goto cleanup;
- }
- }
}
-
+
/* pick first dev with 0 usageCount */
for (ii = 0; ii < netdef->nForwardIfs; ii++) {
if (netdef->forwardIfs[ii].usageCount == 0) {
dev = &netdef->forwardIfs[ii];
break;
- }
+ }
}
- } else if ((netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE)
&&
+ }else if ((netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE)
&&
iface->data.network.actual->data.direct.virtPortProfile
&&
(iface->data.network.actual->data.direct.virtPortProfile->virtPortType
== VIR_NETDEV_VPORT_PROFILE_8021QBH)) {
@@ -3017,9 +3043,6 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
ret = 0;
cleanup:
- for (ii = 0; ii < num_virt_fns; ii++)
- VIR_FREE(vfname[ii]);
- VIR_FREE(vfname);
if (network)
virNetworkObjUnlock(network);
if (ret < 0) {
10:29 a.m.
New subject: [libvirt] [PATCH 3/5] Introduce forward mode='hostdev' for network XML in order to use the functionality of interface pools.
This new forward mode sets up a PCI network device to be assigned to the guest with PCI
passthrough.
The PCI network device is chosen from an interface pool.
Currently there is no support for USB devices when using this forward mode.
Example XML:
Network XML:
<network>
<name>direct-network</name>
<uuid>81ff0d90-c91e-6742-64da-4a736edb9a8f</uuid>
<forward mode="hostdev">
<pf dev="eth2"/>
</forward>
</network>
The above xml leads to the creation of an implicit interface pool and a free network
device
is chosen from the pool. This patch would also work if an interface pool is given
explicitly.
The interface dev value can be an interface name or a PCI device address.
Example XML:
Network XML:
<network>
<name>direct-network</name>
<uuid>81ff0d90-c91e-6742-64da-4a736edb9a8f</uuid>
<forward mode="hostdev">
<interface dev="0000:04:00.1"/>
<interface dev="0000:04:00.2"/>
<interface dev="0000:04:00.3"/>
</forward>
</network>
The MAC address would be provided in the domain xml.
Signed-off-by: Shradha Shah <sshah(a)solarflare.com>
---
docs/schemas/network.rng | 1 +
src/conf/network_conf.c | 3 ++-
src/conf/network_conf.h | 1 +
src/network/bridge_driver.c | 6 ++++--
4 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
index 2ae879e..a0046f1 100644
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -82,6 +82,7 @@
<value>passthrough</value>
<value>private</value>
<value>vepa</value>
+ <value>hostdev</value>
</choice>
</attribute>
</optional>
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 8fcba16..6b346c3 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -51,7 +51,7 @@ VIR_ENUM_DECL(virNetworkForward)
VIR_ENUM_IMPL(virNetworkForward,
VIR_NETWORK_FORWARD_LAST,
- "none", "nat", "route", "bridge",
"private", "vepa", "passthrough" )
+ "none", "nat", "route", "bridge",
"private", "vepa", "passthrough", "hostdev")
#define virNetworkReportError(code, ...) \
virReportErrorHelper(VIR_FROM_NETWORK, code, __FILE__, \
@@ -1251,6 +1251,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) {
virNetworkReportError(VIR_ERR_XML_ERROR,
_("bridge name not allowed in %s mode (network
'%s'"),
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index b205cb0..d473c71 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -45,6 +45,7 @@ enum virNetworkForwardType {
VIR_NETWORK_FORWARD_PRIVATE,
VIR_NETWORK_FORWARD_VEPA,
VIR_NETWORK_FORWARD_PASSTHROUGH,
+ VIR_NETWORK_FORWARD_HOSTDEV,
VIR_NETWORK_FORWARD_LAST,
};
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 8540003..cc53551 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1974,7 +1974,7 @@ networkStartNetworkExternal(struct network_driver *driver
ATTRIBUTE_UNUSED,
virNetworkObjPtr network ATTRIBUTE_UNUSED)
{
/* put anything here that needs to be done each time a network of
- * type BRIDGE, PRIVATE, VEPA, or PASSTHROUGH is started. On
+ * type BRIDGE, PRIVATE, VEPA, HOSTDEV or PASSTHROUGH is started. On
* failure, undo anything you've done, and return -1. On success
* return 0.
*/
@@ -1985,7 +1985,7 @@ static int networkShutdownNetworkExternal(struct network_driver
*driver ATTRIBUT
virNetworkObjPtr network ATTRIBUTE_UNUSED)
{
/* put anything here that needs to be done each time a network of
- * type BRIDGE, PRIVATE, VEPA, or PASSTHROUGH is shutdown. On
+ * type BRIDGE, PRIVATE, VEPA, HOSTDEV or PASSTHROUGH is shutdown. On
* failure, undo anything you've done, and return -1. On success
* return 0.
*/
@@ -2016,6 +2016,7 @@ networkStartNetwork(struct network_driver *driver,
case VIR_NETWORK_FORWARD_PRIVATE:
case VIR_NETWORK_FORWARD_VEPA:
case VIR_NETWORK_FORWARD_PASSTHROUGH:
+ case VIR_NETWORK_FORWARD_HOSTDEV:
ret = networkStartNetworkExternal(driver, network);
break;
}
@@ -2075,6 +2076,7 @@ static int networkShutdownNetwork(struct network_driver *driver,
case VIR_NETWORK_FORWARD_PRIVATE:
case VIR_NETWORK_FORWARD_VEPA:
case VIR_NETWORK_FORWARD_PASSTHROUGH:
+ case VIR_NETWORK_FORWARD_HOSTDEV:
ret = networkShutdownNetworkExternal(driver, network);
break;
}
--
1.7.4.4
12:22 p.m.
New subject: [libvirt] [PATCH 3/5] Introduce forward mode='hostdev' for network XML in order to use the functionality of interface pools.
On 06/08/2012 11:29 AM, Shradha Shah wrote:
This new forward mode sets up a PCI network device to be assigned to
the guest with PCI passthrough.
The PCI network device is chosen from an interface pool.
Currently there is no support for USB devices when using this forward mode.
Example XML:
Network XML:
<network>
<name>direct-network</name>
<uuid>81ff0d90-c91e-6742-64da-4a736edb9a8f</uuid>
<forward mode="hostdev">
<pf dev="eth2"/>
</forward>
</network>
See my comment on PATCH 1/5 for my suggestion on changing the XML format.
The above xml leads to the creation of an implicit interface pool and a free network
device
is chosen from the pool. This patch would also work if an interface pool is given
explicitly.
The interface dev value can be an interface name or a PCI device address.
Example XML:
Network XML:
<network>
<name>direct-network</name>
<uuid>81ff0d90-c91e-6742-64da-4a736edb9a8f</uuid>
<forward mode="hostdev">
<interface dev="0000:04:00.1"/>
<interface dev="0000:04:00.2"/>
<interface dev="0000:04:00.3"/>
</forward>
</network>
The MAC address would be provided in the domain xml.
Signed-off-by: Shradha Shah <sshah(a)solarflare.com>
---
docs/schemas/network.rng | 1 +
src/conf/network_conf.c | 3 ++-
src/conf/network_conf.h | 1 +
src/network/bridge_driver.c | 6 ++++--
4 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
index 2ae879e..a0046f1 100644
--- a/docs/schemas/network.rng
+++ b/docs/schemas/network.rng
@@ -82,6 +82,7 @@
<value>passthrough</value>
<value>private</value>
<value>vepa</value>
+ <value>hostdev</value>
</choice>
</attribute>
</optional>
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 8fcba16..6b346c3 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -51,7 +51,7 @@ VIR_ENUM_DECL(virNetworkForward)
VIR_ENUM_IMPL(virNetworkForward,
VIR_NETWORK_FORWARD_LAST,
- "none", "nat", "route", "bridge",
"private", "vepa", "passthrough" )
+ "none", "nat", "route", "bridge",
"private", "vepa", "passthrough", "hostdev")
#define virNetworkReportError(code, ...) \
virReportErrorHelper(VIR_FROM_NETWORK, code, __FILE__, \
@@ -1251,6 +1251,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) {
virNetworkReportError(VIR_ERR_XML_ERROR,
_("bridge name not allowed in %s mode
(network '%s'"),
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index b205cb0..d473c71 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -45,6 +45,7 @@ enum virNetworkForwardType {
VIR_NETWORK_FORWARD_PRIVATE,
VIR_NETWORK_FORWARD_VEPA,
VIR_NETWORK_FORWARD_PASSTHROUGH,
+ VIR_NETWORK_FORWARD_HOSTDEV,
VIR_NETWORK_FORWARD_LAST,
};
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 8540003..cc53551 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1974,7 +1974,7 @@ networkStartNetworkExternal(struct network_driver *driver
ATTRIBUTE_UNUSED,
virNetworkObjPtr network ATTRIBUTE_UNUSED)
{
/* put anything here that needs to be done each time a network of
- * type BRIDGE, PRIVATE, VEPA, or PASSTHROUGH is started. On
+ * type BRIDGE, PRIVATE, VEPA, HOSTDEV or PASSTHROUGH is started. On
* failure, undo anything you've done, and return -1. On success
* return 0.
*/
@@ -1985,7 +1985,7 @@ static int networkShutdownNetworkExternal(struct network_driver
*driver ATTRIBUT
virNetworkObjPtr network ATTRIBUTE_UNUSED)
{
/* put anything here that needs to be done each time a network of
- * type BRIDGE, PRIVATE, VEPA, or PASSTHROUGH is shutdown. On
+ * type BRIDGE, PRIVATE, VEPA, HOSTDEV or PASSTHROUGH is shutdown. On
* failure, undo anything you've done, and return -1. On success
* return 0.
*/
@@ -2016,6 +2016,7 @@ networkStartNetwork(struct network_driver *driver,
case VIR_NETWORK_FORWARD_PRIVATE:
case VIR_NETWORK_FORWARD_VEPA:
case VIR_NETWORK_FORWARD_PASSTHROUGH:
+ case VIR_NETWORK_FORWARD_HOSTDEV:
ret = networkStartNetworkExternal(driver, network);
break;
}
@@ -2075,6 +2076,7 @@ static int networkShutdownNetwork(struct network_driver *driver,
case VIR_NETWORK_FORWARD_PRIVATE:
case VIR_NETWORK_FORWARD_VEPA:
case VIR_NETWORK_FORWARD_PASSTHROUGH:
+ case VIR_NETWORK_FORWARD_HOSTDEV:
ret = networkShutdownNetworkExternal(driver, network);
break;
}
10:29 a.m.
New subject: [libvirt] [PATCH 4/5] Forward Mode Hostdev Implementation
This patch chooses a free network device from the interface pool and creates a PCI
HostDef
to be passed to the guest, when forward mode is "hostdev".
networkNotifyActualDevice and networkReleaseActualDevice are modified accordingly.
Signed-off-by: Shradha Shah <sshah(a)solarflare.com>
---
src/libvirt_private.syms | 3 +
src/network/bridge_driver.c | 180 +++++++++++++++++++++++++++++++++++++------
src/qemu/qemu_command.c | 14 ++++
src/util/pci.c | 2 +-
src/util/pci.h | 3 +
src/util/virnetdev.c | 128 ++++++++++++++++++++++++++++++
src/util/virnetdev.h | 19 +++++
7 files changed, 325 insertions(+), 24 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index afb308d..5b8ab0b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1261,6 +1261,9 @@ virNetDevGetVLanID;
virNetDevGetVirtualFunctionIndex;
virNetDevGetVirtualFunctionInfo;
virNetDevGetVirtualFunctions;
+virNetDevParsePciConfigAddress;
+virNetDevGetDeviceAddrString;
+virNetDevGetPciAddrFromName;
virNetDevIsOnline;
virNetDevIsVirtualFunction;
virNetDevLinkDump;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index cc53551..691ab07 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2835,6 +2835,8 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
virNetworkObjPtr network;
virNetworkDefPtr netdef;
virPortGroupDefPtr portgroup;
+ virNetDevVPortProfilePtr virtport = NULL;
+ virNetworkForwardIfDefPtr dev = NULL;
int ii;
int ret = -1;
@@ -2906,12 +2908,95 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
virReportOOMError();
goto cleanup;
}
-
+ } else if (netdef->forwardType == VIR_NETWORK_FORWARD_HOSTDEV) {
+ int rc = -1;
+ if (!iface->data.network.actual
+ && (VIR_ALLOC(iface->data.network.actual) < 0)) {
+ virReportOOMError();
+ goto cleanup;
+ }
+ iface->data.network.actual->type = VIR_DOMAIN_NET_TYPE_HOSTDEV;
+ if ((netdef->nForwardPfs > 0) && (netdef->nForwardIfs <= 0))
{
+ if((rc = networkCreateInterfacePool(netdef)) < 0) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not create Interface Pool from
PF"));
+ goto cleanup;
+ }
+ }
+ /* pick first dev with 0 usageCount */
+
+ for (ii = 0; ii < netdef->nForwardIfs; ii++) {
+ if (netdef->forwardIfs[ii].usageCount == 0) {
+ dev = &netdef->forwardIfs[ii];
+ break;
+ }
+ }
+ if (!dev) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ _("network '%s' requires exclusive access to
interfaces, but none are available"),
+ netdef->name);
+ goto cleanup;
+ }
+
+ iface->data.network.actual->data.hostdev.def.parent.type =
VIR_DOMAIN_DEVICE_NET;
+ iface->data.network.actual->data.hostdev.def.parent.data.net = iface;
+ iface->data.network.actual->data.hostdev.def.info = &iface->info;
+ iface->data.network.actual->data.hostdev.def.mode =
VIR_DOMAIN_HOSTDEV_MODE_SUBSYS;
+ iface->data.network.actual->data.hostdev.def.managed = 1;
+ iface->data.network.actual->data.hostdev.def.source.subsys.type =
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI;
+
+ if (dev->isPciAddr == true) {
+ virDomainDevicePCIAddressPtr addr =
&iface->data.network.actual->data.hostdev.def.source.subsys.u.pci;
+ if (virNetDevParsePciConfigAddress(dev->dev,
+ &addr->domain,
+ &addr->bus,
+ &addr->slot,
+ &addr->function) < 0) {
+ goto cleanup;
+ }
+ }
+ else if (dev->isPciAddr == false) {
+ virDomainDevicePCIAddressPtr addr =
&iface->data.network.actual->data.hostdev.def.source.subsys.u.pci;
+ char *device_pci_addr = NULL;
+ if (virNetDevGetPciAddrFromName(dev->dev,
+ &device_pci_addr) < 0) {
+ goto cleanup;
+ }
+ if (virNetDevParsePciConfigAddress(device_pci_addr,
+ &addr->domain,
+ &addr->bus,
+ &addr->slot,
+ &addr->function) < 0) {
+ goto cleanup;
+ }
+ VIR_FREE(device_pci_addr);
+ }
+ dev->usageCount++;
+ VIR_DEBUG("Using physical device %s, usageCount %d",
+ dev->dev, dev->usageCount);
+
+ if (iface->data.network.virtPortProfile) {
+ virtport = iface->data.network.virtPortProfile;
+ } else {
+ if (portgroup)
+ virtport = portgroup->virtPortProfile;
+ else
+ virtport = netdef->virtPortProfile;
+ }
+ if (virtport) {
+ if (VIR_ALLOC(iface->data.network.actual->data.hostdev.virtPortProfile)
< 0) {
+ virReportOOMError();
+ goto cleanup;
+ }
+ /* There are no pointers in a virtualPortProfile, so a shallow copy
+ * is sufficient
+ */
+ *iface->data.network.actual->data.direct.virtPortProfile = *virtport;
+ }
} else if ((netdef->forwardType == VIR_NETWORK_FORWARD_BRIDGE) ||
(netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE) ||
(netdef->forwardType == VIR_NETWORK_FORWARD_VEPA) ||
(netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH)) {
- virNetDevVPortProfilePtr virtport = NULL;
int rc = -1;
/* <forward type='bridge|private|vepa|passthrough'> are all
* VIR_DOMAIN_NET_TYPE_DIRECT.
@@ -2969,7 +3054,6 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
netdef->name);
goto cleanup;
} else {
- virNetworkForwardIfDefPtr dev = NULL;
/* pick an interface from the pool */
@@ -3070,14 +3154,16 @@ networkNotifyActualDevice(virDomainNetDefPtr iface)
struct network_driver *driver = driverState;
virNetworkObjPtr network;
virNetworkDefPtr netdef;
- const char *actualDev;
+ virDomainHostdevDefPtr def = NULL;
+ const char *actualDev = NULL;
int ret = -1;
if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK)
return 0;
if (!iface->data.network.actual ||
- (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_DIRECT)) {
+ ((virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_DIRECT) &&
+ (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_HOSTDEV))) {
VIR_DEBUG("Nothing to claim from network %s",
iface->data.network.name);
return 0;
}
@@ -3092,23 +3178,45 @@ networkNotifyActualDevice(virDomainNetDefPtr iface)
goto cleanup;
}
- actualDev = virDomainNetGetActualDirectDev(iface);
- if (!actualDev) {
- networkReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("the interface uses a direct mode, but
has no source dev"));
- goto cleanup;
+ if (virDomainNetGetActualType(iface) == VIR_DOMAIN_NET_TYPE_DIRECT) {
+ actualDev = virDomainNetGetActualDirectDev(iface);
+ if (!actualDev) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("the interface uses a direct mode,
but has no source dev"));
+ goto cleanup;
+ }
+ }
+
+ if (virDomainNetGetActualType(iface) == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+ def = virDomainNetGetActualHostdev(iface);
+ if (!def) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("the interface uses a hostdev mode,
but has no hostdev"));
+ goto cleanup;
+ }
}
-
netdef = network->def;
if (netdef->nForwardIfs == 0) {
networkReportError(VIR_ERR_INTERNAL_ERROR,
- _("network '%s' uses a direct mode, but has no
forward dev and no interface pool"),
+ _("network '%s' uses a direct/hostdev mode, but
has no forward dev and no interface pool"),
netdef->name);
goto cleanup;
} else {
int ii;
virNetworkForwardIfDefPtr dev = NULL;
-
+
+ virDomainDevicePCIAddressPtr addr = &(def->source.subsys.u.pci);
+ if (virDomainNetGetActualType(iface) == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+ if (virNetDevGetDeviceAddrString(addr->domain,
+ addr->bus,
+ addr->slot,
+ addr->function,
+ &actualDev) < 0) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("Could not get device string
for hostdev"));
+ goto cleanup;
+ }
+ }
/* find the matching interface in the pool and increment its usageCount */
for (ii = 0; ii < netdef->nForwardIfs; ii++) {
@@ -3125,12 +3233,12 @@ networkNotifyActualDevice(virDomainNetDefPtr iface)
goto cleanup;
}
- /* PASSTHROUGH mode, and PRIVATE Mode + 802.1Qbh both require
- * exclusive access to a device, so current usageCount must be
+ /* PASSTHROUGH mode, HOSTDEV mode and PRIVATE Mode + 802.1Qbh both
* require exclusive access to a device, so current usageCount must be
* 0 in those cases.
*/
if ((dev->usageCount > 0) &&
((netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH) ||
+ (netdef->forwardType == VIR_NETWORK_FORWARD_HOSTDEV) ||
((netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE) &&
iface->data.network.actual->data.direct.virtPortProfile &&
(iface->data.network.actual->data.direct.virtPortProfile->virtPortType
@@ -3170,14 +3278,16 @@ networkReleaseActualDevice(virDomainNetDefPtr iface)
struct network_driver *driver = driverState;
virNetworkObjPtr network = NULL;
virNetworkDefPtr netdef;
- const char *actualDev;
+ virDomainHostdevDefPtr def = NULL;
+ const char *actualDev = NULL;
int ret = -1;
if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK)
return 0;
if (!iface->data.network.actual ||
- (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_DIRECT)) {
+ ((virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_DIRECT) &&
+ (virDomainNetGetActualType(iface) != VIR_DOMAIN_NET_TYPE_HOSTDEV))) {
VIR_DEBUG("Nothing to release to network %s",
iface->data.network.name);
ret = 0;
goto cleanup;
@@ -3193,23 +3303,47 @@ networkReleaseActualDevice(virDomainNetDefPtr iface)
goto cleanup;
}
- actualDev = virDomainNetGetActualDirectDev(iface);
- if (!actualDev) {
- networkReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("the interface uses a direct mode, but
has no source dev"));
- goto cleanup;
+ if (virDomainNetGetActualType(iface) == VIR_DOMAIN_NET_TYPE_DIRECT) {
+ actualDev = virDomainNetGetActualDirectDev(iface);
+ if (!actualDev) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("the interface uses a direct mode,
but has no source dev"));
+ goto cleanup;
+ }
+ }
+
+ if (virDomainNetGetActualType(iface) == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+ def = virDomainNetGetActualHostdev(iface);
+ if (!def) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("the interface uses a hostdev mode,
but has no hostdev"));
+ goto cleanup;
+ }
}
netdef = network->def;
if (netdef->nForwardIfs == 0) {
networkReportError(VIR_ERR_INTERNAL_ERROR,
- _("network '%s' uses a direct mode, but has no
forward dev and no interface pool"),
+ _("network '%s' uses a direct/hostdev mode, but
has no forward dev and no interface pool"),
netdef->name);
goto cleanup;
} else {
int ii;
virNetworkForwardIfDefPtr dev = NULL;
+ virDomainDevicePCIAddressPtr addr = &(def->source.subsys.u.pci);
+ if (virDomainNetGetActualType(iface) == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+ if (virNetDevGetDeviceAddrString(addr->domain,
+ addr->bus,
+ addr->slot,
+ addr->function,
+ &actualDev) < 0) {
+ networkReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("Could not get device string
for hostdev"));
+ goto cleanup;
+ }
+ }
+
for (ii = 0; ii < netdef->nForwardIfs; ii++) {
if (STREQ(actualDev, netdef->forwardIfs[ii].dev)) {
dev = &netdef->forwardIfs[ii];
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 117542f..ea1c9c5 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4897,6 +4897,20 @@ qemuBuildCommandLine(virConnectPtr conn,
* code here that adds the newly minted hostdev to the
* hostdevs array).
*/
+ if (qemuAssignDeviceHostdevAlias(def,
+ virDomainNetGetActualHostdev(net),
+ (def->nhostdevs-1)) < 0) {
+ qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Could not assign alias to Net
Hostdev"));
+ goto error;
+ }
+
+ if (virDomainHostdevInsert(def,
+ virDomainNetGetActualHostdev(net)) < 0) {
+ qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Hostdev not inserted into the array"));
+ goto error;
+ }
continue;
}
diff --git a/src/util/pci.c b/src/util/pci.c
index 88e4ac5..6dd9953 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -1812,7 +1812,7 @@ logStrToLong_ui(char const *s,
return ret;
}
-static int
+int
pciParsePciConfigAddress(char *address,
struct pci_config_address *bdf)
{
diff --git a/src/util/pci.h b/src/util/pci.h
index b71bb12..8d85558 100644
--- a/src/util/pci.h
+++ b/src/util/pci.h
@@ -105,6 +105,9 @@ int pciGetVirtualFunctions(const char *sysfs_path,
struct pci_config_address ***virtual_functions,
unsigned int *num_virtual_functions);
+int pciParsePciConfigAddress(char *address,
+ struct pci_config_address *bdf);
+
int pciDeviceIsVirtualFunction(const char *vf_sysfs_device_link);
int pciGetVirtualFunctionIndex(const char *pf_sysfs_device_link,
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index a328294..dad6035 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -38,6 +38,7 @@
#ifdef __linux__
# include <linux/sockios.h>
+# include <linux/ethtool.h>
# include <linux/if_vlan.h>
#elif !defined(AF_PACKET)
# undef HAVE_STRUCT_IFREQ
@@ -463,6 +464,65 @@ int virNetDevSetNamespace(const char *ifname, pid_t pidInNs)
return rc;
}
+#if defined(SIOCETHTOOL) && defined(HAVE_STRUCT_IFREQ)
+/**
+ * virNetDevGetPciAddrFromName:
+ * @device_pci_addr : string containing the BDF of the device
+ * @domain : pointer to the integer domain
+ * @bus : pointer to the integer bus
+ * @slot : pointer to integer slot
+ * @function: pointer to integer function
+ *
+ * Parses the PCI config address string.
+ *
+ * Returns 0 if success and -1 if error
+ *
+ */
+int
+virNetDevGetPciAddrFromName(const char *ifname,
+ char **device_pci_addr)
+{
+ int ret = -1;
+ int fd = -1;
+ struct ifreq ifr;
+ struct ethtool_drvinfo drvinfo;
+
+ memset(&drvinfo, 0, sizeof(drvinfo));
+
+ drvinfo.cmd = ETHTOOL_GDRVINFO;
+
+ if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0)
+ return -1;
+
+ ifr.ifr_ifru.ifru_data = (void *)&drvinfo;
+
+ if (ioctl(fd, SIOCETHTOOL, &ifr) < 0) {
+ virReportSystemError(errno,
+ _("Cannot get driver info on '%s'"),
+ ifname);
+ goto error;
+ }
+
+ if (virAsprintf(device_pci_addr, "%s", drvinfo.bus_info) < 0) {
+ virReportOOMError();
+ goto error;
+ }
+
+ ret = 0;
+error:
+ return ret;
+}
+#else
+int virNetDevGetPciAddrFromName(const char* ifname ATTRIBUTE_UNUSED,
+ char **device_pci_addr ATTRIBUTE_UNUSED)
+{
+ virReportSystemError(errno,
+ _("Cannot get driver info on '%s'"),
+ ifname);
+ return -1;
+}
+#endif
+
#if defined(SIOCSIFNAME) && defined(HAVE_STRUCT_IFREQ)
/**
* virNetDevSetName:
@@ -1040,6 +1100,74 @@ cleanup:
}
/**
+ * virNetDevGetDeviceAddrString:
+ * @domain : pointer to the integer domain
+ * @bus : pointer to the integer bus
+ * @slot : pointer to integer slot
+ * @function: pointer to integer function
+ * @addr: pointer to the memory allocated to hold the BDF string
+ *
+ * Creates the PCI config address string.
+ *
+ * Returns 0 if success and -1 if error
+ *
+ */
+int
+virNetDevGetDeviceAddrString(unsigned domain,
+ unsigned bus,
+ unsigned slot,
+ unsigned function,
+ const char **addr)
+{
+ int ret = -1;
+
+ if (pciGetDeviceAddrString(domain, bus, slot, function, (char **)addr) < 0)
+ goto error;
+
+ ret = 0;
+
+error:
+ return ret;
+}
+
+/**
+ * virNetDevParsePciConfigAddress:
+ * @device_pci_addr : string containing the BDF of the device
+ * @domain : pointer to the integer domain
+ * @bus : pointer to the integer bus
+ * @slot : pointer to integer slot
+ * @function: pointer to integer function
+ *
+ * Parses the PCI config address string.
+ *
+ * Returns 0 if success and -1 if error
+ *
+ */
+int
+virNetDevParsePciConfigAddress(const char *device_pci_addr,
+ unsigned int *domain,
+ unsigned int *bus,
+ unsigned int *slot,
+ unsigned int *function)
+{
+ int ret = -1;
+ struct pci_config_address device;
+
+ if (pciParsePciConfigAddress((char *)device_pci_addr, &device) != 0) {
+ goto error;
+ }
+
+ *domain = device.domain;
+ *bus = device.bus;
+ *slot = device.slot;
+ *function = device.function;
+
+ ret = 0;
+error:
+ return ret;
+}
+
+/**
* virNetDevIsVirtualFunction:
* @ifname : name of the interface
*
diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h
index 660d2db..4248a85 100644
--- a/src/util/virnetdev.h
+++ b/src/util/virnetdev.h
@@ -106,6 +106,25 @@ int virNetDevGetVirtualFunctions(const char *pfname,
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
ATTRIBUTE_RETURN_CHECK;
+int virNetDevGetDeviceAddrString(unsigned domain,
+ unsigned bus,
+ unsigned slot,
+ unsigned function,
+ const char **addr)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
+ ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK;
+
+int virNetDevParsePciConfigAddress(const char *device_pci_addr,
+ unsigned int *domain,
+ unsigned int *bus,
+ unsigned int *slot,
+ unsigned int *function)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
+
+int virNetDevGetPciAddrFromName(const char *ifname,
+ char **device_pci_addr)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
+
int virNetDevLinkDump(const char *ifname, int ifindex,
struct nlattr **tb,
unsigned char **recvbuf,
--
1.7.4.4
10:30 a.m.
New subject: [libvirt] [PATCH 5/5] Supporting managed option for forward devs when using HOSTDEV mode
This patch supports the managed option in a network xml for network devices.
This option is used for pci network devices when forward mode=hostdev.
Hence the example network xml would be:
<network>
<name>direct-network</name>
<uuid>81ff0d90-c91e-6742-64da-4a736edb9a8f</uuid>
<forward mode="hostdev">
<pf dev="eth2" managed='yes'/>
</forward>
</network>
OR
<network>
<name>direct-network</name>
<uuid>81ff0d90-c91e-6742-64da-4a736edb9a8f</uuid>
<forward mode="hostdev">
<interface dev="0000:04:00.1" managed='yes'/>
<interface dev="0000:04:00.2" managed='yes'/>
<interface dev="0000:04:00.3" managed='yes'/>
</forward>
</network>
Signed-off-by: Shradha Shah <sshah(a)solarflare.com>
---
src/conf/network_conf.c | 61 ++++++++++++++++++++++++++++++++++++++++--
src/conf/network_conf.h | 2 +
src/network/bridge_driver.c | 5 +++-
3 files changed, 64 insertions(+), 4 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 6b346c3..18e4ee3 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -986,6 +986,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
char *forwardDev = NULL;
xmlNodePtr save = ctxt->node;
xmlNodePtr bandwidthNode = NULL;
+ char *managed = NULL;
if (VIR_ALLOC(def) < 0) {
virReportOOMError();
@@ -1128,6 +1129,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
}
forwardDev = virXPathString("string(./@dev)", ctxt);
+ managed = virXPathString("string(./managed)", ctxt);
/* all of these modes can use a pool of physical interfaces */
nForwardIfs = virXPathNodeSet("./interface", ctxt,
&forwardIfNodes);
@@ -1151,6 +1153,12 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
goto error;
}
+ if (managed) {
+ virNetworkReportError(VIR_ERR_XML_ERROR,
+ _("A managed field should not be used in this
location when using a SRIOV PF"));
+ goto error;
+ }
+
forwardDev = virXMLPropString(*forwardPfNodes, "dev");
if (!forwardDev) {
virNetworkReportError(VIR_ERR_XML_ERROR,
@@ -1159,9 +1167,16 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
goto error;
}
+ managed = virXMLPropString(*forwardPfNodes, "managed");
+ if(managed != NULL) {
+ if (STREQ(managed, "yes"))
+ def->forwardPfs->managed = 1;
+ }
+
def->forwardPfs->usageCount = 0;
def->forwardPfs->dev = forwardDev;
forwardDev = NULL;
+ managed = NULL;
def->nForwardPfs++;
} else if (nForwardPfs > 1) {
virNetworkReportError(VIR_ERR_XML_ERROR,
@@ -1170,6 +1185,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
}
if (nForwardIfs > 0 || forwardDev) {
int ii;
+ unsigned int managedvalue = 0;
/* allocate array to hold all the portgroups */
if (VIR_ALLOC_N(def->forwardIfs, MAX(nForwardIfs, 1)) < 0) {
@@ -1181,12 +1197,24 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
def->forwardIfs[0].usageCount = 0;
def->forwardIfs[0].dev = forwardDev;
forwardDev = NULL;
+ if (managed != NULL) {
+ if (STREQ(managed, "yes"))
+ def->forwardIfs[0].managed = 1;
+ }
+ managed = NULL;
def->nForwardIfs++;
}
/* parse each forwardIf */
for (ii = 0; ii < nForwardIfs; ii++) {
forwardDev = virXMLPropString(forwardIfNodes[ii], "dev");
+ managed = virXMLPropString(forwardIfNodes[ii], "managed");
+ if (managed != NULL) {
+ if (STREQ(managed, "yes"))
+ managedvalue = 1;
+ else
+ managedvalue = 0;
+ }
if (!forwardDev) {
virNetworkReportError(VIR_ERR_XML_ERROR,
_("Missing required dev attribute in
network '%s' forward interface element"),
@@ -1204,12 +1232,22 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
forwardDev, def->name);
goto error;
}
+
+ if (managedvalue != def->forwardIfs[0].managed) {
+ virNetworkReportError(VIR_ERR_XML_ERROR,
+ _("managed field of forward dev must
match that of the first interface element dev in network '%s'"),
+ def->name);
+ goto error;
+ }
+ VIR_FREE(managed);
VIR_FREE(forwardDev);
continue;
}
def->forwardIfs[ii].dev = forwardDev;
+ def->forwardIfs[ii].managed = managedvalue;
forwardDev = NULL;
+ managed = NULL;
def->forwardIfs[ii].usageCount = 0;
def->nForwardIfs++;
}
@@ -1224,6 +1262,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
}
VIR_FREE(forwardDev);
+ VIR_FREE(managed);
VIR_FREE(forwardPfNodes);
VIR_FREE(forwardIfNodes);
@@ -1541,15 +1580,31 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int
flags)
(def->nForwardIfs || def->nForwardPfs) ? "" :
"/");
/* For now, hard-coded to at most 1 forwardPfs */
- if (def->nForwardPfs)
- virBufferEscapeString(&buf, " <pf
dev='%s'/>\n",
+ if (def->nForwardPfs) {
+ virBufferEscapeString(&buf, " <pf dev='%s'",
def->forwardPfs[0].dev);
+ if (def->forwardType == VIR_NETWORK_FORWARD_HOSTDEV) {
+ if (def->forwardPfs[0].managed == 1)
+ virBufferAddLit(&buf, " managed='yes'");
+ else
+ virBufferAddLit(&buf, " managed='no'");
+ }
+ virBufferAddLit(&buf, "/>\n");
+ }
+
if (def->nForwardIfs &&
(!def->nForwardPfs || !(flags & VIR_NETWORK_XML_INACTIVE))) {
for (ii = 0; ii < def->nForwardIfs; ii++) {
- virBufferEscapeString(&buf, " <interface
dev='%s'/>\n",
+ virBufferEscapeString(&buf, " <interface
dev='%s'",
def->forwardIfs[ii].dev);
+ if (def->forwardType == VIR_NETWORK_FORWARD_HOSTDEV) {
+ if (def->forwardIfs[ii].managed == 1)
+ virBufferAddLit(&buf, " managed='yes'");
+ else
+ virBufferAddLit(&buf, " managed='no'");
+ }
+ virBufferAddLit(&buf, "/>\n");
}
}
if (def->nForwardPfs || def->nForwardIfs)
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index d473c71..4276934 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -135,6 +135,7 @@ struct _virNetworkForwardIfDef {
char *dev; /* name of device */
int usageCount; /* how many guest interfaces are bound to this device? */
bool isPciAddr; /* Differentiate a VF based on interface name or pci addr*/
+ unsigned int managed:1;
};
typedef struct _virNetworkForwardPfDef virNetworkForwardPfDef;
@@ -142,6 +143,7 @@ typedef virNetworkForwardPfDef *virNetworkForwardPfDefPtr;
struct _virNetworkForwardPfDef {
char *dev; /* name of device */
int usageCount; /* how many guest interfaces are bound to this device? */
+ unsigned int managed:1;
};
typedef struct _virPortGroupDef virPortGroupDef;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 691ab07..a2293e6 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2807,6 +2807,9 @@ networkCreateInterfacePool(virNetworkDefPtr netdef) {
netdef->forwardIfs[ii].isPciAddr = true;
else
netdef->forwardIfs[ii].isPciAddr = false;
+
+ if (netdef->forwardPfs[0].managed == 1)
+ netdef->forwardIfs[ii].managed = 1;
}
ret = 0;
@@ -2942,7 +2945,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
iface->data.network.actual->data.hostdev.def.parent.data.net = iface;
iface->data.network.actual->data.hostdev.def.info = &iface->info;
iface->data.network.actual->data.hostdev.def.mode =
VIR_DOMAIN_HOSTDEV_MODE_SUBSYS;
- iface->data.network.actual->data.hostdev.def.managed = 1;
+ iface->data.network.actual->data.hostdev.def.managed = dev->managed;
iface->data.network.actual->data.hostdev.def.source.subsys.type =
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI;
if (dev->isPciAddr == true) {
--
1.7.4.4
12:20 p.m.
New subject: [libvirt] [PATCH 5/5] Supporting managed option for forward devs when using HOSTDEV mode
On 06/08/2012 11:30 AM, Shradha Shah wrote:
This patch supports the managed option in a network xml for network
devices.
This option is used for pci network devices when forward mode=hostdev.
Hence the example network xml would be:
<network>
<name>direct-network</name>
<uuid>81ff0d90-c91e-6742-64da-4a736edb9a8f</uuid>
<forward mode="hostdev">
<pf dev="eth2" managed='yes'/>
</forward>
</network>
OR
<network>
<name>direct-network</name>
<uuid>81ff0d90-c91e-6742-64da-4a736edb9a8f</uuid>
<forward mode="hostdev">
<interface dev="0000:04:00.1" managed='yes'/>
<interface dev="0000:04:00.2" managed='yes'/>
<interface dev="0000:04:00.3" managed='yes'/>
</forward>
</network>
This functionality should be included with the initial patch rather than
as a later add-on.
Signed-off-by: Shradha Shah <sshah(a)solarflare.com>
---
src/conf/network_conf.c | 61 ++++++++++++++++++++++++++++++++++++++++--
src/conf/network_conf.h | 2 +
src/network/bridge_driver.c | 5 +++-
3 files changed, 64 insertions(+), 4 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 6b346c3..18e4ee3 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -986,6 +986,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
char *forwardDev = NULL;
xmlNodePtr save = ctxt->node;
xmlNodePtr bandwidthNode = NULL;
+ char *managed = NULL;
if (VIR_ALLOC(def) < 0) {
virReportOOMError();
@@ -1128,6 +1129,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
}
forwardDev = virXPathString("string(./@dev)", ctxt);
+ managed = virXPathString("string(./managed)", ctxt);
/* all of these modes can use a pool of physical interfaces */
nForwardIfs = virXPathNodeSet("./interface", ctxt,
&forwardIfNodes);
@@ -1151,6 +1153,12 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
goto error;
}
+ if (managed) {
+ virNetworkReportError(VIR_ERR_XML_ERROR,
+ _("A managed field should not be used in this
location when using a SRIOV PF"));
+ goto error;
+ }
+
forwardDev = virXMLPropString(*forwardPfNodes, "dev");
if (!forwardDev) {
virNetworkReportError(VIR_ERR_XML_ERROR,
@@ -1159,9 +1167,16 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
goto error;
}
+ managed = virXMLPropString(*forwardPfNodes, "managed");
+ if(managed != NULL) {
+ if (STREQ(managed, "yes"))
+ def->forwardPfs->managed = 1;
+ }
+
def->forwardPfs->usageCount = 0;
def->forwardPfs->dev = forwardDev;
forwardDev = NULL;
+ managed = NULL;
def->nForwardPfs++;
} else if (nForwardPfs > 1) {
virNetworkReportError(VIR_ERR_XML_ERROR,
@@ -1170,6 +1185,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
}
if (nForwardIfs > 0 || forwardDev) {
int ii;
+ unsigned int managedvalue = 0;
/* allocate array to hold all the portgroups */
if (VIR_ALLOC_N(def->forwardIfs, MAX(nForwardIfs, 1)) < 0) {
@@ -1181,12 +1197,24 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
def->forwardIfs[0].usageCount = 0;
def->forwardIfs[0].dev = forwardDev;
forwardDev = NULL;
+ if (managed != NULL) {
+ if (STREQ(managed, "yes"))
+ def->forwardIfs[0].managed = 1;
+ }
+ managed = NULL;
def->nForwardIfs++;
}
/* parse each forwardIf */
for (ii = 0; ii < nForwardIfs; ii++) {
forwardDev = virXMLPropString(forwardIfNodes[ii], "dev");
+ managed = virXMLPropString(forwardIfNodes[ii], "managed");
+ if (managed != NULL) {
+ if (STREQ(managed, "yes"))
+ managedvalue = 1;
+ else
+ managedvalue = 0;
+ }
if (!forwardDev) {
virNetworkReportError(VIR_ERR_XML_ERROR,
_("Missing required dev attribute in
network '%s' forward interface element"),
@@ -1204,12 +1232,22 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
forwardDev, def->name);
goto error;
}
+
+ if (managedvalue != def->forwardIfs[0].managed) {
+ virNetworkReportError(VIR_ERR_XML_ERROR,
+ _("managed field of forward dev must
match that of the first interface element dev in network '%s'"),
+ def->name);
+ goto error;
+ }
+ VIR_FREE(managed);
VIR_FREE(forwardDev);
continue;
}
def->forwardIfs[ii].dev = forwardDev;
+ def->forwardIfs[ii].managed = managedvalue;
forwardDev = NULL;
+ managed = NULL;
def->forwardIfs[ii].usageCount = 0;
def->nForwardIfs++;
}
@@ -1224,6 +1262,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
}
VIR_FREE(forwardDev);
+ VIR_FREE(managed);
VIR_FREE(forwardPfNodes);
VIR_FREE(forwardIfNodes);
@@ -1541,15 +1580,31 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned
int flags)
(def->nForwardIfs || def->nForwardPfs) ? "" :
"/");
/* For now, hard-coded to at most 1 forwardPfs */
- if (def->nForwardPfs)
- virBufferEscapeString(&buf, " <pf
dev='%s'/>\n",
+ if (def->nForwardPfs) {
+ virBufferEscapeString(&buf, " <pf dev='%s'",
def->forwardPfs[0].dev);
+ if (def->forwardType == VIR_NETWORK_FORWARD_HOSTDEV) {
+ if (def->forwardPfs[0].managed == 1)
+ virBufferAddLit(&buf, " managed='yes'");
+ else
+ virBufferAddLit(&buf, " managed='no'");
+ }
+ virBufferAddLit(&buf, "/>\n");
+ }
+
if (def->nForwardIfs &&
(!def->nForwardPfs || !(flags & VIR_NETWORK_XML_INACTIVE))) {
for (ii = 0; ii < def->nForwardIfs; ii++) {
- virBufferEscapeString(&buf, " <interface
dev='%s'/>\n",
+ virBufferEscapeString(&buf, " <interface
dev='%s'",
def->forwardIfs[ii].dev);
+ if (def->forwardType == VIR_NETWORK_FORWARD_HOSTDEV) {
+ if (def->forwardIfs[ii].managed == 1)
+ virBufferAddLit(&buf, " managed='yes'");
+ else
+ virBufferAddLit(&buf, " managed='no'");
+ }
+ virBufferAddLit(&buf, "/>\n");
}
}
if (def->nForwardPfs || def->nForwardIfs)
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index d473c71..4276934 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -135,6 +135,7 @@ struct _virNetworkForwardIfDef {
char *dev; /* name of device */
int usageCount; /* how many guest interfaces are bound to this device? */
bool isPciAddr; /* Differentiate a VF based on interface name or pci addr*/
+ unsigned int managed:1;
};
typedef struct _virNetworkForwardPfDef virNetworkForwardPfDef;
@@ -142,6 +143,7 @@ typedef virNetworkForwardPfDef *virNetworkForwardPfDefPtr;
struct _virNetworkForwardPfDef {
char *dev; /* name of device */
int usageCount; /* how many guest interfaces are bound to this device? */
+ unsigned int managed:1;
};
typedef struct _virPortGroupDef virPortGroupDef;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 691ab07..a2293e6 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2807,6 +2807,9 @@ networkCreateInterfacePool(virNetworkDefPtr netdef) {
netdef->forwardIfs[ii].isPciAddr = true;
else
netdef->forwardIfs[ii].isPciAddr = false;
+
+ if (netdef->forwardPfs[0].managed == 1)
+ netdef->forwardIfs[ii].managed = 1;
}
ret = 0;
@@ -2942,7 +2945,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
iface->data.network.actual->data.hostdev.def.parent.data.net = iface;
iface->data.network.actual->data.hostdev.def.info = &iface->info;
iface->data.network.actual->data.hostdev.def.mode =
VIR_DOMAIN_HOSTDEV_MODE_SUBSYS;
- iface->data.network.actual->data.hostdev.def.managed = 1;
+ iface->data.network.actual->data.hostdev.def.managed = dev->managed;
iface->data.network.actual->data.hostdev.def.source.subsys.type =
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI;
if (dev->isPciAddr == true) {
11:06 a.m.
Hello all,
I have actually based these patches of v0.9.12.
Top of tree libvirt had some errors due to which I was not able to use virsh and test
these patches so I based them off the last release.
But these patches should work equivalently well with top of tree.
Many Thanks,
Regards,
Shradha Shah
On 06/08/2012 04:26 PM, Shradha Shah wrote:
This patch series supports the forward mode='hostdev'. The
functionality
of this mode is the same as interface type='hostdev' but with the added
benefit of using interface pools.
The patch series also contains a patch to support use of interface names
and PCI device addresses interchangeably in a network xml, and return
the appropriate one in actualDevice when networkAllocateActualDevice is
called. This functionality is not supported for any other forward mode
except hostdev.
Currently this patch series also does not support USB hostdev
passthrough.
At the top level managed attribute can be specified for a pf dev or an
interface dev (with identical results as when it's specified for a
hostdev
Shradha Shah (5):
Code to return interface name or pci_addr of the VF in actualDevice
Moved the code to create implicit interface pool from PF to a new
function
Introduce forward mode='hostdev' for network XML in order to use the
functionality of interface pools.
Forward Mode Hostdev Implementation
Supporting managed option for forward devs when using HOSTDEV mode
docs/schemas/network.rng | 1 +
src/conf/network_conf.c | 119 +++++++++++++++-
src/conf/network_conf.h | 13 ++-
src/libvirt_private.syms | 3 +
src/network/bridge_driver.c | 325 +++++++++++++++++++++++++++++++++++--------
src/qemu/qemu_command.c | 14 ++
src/util/pci.c | 7 +-
src/util/pci.h | 3 +
src/util/virnetdev.c | 134 +++++++++++++++++-
src/util/virnetdev.h | 19 +++
10 files changed, 568 insertions(+), 70 deletions(-)
1:27 p.m.
On 06/08/2012 12:06 PM, Shradha Shah wrote:
Hello all,
I have actually based these patches of v0.9.12.
Top of tree libvirt had some errors due to which I was not able to use virsh and test
these patches so I based them off the last release.
Do you still have the same problem with the latest from git?
It will be really nice to have this functionality in - I've considered
the type='hostdev' support incomplete without it.
Some general comments beyond (or reinforcing) those I already made in
responses to the individual patches:
* You have no documentation (should be added to
docs/formatnetwork.html.in and docs/formatdomain.html.in), which really
needs to be in the same patch that enables the new functionality.
* Likewise I didn't see any new test case (tests/networkxml2xml*).
* In general, I think it's useful to arrange multiple patches like this:
* One or more patches that re-factor existing code (without causing
any functional
change) to prepare for the new code.
* A patch that adds the new XML parsing/formatting, updates the RNG,
and adds xml2xml tests.
* A patch that connects the new functionality to the backend (in this
case, much
of that is already done, since the "<interface
type='hostdev'>"
patches were
written such that networkAllocationActualDevice is called at the
appropriate place
and the actualdevice object is queried when appropriate (obviously
there will be
bugs in this, since it couldn't be properly tested until now).
You may find that it useful to split the 3rd item into 2 parts: 1) a
patch that updates the network driver to properly utilize the new
attributes/elements that are now in virNetworkDef (i.e. allowing the pf
element to populate all the devs/addresses) and 2) A patch that fixes
any latent bugs in the qemu driver utilizing the returned dev/address).
I guess in the end there are 4 rules of thumb I try to think of when I'm
putting together a patch series (others probably have their own set of
personal rules):
1) try to make each patch as easy to understand as possible, while
avoiding having a large number of related, yet trivial, patches.
2) unrelated changes (or changes that could be useful separately from
the rest of the series, e.g. general purpose utility functions) should
be in separate patches.
3) try to avoid code churn - if something will move, move it once; when
adding new code, the patch that introduces it should introduce it where
you want it to be at the end of the series.
4) above anything else, it is a requirement that make check (and other
existing libvirt operation) work correctly at any step in applying the
patches. Sometimes this will require temporarily putting in a small bit
of stub code, omitting/modifying a test, or even combining two patches
that you would rather have separated, but it is necessary in order for
git bisect to be useful.
4547
days inactive
4551
days old
14 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Laine Stump -
Shradha Shah