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 09:42, Cornelia Huck wrote:
> On Thu, 14 Mar 2013 09:37:54 +0100
> KONRAD Frédéric <fred.konrad@greensocs.com> wrote:
>
>> 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.
> See the second patch :)
>
>
>
good, sorry for the noise :).

Thanks,

Fred