Re: [Qemu-devel] [PATCH v2 4/4] raw-posix: Clean up around raw_co_get_block_status()

[Markus Armbruster <armbru@redhat.com>]

Max Reitz <mreitz@redhat.com> writes:

> On 2014-11-13 at 11:17, Markus Armbruster wrote:
>> try_seek_hole() doesn't really seek to a hole, it tries to find out
>> whether its argument is in a hole or not, and where the hole or
>> non-hole ends.  Rename to find_allocation() and add a proper function
>> comment.
>>
>> Using arguments passed by reference like local variables is a bad
>> habit.  Only assign to them right before return.
>>
>> Avoid nesting of conditionals.
>>
>> When find_allocation() fails, don't make up a range that'll get mapped
>> to nb_sectors, simply set *pnum = nb_sectors directly.
>>
>> Don't repeat BDRV_BLOCK_OFFSET_VALID | start.
>>
>> Drop a pointless assertion, add some meaningful ones.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   block/raw-posix.c | 62 +++++++++++++++++++++++++++++++++----------------------
>>   1 file changed, 37 insertions(+), 25 deletions(-)
>>
>> diff --git a/block/raw-posix.c b/block/raw-posix.c
>> index 2a12a50..ea5b3b8 100644
>> --- a/block/raw-posix.c
>> +++ b/block/raw-posix.c
>> @@ -1478,28 +1478,43 @@ out:
>>       return result;
>>   }
>>   -static int try_seek_hole(BlockDriverState *bs, off_t start, off_t
>> *data,
>> -                         off_t *hole)
>> +/*
>> + * Find allocation range in @bs around offset @start.
>> + * If @start is in a hole, store @start in @hole and the end of the
>> + * hole in @data, and return 0.
>> + * If @start is in a data, store @start to @data, and the end of the
>
> "is in a data" sounds funny enough I'd even like to keep it. Probably
> should be "data extent" or something similar.

Okay.

>> + * data to @hole, and return 0.
>
> Here, too.
>
> With or without that changed:
>
> Reviewed-by: Max Reitz <mreitz@redhat.com>

Thanks!