Re: [PATCH 2/8] qemu: Add optional unit to qemuMigrationParamsTPMapItem

[Ján Tomko <jtomko@xxxxxxxxxx>]

On Tue, Feb 05, 2019 at 04:23:05PM +0100, Jiri Denemark wrote:
> Some migration parameters supported by libvirt may use units that differ
> from the units used by QEMU for the corresponding parameters. For
> example, libvirt defines migration bandwidth in MiB/s while QEMU expects
> B/s. Let's add a unit field to qemuMigrationParamsTPMapItem for
> automatic conversion when translating between libvirt's migration typed
> parameters and QEMU's migration paramteres.
>
> This patch is a preparation for future parameters as the existing
> VIR_MIGRATE_PARAM_BANDWIDTH parameter is set using "migrate_set_speed"
> QMP command rather than "migrate-set-parameters" for backward
> compatibility.
>
> Signed-off-by: Jiri Denemark <jdenemar@xxxxxxxxxx>
> ---
> src/qemu/qemu_migration_params.c | 63 ++++++++++++++++++++++++++------
> 1 file changed, 51 insertions(+), 12 deletions(-)
>
> diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c
> index 658e785059..c36bed5744 100644
> --- a/src/qemu/qemu_migration_params.c
> +++ b/src/qemu/qemu_migration_params.c
> @@ -120,6 +120,7 @@ struct _qemuMigrationParamsFlagMapItem {
> typedef struct _qemuMigrationParamsTPMapItem qemuMigrationParamsTPMapItem;
> struct _qemuMigrationParamsTPMapItem {
>     const char *typedParam;
> +    unsigned int unit;
>     qemuMigrationParam param;
>     int party; /* bit-wise OR of qemuMigrationParty */
> };
> @@ -273,7 +274,8 @@ qemuMigrationParamsGetTPInt(qemuMigrationParamsPtr migParams,
>                             qemuMigrationParam param,
>                             virTypedParameterPtr params,
>                             int nparams,
> -                            const char *name)
> +                            const char *name,
> +                            unsigned int unit)
> {
>     int rc;
>
> @@ -287,6 +289,17 @@ qemuMigrationParamsGetTPInt(qemuMigrationParamsPtr migParams,
>                                    &migParams->params[param].value.i)) < 0)
>         return -1;
>
> +    if (unit > 0) {
> +        unsigned int max = UINT_MAX / unit;
> +        if (migParams->params[param].value.i > max) {
> +            virReportError(VIR_ERR_OVERFLOW,
> +                           _("%s migration parameter must be less than %u"),

Sounds better in my head as:
migration parameter '%s' must be...
(which should also be easier to translate with the parameter name
quoted)

> +                           name, max + 1);
> +            return -1;
> +        }
> +        migParams->params[param].value.i *= unit;
> +    }
> +
>     migParams->params[param].set = !!rc;
>     return 0;
> }
> @@ -298,16 +311,22 @@ qemuMigrationParamsSetTPInt(qemuMigrationParamsPtr migParams,
>                             virTypedParameterPtr *params,
>                             int *nparams,
>                             int *maxparams,
> -                            const char *name)
> +                            const char *name,
> +                            unsigned int unit)
> {
> +    int value;
> +
>     if (qemuMigrationParamsCheckType(param, QEMU_MIGRATION_PARAM_TYPE_INT) < 0)
>         return -1;
>
>     if (!migParams->params[param].set)
>         return 0;
>
> -    return virTypedParamsAddInt(params, nparams, maxparams, name,
> -                                migParams->params[param].value.i);
> +    value = migParams->params[param].value.i;
> +    if (unit > 0)
> +        value /= unit;
> +
> +    return virTypedParamsAddInt(params, nparams, maxparams, name, value);
> }
>
>
> @@ -316,7 +335,8 @@ qemuMigrationParamsGetTPULL(qemuMigrationParamsPtr migParams,
>                             qemuMigrationParam param,
>                             virTypedParameterPtr params,
>                             int nparams,
> -                            const char *name)
> +                            const char *name,
> +                            unsigned int unit)
> {
>     int rc;
>
> @@ -330,6 +350,17 @@ qemuMigrationParamsGetTPULL(qemuMigrationParamsPtr migParams,
>                                       &migParams->params[param].value.ull)) < 0)
>         return -1;
>
> +    if (unit > 0) {
> +        unsigned long long max = ULLONG_MAX / unit;
> +        if (migParams->params[param].value.ull > max) {
> +            virReportError(VIR_ERR_OVERFLOW,
> +                           _("%s migration parameter must be less than %llu"),
> +                           name, max + 1);

Same here.

> +            return -1;
> +        }
> +        migParams->params[param].value.ull *= unit;
> +    }
> +
>     migParams->params[param].set = !!rc;
>     return 0;
> }

Reviewed-by: Ján Tomko <jtomko@xxxxxxxxxx>

Jano

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list