Re: [PULL 15/18] qapi: introduce x-query-ramblock QMP command

[Claudio Fontana]

Hello all,

it would be really good to be able to rely on this command or something similar,
to be able to know the approximate size of a migration before starting it.

in QEMU ram_bytes_total() returns what I would like to have,
but there is currently no QMP way to get it without starting a migration,
which when trying to optimize it/size it is just about too late.

Do you think x-query-ramblock could be promoted to non-experimental?

Should another one be made available instead, like :
query-ram-bytes-total ?

Thanks,

Claudio


On 11/2/21 18:56, Daniel P. Berrangé wrote:
> This is a counterpart to the HMP "info ramblock" command. It is being
> added with an "x-" prefix because this QMP command is intended as an
> adhoc debugging tool and will thus not be modelled in QAPI as fully
> structured data, nor will it have long term guaranteed stability.
> The existing HMP command is rewritten to call the QMP command.
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  hmp-commands-info.hx   |  2 +-
>  include/exec/ramlist.h |  2 +-
>  monitor/hmp-cmds.c     |  6 ------
>  monitor/qmp-cmds.c     |  8 ++++++++
>  qapi/machine.json      | 12 ++++++++++++
>  softmmu/physmem.c      | 19 +++++++++++--------
>  6 files changed, 33 insertions(+), 16 deletions(-)
> 
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index d9af216473..c2d7275bf5 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -772,7 +772,7 @@ ERST
>          .args_type  = "",
>          .params     = "",
>          .help       = "Display system ramblock information",
> -        .cmd        = hmp_info_ramblock,
> +        .cmd_info_hrt = qmp_x_query_ramblock,
>      },
>  
>  SRST
> diff --git a/include/exec/ramlist.h b/include/exec/ramlist.h
> index ece6497ee2..2ad2a81acc 100644
> --- a/include/exec/ramlist.h
> +++ b/include/exec/ramlist.h
> @@ -80,6 +80,6 @@ void ram_block_notify_add(void *host, size_t size, size_t 
> max_size);
>  void ram_block_notify_remove(void *host, size_t size, size_t max_size);
>  void ram_block_notify_resize(void *host, size_t old_size, size_t new_size);
>  
> -void ram_block_dump(Monitor *mon);
> +GString *ram_block_format(void);
>  
>  #endif /* RAMLIST_H */
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 9d221622d7..90f9a64573 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -52,7 +52,6 @@
>  #include "ui/console.h"
>  #include "qemu/cutils.h"
>  #include "qemu/error-report.h"
> -#include "exec/ramlist.h"
>  #include "hw/intc/intc.h"
>  #include "migration/snapshot.h"
>  #include "migration/misc.h"
> @@ -2176,11 +2175,6 @@ void hmp_rocker_of_dpa_groups(Monitor *mon, const 
> QDict *qdict)
>      qapi_free_RockerOfDpaGroupList(list);
>  }
>  
> -void hmp_info_ramblock(Monitor *mon, const QDict *qdict)
> -{
> -    ram_block_dump(mon);
> -}
> -
>  void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict)
>  {
>      Error *err = NULL;
> diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
> index 0a9ba7595c..a9766fa38d 100644
> --- a/monitor/qmp-cmds.c
> +++ b/monitor/qmp-cmds.c
> @@ -38,6 +38,7 @@
>  #include "qapi/qapi-commands-ui.h"
>  #include "qapi/type-helpers.h"
>  #include "qapi/qmp/qerror.h"
> +#include "exec/ramlist.h"
>  #include "hw/mem/memory-device.h"
>  #include "hw/acpi/acpi_dev_interface.h"
>  #include "hw/rdma/rdma.h"
> @@ -414,3 +415,10 @@ HumanReadableText *qmp_x_query_rdma(Error **errp)
>  
>      return human_readable_text_from_str(buf);
>  }
> +
> +HumanReadableText *qmp_x_query_ramblock(Error **errp)
> +{
> +    g_autoptr(GString) buf = ram_block_format();
> +
> +    return human_readable_text_from_str(buf);
> +}
> diff --git a/qapi/machine.json b/qapi/machine.json
> index 1b2748c77a..be81170c2b 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -1436,6 +1436,18 @@
>  { 'command': 'x-query-profile',
>    'returns': 'HumanReadableText' }
>  
> +##
> +# @x-query-ramblock:
> +#
> +# Query system ramblock information
> +#
> +# Returns: system ramblock information
> +#
> +# Since: 6.2
> +##
> +{ 'command': 'x-query-ramblock',
> +  'returns': 'HumanReadableText' }
> +
>  ##
>  # @x-query-rdma:
>  #
> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> index b9a8c1d1f4..314f8b439c 100644
> --- a/softmmu/physmem.c
> +++ b/softmmu/physmem.c
> @@ -1296,23 +1296,26 @@ void qemu_mutex_unlock_ramlist(void)
>      qemu_mutex_unlock(&ram_list.mutex);
>  }
>  
> -void ram_block_dump(Monitor *mon)
> +GString *ram_block_format(void)
>  {
>      RAMBlock *block;
>      char *psize;
> +    GString *buf = g_string_new("");
>  
>      RCU_READ_LOCK_GUARD();
> -    monitor_printf(mon, "%24s %8s  %18s %18s %18s\n",
> -                   "Block Name", "PSize", "Offset", "Used", "Total");
> +    g_string_append_printf(buf, "%24s %8s  %18s %18s %18s\n",
> +                           "Block Name", "PSize", "Offset", "Used", "Total");
>      RAMBLOCK_FOREACH(block) {
>          psize = size_to_str(block->page_size);
> -        monitor_printf(mon, "%24s %8s  0x%016" PRIx64 " 0x%016" PRIx64
> -                       " 0x%016" PRIx64 "\n", block->idstr, psize,
> -                       (uint64_t)block->offset,
> -                       (uint64_t)block->used_length,
> -                       (uint64_t)block->max_length);
> +        g_string_append_printf(buf, "%24s %8s  0x%016" PRIx64 " 0x%016" 
> PRIx64
> +                               " 0x%016" PRIx64 "\n", block->idstr, psize,
> +                               (uint64_t)block->offset,
> +                               (uint64_t)block->used_length,
> +                               (uint64_t)block->max_length);
>          g_free(psize);
>      }
> +
> +    return buf;
>  }
>  
>  #ifdef __linux__