On 04/04/2018 12:29 PM, Jai Singh Rana wrote:
In case of pci SR-IOV device with interface_type as 'hostdev', return network stats if it has a VF Representor interface on host for pci SR-IOV device according to switchdev model. --- v3 includes changes based on v2's[1] feedback and suggestions. Includes fix for hostdev to net mapping in a given domain. [1] https://www.redhat.com/archives/libvir-list/2018-February/msg00563.html
docs/news.xml | 9 +++++++++ src/conf/domain_conf.c | 15 +++++++++++++++ src/qemu/qemu_driver.c | 33 +++++++++++++++++++++++++++++---- 3 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/docs/news.xml b/docs/news.xml index 87f52e83e..04c18495f 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -47,6 +47,15 @@ supported. In fact, kernel has been supporting this since 4.10. </description> </change> + <change> + <summary> + qemu: Support interface network stats for VF Representors + </summary> + <description> + Interface network stats are supported now for SR-IOV device(hostdev) + if this interface has VF representor on host in switchdev mode. + </description> + </change> </section> <section title="Bug fixes"> </section>
This needs to be it's own separate and last patch.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ef16431aa..50813701c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -56,6 +56,7 @@ #include "virsecret.h" #include "virstring.h" #include "virnetdev.h" +#include "virnetdevhostdev.h" #include "virnetdevmacvlan.h" #include "virhostdev.h" #include "virmdev.h" @@ -28264,12 +28265,26 @@ virDomainNetFindByName(virDomainDefPtr def, const char *ifname) { size_t i; + size_t j;
for (i = 0; i < def->nnets; i++) { if (STREQ_NULLABLE(ifname, def->nets[i]->ifname)) return def->nets[i]; }
+ /* Give a try to hostdev */ + for (i = 0; i < def->nhostdevs; i++) { + if (virNetdevHostdevCheckVFRIfName(def->hostdevs[i], ifname)) {
You probably need to gate the above a bit more as a hostdev can be non network... No sense in calling the very heavy above function for scsi disks is there? There's also interesting relationship between nnets and nhostdevs - see virDomainNetInsert, which is something I think you need to understand before unnecessarily walking this hostdev's list.
+ for (j = 0; j < def->nnets; j++) { + if (def->nets[j]->type != VIR_DOMAIN_NET_TYPE_HOSTDEV) + continue;
We already walked the nnets and what about virDomainNetGetActualType... Not so sure about this. Laine perhaps understand all these relationships a bit better than I do.
+ if (memcmp(def->hostdevs[i], &def->nets[j]->data.hostdev, + sizeof(virDomainHostdevDef)) == 0) + return def->nets[j]; + } + } + } + return NULL; }
If it stays, I think this hunk is separable into it's own patch...
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5c31dfdd5..f2f9d290b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -66,6 +66,7 @@ #include "virbuffer.h" #include "virhostcpu.h" #include "virhostmem.h" +#include "virnetdevhostdev.h" #include "virnetdevtap.h" #include "virnetdevopenvswitch.h" #include "capabilities.h" @@ -11156,6 +11157,11 @@ qemuDomainInterfaceStats(virDomainPtr dom, if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_VHOSTUSER) { if (virNetDevOpenvswitchInterfaceStats(net->ifname, stats) < 0) goto cleanup; + } else if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
bool swapped = virDomainNetTypeSharesHostView(net);
+ if (virNetdevHostdevVFRIfStats(device, stats, + !virDomainNetTypeSharesHostView(net))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer to see a local "swapped" boolean used/defined than a function call within a function call.
+ < 0)
That would at least not leave this naked < 0) on one line
+ goto cleanup; } else { if (virNetDevTapInterfaceStats(net->ifname, stats, !virDomainNetTypeSharesHostView(net)) < 0) @@ -19818,6 +19824,7 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, { size_t i; struct _virDomainInterfaceStats tmp; + char *vf_ifname = NULL; int ret = -1;
if (!virDomainObjIsActive(dom)) @@ -19830,21 +19837,39 @@ qemuDomainGetStatsInterface(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, virDomainNetDefPtr net = dom->def->nets[i]; virDomainNetType actualType;
- if (!net->ifname) + actualType = virDomainNetGetActualType(net); + + if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + vf_ifname = virNetdevHostdevGetVFRIfName(dom->def->hostdevs[i]); + if (!vf_ifname) + continue; + } + else if (!net->ifname) continue;
memset(&tmp, 0, sizeof(tmp));
- actualType = virDomainNetGetActualType(net);
- QEMU_ADD_NAME_PARAM(record, maxparams, - "net", "name", i, net->ifname); + if (actualType != VIR_DOMAIN_NET_TYPE_HOSTDEV) + QEMU_ADD_NAME_PARAM(record, maxparams, + "net", "name", i, net->ifname); + else + QEMU_ADD_NAME_PARAM(record, maxparams, + "net", "name", i, vf_ifname);
if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) { if (virNetDevOpenvswitchInterfaceStats(net->ifname, &tmp) < 0) { virResetLastError(); continue; } + } else if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { + if (virNetdevHostdevVFRIfStats(vf_ifname, &tmp, + !virDomainNetTypeSharesHostView(net)) < 0) { + VIR_FREE(vf_ifname);
Same here w/r/t boolean....
+ virResetLastError(); + continue; + } + VIR_FREE(vf_ifname);
consider: rc = virNetdevHostdevVFRIfStats(); VIR_FREE(vf_ifname); if (rc < 0) { virResetLastError continue; }
} else { if (virNetDevTapInterfaceStats(net->ifname, &tmp, !virDomainNetTypeSharesHostView(net)) < 0) {
Then it can be used here too John