Re: [Qemu-devel] [PATCH v2 5/8] dump-query: add "dump-query" command to query dump status

[Peter Xu <peterx@redhat.com>]

On 11/27/2015 01:25 PM, Fam Zheng wrote:
> On Fri, 11/27 10:48, Peter Xu wrote:
>> This patch is only adding the QMP/HMP interface for "dump-query"
>> command, but not implementing them. This command could be used to
>> query background dump status. Please refer to the next patch to see
>> how dump status are defined.
>>
>> Currently, only fake results are returned.
>>
>> Signed-off-by: Peter Xu <peterx@redhat.com>
>> ---
>>  dump.c           |  9 +++++++++
>>  hmp-commands.hx  | 15 +++++++++++++++
>>  hmp.c            |  6 ++++++
>>  hmp.h            |  1 +
>>  qapi-schema.json | 21 +++++++++++++++++++++
>>  qmp-commands.hx  | 29 ++++++++++++++++++++++++++++-
>>  6 files changed, 80 insertions(+), 1 deletion(-)
>>
>> diff --git a/dump.c b/dump.c
>> index 0d321d4..446a991 100644
>> --- a/dump.c
>> +++ b/dump.c
>> @@ -1777,6 +1777,15 @@ void qmp_dump_guest_memory(bool paging, const char *file,
>>      }
>>  }
>>  
>> +DumpStatus *qmp_dump_query(Error **errp)
>> +{
>> +    DumpStatus *status = g_malloc0(sizeof(*status));
>> +    /* TBD */
>> +    status->status = g_strdup("WORKING");
>> +    status->percentage = g_strdup("50%");
>> +    return status;
>> +}
>> +
>>  DumpGuestMemoryCapability *qmp_query_dump_guest_memory_capability(Error **errp)
>>  {
>>      DumpGuestMemoryFormatList *item;
>> diff --git a/hmp-commands.hx b/hmp-commands.hx
>> index 664d794..4ce7721 100644
>> --- a/hmp-commands.hx
>> +++ b/hmp-commands.hx
>> @@ -1087,6 +1087,21 @@ gdb. Without -z|-l|-s, the dump format is ELF.
>>              together with begin.
>>  ETEXI
>>  
>> +    {
>> +        .name       = "dump-query",
>> +        .args_type  = "",
>> +        .params     = "",
>> +        .help       = "query last guest memory dump status.\n\t\t\t",
> 
> What are "\n\t\t\t" doing here?

Sorry I should remove that.

> 
>> +        .mhandler.cmd = hmp_dump_query,
>> +    },
>> +
>> +
>> +STEXI
>> +@item dump-query
>> +@findex dump-query
>> +Query latest dump status.
>> +ETEXI
>> +
>>  #if defined(TARGET_S390X)
>>      {
>>          .name       = "dump-skeys",
>> diff --git a/hmp.c b/hmp.c
>> index dccb457..6d9c127 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -1576,6 +1576,12 @@ void hmp_device_del(Monitor *mon, const QDict *qdict)
>>      hmp_handle_error(mon, &err);
>>  }
>>  
>> +void hmp_dump_query(Monitor *mon, const QDict *qdict)
>> +{
>> +    /* TBD */
>> +    monitor_printf(mon, "QUERY DUMP STATUS\n");
>> +}
>> +
>>  void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
>>  {
>>      Error *err = NULL;
>> diff --git a/hmp.h b/hmp.h
>> index a8c5b5a..fdde4a3 100644
>> --- a/hmp.h
>> +++ b/hmp.h
>> @@ -85,6 +85,7 @@ void hmp_migrate(Monitor *mon, const QDict *qdict);
>>  void hmp_device_add(Monitor *mon, const QDict *qdict);
>>  void hmp_device_del(Monitor *mon, const QDict *qdict);
>>  void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
>> +void hmp_dump_query(Monitor *mon, const QDict *qdict);
>>  void hmp_netdev_add(Monitor *mon, const QDict *qdict);
>>  void hmp_netdev_del(Monitor *mon, const QDict *qdict);
>>  void hmp_getfd(Monitor *mon, const QDict *qdict);
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index fd81ce2..5db615d 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -2139,6 +2139,27 @@
>>              '*format': 'DumpGuestMemoryFormat'} }
>>  
>>  ##
>> +# @DumpStatus
>> +#
>> +# Status for the last guest memory dump.
>> +#
>> +# Since: 2.6
>> +##
>> +{ 'struct': 'DumpStatus',
>> +  'data': { 'status': 'str', 'percentage': 'str' } }
> 
> I suggest using enum for "status" and int for "percentage" (or two ints for
> "total" and "current").

Yes, I picked string just for flexibility. Regarding to this
interface issue, I'd like to wait for others' comments too before
modification.

> 
>> +
>> +##
>> +# @dump-query
>> +#
>> +# Query latest dump status.
>> +#
>> +# Returns: A @DumpStatus object showing the dump status.
>> +#
>> +# Since: 2.6
>> +##
>> +{ 'command': 'dump-query', 'returns': 'DumpStatus' }
>> +
>> +##
>>  # @DumpGuestMemoryCapability:
>>  #
>>  # A list of the available formats for dump-guest-memory
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index bbb08e1..6d13778 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -879,9 +879,36 @@ Notes:
>>  EQMP
>>  
>>      {
>> +        .name       = "dump-query",
>> +        .args_type  = "",
>> +        .params     = "",
>> +        .help       = "query background dump status",
>> +        .mhandler.cmd_new = qmp_marshal_dump_query,
>> +    },
>> +
>> +SQMP
>> +dump-query
>> +----------
>> +
>> +Query background dump status.
>> +
>> +Arguments: None.
>> +
>> +Example:
>> +
>> +-> { "execute": "dump-query" }
>> +<- { "return": {"status": "IN_PROGRESS", "percentage": "85%" } }
>> +
>> +Notes:
>> +
>> +(1) All boolean arguments default to false
> 
> Which "boolean arguments"?

Should remove. Sorry.

Peter

> 
> 
>> +
>> +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,
>>      },
>>  
>>  SQMP
>> -- 
>> 2.4.3
>>
>>