From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46319) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bDrCM-0007nO-KD for qemu-devel@nongnu.org; Fri, 17 Jun 2016 06:40:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bDrCI-0005MR-Iw for qemu-devel@nongnu.org; Fri, 17 Jun 2016 06:40:26 -0400 Received: from mx2.suse.de ([195.135.220.15]:54104) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bDrCI-0005MN-6x for qemu-devel@nongnu.org; Fri, 17 Jun 2016 06:40:22 -0400 References: <1466154874-6218-1-git-send-email-jgross@suse.com> <5763DE5D02000078000F5FF8@prv-mh.provo.novell.com> <5763C6CA.4090705@suse.com> <5763CC05.3030008@suse.com> From: Juergen Gross Message-ID: <5763D394.8000102@suse.com> Date: Fri, 17 Jun 2016 12:40:20 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Xen-devel] [PATCH v2] xen: fix qdisk BLKIF_OP_DISCARD for 32/64 word size mix List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paul Durrant , Jan Beulich Cc: Anthony Perard , xen-devel , "sstabellini@kernel.org" , "qemu-devel@nongnu.org" , "kraxel@redhat.com" On 17/06/16 12:15, Paul Durrant wrote: >> -----Original Message----- >> From: Xen-devel [mailto:xen-devel-bounces@lists.xen.org] On Behalf Of >> Juergen Gross >> Sent: 17 June 2016 11:08 >> To: Paul Durrant; Jan Beulich >> Cc: Anthony Perard; xen-devel; sstabellini@kernel.org; qemu- >> devel@nongnu.org; kraxel@redhat.com >> Subject: Re: [Xen-devel] [PATCH v2] xen: fix qdisk BLKIF_OP_DISCARD for >> 32/64 word size mix >> >> On 17/06/16 11:50, Paul Durrant wrote: >>>> -----Original Message----- >>>> From: Juergen Gross [mailto:jgross@suse.com] >>>> Sent: 17 June 2016 10:46 >>>> To: Paul Durrant; Jan Beulich >>>> Cc: Anthony Perard; xen-devel; sstabellini@kernel.org; qemu- >>>> devel@nongnu.org; kraxel@redhat.com >>>> Subject: Re: [Xen-devel] [PATCH v2] xen: fix qdisk BLKIF_OP_DISCARD for >>>> 32/64 word size mix >>>> >>>> On 17/06/16 11:37, Paul Durrant wrote: >>>>>> -----Original Message----- >>>>>> From: Xen-devel [mailto:xen-devel-bounces@lists.xen.org] On Behalf >> Of >>>> Jan >>>>>> Beulich >>>>>> Sent: 17 June 2016 10:26 >>>>>> To: Juergen Gross >>>>>> Cc: Anthony Perard; xen-devel; sstabellini@kernel.org; qemu- >>>>>> devel@nongnu.org; kraxel@redhat.com >>>>>> Subject: Re: [Xen-devel] [PATCH v2] xen: fix qdisk BLKIF_OP_DISCARD >> for >>>>>> 32/64 word size mix >>>>>> >>>>>>>>> On 17.06.16 at 11:14, wrote: >>>>>>> In case the word size of the domU and qemu running the qdisk >> backend >>>>>>> differ BLKIF_OP_DISCARD will not work reliably, as the request >>>>>>> structure in the ring have different layouts for different word size. >>>>>>> >>>>>>> Correct this by copying the request structure in case of different >>>>>>> word size element by element in the BLKIF_OP_DISCARD case, too. >>>>>>> >>>>>>> The easiest way to achieve this is to resync hw/block/xen_blkif.h with >>>>>>> its original source from the Linux kernel. >>>>>>> >>>>>>> Signed-off-by: Juergen Gross >>>>>>> --- >>>>>>> V2: resync with Linux kernel version of hw/block/xen_blkif.h as >>>>>>> suggested by Paul Durrant >>>>>> >>>>>> Oh, I didn't realize he suggested syncing with the Linux variant. >>>>>> Why not with the canonical one? I have to admit that I particularly >>>>>> dislike Linux'es strange union-izng, mainly because of it requiring >>>>>> this myriad of __attribute__((__packed__)). >>>>>> >>>>> >>>>> Yes, it's truly grotesque and such things should be blown away with >>>> extreme prejudice. >>>> >>>> Sorry, I'm confused now. >>>> >>>> Do you still mandate for the resync or not? >>>> >>>> Resyncing with elimination of all the __packed__ stuff seems not to be >>>> a proper alternative as this would require a major rework. >>> >>> Why? Replacing the existing horribleness with the canonical header (fixed >> for style) might mean a large diff but it should be functionally the same or >> something has gone very seriously wrong. If the extra part you need is not in >> the canonical header then adding this as a second patch seems like a >> reasonable plan. >> >> I think you don't realize that qemu is built using the public headers >> from the Xen build environment. So there is no way to resync with the >> canonical header as this isn't part of the qemu tree. >> > > Now I'm confused... you're posting a patch to hw/block/xen_blkif.h. That's in the QEMU source, right? That's not a Xen public header but is a Linux mangled variant of a Xen public header. So, actually, I guess the question is why can't this header just go away and QEMU use the canonical header directly from Xen? No, hw/block/xen_blkif.h is based on the Linux header drivers/block/xen-blkback/common.h which is an add-on header to the canonical-based Linux header include/xen/interface/io/blkif.h >> The header in question is originating from the Linux one which is an >> add-on of the canonical header containing the explicit 32- and 64-bit >> variants of the xenbus protocol and the conversion routines between >> those. >> >> It would be possible to add these parts to the canonical header, but >> do we really want that? >> > > No, we shouldn't be taking Linux brokenness into the canonical header. Okay, so then back to the first approach using hw/block/xen_blkif.h as today and adapting the style first and then doing the necessary code correction? Juergen