Re: [Qemu-devel] [PATCH 08/11] Add migrate_set_cachesize command

[Luiz Capitulino <lcapitulino@redhat.com>]

On Thu,  2 Aug 2012 15:44:51 +0300
Orit Wasserman <owasserm@redhat.com> wrote:

> Change XBZRLE cache size in bytes (the size should be a power of 2, it will be
> rounded down to the nearest power of 2).
> If XBZRLE cache size is too small there will be many cache miss.
> 
> New query-migrate-cache-size QMP command and 'info migrate-cache-size' HMP
> command to query cache value.
> 
> Signed-off-by: Benoit Hudzia <benoit.hudzia@sap.com>
> Signed-off-by: Petter Svard <petters@cs.umu.se>
> Signed-off-by: Aidan Shribman <aidan.shribman@sap.com>
> Signed-off-by: Orit Wasserman <owasserm@redhat.com>

Apart from what Eric spotted, this looks good to me.

> ---
>  arch_init.c      |   10 ++++++++++
>  hmp-commands.hx  |   22 ++++++++++++++++++++++
>  hmp.c            |   19 +++++++++++++++++++
>  hmp.h            |    2 ++
>  migration.c      |   19 +++++++++++++++++++
>  migration.h      |    2 ++
>  monitor.c        |    7 +++++++
>  qapi-schema.json |   27 +++++++++++++++++++++++++++
>  qmp-commands.hx  |   44 ++++++++++++++++++++++++++++++++++++++++++++
>  9 files changed, 152 insertions(+), 0 deletions(-)
> 
> diff --git a/arch_init.c b/arch_init.c
> index 410ba4d..d709ccb 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -189,6 +189,16 @@ static struct {
>      .cache = NULL,
>  };
>  
> +
> +int64_t xbzrle_cache_resize(int64_t new_size)
> +{
> +    if (XBZRLE.cache != NULL) {
> +        return cache_resize(XBZRLE.cache, new_size / TARGET_PAGE_SIZE) *
> +            TARGET_PAGE_SIZE;
> +    }
> +    return pow2floor(new_size);
> +}
> +
>  static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset,
>          int cont, int flag)
>  {
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index f4c8495..ae98c12 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -829,6 +829,26 @@ STEXI
>  @item migrate_cancel
>  @findex migrate_cancel
>  Cancel the current VM migration.
> +
> +ETEXI
> +
> +    {
> +        .name       = "migrate_set_cachesize",
> +        .args_type  = "value:o",
> +        .params     = "value",
> +        .help       = "set cache size (in bytes) for XBZRLE migrations,"
> +                      "the cache size will be rounded down to the nearest "
> +                      "power of 2.\n"
> +                      "The cache size affects the number of cache misses."
> +                      "In case of a high cache miss ratio you need to increase"
> +                      " the cache size",
> +        .mhandler.cmd = hmp_migrate_set_cachesize,
> +    },
> +
> +STEXI
> +@item migrate_set_cachesize @var{value}
> +@findex migrate_set_cachesize
> +Set cache size to @var{value} (in bytes) for xbzrle migrations.
>  ETEXI
>  
>      {
> @@ -1435,6 +1455,8 @@ show migration status
>  show migration supported capabilities
>  @item info migrate_capabilities
>  show current migration capabilities
> +@item info migrate-cache-size
> +show current migration XBZRLE cache size
>  @item info balloon
>  show balloon information
>  @item info qtree
> diff --git a/hmp.c b/hmp.c
> index 463b730..3a61da5 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -215,6 +215,12 @@ void hmp_info_migrate_capabilities(Monitor *mon)
>      qapi_free_MigrationCapabilityStatusList(caps);
>  }
>  
> +void hmp_info_migrate_cache_size(Monitor *mon)
> +{
> +    monitor_printf(mon, "xbzrel cache size: %" PRId64 " kbytes\n",
> +                   qmp_query_migrate_cache_size(NULL) >> 10);
> +}
> +
>  void hmp_info_cpus(Monitor *mon)
>  {
>      CpuInfoList *cpu_list, *cpu;
> @@ -783,6 +789,19 @@ void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict)
>      qmp_migrate_set_downtime(value, NULL);
>  }
>  
> +void hmp_migrate_set_cachesize(Monitor *mon, const QDict *qdict)
> +{
> +    int64_t value = qdict_get_int(qdict, "value");
> +    Error *err = NULL;
> +
> +    qmp_migrate_set_cache_size(value, &err);
> +    if (err) {
> +        monitor_printf(mon, "%s\n", error_get_pretty(err));
> +        error_free(err);
> +        return;
> +    }
> +}
> +
>  void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict)
>  {
>      int64_t value = qdict_get_int(qdict, "value");
> diff --git a/hmp.h b/hmp.h
> index f2a890f..0951a0e 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -27,6 +27,7 @@ void hmp_info_mice(Monitor *mon);
>  void hmp_info_migrate(Monitor *mon);
>  void hmp_info_migration_supported_capabilities(Monitor *mon);
>  void hmp_info_migrate_capabilities(Monitor *mon);
> +void hmp_info_migrate_cache_size(Monitor *mon);
>  void hmp_info_cpus(Monitor *mon);
>  void hmp_info_block(Monitor *mon);
>  void hmp_info_blockstats(Monitor *mon);
> @@ -54,6 +55,7 @@ void hmp_migrate_cancel(Monitor *mon, const QDict *qdict);
>  void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict);
>  void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict);
>  void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict);
> +void hmp_migrate_set_cachesize(Monitor *mon, const QDict *qdict);
>  void hmp_set_password(Monitor *mon, const QDict *qdict);
>  void hmp_expire_password(Monitor *mon, const QDict *qdict);
>  void hmp_eject(Monitor *mon, const QDict *qdict);
> diff --git a/migration.c b/migration.c
> index 497efc6..d007e28 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -523,6 +523,25 @@ void qmp_migrate_cancel(Error **errp)
>      migrate_fd_cancel(migrate_get_current());
>  }
>  
> +void qmp_migrate_set_cache_size(int64_t value, Error **errp)
> +{
> +    MigrationState *s = migrate_get_current();
> +
> +    /* Check for truncation */
> +    if (value != (size_t)value) {
> +        error_set(errp, QERR_INVALID_PARAMETER_VALUE, "cache size",
> +                  "exceeding address space");
> +        return;
> +    }
> +
> +    s->xbzrle_cache_size = xbzrle_cache_resize(value);
> +}
> +
> +int64_t qmp_query_migrate_cache_size(Error **errp)
> +{
> +    return migrate_xbzrle_cache_size();
> +}
> +
>  void qmp_migrate_set_speed(int64_t value, Error **errp)
>  {
>      MigrationState *s;
> diff --git a/migration.h b/migration.h
> index cdf6787..337e225 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -108,4 +108,6 @@ int xbzrle_decode_buffer(uint8_t *src, int slen, uint8_t *dst, int dlen);
>  int migrate_use_xbzrle(void);
>  int64_t migrate_xbzrle_cache_size(void);
>  
> +int64_t xbzrle_cache_resize(int64_t new_size);
> +
>  #endif
> diff --git a/monitor.c b/monitor.c
> index 58d6e2a..6f10304 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2676,6 +2676,13 @@ static mon_cmd_t info_cmds[] = {
>          .mhandler.info = hmp_info_migrate_capabilities,
>      },
>      {
> +        .name       = "migrate-cache-size",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show current migration xbzrle cache size",
> +        .mhandler.info = hmp_info_migrate_cache_size,
> +    },
> +    {
>          .name       = "balloon",
>          .args_type  = "",
>          .params     = "",
> diff --git a/qapi-schema.json b/qapi-schema.json
> index f38dfc6..819e45f 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1412,6 +1412,33 @@
>  { 'command': 'migrate_set_speed', 'data': {'value': 'int'} }
>  
>  ##
> +# @migrate-set-cache-size
> +#
> +# Set XBZRLE cache size
> +#
> +# @value: cache size in bytes
> +#
> +# The size will be rounded down to the nearest power of 2.
> +# The cache size can be modified before and during ongoing migration
> +#
> +# Returns: nothing on success
> +#
> +# Since: 1.2
> +##
> +{ 'command': 'migrate-set-cache-size', 'data': {'value': 'int'} }
> +
> +##
> +# @query-migrate-cache-size
> +#
> +# query XBZRLE cache size
> +#
> +# Returns: XBZRLE cache size in bytes
> +#
> +# Since: 1.2
> +##
> +{ 'command': 'query-migrate-cache-size', 'returns': 'int' }
> +
> +##
>  # @ObjectPropertyInfo:
>  #
>  # @name: the name of the property
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index e96a993..25a26fa 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -520,6 +520,50 @@ Example:
>  <- { "return": {} }
>  
>  EQMP
> +{
> +        .name       = "migrate-set-cache-size",
> +        .args_type  = "value:o",
> +        .mhandler.cmd_new = qmp_marshal_input_migrate_set_cache_size,
> +    },
> +
> +SQMP
> +migrate-set-cache-size
> +---------------------
> +
> +Set cache size to be used by XBZRLE migration, the cache size will be rounded
> +down to the nearest power of 2
> +
> +Arguments:
> +
> +- "value": cache size in bytes (json-int)
> +
> +Example:
> +
> +-> { "execute": "migrate-set-cache-size", "arguments": { "value": 536870912 } }
> +<- { "return": {} }
> +
> +EQMP
> +    {
> +        .name       = "query-migrate-cache-size",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_marshal_input_query_migrate_cache_size,
> +    },
> +
> +SQMP
> +query-migrate-cache-size
> +---------------------
> +
> +Show cache size to be used by XBZRLE migration
> +
> +returns a json-object with the following information:
> +- "size" : json-int
> +
> +Example:
> +
> +-> { "execute": "query-migrate-cache-size" }
> +<- { "return": { "size" : "123456" } }
> +
> +EQMP
>  
>      {
>          .name       = "migrate_set_speed",