On 05.11.2014 18:41, 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 (also reverts adding variable @cfg in qemuDomainGetNumaParameters which does not have any use at the moment, but getting and unreferencing driver's config) 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 | 9 --------- 2 files changed, 20 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 917639e..956bb14 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7841,17 +7841,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 6acaea8..edd82c1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9383,7 +9383,6 @@ qemuDomainGetNumaParameters(virDomainPtr dom, size_t i; virDomainObjPtr vm = NULL; virDomainDefPtr persistentDef = NULL; - virQEMUDriverConfigPtr cfg = NULL; char *nodeset = NULL; int ret = -1; virCapsPtr caps = NULL; @@ -9402,7 +9401,6 @@ qemuDomainGetNumaParameters(virDomainPtr dom, return -1;

priv = vm->privateData; - cfg = virQEMUDriverGetConfig(driver);

if (virDomainGetNumaParametersEnsureACL(dom->conn, vm->def) < 0) goto cleanup; @@ -9476,7 +9474,6 @@ qemuDomainGetNumaParameters(virDomainPtr dom, if (vm) virObjectUnlock(vm); virObjectUnref(caps); - virObjectUnref(cfg); return ret; }

@@ -10447,12 +10444,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;

ACK, however, I'm not pushing this one until 2/2 is fixed. Michal

On 05.11.2014 18:41, Erik Skultety wrote:

Patch 43b67f2e disallowed network tuning only with qemu driver, however this patch moved the check for root privileges into virNetDevBandwidthSet function, so the call should now fail in all possible cases. A mock function was created so that the test suite doesn't fail because of unsufficient privileges. --- src/util/virnetdevbandwidth.c | 8 ++++++++ tests/Makefile.am | 11 ++++++++++- tests/virnetdevbandwidthtest.c | 14 +++++++++++++- 3 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c index 5fa231a..8360fd4 100644 --- a/src/util/virnetdevbandwidth.c +++ b/src/util/virnetdevbandwidth.c @@ -27,6 +27,7 @@ #include "viralloc.h" #include "virerror.h" #include "virstring.h" +#include "unistd.h"

#define VIR_FROM_THIS VIR_FROM_NONE

@@ -74,6 +75,13 @@ virNetDevBandwidthSet(const char *ifname, goto cleanup; }

+ if (geteuid() != 0) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Network bandwidth tuning is not available" + " in session mode")); + return -1; + } + virNetDevBandwidthClear(ifname);

if (bandwidth->in && bandwidth->in->average) { diff --git a/tests/Makefile.am b/tests/Makefile.am index 7b22f90..7fa4575 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -392,6 +392,7 @@ test_libraries = libshunload.la \ virnetserverclientmock.la \ vircgroupmock.la \ virpcimock.la \ + virnetdevbandwidthmock.la \ $(NULL) if WITH_QEMU test_libraries += libqemumonitortestutils.la \ @@ -409,7 +410,9 @@ test_libraries += \ endif WITH_DBUS

if WITH_LINUX -test_libraries += virusbmock.la +test_libraries += \ + virusbmock.la + $(NULL) endif WITH_LINUX

This chunk seem unrelated.

if WITH_TESTS @@ -829,6 +832,12 @@ virnetdevbandwidthtest_SOURCES = \ virnetdevbandwidthtest.c testutils.h testutils.c virnetdevbandwidthtest_LDADD = $(LDADDS) $(LIBXML_LIBS)

+virnetdevbandwidthmock_la_SOURCES = \ + virnetdevbandwidthmock.c +virnetdevbandwidthmock_la_CFLAGS = $(AM_CFLAGS) +virnetdevbandwidthmock_la_LDFLAGS = -module -avoid-version \ + -rpath /evil/libtool/hack/to/force/shared/lib/creation +

Did you forget to 'git add virnetdevbandwidthmock.c'? It's not in the patch anywhere.

virkmodtest_SOURCES = \ virkmodtest.c testutils.h testutils.c virkmodtest_LDADD = $(LDADDS) diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c index 384991e..df69bac 100644 --- a/tests/virnetdevbandwidthtest.c +++ b/tests/virnetdevbandwidthtest.c @@ -21,6 +21,9 @@ #include <config.h>

#include "testutils.h" + +#ifdef WITH_LINUX + #define __VIR_COMMAND_PRIV_H_ALLOW__ #include "vircommandpriv.h" #include "virnetdevbandwidth.h" @@ -167,4 +170,13 @@ mymain(void) return ret; }

-VIRT_TEST_MAIN(mymain); +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virnetdevbandwidthmock.so") + +#else + +int main(void) +{ + return EXIT_AM_SKIP; +} + +#endif /* WITH_LINUX */

Okay, we can make this test run on Linux only. Well, I don't think they have tc in *BSD anyway, do they? Michal