On 12/01/2017 09:24 AM, Kevin Wolf wrote: > Am 01.12.2017 um 16:03 hat Eric Blake geschrieben: >> On 12/01/2017 08:40 AM, Kevin Wolf wrote: >>>> Note that most drivers give sector-aligned answers, except at >>>> end-of-file, even when request_alignment is smaller than a sector. >>>> However, bdrv_getlength() is sector-aligned (even though it gives a >>>> byte answer), often by exceeding the actual file size. If we were to >>>> give back strict results, at least file-posix.c would report a >>>> transition from DATA to HOLE at the end of a file even in the middle >>>> of a sector, which can throw off callers; so we intentionally lie and >>>> state that any partial sector at the end of a file has the same >>>> status for the entire sector. Maybe at some future day we can >>>> report actual file size instead of rounding up, but not for this >>>> series. >>> >>> In what way does this throw off callers in practice? >> >> Several iotests failed if I didn't do that (it's been a few months, so the >> details are a bit fuzzy). I think the biggest problem is that because we >> round the size up in bdrv_getlength(), but CANNOT access those rounded >> bytes, then reporting the status of those bytes as a hole (which is the only >> sane thing that file-posix can do) can cause grief when the rest of the >> sector (which we CAN access) is data. > > Does this indicate caller bugs? If they call byte-based > bdrv_co_block_status(), they should surely be able to handle this > situation in theory?
Hmm, the more I try to reproduce the failure, the more I'm coming to the conclusion that I needed this back when we returned a rounded offset via the return value (my tag nbd-byte-callback-v2), but now that we return the actual offset rather than a rounded offset via a secondary parameter (my tag nbd-byte-callback-v4), we don't need to compensate for end-of-file at all! That means even less code in my upcoming v5 submission. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
