From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:34146) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UG8wn-00078o-Hm for qemu-devel@nongnu.org; Thu, 14 Mar 2013 10:16:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UG8wd-0001VB-Rk for qemu-devel@nongnu.org; Thu, 14 Mar 2013 10:15:57 -0400 Received: from greensocs.com ([87.106.252.221]:44209 helo=s15328186.onlinehome-server.info) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UG8wd-0001V1-24 for qemu-devel@nongnu.org; Thu, 14 Mar 2013 10:15:47 -0400 Message-ID: <5141CB27.4050906@greensocs.com> Date: Thu, 14 Mar 2013 14:05:43 +0100 From: =?UTF-8?B?S09OUkFEIEZyw6lkw6lyaWM=?= MIME-Version: 1.0 References: <1363080131-16427-1-git-send-email-fred.konrad@greensocs.com> <1363080131-16427-3-git-send-email-fred.konrad@greensocs.com> <513F3DAF.8050405@greensocs.com> <513F44E7.2020306@greensocs.com> <513F482E.1000106@greensocs.com> <20130312173102.3e86ef76@gondolin> <514037CA.1010607@greensocs.com> <51409C0F.4000007@greensocs.com> <20130314082535.64c08ea5@gondolin> <51418C62.3080800@greensocs.com> <20130314094248.4d76975f@gondolin> In-Reply-To: <20130314094248.4d76975f@gondolin> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v6 2/8] virtio-blk: add the virtio-blk device. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cornelia Huck Cc: Kevin Wolf , Peter Maydell , aliguori@us.ibm.com, mst@redhat.com, mark.burton@greensocs.com, Alexander Graf , qemu-devel@nongnu.org, Stefan Hajnoczi , afaerber@suse.de On 14/03/2013 09:42, Cornelia Huck wrote: > On Thu, 14 Mar 2013 09:37:54 +0100 > KONRAD Fr=C3=A9d=C3=A9ric wrote: > >> On 14/03/2013 08:25, Cornelia Huck wrote: >>> On Wed, 13 Mar 2013 16:32:31 +0100 >>> KONRAD Fr=C3=A9d=C3=A9ric wrote: >>> >>>> On 13/03/2013 09:24, KONRAD Fr=C3=A9d=C3=A9ric wrote: >>>>> On 12/03/2013 17:31, Cornelia Huck wrote: >>>>>> On Tue, 12 Mar 2013 16:22:22 +0100 >>>>>> KONRAD Fr=C3=A9d=C3=A9ric wrote: >>>>>> >>>>>>> On 12/03/2013 16:12, Peter Maydell wrote: >>>>>>>> On 12 March 2013 15:08, KONRAD Fr=C3=A9d=C3=A9ric >>>>>>>> wrote: >>>>>>>>> On 12/03/2013 15:42, Peter Maydell wrote: >>>>>>>>>> Yes, I see your problem there, but DEFINE_VIRTIO_BLK_SCSI_PROP= ERTY >>>>>>>>>> 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 obvio= us >>>>>>>>>> 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 macr= os >>>>>>>>>> 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 follow= ing: >>>>>> >>>>>> - CHS - missing on virtio-ccw, I'll do a patch. >>>>>> - config_wce - missing on s390-virtio and virtio-ccw, should proba= bly >>>>>> 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 prop= erties? >>>>>> >>>>> 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 queu= e >>> (git://github.com/cohuck/qemu virtio-ccw-pending); I'll probably subm= it >>> them once my current pull request is through. >>> >>> On top of this, s390-virtio and virtio-ccw could use the generic macr= o >>> 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