Re: [Qemu-block] Unaligned images with O_DIRECT

[Max Reitz]

On 14.05.19 17:45, Eric Blake wrote:
> On 5/14/19 10:06 AM, Max Reitz wrote:
>> Hi,
>>
>> Unaligned images don’t work so well with O_DIRECT:
>>
>> $ echo > foo
>> $ qemu-img map --image-opts driver=file,filename=foo,cache.direct=on
>> Offset          Length          Mapped to       File
>> qemu-img: block/io.c:2093: bdrv_co_block_status: Assertion `*pnum &&
>> QEMU_IS_ALIGNED(*pnum, align) && align > offset - aligned_offset' failed.
>> [1]    10954 abort (core dumped)  qemu-img map --image-opts
>> driver=file,filename=foo,cache.direct=on
>>
>> (compare https://bugzilla.redhat.com/show_bug.cgi?id=1588356)
>>
>> This is because the request_alignment is 512 (in my case), but the EOF
>> is not aligned accordingly, so raw_co_block_status() returns an aligned
>> *pnum.
> 
> Uggh. Yet another reason why I want qemu to support byte-accurate
> sizing, instead of rounding up. The rounding keeps raising its head in
> more and more places. I have pending patches that are trying to improve
> block status to round driver answers up to match request_alignment (when
> the protocol layer has finer granularity than the format layer); but
> this sounds like it is a bug in the file driver itself for returning an
> answer that is not properly rounded according to its own
> request_alignment boundary, and not one where my pending patches would help.

Yes, I think so, too.

>> I suppose having an unaligned tail is not so bad and maybe we can just
>> adjust the assertion accordingly.  On the other hand, this has been
>> broken for a while.  Does it even make sense to use O_DIRECT with
>> unaligned images?  Shouldn’t we just reject them outright?
> 
> The tail of an unaligned file is generally inaccessible to O_DIRECT,

Especially with this.

> where it is easier to use ftruncate() up to an aligned boundary if you
> really must play with that region of the file, and then ftruncate() back
> to the intended size after I/O. But that sounds hairy.  We could also
> round down and silently ignore the tail of the file, but that is at odds
> with our practice of rounding size up.  So for the short term, I'd be
> happy with a patch that just rejects any attempt to use cache.direct=on
> (O_DIRECT) with a file that is not already a multiple of the alignment
> required thereby. (For reference, that's what qemu as NBD client
> recently did when talking to a server that advertises a size
> inconsistent with forced minimum block access: commit 3add3ab7)

OK, I’ll send a patch.  Thanks for you explanation!

Max

signature.asc
Description: OpenPGP digital signature