Bug 1038363 - qemu guests cannot raise cpu/memory above live maxvcpus/maxmem while running
qemu guests cannot raise cpu/memory above live maxvcpus/maxmem while running
Status: CLOSED CURRENTRELEASE
Product: Virtualization Tools
Classification: Community
Component: libvirt (Show other bugs)
unspecified
x86_64 Linux
unspecified Severity medium
: ---
: ---
Assigned To: Libvirt Maintainers
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2013-12-04 20:22 EST by Geoff Franks
Modified: 2015-08-12 10:28 EDT (History)
4 users (show)

See Also:
Fixed In Version: libvirt-1.2.2
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2015-03-16 09:14:51 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Geoff Franks 2013-12-04 20:22:52 EST
Description of problem:
When attempting to raise the configured memory of a running guest, you cannot raise memory beyond the running maxmem value (even if you have raised the configured maxmem value). It seems that raising the configured memory value should verify against the configured maxmem value, rather than the current/live maxmem value.

Version-Release number of selected component (if applicable):
1.1.1

How reproducible:
Very

Steps to Reproduce:
1. Install libvirt, create and start a new domain called test_vm with 1G of memory
2. virsh setmaxmem test_vm 2097152 --config
3. virsh setmem test_vm 2097152 --config

Actual results:
Step 2 succeeds, and if you later go in and inspect the maxmem configured value, it is 2097152 (rather than the running value of 1G).
Step 3 fails with this message:
error: invalid argument: cannot set memory higher than max memory

Expected results:
Step 3 would succeed.

Additional info:
This is using QEMU/KVM as the virtualization technology.

Looks like this is in qemu_driver.c, lines 2239-2268. 

Suggested patch:

--- a/libvirt0/libvirt-1.1.1/src/qemu/qemu_driver.c
+++ b/libvirt0/libvirt-1.1.1/src/qemu/qemu_driver.c
@@ -2236,13 +2236,13 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
     } else {
         /* resize the current memory */
 
-        if (newmem > vm->def->mem.max_balloon) {
-            virReportError(VIR_ERR_INVALID_ARG, "%s",
-                           _("cannot set memory higher than max memory"));
-            goto endjob;
-        }
-
         if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+            if (newmem > vm->def->mem.max_balloon) {
+                virReportError(VIR_ERR_INVALID_ARG, "%s",
+                    _("cannot set memory higher than max memory"));
+                goto endjob;
+            }
+
             priv = vm->privateData;
             qemuDomainObjEnterMonitor(driver, vm);
             r = qemuMonitorSetBalloon(priv->mon, newmem);
@@ -2262,6 +2262,12 @@ static int qemuDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
         }
 
         if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+            if (newmem > persistentDef->mem.max_balloon) {
+                virReportError(VIR_ERR_INVALID_ARG, "%s",
+                    _("cannot set configured memory higher than configured max memory"));
+                goto endjob;
+            }
+
             sa_assert(persistentDef);
             persistentDef->mem.cur_balloon = newmem;
             ret = virDomainSaveConfig(cfg->configDir, persistentDef);

This will probably need more work to handle the localization of a new error message.
Comment 1 Geoff Franks 2013-12-04 20:28:58 EST
Disclaimer: The suggested patch is just a guess, I'm not positive if it will work the way I hope it will.
Comment 2 Eric Blake 2013-12-04 20:53:14 EST
Can you please post your proposed patch upstream to libvir-list@redhat.com so that it will get a wider review?
Comment 3 Geoff Franks 2014-01-23 21:52:10 EST
This also affects raising configured vcpu + maxvcpu values.

Steps to reproduce:

1. Install libvirt, create and start a new domain called test_vm with 1 vcpu
2. virsh setvcpus test_vm 2 --config --maximum
3. virsh setvcpus test_vm 2 --config

Expected results:

Either step 2 would set maxvcpus on persistent domain's definition to 2 as well as the vcpu count on the persistent config, or step 2 would set the maximum, and step 3 would allow the persistent vcpu count to be raised to 2.

Actual results:

Step 2 raises the maxvcpus value on the running domain. Step 3 errors with: 
error: invalid argument: requested vcpus is greater than max allowable vcpus for the domain: 2 > 1

Notes:

I will post the combined patch for this + the memory issue to the requested list.

Suggested Patch for CPU issue:
@@ -4211,7 +4217,7 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
         virReportError(VIR_ERR_INVALID_ARG,
                        _("requested vcpus is greater than max allowable"
                          " vcpus for the domain: %d > %d"),
-                       nvcpus, vm->def->maxvcpus);
+                       nvcpus, persistentDef->maxvcpus);
         goto endjob;
     }
Comment 4 Geoff Franks 2014-01-23 22:25:06 EST
Strike that patch for vpcus, this one is probably more accurate:

@@ -4160,6 +4166,7 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
     virDomainDefPtr persistentDef;
     int ret = -1;
     bool maximum;
+    unsigned int maxvcpus;
     virQEMUDriverConfigPtr cfg = NULL;
     virCapsPtr caps = NULL;
     qemuAgentCPUInfoPtr cpuinfo = NULL;
@@ -4207,11 +4214,12 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
         goto endjob;
     }
 
-    if (!maximum && nvcpus > vm->def->maxvcpus) {
+    maxvcpus = (flags & VIR_DOMAIN_AFFECT_LIVE) ? vm->def->maxvcpus : persistentDef->maxvcpus;
+    if (!maximum && nvcpus > maxvcpus) {
         virReportError(VIR_ERR_INVALID_ARG,
                        _("requested vcpus is greater than max allowable"
                          " vcpus for the domain: %d > %d"),
-                       nvcpus, vm->def->maxvcpus);
+                       nvcpus, maxvcpus);
         goto endjob;
     }
Comment 5 Eric Blake 2014-01-23 23:25:25 EST
(In reply to Geoff Franks from comment #4)
> Strike that patch for vpcus, this one is probably more accurate:
> 
> @@ -4160,6 +4166,7 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int
> nvcpus,

Again, please post your patch upstream to libvir-list@redhat.com for faster response.
Comment 6 Geoff Franks 2014-01-24 08:48:03 EST
I posted the patches to the mailing list last night.
Comment 7 Eric Blake 2014-01-29 14:05:19 EST
Thanks for persisting; I've posted another variant based on your initial work:
https://www.redhat.com/archives/libvir-list/2014-January/msg01467.html
Comment 8 Ján Tomko 2015-03-16 09:14:51 EDT
Pushed upstream as of:
commit 60f7303c151cccdbe214b9f9ac59ecaf95cbf24b
Author:     Eric Blake <eblake@redhat.com>
AuthorDate: 2014-01-24 10:52:23 -0700
Commit:     Eric Blake <eblake@redhat.com>
CommitDate: 2014-02-20 11:27:16 -0700

    qemu: adjust maxmem/maxvcpu computation
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1038363
    
    If a domain has a different maximum for persistent and live maxmem
    or max vcpus, then it is possible to hit cases where libvirt
    refuses to adjust the current values or gets halfway through
    the adjustment before failing.  Better is to determine up front
    if the change is possible for all requested flags.
    
    Based on an idea by Geoff Franks.
    
    * src/qemu/qemu_driver.c (qemuDomainSetMemoryFlags): Compute
    correct maximum if both live and config are being set.
    (qemuDomainSetVcpusFlags): Likewise.
    
    Signed-off-by: Eric Blake <eblake@redhat.com>

git describe: v1.2.1-280-g60f7303 contains: v1.2.2-rc1~12

Note You need to log in before you can comment on or make changes to this bug.