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

On Mon, 15 Feb 2021 18:22:41 +0100 Erik Skultety <eskultet(a)redhat.com&gt; wrote: > On Wed, Feb 03, 2021 at 11:38:47AM -0600, 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 <jjongsma(a)redhat.com&gt; > > Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; > > --- > > include/libvirt/libvirt-nodedev.h | 9 +++-- > > src/conf/node_device_conf.h | 8 ++++ > > src/conf/virnodedeviceobj.c | 56 > > ++++++++++++++++------------ src/libvirt-nodedev.c | > > 2 + src/node_device/node_device_driver.c | 2 +- > > 5 files changed, 50 insertions(+), 27 deletions(-) > > > > diff --git a/include/libvirt/libvirt-nodedev.h > > b/include/libvirt/libvirt-nodedev.h index eab8abf6ab..d304283871 > > 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 */ @@ -86,6 +85,10 @@ typedef enum { > > VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_CARD = 1 << 18, /* s390 > > AP Card device */ VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_QUEUE = > > 1 << 19, /* s390 AP Queue */ > > VIR_CONNECT_LIST_NODE_DEVICES_CAP_AP_MATRIX = 1 << 20, /* s390 > > AP Matrix */ + > > + /* filter the devices by active state */ > > + VIR_CONNECT_LIST_NODE_DEVICES_INACTIVE = 1 << 29, /* > > Inactive devices */ > > + VIR_CONNECT_LIST_NODE_DEVICES_ACTIVE = 1 << 30, /* > > Active devices */ > > We don't define sentinels on public flags, so what are we saving the > last value for? Why couldn't ^this become 1U << 31? > > ... Because gcc implements the enum as a signed int and 1<<31 overflows the maximum positive integer value: In file included from ../include/libvirt/libvirt.h:42, from ../src/internal.h:65, from ../src/util/vircgroupv1.c:30: ../include/libvirt/libvirt-nodedev.h:91:57: error: result of ‘1 << 31’ requires 33 bits to represent, but ‘int’ only has 32 bits [-Werror=shift-overflow=] 91 | VIR_CONNECT_LIST_NODE_DEVICES_ACTIVE = 1 << 31, /* Active devices */ | ^~ cc1: all warnings being treated as errors