Re: [PATCH 1/2] LXC: add support for --config in setmaxmem command

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 16.07.2014 11:51, Chen Hanxiao wrote:
In lxc, we could not use setmaxmem command
with --config options.
This patch will add support for this.

Signed-off-by: Chen Hanxiao <chenhanxiao@xxxxxxxxxxxxxx>
---
  src/lxc/lxc_driver.c | 69 ++++++++++++++++++++++++++++++++++------------------
  1 file changed, 46 insertions(+), 23 deletions(-)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index b7b4b02..be6ee19 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -721,10 +721,10 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
      virLXCDomainObjPrivatePtr priv;
      virLXCDriverPtr driver = dom->conn->privateData;
      virLXCDriverConfigPtr cfg = NULL;
-    unsigned long oldmax = 0;

      virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
-                  VIR_DOMAIN_AFFECT_CONFIG, -1);
+                  VIR_DOMAIN_AFFECT_CONFIG |
+                  VIR_DOMAIN_MEM_MAXIMUM, -1);

      if (!(vm = lxcDomObjFromDomain(dom)))
          goto cleanup;
@@ -743,32 +743,55 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
                                          &persistentDef) < 0)
          goto cleanup;

-    if (flags & VIR_DOMAIN_AFFECT_LIVE)
-        oldmax = vm->def->mem.max_balloon;
-    if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
-        if (!oldmax || oldmax > persistentDef->mem.max_balloon)
-            oldmax = persistentDef->mem.max_balloon;
-    }
+    if (flags & VIR_DOMAIN_MEM_MAXIMUM) {
+        if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+            if (newmem < vm->def->mem.cur_balloon) {
+                virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+                           _("Cannot resize the max memory less than current"
+                             " memory for an active domain"));
+                goto cleanup;
+            }
+            vm->def->mem.max_balloon = newmem;

Are things that easy? Don't we need to communicate this with the lxc_controler somehow? Even though you allow only extending, I think unless we are 100% sure guest will see the resize, we shouldn't allow this.

+        }

-    if (newmem > oldmax) {
-        virReportError(VIR_ERR_INVALID_ARG,
-                       "%s", _("Cannot set memory higher than max memory"));
-        goto cleanup;
-    }
+        if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+            sa_assert(persistentDef);

Is this assert needed? Did clang complain or is this just a pure lefover from copying from qemu_driver.c?

+            persistentDef->mem.max_balloon = newmem;
+            if (persistentDef->mem.cur_balloon > newmem)
+                persistentDef->mem.cur_balloon = newmem;
+            if (virDomainSaveConfig(cfg->configDir, persistentDef) < 0)
+                goto cleanup;
+        }
+    } else {
+        unsigned long oldmax = 0;

-    if (flags & VIR_DOMAIN_AFFECT_LIVE) {
-        if (virCgroupSetMemory(priv->cgroup, newmem) < 0) {
-            virReportError(VIR_ERR_OPERATION_FAILED,
-                           "%s", _("Failed to set memory for domain"));
-            goto cleanup;
+        if (flags & VIR_DOMAIN_AFFECT_LIVE)
+            oldmax = vm->def->mem.max_balloon;
+        if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+            if (!oldmax || oldmax > persistentDef->mem.max_balloon)
+                oldmax = persistentDef->mem.max_balloon;
          }
-    }

-    if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
-        sa_assert(persistentDef);

Well, since it has been here already, I think we can leave it in your patch too.

-        persistentDef->mem.cur_balloon = newmem;
-        if (virDomainSaveConfig(cfg->configDir, persistentDef) < 0)
+        if (newmem > oldmax) {
+            virReportError(VIR_ERR_INVALID_ARG,
+                           "%s", _("Cannot set memory higher than max memory"));
              goto cleanup;
+        }
+
+        if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+            if (virCgroupSetMemory(priv->cgroup, newmem) < 0) {
+                virReportError(VIR_ERR_OPERATION_FAILED,
+                               "%s", _("Failed to set memory for domain"));
+                goto cleanup;
+            }
+        }
+
+        if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+            sa_assert(persistentDef);
+            persistentDef->mem.cur_balloon = newmem;
+            if (virDomainSaveConfig(cfg->configDir, persistentDef) < 0)
+                goto cleanup;
+        }
      }

      ret = 0;


But what I miss in this patch is, what would:

virDomainSetMaxMemoryFlags(dom, newmem, VIR_DOMAIN_AFFECT_CURRENT | VIR_DOMAIN_MEM_MAXIMUM)

do? I mean, the _CURRENT is not translated to _LIVE or _CONFIG anywhere in the function.

Michal

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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]