10:11 a.m.
$SUBJ:
"conf: Introduce virDomainNetFindByHostdev"
Is fine.
then the commit message can be more descriptive.
On 06/28/2018 09:22 AM, Jai Singh Rana wrote:
Introduce API virDomainNetFindByHostdev which retrieves net def in
given domain for the given hostdev def. This API will now be used by
virDomainNetFind to further probe net for hostdev network devices.
Signed-off-by: Jai Singh Rana <JaiSingh.Rana(a)cavium.com>
---
src/conf/domain_conf.c | 43 +++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_conf.h | 2 ++
src/libvirt_private.syms | 2 ++
src/util/virhostdev.c | 2 +-
src/util/virhostdev.h | 3 +++
5 files changed, 51 insertions(+), 1 deletion(-)
You could split out the last 3 into their own patch to promote
virHostdevIsPCINetDevice from static to global, but that's just a nit.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d8cb7f37f3..8432215d19 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"
@@ -29064,6 +29065,37 @@ virDomainNetFind(virDomainDefPtr def, const char *device)
/**
+ * virDomainNetFindByHostdev:
+ * @def: domain's def
+ * @hostdev: hostdev whose net def if exists to be retrieved
+ *
+ * Finds net def in domain given the domain's hostdev.
+ *
+ * Returns a pointer to the net def or NULL if not found.
+ */
+virDomainNetDefPtr
+virDomainNetFindByHostdev(virDomainDefPtr def,
+ virDomainHostdevDefPtr hostdev)
+{
+ size_t i;
+ virDomainNetType actualType;
+ virDomainHostdevDefPtr hostdef = NULL;
+
+ for (i = 0; i < def->nnets; i++) {
+ actualType = virDomainNetGetActualType(def->nets[i]);
+ if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV)
+ hostdef = virDomainNetGetActualHostdev(def->nets[i]);
Of concern, virDomainNetGetActualHostdev can return NULL...
+ else
+ continue;
+ if (!memcmp(hostdev, hostdef, sizeof(virDomainHostdevDef)))
Is comparing the whole thing necessary? Is perhaps just the name good
enough? There's a lot of data in the HostdevDef - if something doesn't
match then I suppose you could conceivably fail here even though you're
not expecting to.
+ return def->nets[i];
+ }
Does the following work?
for (i = 0; i < def->nnets; i++) {
virDomainHostdevDefPtr net_hostdev;
if (!(net_hostdev = virDomainNetGetActualHostdev(def->nets[i])))
continue;
[whatever comparison is appropriate for whether net_hostdev
matches the passed hostdev]
+
+ return NULL;
+}
+
+
+/**
* virDomainNetFindByName:
* @def: domain's def
* @ifname: interface name
@@ -29077,12 +29109,23 @@ virDomainNetFindByName(virDomainDefPtr def,
const char *ifname)
{
size_t i;
+ virDomainNetDefPtr net = NULL;
for (i = 0; i < def->nnets; i++) {
if (STREQ_NULLABLE(ifname, def->nets[i]->ifname))
return def->nets[i];
}
+ /* Give a try to hostdev if its a switchdev network device*/
s/its/it's/
s/device*/device */
+ for (i = 0; i < def->nhostdevs; i++) {
+ if (!virHostdevIsPCINetDevice(def->hostdevs[i]))
+ continue;
+ if (virNetdevHostdevCheckVFRIfName(def->hostdevs[i], ifname)) {
+ if ((net = virDomainNetFindByHostdev(def, def->hostdevs[i])))
+ return net;
+ }
+ }
+
IIUC, this is the "magic" place where as stated in the doc patch6 where
the hostdev being used to fetch the stats is determined to be an SR-IOV
device w/ VF representor on host in switchdev mode.
There is some concern over adding this without some sort of "filter" -
mainly because of the extra work, but also because it's not 100% clear
to me all callers would expect to get this data.
Anyway, for callers of virDomainNetFind that end up just calling
virNetDevTapInterfaceStats - it would seem to be OK, but I'm not 100%
sure. For callers that are getting/setting interface parameters - would
returning the @net that is actually a hostdev type be expected?
Maybe it'd be best to introduce a boolean 'wantSRIOVHostdevVFR' that
would be "true" only from qemuDomainInterfaceStats since it then has the
logic to detect if the ActualType(net) is HOSTDEV (name chosen from my
IIUC above - feel free to adjust to match reality).
That way we only check the hostdev's if that's what we want with the
logic being:
if (!wantSRIOVHostdevVFR)
return NULL;
for (i = 0; i < def->nhostdevs; i++) {
...
John
return NULL;
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0924fc4f3c..ccec74e51d 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3146,6 +3146,8 @@ int virDomainDiskSourceParse(xmlNodePtr node,
int virDomainNetFindIdx(virDomainDefPtr def, virDomainNetDefPtr net);
virDomainNetDefPtr virDomainNetFind(virDomainDefPtr def, const char *device);
+virDomainNetDefPtr virDomainNetFindByHostdev(virDomainDefPtr def,
+ virDomainHostdevDefPtr hostdev);
virDomainNetDefPtr virDomainNetFindByName(virDomainDefPtr def, const char *ifname);
bool virDomainHasNet(virDomainDefPtr def, virDomainNetDefPtr net);
int virDomainNetInsert(virDomainDefPtr def, virDomainNetDefPtr net);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 5aa8f7ed64..e4d8583d41 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -449,6 +449,7 @@ virDomainNetDefClear;
virDomainNetDefFormat;
virDomainNetDefFree;
virDomainNetFind;
+virDomainNetFindByHostdev;
virDomainNetFindByName;
virDomainNetFindIdx;
virDomainNetGenerateMAC;
@@ -1966,6 +1967,7 @@ virHostCPUStatsAssign;
# util/virhostdev.h
virHostdevFindUSBDevice;
virHostdevIsMdevDevice;
+virHostdevIsPCINetDevice;
virHostdevIsSCSIDevice;
virHostdevManagerGetDefault;
virHostdevNetDevice;
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index c6ee725860..2b8ecb9649 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -348,7 +348,7 @@ virHostdevNetDevice(virDomainHostdevDefPtr hostdev,
}
-static bool
+bool
virHostdevIsPCINetDevice(virDomainHostdevDefPtr hostdev)
{
return hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
diff --git a/src/util/virhostdev.h b/src/util/virhostdev.h
index 7412a20aa9..71faaf4e7a 100644
--- a/src/util/virhostdev.h
+++ b/src/util/virhostdev.h
@@ -197,6 +197,9 @@ virHostdevReAttachDomainDevices(virHostdevManagerPtr mgr,
const char *oldStateDir)
ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
bool
+virHostdevIsPCINetDevice(virDomainHostdevDefPtr hostdev)
+ ATTRIBUTE_NONNULL(1);
+bool
virHostdevIsSCSIDevice(virDomainHostdevDefPtr hostdev)
ATTRIBUTE_NONNULL(1);
bool