On 09/25/2017 05:43 PM, John Snow wrote:
>
>
> On 09/13/2017 12:03 PM, Eric Blake wrote:
>> 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.
>>
>
> Function reads slightly worse for the wear now with all of the return
> logic handled at various places within, but unifying it might be even
> stranger, perhaps..
>
> Let's see if I hate this more:
>
> out:
> bdrv_dec_in_flight(bs);
> bdrv_dec_in_flight(bs);
> if (ret >= 0 && sector_num + *pnum == total_sectors) {
> ret |= BDRV_BLOCK_EOF;
> }
> early_out:
> if (file) {
> *file = local_file;
> }
> return ret;
>
>
> and then earlier in the function, we can just:
>
> if (total_sectors < 0) {
> ret = total_sectors;
> goto early_out;
> }Seems reasonable enough, I'll work that in to v5, since there are other reasons to respin the series anyway. > > It's only shed paint, though: > > Reviewed-by: John Snow <[email protected]> > > I'm looking at the rest of the series now, so please stand by. > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
