On 03/23/2016 09:17 PM, Wen Congyang wrote: > The monitor command 'query-block' or 'info block' will output the format > specific > information. So we can get each child's child-name after this patch. This > useful > for dynamic reconfiguration. > > Signed-off-by: Wen Congyang <we...@cn.fujitsu.com> > --- > block/quorum.c | 27 +++++++++++++++++++++++++++ > qapi/block-core.json | 15 ++++++++++++++- > 2 files changed, 41 insertions(+), 1 deletion(-)
Can you add an example QMP session with the new information included, as
part of the commit message, to make it easier to see in context what you
are adding?
>
> diff --git a/block/quorum.c b/block/quorum.c
> index da15465..afe6c3f 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -1054,6 +1054,31 @@ static void quorum_refresh_filename(BlockDriverState
> *bs, QDict *options)
> bs->full_open_options = opts;
> }
>
> +static ImageInfoSpecific *quorum_get_specific_info(BlockDriverState *bs)
> +{
> + int i;
> + BDRVQuorumState *s = bs->opaque;
> + ImageInfoSpecific *spec_info = g_new0(ImageInfoSpecific, 1);
Others have pointed out that this can be g_new(), since...
> + strList **next;
> +
> + *spec_info = (ImageInfoSpecific){
> + .type = IMAGE_INFO_SPECIFIC_KIND_QUORUM,
...you are assigning all fields here.
> + .u = {
> + .quorum.data = g_new0(ImageInfoSpecificQuorum, 1),
> + },
I think you could just directly do:
.u.quorum.data = ...
instead of nesting {}.
> + };
> +
> + next = &spec_info->u.quorum.data->child_name;
> + for (i = 0; i < s->num_children; i++) {
> + *next = g_new0(strList, 1);
> + (*next)->value = g_strdup(s->children[i]->name);
> + (*next)->next = NULL;
Dead assignment, thanks to the g_new0() above.
> + next = &(*next)->next;
> + }
> +
> + return spec_info;
> +}
> +
> +++ b/qapi/block-core.json
> @@ -75,6 +75,18 @@
> } }
>
> ##
> +# @ImageInfoSpecificQuorum:
> +#
> +# @child-name: List of child name
As others have pointed out, I'd prefer:
@children: list of children's names
> +#
> +# Since: 2.7
Is this information needed in 2.6 (basically, a bug fix to finish an
incomplete feature addition), or are you really okay deferring it to 2.7?
> +##
> +{ 'struct': 'ImageInfoSpecificQuorum',
> + 'data': {
> + 'child-name': ['str']
> + } }
Other than the naming, it looks okay.
> @@ -85,7 +97,8 @@
> { 'union': 'ImageInfoSpecific',
> 'data': {
> 'qcow2': 'ImageInfoSpecificQCow2',
> - 'vmdk': 'ImageInfoSpecificVmdk'
> + 'vmdk': 'ImageInfoSpecificVmdk',
> + 'quorum': 'ImageInfoSpecificQuorum'
Worth keeping this list sorted? QAPI doesn't care, but as the list gets
longer, sorted is easier to maintain.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
