On 10/10/2017 08:59 AM, Kevin Wolf wrote: > Am 04.10.2017 um 04:00 hat Eric Blake geschrieben: >> Not all callers care about which BDS owns the mapping for a given >> range of the file. This patch merely simplifies the callers by >> consolidating the logic in the common call point, while guaranteeing >> a non-NULL file to all the driver callbacks, for no semantic change. >> The only caller that does not care about pnum is bdrv_is_allocated, >> as invoked by vvfat; we can likewise add assertions that the rest >> of the stack does not have to worry about a NULL pnum. >> >> Furthermore, this will also set the stage for a future cleanup: when >> a caller does not care about which BDS owns an offset, it would be >> nice to allow the driver to optimize things to not have to return >> BDRV_BLOCK_OFFSET_VALID in the first place. In the case of fragmented >> allocation (for example, it's fairly easy to create a qcow2 image >> where consecutive guest addresses are not at consecutive host >> addresses), the current contract requires bdrv_get_block_status() >> to clamp *pnum to the limit where host addresses are no longer >> consecutive, but allowing a NULL file means that *pnum could be >> set to the full length of known-allocated data. >> >> Signed-off-by: Eric Blake <ebl...@redhat.com> >> >> --- >> v5: use second label for cleaner exit logic [John], use local_pnum
>> @@ -1811,16 +1811,19 @@ static int64_t coroutine_fn >> bdrv_co_get_block_status(BlockDriverState *bs, >> int64_t total_sectors; >> int64_t n; >> int64_t ret, ret2; >> + BlockDriverState *local_file = NULL; >> + int local_pnum = 0; > > I don't quite see what the point of local_pnum is if we assert anyway > that the real pnum is non-NULL. I did it in parallel with fallout from John's review on v4: https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg06958.html but since it wasn't specifically asked for, and is now getting questions, I'm fine with not having it in v6. >> >> -out: >> + out: > > git grep tells me that the old style (unindented label, as opposed to > indented by a single space) is a lot more common both within the block > layer and across qemu as a whole. emacs likes to add the space (so that grepping for content in column 1 sees only function declarations, not mid-function labels - which in turn makes 'diff -p' output nicer to read). But I can override emacs' insistence, and go with the label flush to the left. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature