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

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

On 14/03/2013 08:25, Cornelia Huck wrote:
> On Wed, 13 Mar 2013 16:32:31 +0100
> KONRAD Frédéric <fred.konrad@greensocs.com> wrote:
>
>> 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?
> I currently have the following two patches sitting in my pending queue
> (git://github.com/cohuck/qemu virtio-ccw-pending); I'll probably submit
> them once my current pull request is through.
>
> On top of this, s390-virtio and virtio-ccw could use the generic macro
> for the virtio-blk properties from the start if x-data-plane is split
> out (I can add it to virtio-ccw once we support it).
>
Ok good,

And what about config-wce property for virtio-blk-s390x? It is missing too.

Fred