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@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) ||