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

["KONRAD Frédéric" <fred.konrad@greensocs.com>]

On 13/03/2013 09:24, KONRAD Frédéric wrote:
> On 12/03/2013 17:31, Cornelia Huck wrote:
>> On Tue, 12 Mar 2013 16:22:22 +0100
>> KONRAD Frédéric <fred.konrad@greensocs.com> wrote:
>>
>>> On 12/03/2013 16:12, Peter Maydell wrote:
>>>> On 12 March 2013 15:08, KONRAD Frédéric <fred.konrad@greensocs.com> 
>>>> wrote:
>>>>> On 12/03/2013 15:42, Peter Maydell wrote:
>>>>>> 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
>>>>> ok, I can put a comment which say not to use them?
>>>> And suitable macro names (ie not ones which look like all
>>>> the other DEFINE_FOO_PROPERTIES ones). Alternatively since the
>>>> macro's only used once as far as I can see, you could just not
>>>> bother to abstract it out. The virtio-ccw blk properties still
>>>> just have inline #ifdefs for the scsi prop for instance.
>>>>
>>>> -- PMM
>>> The macro is used for virtio-blk device and virtio-blk-pci.
>>> s390x devices don't use the same properties.
>>>
>> Looking at the s390 devices, the difference seems to be the following:
>>
>> - CHS - missing on virtio-ccw, I'll do a patch.
>> - config_wce - missing on s390-virtio and virtio-ccw, should probably
>>    be added.
>> - x-data-plane - we plan to add this eventually to virtio-ccw, but not
>>    to s390-virtio. Could that be split out from the generic properties?
>>
> ok, so what I can do is:
>
> - split up x-data-plane property (so it will be only in virtio-pci.c).
> - fix this comma thing.
>
> Then when you put these two missing properties you can just replace 
> all of them
>  with the macro.
>
> Is that ok for everybody? Peter? Stefan?
>
Any other suggestion?

Thanks,

Fred