[libvirt] [PATCHv2] qemu: numatune/domiftune no support in session mode

[Martin Kletzander]

On Fri, Sep 05, 2014 at 02:20:18PM +0200, Erik Skultety wrote:

The $SUBJ doesn't make sense, try describing what this patch is doing
using few words or a sentence.

>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  |
>------------|---------------|--------|
>numatune    | shut off only | always |
>memtune     |     never     | never  |
>domiftune   |     never     | always |
>

You don't mention anything about the bandwidth in here, it would be
nice to add it to the commit message.

>Resolves https://bugzilla.redhat.com/show_bug.cgi?id=1126762
>---
> src/qemu/qemu_command.c | 14 +++++++++++++-
> src/qemu/qemu_driver.c  | 35 +++++++++++++++++++++++++----------
> 2 files changed, 38 insertions(+), 11 deletions(-)
>
>diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>index 1ca98fb..5d4838e 100644
>--- a/src/qemu/qemu_command.c
>+++ b/src/qemu/qemu_command.c
>@@ -7442,7 +7442,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 ||
>@@ -7465,6 +7465,18 @@ qemuBuildCommandLine(virConnectPtr conn,
>                            _("CPU tuning is not available in session mode"));
>             goto error;
>         }
>+
>+        virDomainNetDefPtr *nets = def->nets;
>+        size_t nnets = def->nnets;
>+        for (i = 0; i < nnets; i++) {
>+            if (nets[i]->bandwidth) {
>+                if (nets[i]->bandwidth->in || nets[i]->bandwidth->out) {
>+                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>+                                   _("Network bandwidth tuning is not available in session mode"));
>+                    goto error;
>+                }
>+            }
>+        }

If the interface is of type network, then this will fail checking
that.  You should use virDomainNetGetActualBandwidth() to check the
proper bandwidth for each interface.

Sorry for not noticing this earlier.

The rest looks fine,

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140926/f51a46c8/attachment-0001.sig>