Re: [libvirt] [PATCH] conf: nodedev: Update capabilities from within virNodeDeviceObjHasCap
On 03/06/2018 10:32 AM, Erik Skultety wrote:
Commit d18feadc0c put this into virNodeDeviceMatch, but this should
have
rather been part of virNodeDeviceObjHasCap from the beginning, since
virNodeDeviceObjHasCap is the last helper to be called (in the calltree)
whereas virNodeDeviceMatch is called from a single place only -
virNodeDeviceObjListExportCallback.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
src/conf/virnodedeviceobj.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index ad0f27ee4..0417664ad 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -644,6 +644,10 @@ virNodeDeviceObjHasCap(const virNodeDeviceObj *obj,
{
virNodeDevCapsDefPtr cap = NULL;
+ /* Refresh the capabilities first, e.g. due to a driver change */
+ if (virNodeDeviceUpdateCaps(obj->def) < 0)
+ return false;
+
While I understand why you put it here - in order to be somewhere common
for nodeDeviceCreateXML, nodeNumOfDevices, nodeListDevices, and
nodeConnectListAllNodeDevices.
However, I think this'll impact "performance" of virNodeDeviceMatch as
virNodeDeviceUpdateCaps would be called 16 times due to the logic of "if
!(MATCH() || MATCH() || MATCH() ...)))"
Of course the alternative is calling virNodeDeviceUpdateCaps from
nodeDeviceCreateXML, nodeNumOfDevices, nodeListDevices, and
nodeConnectListAllNodeDevices.
(or am I reading the !(a || b || c || ... ) logic wrong?
John
for (cap = obj->def->caps; cap; cap = cap->next) {
if (type == cap->data.type)
return true;
@@ -811,10 +815,6 @@ static bool
virNodeDeviceMatch(virNodeDeviceObjPtr obj,
unsigned int flags)
{
- /* Refresh the capabilities first, e.g. due to a driver change */
- if (virNodeDeviceUpdateCaps(obj->def) < 0)
- return false;
-
/* filter by cap type */
if (flags & VIR_CONNECT_LIST_NODE_DEVICES_FILTERS_CAP) {
if (!(MATCH(SYSTEM) ||