On Thu, 14 Mar 2013 09:37:54 +0100 KONRAD Frédéric <fred.kon...@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.kon...@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.kon...@greensocs.com> wrote: > >>>> > >>>>> On 12/03/2013 16:12, Peter Maydell wrote: > >>>>>> On 12 March 2013 15:08, KONRAD Frédéric <fred.kon...@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 :)