Re: [Qemu-devel] [Xen-devel] [PATCH v2] xen: fix qdisk BLKIF_OP_DISCARD for 32/64 word size mix

[Juergen Gross]

On 17/06/16 11:50, Paul Durrant wrote:
>> -----Original Message-----
>> From: Juergen Gross [mailto:address@hidden
>> Sent: 17 June 2016 10:46
>> To: Paul Durrant; Jan Beulich
>> Cc: Anthony Perard; xen-devel; address@hidden; qemu-
>> address@hidden; address@hidden
>> 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:address@hidden On Behalf Of
>> Jan
>>>> Beulich
>>>> Sent: 17 June 2016 10:26
>>>> To: Juergen Gross
>>>> Cc: Anthony Perard; xen-devel; address@hidden; qemu-
>>>> address@hidden; address@hidden
>>>> 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, <address@hidden> 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 <address@hidden>
>>>>> ---
>>>>> 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.

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?


Juergen