Re: [PATCH 3/5] setmem: implement the code to address the new API in the qemu driver

[Eric Blake <eblake@xxxxxxxxxx>]

On 03/02/2011 01:13 AM, Taku Izumi wrote:
> This patch implements the code to address the new API
> in the qemu driver.
> 
> +    if (!vm->persistent && (flags & VIR_DOMAIN_MEM_CONFIG)) {
> +        qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                        _("cannot change persistent config of a transient domain"));
>          goto endjob;
> +    }
> 
> +    if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm)))
>          goto endjob;

Doesn't that fail for a transient domain?  That is, shouldn't the
persistentDef only be set when flags & VIR_DOMAIN_MEM_CONFIG is
requested and validated?

> +
> +    switch (flags) {
> +    case VIR_DOMAIN_MEM_CONFIG:
> +        persistentDef->mem.cur_balloon = newmem;
> +        ret = 0;
> +        break;
> +
> +    case VIR_DOMAIN_MEM_LIVE:
> +    case VIR_DOMAIN_MEM_LIVE | VIR_DOMAIN_MEM_CONFIG:
> +        priv = vm->privateData;
> +        qemuDomainObjEnterMonitor(vm);
> +        r = qemuMonitorSetBalloon(priv->mon, newmem);

Merge conflict with my audit patches.  Plus Daniel's request to
rearrange the switch statement into if statements.

> +        qemuDomainObjExitMonitor(vm);

> @@ -6789,7 +6827,7 @@ static virDriver qemuDriver = {
>      qemudDomainGetMaxMemory, /* domainGetMaxMemory */
>      NULL, /* domainSetMaxMemory */
>      qemudDomainSetMemory, /* domainSetMemory */
> -    NULL, /* domainSetMemoryFlags */
> +    qemudDomainSetMemoryFlags, /* domainSetMemoryFlags */

Hmm, for other API additions, I have provided dummy wrappers for the new
function anywhere the old function existed, that merely require the same
flags as the old function would use.  I'll probably propose that as a
followup patch.

Here's what I ended up with before pushing:

diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index 2f15144..7921d98 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -1568,12 +1568,22 @@ cleanup:
     return ret;
 }

-static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) {
+static int qemudDomainSetMemoryFlags(virDomainPtr dom, unsigned long
newmem,
+                                     unsigned int flags) {
     struct qemud_driver *driver = dom->conn->privateData;
     qemuDomainObjPrivatePtr priv;
     virDomainObjPtr vm;
+    virDomainDefPtr persistentDef = NULL;
     int ret = -1, r;

+    virCheckFlags(VIR_DOMAIN_MEM_LIVE |
+                  VIR_DOMAIN_MEM_CONFIG, -1);
+
+    if ((flags & (VIR_DOMAIN_MEM_LIVE | VIR_DOMAIN_MEM_CONFIG)) == 0) {
+        qemuReportError(VIR_ERR_INVALID_ARG,
+                        _("invalid flag combination: (0x%x)"), flags);
+    }
+
     qemuDriverLock(driver);
     vm = virDomainFindByUUID(&driver->domains, dom->uuid);
     qemuDriverUnlock(driver);
@@ -1594,24 +1604,42 @@ static int qemudDomainSetMemory(virDomainPtr
dom, unsigned long newmem) {
     if (qemuDomainObjBeginJob(vm) < 0)
         goto cleanup;

-    if (!virDomainObjIsActive(vm)) {
+    if (!virDomainObjIsActive(vm) && (flags & VIR_DOMAIN_MEM_LIVE)) {
         qemuReportError(VIR_ERR_OPERATION_INVALID,
                         "%s", _("domain is not running"));
         goto endjob;
     }

-    priv = vm->privateData;
-    qemuDomainObjEnterMonitor(vm);
-    r = qemuMonitorSetBalloon(priv->mon, newmem);
-    qemuDomainObjExitMonitor(vm);
-    qemuAuditMemory(vm, vm->def->mem.cur_balloon, newmem, "update", r
== 1);
-    if (r < 0)
-        goto endjob;
+    if (flags & VIR_DOMAIN_MEM_CONFIG) {
+        if (!vm->persistent) {
+            qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
+                            _("cannot change persistent config of a
transient domain"));
+            goto endjob;
+        }
+        if (!(persistentDef =
virDomainObjGetPersistentDef(driver->caps, vm)))
+            goto endjob;
+    }

-    /* Lack of balloon support is a fatal error */
-    if (r == 0) {
-        qemuReportError(VIR_ERR_OPERATION_INVALID,
-                        "%s", _("cannot set memory of an active domain"));
+    if (flags & VIR_DOMAIN_MEM_LIVE) {
+        priv = vm->privateData;
+        qemuDomainObjEnterMonitor(vm);
+        r = qemuMonitorSetBalloon(priv->mon, newmem);
+        qemuDomainObjExitMonitor(vm);
+        qemuAuditMemory(vm, vm->def->mem.cur_balloon, newmem, "update",
r == 1);
+        if (r < 0)
+            goto endjob;
+
+        /* Lack of balloon support is a fatal error */
+        if (r == 0) {
+            qemuReportError(VIR_ERR_OPERATION_INVALID,
+                            "%s", _("cannot set memory of an active
domain"));
+            goto endjob;
+        }
+    }
+
+    if (flags& VIR_DOMAIN_MEM_CONFIG) {
+        persistentDef->mem.cur_balloon = newmem;
+        ret = virDomainSaveConfig(driver->configDir, persistentDef);
         goto endjob;
     }

@@ -1626,6 +1654,10 @@ cleanup:
     return ret;
 }

+static int qemudDomainSetMemory(virDomainPtr dom, unsigned long newmem) {
+    return qemudDomainSetMemoryFlags(dom, newmem, VIR_DOMAIN_MEM_LIVE);
+}
+
 static int qemudDomainGetInfo(virDomainPtr dom,
                               virDomainInfoPtr info) {
     struct qemud_driver *driver = dom->conn->privateData;
@@ -6854,7 +6886,7 @@ static virDriver qemuDriver = {
     qemudDomainGetMaxMemory, /* domainGetMaxMemory */
     NULL, /* domainSetMaxMemory */
     qemudDomainSetMemory, /* domainSetMemory */
-    NULL, /* domainSetMemoryFlags */
+    qemudDomainSetMemoryFlags, /* domainSetMemoryFlags */
     qemudDomainGetInfo, /* domainGetInfo */
     qemudDomainSave, /* domainSave */
     qemudDomainRestore, /* domainRestore */


-- 
Eric Blake   eblake@xxxxxxxxxx    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list