On Mon, Nov 30, 2015 at 03:17:01PM -0700, Eric Blake wrote:
> On 11/30/2015 04:32 AM, Peter Xu wrote:
> > +DumpQueryResult *qmp_query_dump(Error **errp)
> > +{
> > + DumpQueryResult *result = g_malloc0(sizeof(*result));
>
> Might be nicer as g_new0(DumpQueryResult, 1). Markus has been switching
> to g_new0 where a type name was already mentioned, although here you
> used *result rather than a typename.
Sure. Thanks.
>
>
> > +++ b/qapi-schema.json
> > @@ -2157,6 +2157,35 @@
> > 'data': [ 'none', 'active', 'completed', 'failed' ] }
> >
> > ##
> > +# @DumpQueryResult
> > +#
> > +# The result format for 'query-dump'.
> > +#
> > +# @status: enum of @DumpStatus, which shows current dump status
> > +#
> > +# @written_bytes: bytes written in latest dump (uncompressed)
> > +#
> > +# @total_bytes: total bytes to be write in latest dump (uncompressed)
>
> s/be write/written/
>
> > +#
> > +# Since 2.6
> > +##
> > +{ 'struct': 'DumpQueryResult',
> > + 'data': { 'status': 'DumpStatus',
> > + 'written_bytes': 'int',
> > + 'total_bytes': 'int' } }
>
> Prefer '-' over '_' in new QMP (as in 'total-bytes' rather than
> 'total_bytes'). Furthermore, QMP already defaults to bytes, so it would
> be sufficient to name these merely 'written'/'total' or even
> 'complete'/'total'.
I will directly take "completed" and "total" then.
>
> > +++ b/qmp-commands.hx
> > @@ -881,7 +881,7 @@ EQMP
> > {
> > .name = "query-dump-guest-memory-capability",
> > .args_type = "",
> > - .mhandler.cmd_new = qmp_marshal_query_dump_guest_memory_capability,
> > + .mhandler.cmd_new = qmp_marshal_query_dump_guest_memory_capability,
> > },
>
> Unrelated hunk.
Is it ok to have this line here? I just saw it and fixed it. Please
let me know if I should move it out of this patch.
Thanks.
Peter
>
> --
> Eric Blake eblake redhat com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
