| Message ID | 1343203902-10524-3-git-send-email-benoit@irqsave.net |
|---|---|
| State | New |
| Headers | show |
Am 25.07.2012 10:11, schrieb benoit.canet@gmail.com: > From: Benoît Canet <benoit@irqsave.net> > > Use the dedicated counting function in qmp_query_block in order to > propagate the backing file count to HMP. > > Signed-off-by: Benoit Canet <benoit@irqsave.net> > --- > 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? Could also use a "(since: 1.2)" note. Kevin
Le Wednesday 25 Jul 2012 à 12:57:05 (+0200), Kevin Wolf a écrit : > Am 25.07.2012 10:11, schrieb benoit.canet@gmail.com: > > From: Benoît Canet <benoit@irqsave.net> > > > > Use the dedicated counting function in qmp_query_block in order to > > propagate the backing file count to HMP. > > > > Signed-off-by: Benoit Canet <benoit@irqsave.net> > > --- > > 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. > > Could also use a "(since: 1.2)" note. > > Kevin
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 benoit.canet@gmail.com: >>> From: Benoît Canet <benoit@irqsave.net> >>> >>> Use the dedicated counting function in qmp_query_block in order to >>> propagate the backing file count to HMP. >>> >>> Signed-off-by: Benoit Canet <benoit@irqsave.net> >>> --- >>> 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. Kevin
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 benoit.canet@gmail.com: > >>> From: Benoît Canet <benoit@irqsave.net> > >>> > >>> Use the dedicated counting function in qmp_query_block in order to > >>> propagate the backing file count to HMP. > >>> > >>> Signed-off-by: Benoit Canet <benoit@irqsave.net> > >>> --- > >>> 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 ? Benoît > > Kevin >
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 benoit.canet@gmail.com: >>>>> From: Benoît Canet <benoit@irqsave.net> >>>>> >>>>> Use the dedicated counting function in qmp_query_block in order to >>>>> propagate the backing file count to HMP. >>>>> >>>>> Signed-off-by: Benoit Canet <benoit@irqsave.net> >>>>> --- >>>>> 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
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) +# # @encrypted: true if the backing device is encrypted # # @bps: total throughput limit in bytes per second is specified @@ -418,9 +420,10 @@ ## { 'type': 'BlockDeviceInfo', 'data': { 'file': 'str', 'ro': 'bool', 'drv': 'str', - '*backing_file': 'str', 'encrypted': 'bool', - 'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int', - 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int'} } + '*backing_file': 'str', 'backing_file_ancestors_count': 'int', + 'encrypted': 'bool', 'bps': 'int', 'bps_rd': 'int', + 'bps_wr': 'int', 'iops': 'int', 'iops_rd': 'int', + 'iops_wr': 'int'} } ## # @BlockDeviceIoStatus: