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.

> 
> 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,
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)

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org