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

[Jonathon Jongsma]

On Mon, 15 Feb 2021 18:22:41 +0100
Erik Skultety <eskultet at redhat.com> 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 at redhat.com>
> > Reviewed-by: Erik Skultety <eskultet at redhat.com>
> > ---
> >  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


Jonathon