Re: [Qemu-devel] [PATCH v6 2/8] virtio-blk: add the virtio-blk device.

[Peter Maydell]

On 12 March 2013 09:22,  <address@hidden> wrote:
>  /* The ID for virtio_block */
> @@ -130,4 +134,28 @@ typedef struct VirtIOBlock {
>  #define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \
>          DEFINE_VIRTIO_COMMON_FEATURES(_state, _field)
>
> +#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
> +#define DEFINE_DATA_PLANE_PROPERTIES(_state, _field)                         
>  \
> +        DEFINE_PROP_BIT("x-data-plane", _state, _field.data_plane, 0, false),
> +#else
> +#define DEFINE_DATA_PLANE_PROPERTIES(_state, _field)
> +#endif /* CONFIG_VIRTIO_BLK_DATA_PLANE */
> +
> +#ifdef __linux__
> +#define DEFINE_VIRTIO_BLK_SCSI_PROPERTY(_state, _field)                      
>  \
> +        DEFINE_PROP_BIT("scsi", _state, _field.scsi, 0, true),
> +#else
> +#define DEFINE_VIRTIO_BLK_SCSI_PROPERTY(_state, _field)
> +#endif /* __linux__ */
> +
> +#define DEFINE_VIRTIO_BLK_PROPERTIES(_state, _field)                         
>  \
> +        DEFINE_BLOCK_PROPERTIES(_state, _field.conf),                        
>  \
> +        DEFINE_BLOCK_CHS_PROPERTIES(_state, _field.conf),                    
>  \
> +        DEFINE_PROP_STRING("serial", _state, _field.serial),                 
>  \
> +        DEFINE_PROP_BIT("config-wce", _state, _field.config_wce, 0, true),   
>  \
> +        DEFINE_VIRTIO_BLK_SCSI_PROPERTY(_state, _field)                      
>  \
> +        DEFINE_DATA_PLANE_PROPERTIES(_state, _field)
> +
> +void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk);
> +
>  #endif
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index 39c1966..9ed0228 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -1084,19 +1084,10 @@ static void virtio_rng_exit_pci(PCIDevice *pci_dev)
>
>  static Property virtio_blk_properties[] = {
>      DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
> -    DEFINE_BLOCK_PROPERTIES(VirtIOPCIProxy, blk.conf),
> -    DEFINE_BLOCK_CHS_PROPERTIES(VirtIOPCIProxy, blk.conf),
> -    DEFINE_PROP_STRING("serial", VirtIOPCIProxy, blk.serial),
> -#ifdef __linux__
> -    DEFINE_PROP_BIT("scsi", VirtIOPCIProxy, blk.scsi, 0, true),
> -#endif
> -    DEFINE_PROP_BIT("config-wce", VirtIOPCIProxy, blk.config_wce, 0, true),
>      DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, 
> VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
> -#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
> -    DEFINE_PROP_BIT("x-data-plane", VirtIOPCIProxy, blk.data_plane, 0, 
> false),
> -#endif
>      DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
>      DEFINE_VIRTIO_BLK_FEATURES(VirtIOPCIProxy, host_features),
> +    DEFINE_VIRTIO_BLK_PROPERTIES(VirtIOPCIProxy, blk)
>      DEFINE_PROP_END_OF_LIST(),

You need to tweak your macro definitions so that the user can
put a comma after DEFINE_VIRTIO_BLK_PROPERTIES() [compare
DEFINE_BLOCK_PROPERTIES and DEFINE_BLOCK_CHS_PROPERTIES].
Otherwise it's going to get confusing and somebody's going
to add one without noticing that that gets you an extra
empty properties array element.

>  };
>
> --
> 1.7.11.7
>

-- PMM