On 30.10.2014 16:14, Erik Skultety wrote:
Since there was a valid note to patch 43b67f2e about the best spot to check for bandwidth set call while having libvirt daemon run in session mode, this patch reverts previous changes dealing with bandwith (excluding NUMA!) in qemu_driver.c and qemu_command.c. There will be another patch in the series which introduces the fix itself. --- src/qemu/qemu_command.c | 11 ----------- src/qemu/qemu_driver.c | 6 ------ 2 files changed, 17 deletions(-)
Looking at the commit you are referring to, I spot other interesting things too. For instance, you are introducing @cfg variable to qemuDomainGetNumaParameters() but it's not used anywhere in the function (besides getting and unrefing driver's config) which is weird. So while you are at revert, you may revert that part too.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2e5af4f..be8e335 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7835,17 +7835,6 @@ 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 373daab..631cb5f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10437,12 +10437,6 @@ 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;
I've always felt we should use a different approach. If we want to check the permissions we should do that as close to the actual place needing root permissions as possible. With the complexity of call tree in libvirt it's easy to forget about one possible path. I mean, I like the approach 2/2 more then this old one.
Michal -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list