Re: [Qemu-devel] [RFC v3 07/10] vfio_pci: change vfio device features bit macro to enum definition

[Alex Williamson <alex.williamson@redhat.com>]

On Tue, 2015-02-10 at 15:03 +0800, Chen Fan wrote:
> Introduce an independent enum structure to define the features bitmap,
> it would be good for adding new features definition.
> 
> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> ---
>  hw/vfio/pci.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 75c932b..bf314a1 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -134,6 +134,12 @@ typedef struct VFIOMSIXInfo {
>      void *mmap;
>  } VFIOMSIXInfo;
>  
> +/* Bits in VFIOPCIDevice features field. */
> +enum {
> +#define VFIO_FEATURE_ENABLE_VGA_BIT 0
> +    VFIO_FEATURE_ENABLE_VGA = (1 << VFIO_FEATURE_ENABLE_VGA_BIT),
> +};
> +

As commented last time, I don't see why making this an enum helps
anything.  If anything, the bits could be an enum, but it makes no sense
to me to put the feature mask into an enum.

>  typedef struct VFIOPCIDevice {
>      PCIDevice pdev;
>      VFIODevice vbasedev;
> @@ -154,8 +160,6 @@ typedef struct VFIOPCIDevice {
>      PCIHostDeviceAddress host;
>      EventNotifier err_notifier;
>      uint32_t features;
> -#define VFIO_FEATURE_ENABLE_VGA_BIT 0
> -#define VFIO_FEATURE_ENABLE_VGA (1 << VFIO_FEATURE_ENABLE_VGA_BIT)
>      int32_t bootindex;
>      uint8_t pm_cap;
>      bool has_vga;