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 > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
