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

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

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?

Thanks for replies,

Fred