[libvirt] [PATCHv3] qemu: NUMA/network tuning shouldn't be supported in session mode

[John Ferlan]

On 10/01/2014 08:57 AM, Erik Skultety wrote:
> Tuning NUMA or network interface parameters require root
> privileges to manage cgroups, thus an attempt to set some of these
> parameters in session mode on a running domain should be invalid
> followed by an error.
> As an example might be memory tuning which raises an error in such case.
> Following behavior in session mode will be present after applying
> this patch:
> 
>   Tuning  |      SET      |   GET  |
> ----------|---------------|--------|
> NUMA      | shut off only | always |
> Memory    |     never     | never  |
> Interface |     never     | always |
> 
> Resolves https://bugzilla.redhat.com/show_bug.cgi?id=1126762
> ---
>  src/qemu/qemu_command.c | 13 ++++++++++++-
>  src/qemu/qemu_driver.c  | 35 +++++++++++++++++++++++++----------
>  2 files changed, 37 insertions(+), 11 deletions(-)
> 


I was going through some of my list backlog - it seems this was orphaned
:-)...  Since v3 addressed Mark's comment, I rebased it to top of
tree... adjusted the title to be just:

"qemu: Disallow NUMA/network tuning for session mode"

adjusted the grammar of the commit message a bit, and pushed

John


> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index eb72451..4c335dc 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -7671,7 +7671,7 @@ qemuBuildCommandLine(virConnectPtr conn,
>      emulator = def->emulator;
>  
>      if (!cfg->privileged) {
> -        /* If we have no cgroups than we can have no tunings that
> +        /* If we have no cgroups then we can have no tunings that
>           * require them */
>  
>          if (def->mem.hard_limit || def->mem.soft_limit ||
> @@ -7694,6 +7694,17 @@ qemuBuildCommandLine(virConnectPtr conn,
>                             _("CPU tuning is not available in session mode"));
>              goto error;
>          }
> +
> +        virDomainNetDefPtr *nets = def->nets;
> +        virNetDevBandwidthPtr bandwidth = NULL;
> +        size_t nnets = def->nnets;
> +        for (i = 0; i < nnets; i++) {
> +            if ((bandwidth = virDomainNetGetActualBandwidth(nets[i])) != NULL) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                    _("Network bandwidth tuning is not available in session mode"));
> +                goto error;
> +            }
> +        }
>      }
>  
>      for (i = 0; i < def->ngraphics; ++i) {
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 6606154..c64d272 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -8974,6 +8974,13 @@ qemuDomainSetNumaParameters(virDomainPtr dom,
>                                          &persistentDef) < 0)
>          goto cleanup;
>  
> +    if (!cfg->privileged &&
> +        flags & VIR_DOMAIN_AFFECT_LIVE) {
> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> +                       _("NUMA tuning is not available in session mode"));
> +        goto cleanup;
> +    }
> +
>      if (flags & VIR_DOMAIN_AFFECT_LIVE) {
>          if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) {
>              virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> @@ -9058,6 +9065,7 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
>      size_t i;
>      virDomainObjPtr vm = NULL;
>      virDomainDefPtr persistentDef = NULL;
> +    virQEMUDriverConfigPtr cfg = NULL;
>      char *nodeset = NULL;
>      int ret = -1;
>      virCapsPtr caps = NULL;
> @@ -9076,6 +9084,7 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
>          return -1;
>  
>      priv = vm->privateData;
> +    cfg = virQEMUDriverGetConfig(driver);
>  
>      if (virDomainGetNumaParametersEnsureACL(dom->conn, vm->def) < 0)
>          goto cleanup;
> @@ -9093,14 +9102,6 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
>          goto cleanup;
>      }
>  
> -    if (flags & VIR_DOMAIN_AFFECT_LIVE) {
> -        if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_MEMORY)) {
> -            virReportError(VIR_ERR_OPERATION_INVALID,
> -                           "%s", _("cgroup memory controller is not mounted"));
> -            goto cleanup;
> -        }
> -    }
> -
>      for (i = 0; i < QEMU_NB_NUMA_PARAM && i < *nparams; i++) {
>          virMemoryParameterPtr param = &params[i];
>  
> @@ -9123,9 +9124,16 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
>                  if (!nodeset)
>                      goto cleanup;
>              } else {
> -                if (virCgroupGetCpusetMems(priv->cgroup, &nodeset) < 0)
> -                    goto cleanup;
> +                if (!virCgroupHasController(priv->cgroup,
> +                                            VIR_CGROUP_CONTROLLER_MEMORY) ||
> +                    virCgroupGetCpusetMems(priv->cgroup, &nodeset) < 0) {
> +                    nodeset = virDomainNumatuneFormatNodeset(vm->def->numatune,
> +                                                             NULL, -1);
> +                    if (!nodeset)
> +                        goto cleanup;
> +                }
>              }
> +
>              if (virTypedParameterAssign(param, VIR_DOMAIN_NUMA_NODESET,
>                                          VIR_TYPED_PARAM_STRING, nodeset) < 0)
>                  goto cleanup;
> @@ -9150,6 +9158,7 @@ qemuDomainGetNumaParameters(virDomainPtr dom,
>      if (vm)
>          virObjectUnlock(vm);
>      virObjectUnref(caps);
> +    virObjectUnref(cfg);
>      return ret;
>  }
>  
> @@ -10120,6 +10129,12 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
>      if (virDomainSetInterfaceParametersEnsureACL(dom->conn, vm->def, flags) < 0)
>          goto cleanup;
>  
> +    if (!cfg->privileged) {
> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> +                       _("Network bandwidth tuning is not available in session mode"));
> +        goto cleanup;
> +    }
> +
>      if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
>          goto cleanup;
>  
>