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

[Erik Skultety <eskultet@xxxxxxxxxx>]

Splendid, thank you John :).
Erik

On 10/22/2014 09:00 PM, John Ferlan wrote:
> 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;
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list

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