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

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(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list