[libvirt] [PATCH v3 3/7] qemu monitor: add multithread compress parameters accessors

[Jiri Denemark]

On Thu, Jan 28, 2016 at 10:04:29 +0300, Nikolay Shirokovskiy wrote:
> From: ShaoHe Feng <shaohe.feng at intel.com>
> 
> Current compression does not use all range of parameter values
> so let's use some of them as 'unspecified' values. These
> values will be used to mark parameters that were not specified
> on migrate command line. Thus we check that qemu does not
> use these values when we read parameters.
> 
> Signed-off-by: ShaoHe Feng <shaohe.feng at intel.com>
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
> ---
>  src/qemu/qemu_monitor.c      |  29 +++++++++++++
>  src/qemu/qemu_monitor.h      |  16 +++++++
>  src/qemu/qemu_monitor_json.c |  87 +++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_monitor_json.h |   5 +++
>  src/qemu/qemu_monitor_text.c | 100 +++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_monitor_text.h |   5 +++

Please, remove everything related to HMP monitor, multithreaded
compression is not supported by any QEMU for which we would use text
monitor.

>  6 files changed, 242 insertions(+)
> 
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index cf1cdfb..e9b1ce4 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -2116,6 +2116,35 @@ qemuMonitorSetMigrationCacheSize(qemuMonitorPtr mon,
>  
>  
>  int
> +qemuMonitorGetMigrationCompressParametersMT(qemuMonitorPtr mon,
> +                                            qemuMonitorMigrationMTParametersPtr params)

I think a more general qemuMonitor[GS]etMigrationParameters [gs]etting
all parameters at once would be a bit better. After all, it all boils
down to query-migrate-parameters and migrate-set-parameters and having
separate functions for each group of parameters would mean we'd have to
call the QMP commands several times.

> +{
> +    QEMU_CHECK_MONITOR(mon);
> +
> +    if (mon->json)
> +        return qemuMonitorJSONGetMigrationCompressParametersMT(mon, params);
> +    else
> +        return qemuMonitorTextGetMigrationCompressParametersMT(mon, params);

Since support for text monitor is not desirable, this function will be
as simple as

    QEMU_CHECK_MONITOR_JSON(mon);

    return qemuMonitorJSON...;

> +}
> +
> +int
> +qemuMonitorSetMigrationCompressParametersMT(qemuMonitorPtr mon,
> +                                            qemuMonitorMigrationMTParametersPtr params)
> +{
> +    VIR_DEBUG("level=%d threads=%d dthreads=%d", params->level,
> +                                                 params->threads,
> +                                                 params->dthreads);

Either

    VIR_DEBUG("level=%d threads=%d dthreads=%d",
              params->level,
              params->threads,
              params->dthreads);

or

    VIR_DEBUG("level=%d threads=%d dthreads=%d",
              params->level, params->threads, params->dthreads);

> +
> +    QEMU_CHECK_MONITOR(mon);
> +
> +    if (mon->json)
> +        return qemuMonitorJSONSetMigrationCompressParametersMT(mon, params);
> +    else
> +        return qemuMonitorTextSetMigrationCompressParametersMT(mon, params);
> +}
> +
> +
> +int
>  qemuMonitorGetMigrationStats(qemuMonitorPtr mon,
>                               qemuMonitorMigrationStatsPtr stats)
>  {
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index c2a0ed6..5a5e0e2 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -463,6 +463,22 @@ int qemuMonitorGetMigrationCacheSize(qemuMonitorPtr mon,
>  int qemuMonitorSetMigrationCacheSize(qemuMonitorPtr mon,
>                                       unsigned long long cacheSize);
>  
> +typedef struct _qemuMonitorMigrationMTParameters qemuMonitorMigrationMTParameters;
> +typedef qemuMonitorMigrationMTParameters *qemuMonitorMigrationMTParametersPtr;
> +struct _qemuMonitorMigrationMTParameters {
> +    /* -1 is value of unspecified */
> +    int level;
> +    /* 0 is value of unspecified */
> +    unsigned int threads;
> +    /* 0 is value of unspecified */
> +    unsigned int dthreads;
> +};
> +
> +int qemuMonitorGetMigrationCompressParametersMT(qemuMonitorPtr mon,
> +                                                qemuMonitorMigrationMTParametersPtr params);
> +int qemuMonitorSetMigrationCompressParametersMT(qemuMonitorPtr mon,
> +                                                qemuMonitorMigrationMTParametersPtr params);
> +
>  typedef enum {
>      QEMU_MONITOR_MIGRATION_STATUS_INACTIVE,
>      QEMU_MONITOR_MIGRATION_STATUS_SETUP,
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 24a8865..62aba88 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -2437,6 +2437,93 @@ qemuMonitorJSONSetMigrationCacheSize(qemuMonitorPtr mon,
>  }
>  
>  
> +int qemuMonitorJSONGetMigrationCompressParametersMT(qemuMonitorPtr mon,
> +                                                    qemuMonitorMigrationMTParametersPtr params)
> +{
> +    int ret = -1;
> +    virJSONValuePtr result;
> +    virJSONValuePtr cmd;
> +    virJSONValuePtr reply = NULL;
> +
> +    if ((cmd = qemuMonitorJSONMakeCommand("query-migrate-parameters", NULL)) == NULL)
> +        return -1;
> +
> +    if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0)
> +        goto cleanup;
> +
> +    if ((ret = qemuMonitorJSONCheckError(cmd, reply)) < 0)
> +        goto cleanup;
> +
> +    if (!(result = virJSONValueObjectGet(reply, "return"))) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("query-migrate-parameters reply was missing "
> +                         "'return' data"));
> +        goto cleanup;
> +    }
> +
> +    if (virJSONValueObjectGetNumberInt(result, "compress-level",
> +                                        &params->level) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("malformed/missing compress-level "
> +                         "in migrate parameters"));
> +        goto cleanup;
> +    }
> +
> +    if (virJSONValueObjectGetNumberUint(result, "compress-threads",
> +                                        &params->threads) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("malformed/missing compress-threads "
> +                         "in migrate parameters"));
> +        goto cleanup;
> +    }
> +
> +    if (virJSONValueObjectGetNumberUint(result, "decompress-threads",
> +                                        &params->dthreads) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("malformed/missing decompress-threads "
> +                         "in migrate parameters"));
> +        goto cleanup;
> +    }
> +
> +    if (params->level < 0 || params->threads < 1 || params->dthreads < 1) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("unexpected compress parameters"));
> +        goto cleanup;
> +    }
> +
> +    ret = 0;
> + cleanup:
> +    virJSONValueFree(cmd);
> +    virJSONValueFree(reply);
> +    return ret;
> +}
> +
> +int qemuMonitorJSONSetMigrationCompressParametersMT(qemuMonitorPtr mon,
> +                                                    qemuMonitorMigrationMTParametersPtr params)
> +{
> +    int ret = -1;
> +    virJSONValuePtr cmd;
> +    virJSONValuePtr reply = NULL;
> +
> +    cmd = qemuMonitorJSONMakeCommand("migrate-set-parameters",
> +                                     "i:compress-level", params->level,
> +                                     "u:compress-threads", params->threads,
> +                                     "u:decompress-threads", params->dthreads,
> +                                     NULL);

Is passing an "undefined" value for any of these parameters allowed
(i.e., will QEMU use a default value) or do we always have to set all of
them?

> +    if (!cmd)
> +        return -1;
> +
> +    if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0)
> +        goto cleanup;
> +
> +    ret = qemuMonitorJSONCheckError(cmd, reply);
> +
> + cleanup:
> +    virJSONValueFree(cmd);
> +    virJSONValueFree(reply);
> +    return ret;
> +}
> +
...

Jirka