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

2014-07-28 Thread chenhanx...@cn.fujitsu.com 


 -Original Message-
 From: Michal Privoznik [mailto:mpriv...@redhat.com]
 Sent: Thursday, July 24, 2014 5:00 PM
 To: Chen, Hanxiao/陈 晗霄; libvir-list@redhat.com
 Subject: Re: [libvirt] [PATCH 1/2] LXC: add support for --config in setmaxmem
 command
 
 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 chenhanx...@cn.fujitsu.com
  ---
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.
 
I focused on '--config', 
so I kept what the original lxcDomainSetMaxMemory did.
It looks that guest could not see the resize, and 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

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

2014-07-24 Thread Michal Privoznik 

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 chenhanx...@cn.fujitsu.com
---
  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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2014-07-16 Thread Chen Hanxiao 
In lxc, we could not use setmaxmem command
with --config options.
This patch will add support for this.

Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 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;
+}
 
-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);
+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);
-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;
-- 
1.9.0

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