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