From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38268) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bEsIO-0008WB-Ih for qemu-devel@nongnu.org; Mon, 20 Jun 2016 02:02:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bEsIJ-0000vA-HF for qemu-devel@nongnu.org; Mon, 20 Jun 2016 02:02:51 -0400 Received: from mx2.suse.de ([195.135.220.15]:38565) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bEsIJ-0000v3-6W for qemu-devel@nongnu.org; Mon, 20 Jun 2016 02:02:47 -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> <5763D394.8000102@suse.com> <8b53bcd700444ee883e0450c51ca7cb9@AMSPEX02CL03.citrite.net> From: Juergen Gross Message-ID: <57678703.4040408@suse.com> Date: Mon, 20 Jun 2016 08:02:43 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 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: Stefano Stabellini , Paul Durrant Cc: Jan Beulich , Anthony Perard , xen-devel , "qemu-devel@nongnu.org" , "kraxel@redhat.com" On 17/06/16 18:10, Stefano Stabellini wrote: > On Fri, 17 Jun 2016, Paul Durrant wrote: >>> -----Original Message----- >>> From: Juergen Gross [mailto:jgross@suse.com] >>> Sent: 17 June 2016 11:40 >>> 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 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? >>> >> >> I guess re-syncing with the Linux header as in your v2 patch is the least worst option then. > > As the Linux header is by no means canonical or special, I prefer patch > #1 (with the indentation fixed). Could you please specify in which way the indentation should be fixed? Should I keep the current indentation of the file via tabs or should I prepend a patch to change the whole file to qemu style? Juergen