On 12 March 2013 14:37, KONRAD Frédéric <fred.kon...@greensocs.com> wrote:
> On 12/03/2013 15:28, Peter Maydell wrote:
>>
>> On 12 March 2013 09:22, <fred.kon...@greensocs.com> 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.
>
> Do you have any idea for that?
>
> It's a little tricky with the two conditions CONFIG_VIRTIO_BLK_DATA_PLANE
> and __linux__,
>
> I can't have #define with an #ifdef inside..
>
> Do you see what I mean?
Yes, I see your problem there, but DEFINE_VIRTIO_BLK_SCSI_PROPERTY
and DEFINE_DATA_PLANE_PROPERTIES are just convenience macros, not
ones that are expected to be used by other code, right? So you can
define them with commas (and name them something so it's obvious
they're not intended for wider use as property array elements),
and then just make sure your public-facing DEFINE_VIRTIO_BLK_PROPERTIES
doesn't end with a comma. (You can do that by putting the macros
that expand to maybe-comma-or-not at the front, not the end.)
-- PMM
