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