[libvirt] [PATCH 2/2] test_driver: implement virDomainSetNumaParameters

[Erik Skultety]

On Thu, Jul 04, 2019 at 11:32:07PM +0200, Ilias Stamatis wrote:
> Signed-off-by: Ilias Stamatis <stamatis.iliass at gmail.com>
> ---
>  src/test/test_driver.c | 82 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 82 insertions(+)
>
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 74de8f13ad..fa690e2bd6 100755
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -2928,6 +2928,87 @@ testDomainGetMemoryParameters(virDomainPtr dom,
>  }
>
>
> +static int
> +testDomainSetNumaParameters(virDomainPtr dom,
> +                            virTypedParameterPtr params,
> +                            int nparams,
> +                            unsigned int flags)
> +{
> +    virDomainObjPtr vm = NULL;
> +    virDomainDefPtr def = NULL;
> +    virBitmapPtr nodeset = NULL;
> +    size_t i;
> +    virDomainNumatuneMemMode config_mode;
> +    int mode = -1;
> +    int ret = -1;
> +
> +    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> +                  VIR_DOMAIN_AFFECT_CONFIG, -1);
> +
> +    if (virTypedParamsValidate(params, nparams,
> +                               VIR_DOMAIN_NUMA_MODE,
> +                               VIR_TYPED_PARAM_INT,
> +                               VIR_DOMAIN_NUMA_NODESET,
> +                               VIR_TYPED_PARAM_STRING,
> +                               NULL) < 0)
> +        return -1;
> +
> +    if (!(vm = testDomObjFromDomain(dom)))
> +        return -1;
> +
> +    if (!(def = virDomainObjGetOneDef(vm, flags)))
> +        goto cleanup;
> +
> +    for (i = 0; i < nparams; i++) {
> +        virTypedParameterPtr param = &params[i];
> +
> +        if (STREQ(param->field, VIR_DOMAIN_NUMA_MODE)) {
> +            mode = param->value.i;
> +
> +            if (mode < 0 || mode >= VIR_DOMAIN_NUMATUNE_MEM_LAST) {
> +                virReportError(VIR_ERR_INVALID_ARG,
> +                               _("unsupported numatune mode: '%d'"), mode);
> +                goto cleanup;
> +            }
> +
> +        } else if (STREQ(param->field, VIR_DOMAIN_NUMA_NODESET)) {
> +            if (virBitmapParse(param->value.s, &nodeset,
> +                               VIR_DOMAIN_CPUMASK_LEN) < 0)
> +                goto cleanup;
> +
> +            if (virBitmapIsAllClear(nodeset)) {
> +                virReportError(VIR_ERR_OPERATION_INVALID,
> +                               _("Invalid nodeset of 'numatune': %s"),
> +                               param->value.s);
> +                goto cleanup;
> +            }
> +        }
> +    }
> +
> +    if (virDomainObjIsActive(vm) &&
> +        (flags & VIR_DOMAIN_AFFECT_LIVE) &&

^This doesn't work with VIR_DOMAIN_AFFECT_CURRENT. You're calling
virDomainObjGetOneDef, in this case you want virDomainObjGetOneDefState which
allows you to pass a boolean ptr which you can then harness here as (dropping
the above 2 lines):

if (live &&

> +        mode != -1 &&
> +        virDomainNumatuneGetMode(def->numa, -1, &config_mode) == 0 &&
> +        config_mode != mode) {
> +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                       _("can't change numatune mode for running domain"));
> +        goto cleanup;
> +    }
> +
> +    if (virDomainNumatuneSet(def->numa,
> +                             def->placement_mode ==
> +                             VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC,
> +                             -1, mode, nodeset) < 0)
> +        goto cleanup;
> +
> +    ret = 0;
> + cleanup:
> +    virBitmapFree(nodeset);
> +    virDomainObjEndAPI(&vm);
> +    return ret;
> +}
> +
> +
>  static int
>  testDomainGetNumaParameters(virDomainPtr dom,
>                              virTypedParameterPtr params,
> @@ -7645,6 +7726,7 @@ static virHypervisorDriver testHypervisorDriver = {
>      .domainGetMaxVcpus = testDomainGetMaxVcpus, /* 0.7.3 */
>      .domainGetXMLDesc = testDomainGetXMLDesc, /* 0.1.4 */
>      .domainGetMemoryParameters = testDomainGetMemoryParameters, /* 5.6.0 */
> +    .domainSetNumaParameters = testDomainSetNumaParameters, /* 5.6.0 */
>      .domainGetNumaParameters = testDomainGetNumaParameters, /* 5.6.0 */
>      .domainGetInterfaceParameters = testDomainGetInterfaceParameters, /* 5.6.0 */
>      .connectListDefinedDomains = testConnectListDefinedDomains, /* 0.1.11 */

Normally I'd have fixed the issues and pushed, but patch 2/2 cannot be applied
because it expects a different ancestor rather than 1/2 which means it was
probably created on a different branch not containing 1/2. Rather than forging
patches, could you fix and resend?

To both:
Reviewed-by: Erik Skultety <eskultet at redhat.com>