Re: [Qemu-trivial] [Qemu-devel] [PATCH] block: Fix comment for bdrv_co_get_block_status

[Markus Armbruster]

Max Reitz <address@hidden> writes:

> On 2014-11-10 at 10:10, Fam Zheng wrote:
>> It returns more information than binary, fix the comment.
>>
>> Signed-off-by: Fam Zheng <address@hidden>
>> ---
>>   block.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index dacd881..42c914e 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -3903,9 +3903,9 @@ typedef struct BdrvCoGetBlockStatusData {
>>   } BdrvCoGetBlockStatusData;
>>     /*
>> - * Returns true iff the specified sector is present in the disk image. 
>> Drivers
>> - * not implementing the functionality are assumed to not support backing 
>> files,
>> - * hence all their sectors are reported as allocated.
>> + * Returns the status of the specified sectors. Drivers not implementing the
>> + * functionality are assumed to not support backing files, hence all their
>> + * sectors are reported as allocated.
>>    *
>>    * If 'sector_num' is beyond the end of the disk image the return value is >> 0
>>    * and 'pnum' is set to 0.
>
> "status" is a bit broad, but pointing to "line 86 in
> include/block/block.h" for a further explanation will probably not be
> very stable.
>
> Reviewed-by: Max Reitz <address@hidden>

Could perhaps call it "allocation status", and squash in something like

diff --git a/include/block/block.h b/include/block/block.h
index 13e4537..5450610 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -83,7 +83,9 @@ typedef enum {
 #define BDRV_SECTOR_SIZE   (1ULL << BDRV_SECTOR_BITS)
 #define BDRV_SECTOR_MASK   ~(BDRV_SECTOR_SIZE - 1)
 
-/* BDRV_BLOCK_DATA: data is read from bs->file or another file
+/*
+ * Allocation status flags
+ * BDRV_BLOCK_DATA: data is read from bs->file or another file
  * BDRV_BLOCK_ZERO: sectors read as zero
  * BDRV_BLOCK_OFFSET_VALID: sector stored in bs->file as raw data
  * BDRV_BLOCK_ALLOCATED: the content of the block is determined by this

Anyway, it's an improvement already, so

Reviewed-by: Markus Armbruster <address@hidden>