Re: [Qemu-devel] [PATCH V7 02/14] block: add bdrv_can_read_snapshot() function

[Wenchao Xia]

于 2013-2-27 21:58, Markus Armbruster 写道:
> Wenchao Xia <address@hidden> writes:
> 
>>    Compared to bdrv_can_snapshot(), this function return whether
>> bs* is ready to read snapshot info from instead of write. If yes,
>> caller can then query snapshot information, but taking snapshot
>> is not always possible for that *bs may be read only.
>>
>> Signed-off-by: Wenchao Xia <address@hidden>
>> Reviewed-by: Eric Blake <address@hidden>
>> ---
>>   block.c               |   19 +++++++++++++++++++
>>   include/block/block.h |    1 +
>>   2 files changed, 20 insertions(+), 0 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 50dab8e..19c2d7b 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -3058,6 +3058,25 @@ bool bdrv_debug_is_suspended(BlockDriverState *bs, 
>> const char *tag)
>>   /**************************************************************/
>>   /* handling of snapshots */
>>   
>> +/* return whether internal snapshot can be read on @bs */
>> +bool bdrv_can_read_snapshot(BlockDriverState *bs)
>> +{
>> +    BlockDriver *drv = bs->drv;
>> +    if (!drv || !bdrv_is_inserted(bs)) {
>> +        return false;
>> +    }
>> +
>> +    if (!drv->bdrv_snapshot_create) {
>> +        if (bs->file != NULL) {
>> +            return bdrv_can_read_snapshot(bs->file);
>> +        }
>> +        return false;
>> +    }
>> +
>> +    return true;
>> +}
> 
> Looks like
> 
> bdrv_can_read_snapshot(bs) && !bdrv_is_read_only(bs) == bdrv_can_snapshot(bs)
> 
> Correct?
> 
  not equal, it is tricky for that recusively check is done inside
bdrv_can_snapshot(bs), that is why I did not use a common internal
static function.

>> +
>> +/* return whether internal snapshot can be write on @bs */
> 
> "be written".
> 
> Or more succinctly:
> 
> /* Can @bs take a snapshot?  */
> 
  OK, this seems better.

>>   int bdrv_can_snapshot(BlockDriverState *bs)
>>   {
>>       BlockDriver *drv = bs->drv;
>> diff --git a/include/block/block.h b/include/block/block.h
>> index 5c3b911..4c48052 100644
>> --- a/include/block/block.h
>> +++ b/include/block/block.h
>> @@ -321,6 +321,7 @@ void bdrv_get_full_backing_filename(BlockDriverState *bs,
>>                                       char *dest, size_t sz);
>>   BlockInfo *bdrv_query_info(BlockDriverState *s);
>>   BlockStats *bdrv_query_stats(const BlockDriverState *bs);
>> +bool bdrv_can_read_snapshot(BlockDriverState *bs);
>>   int bdrv_can_snapshot(BlockDriverState *bs);
>>   int bdrv_is_snapshot(BlockDriverState *bs);
>>   BlockDriverState *bdrv_snapshots(void);
> 


-- 
Best Regards

Wenchao Xia