Re: [libvirt PATCH v2 03/16] nodedev: Add ability to filter by active state

2020-08-20 Thread Erik Skultety 
On Tue, Aug 18, 2020 at 09:47:53AM -0500, Jonathon Jongsma wrote:
> Add two flag values for virConnectListAllNodeDevices() so that we can
> list only node devices that are active or inactive.
>
> Signed-off-by: Jonathon Jongsma 
> ---
...

> diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
> index 88a7746b27..9838513b69 100644
> --- a/src/conf/virnodedeviceobj.c
> +++ b/src/conf/virnodedeviceobj.c
> @@ -865,6 +865,16 @@ virNodeDeviceObjMatch(virNodeDeviceObjPtr obj,
>  return false;
>  }
>
> +#undef MATCH
> +#define MATCH(FLAG) (flags & (FLAG))
> +
> +if (MATCH(VIR_CONNECT_LIST_NODE_DEVICES_FILTERS_ACTIVE) &&
> +!((MATCH(VIR_CONNECT_LIST_NODE_DEVICES_ACTIVE) &&
> +   virNodeDeviceObjIsActive(obj)) ||
> +  (MATCH(VIR_CONNECT_LIST_NODE_DEVICES_INACTIVE) &&
> +   !virNodeDeviceObjIsActive(obj
> +return false;
> +
>  return true;
>  }
>  #undef MATCH

^This produces a significantly smaller diff, but I'm not really a fan of
re-defining the same macro in a different way, it can turn out being confusing
in the end, since it's a different macro. I suggest renaming the original MATCH
macro to something like MATCH_CAP(CAP) and defining this new one as MATCH.

ACK to the rest.

Erik

[libvirt PATCH v2 03/16] nodedev: Add ability to filter by active state

2020-08-18 Thread Jonathon Jongsma 
Add two flag values for virConnectListAllNodeDevices() so that we can
list only node devices that are active or inactive.

Signed-off-by: Jonathon Jongsma 
---
 include/libvirt/libvirt-nodedev.h|  9 ++---
 src/conf/node_device_conf.h  |  8 
 src/conf/virnodedeviceobj.c  | 10 ++
 src/libvirt-nodedev.c|  2 ++
 src/node_device/node_device_driver.c |  2 +-
 5 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/include/libvirt/libvirt-nodedev.h 
b/include/libvirt/libvirt-nodedev.h
index a2ad61ac6d..98a972e60d 100644
--- a/include/libvirt/libvirt-nodedev.h
+++ b/include/libvirt/libvirt-nodedev.h
@@ -61,10 +61,9 @@ int virNodeListDevices  
(virConnectPtr conn,
  * virConnectListAllNodeDevices:
  *
  * Flags used to filter the returned node devices. Flags in each group
- * are exclusive. Currently only one group to filter the devices by cap
- * type.
- */
+ * are exclusive.  */
 typedef enum {
+/* filter the devices by cap type */
 VIR_CONNECT_LIST_NODE_DEVICES_CAP_SYSTEM= 1 << 0,  /* System 
capability */
 VIR_CONNECT_LIST_NODE_DEVICES_CAP_PCI_DEV   = 1 << 1,  /* PCI device */
 VIR_CONNECT_LIST_NODE_DEVICES_CAP_USB_DEV   = 1 << 2,  /* USB device */
@@ -81,6 +80,10 @@ typedef enum {
 VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV_TYPES= 1 << 13, /* Capable of 
mediated devices */
 VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV  = 1 << 14, /* Mediated 
device */
 VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV   = 1 << 15, /* CCW device */
+
+/* filter the devices by active state */
+VIR_CONNECT_LIST_NODE_DEVICES_INACTIVE  = 1 << 16, /* Inactive 
devices */
+VIR_CONNECT_LIST_NODE_DEVICES_ACTIVE= 1 << 17, /* Active 
devices */
 } virConnectListAllNodeDeviceFlags;
 
 int virConnectListAllNodeDevices (virConnectPtr conn,
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index 9b8c7aadea..353dbebaf0 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -369,6 +369,14 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps);
  VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV  | \
  VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV)
 
+#define VIR_CONNECT_LIST_NODE_DEVICES_FILTERS_ACTIVE \
+VIR_CONNECT_LIST_NODE_DEVICES_ACTIVE |\
+VIR_CONNECT_LIST_NODE_DEVICES_INACTIVE
+
+#define VIR_CONNECT_LIST_NODE_DEVICES_FILTERS_ALL \
+VIR_CONNECT_LIST_NODE_DEVICES_FILTERS_CAP | \
+VIR_CONNECT_LIST_NODE_DEVICES_FILTERS_ACTIVE
+
 int
 virNodeDeviceGetSCSIHostCaps(virNodeDevCapSCSIHostPtr scsi_host);
 
diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index 88a7746b27..9838513b69 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -865,6 +865,16 @@ virNodeDeviceObjMatch(virNodeDeviceObjPtr obj,
 return false;
 }
 
+#undef MATCH
+#define MATCH(FLAG) (flags & (FLAG))
+
+if (MATCH(VIR_CONNECT_LIST_NODE_DEVICES_FILTERS_ACTIVE) &&
+!((MATCH(VIR_CONNECT_LIST_NODE_DEVICES_ACTIVE) &&
+   virNodeDeviceObjIsActive(obj)) ||
+  (MATCH(VIR_CONNECT_LIST_NODE_DEVICES_INACTIVE) &&
+   !virNodeDeviceObjIsActive(obj
+return false;
+
 return true;
 }
 #undef MATCH
diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c
index ca6c2bb057..6dc61304a0 100644
--- a/src/libvirt-nodedev.c
+++ b/src/libvirt-nodedev.c
@@ -101,6 +101,8 @@ virNodeNumOfDevices(virConnectPtr conn, const char *cap, 
unsigned int flags)
  *   VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV_TYPES
  *   VIR_CONNECT_LIST_NODE_DEVICES_CAP_MDEV
  *   VIR_CONNECT_LIST_NODE_DEVICES_CAP_CCW_DEV
+ *   VIR_CONNECT_LIST_NODE_DEVICES_ACTIVE
+ *   VIR_CONNECT_LIST_NODE_DEVICES_INACTIVE
  *
  * Returns the number of node devices found or -1 and sets @devices to NULL in
  * case of error.  On success, the array stored into @devices is guaranteed to
diff --git a/src/node_device/node_device_driver.c 
b/src/node_device/node_device_driver.c
index e89c8b0ee5..f766fd9f32 100644
--- a/src/node_device/node_device_driver.c
+++ b/src/node_device/node_device_driver.c
@@ -218,7 +218,7 @@ nodeConnectListAllNodeDevices(virConnectPtr conn,
   virNodeDevicePtr **devices,
   unsigned int flags)
 {
-virCheckFlags(VIR_CONNECT_LIST_NODE_DEVICES_FILTERS_CAP, -1);
+virCheckFlags(VIR_CONNECT_LIST_NODE_DEVICES_FILTERS_ALL, -1);
 
 if (virConnectListAllNodeDevicesEnsureACL(conn) < 0)
 return -1;
-- 
2.26.2