Re: [Qemu-devel] [PATCH V2 2/3] block: Use bdrv_get_backing_file_ancestors_count()

[Kevin Wolf]

Am 25.07.2012 13:42, schrieb Benoît Canet:
> Le Wednesday 25 Jul 2012 à 13:19:15 (+0200), Kevin Wolf a écrit :
>> Am 25.07.2012 13:15, schrieb Benoît Canet:
>>> Le Wednesday 25 Jul 2012 à 12:57:05 (+0200), Kevin Wolf a écrit :
>>>> Am 25.07.2012 10:11, schrieb address@hidden:
>>>>> From: Benoît Canet <address@hidden>
>>>>>
>>>>> Use the dedicated counting function in qmp_query_block in order to
>>>>> propagate the backing file count to HMP.
>>>>>
>>>>> Signed-off-by: Benoit Canet <address@hidden>
>>>>> ---
>>>>>  block.c          |    2 ++
>>>>>  qapi-schema.json |    9 ++++++---
>>>>>  2 files changed, 8 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/block.c b/block.c
>>>>> index 03e0860..4aa3ea9 100644
>>>>> --- a/block.c
>>>>> +++ b/block.c
>>>>> @@ -2448,6 +2448,8 @@ BlockInfoList *qmp_query_block(Error **errp)
>>>>>              if (bs->backing_file[0]) {
>>>>>                  info->value->inserted->has_backing_file = true;
>>>>>                  info->value->inserted->backing_file = 
>>>>> g_strdup(bs->backing_file);
>>>>> +                info->value->inserted->backing_file_ancestors_count =
>>>>> +                    bdrv_get_backing_file_ancestors_count(bs);
>>>>>              }
>>>>>  
>>>>>              if (bs->io_limits_enabled) {
>>>>> diff --git a/qapi-schema.json b/qapi-schema.json
>>>>> index a92adb1..eb72c16 100644
>>>>> --- a/qapi-schema.json
>>>>> +++ b/qapi-schema.json
>>>>> @@ -398,6 +398,8 @@
>>>>>  #
>>>>>  # @backing_file: #optional the name of the backing file (for 
>>>>> copy-on-write)
>>>>>  #
>>>>> +# @backing_file_ancestors_count: #optional the count of ancestors 
>>>>> backing files (for copy-on-write)
>>>>> +#
>>>>
>>>> Why is it optional? Would it be omitted rather than set to 0 if there
>>>> are no backing files?
>>>
>>> I made it optional because backing_file=something is optional.
>>> So It seemed coherent to make it also optional.
>>> However I'll change it if you confirm it should be changed.
>>
>> The reason why the backing file naem is optional is because there is no
>> valid value to use when there is no backing file. But for the count, 0
>> makes perfect sense.
> 
> Should I also make is alway present in the HMP output ?

I think I would hide it in HMP if it is 0, but this isn't a very strong
opinion. You can do it as you like.

Kevin