[libvirt] [PATCH 0/3] support for changing memory parameters for inactive domains

This series enables user to change memory parameters for inactive domains from virsh command line. Hu Tao (3): Add new flags for setting memory parameters update qemuDomainGetMemoryParameters to use flags update qemuDomainSetMemoryParameters to use flags include/libvirt/libvirt.h.in | 7 ++ src/qemu/qemu_driver.c | 206 +++++++++++++++++++++++++++++++++--------- tools/virsh.c | 24 +++++- tools/virsh.pod | 7 ++ 4 files changed, 198 insertions(+), 46 deletions(-) -- 1.7.3.1

This enables users to modify memory parameters for inactive domains --- include/libvirt/libvirt.h.in | 7 +++++++ tools/virsh.c | 22 +++++++++++++++++++++- tools/virsh.pod | 7 +++++++ 3 files changed, 35 insertions(+), 1 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index a174201..b213b13 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -840,6 +840,13 @@ typedef enum { VIR_DOMAIN_MEMORY_PARAM_BOOLEAN = 6 /* boolean(character) case */ } virMemoryParameterType; +/* flags for setting memory parameters */ +typedef enum { + VIR_DOMAIN_MEMORY_PARAM_CURRENT = 0, /* affect current domain state */ + VIR_DOMAIN_MEMORY_PARAM_LIVE = (1 << 0), /* affect active domain */ + VIR_DOMAIN_MEMORY_PARAM_CONFIG = (1 << 1) /* affect next boot */ +} virMemoryParamFlags; + /** * VIR_DOMAIN_MEMORY_FIELD_LENGTH: * diff --git a/tools/virsh.c b/tools/virsh.c index 77cadcb..b2a5a8d 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -3294,6 +3294,9 @@ static const vshCmdOptDef opts_memtune[] = { N_("Max memory plus swap in kilobytes")}, {"min-guarantee", VSH_OT_INT, VSH_OFLAG_NONE, N_("Min guaranteed memory in kilobytes")}, + {"config", VSH_OT_BOOL, 0, N_("affect next boot")}, + {"live", VSH_OT_BOOL, 0, N_("affect running domain")}, + {"current", VSH_OT_BOOL, 0, N_("affect current domain")}, {NULL, 0, 0, NULL} }; @@ -3307,6 +3310,23 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd) unsigned int i = 0; virMemoryParameterPtr params = NULL, temp = NULL; bool ret = false; + unsigned int flags = 0; + int current = vshCommandOptBool(cmd, "current"); + int config = vshCommandOptBool(cmd, "config"); + int live = vshCommandOptBool(cmd, "live"); + + if (current) { + if (live || config) { + vshError(ctl, "%s", _("--current must be specified exclusively")); + return false; + } + flags = VIR_DOMAIN_MEMORY_PARAM_CURRENT; + } else { + if (config) + flags |= VIR_DOMAIN_MEMORY_PARAM_CONFIG; + if (live) + flags |= VIR_DOMAIN_MEMORY_PARAM_LIVE; + } if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -3431,7 +3451,7 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd) if (temp->value.ul == -1) temp->value.ul = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; } - if (virDomainSetMemoryParameters(dom, params, nparams, 0) != 0) + if (virDomainSetMemoryParameters(dom, params, nparams, flags) != 0) vshError(ctl, "%s", _("Unable to change memory parameters")); else ret = true; diff --git a/tools/virsh.pod b/tools/virsh.pod index ef01f41..9251db6 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -644,6 +644,13 @@ flags, the current settings are displayed; with a flag, the appropriate limit is adjusted if supported by the hypervisor. LXC and QEMU/KVM support I<--hard-limit>, I<--soft-limit>, and I<--swap-hard-limit>. +If I<--live> is specified, affect a running guest. +If I<--config> is specified, affect the next boot of a persistent guest. +If I<--current> is specified, affect the current guest state. +Both I<--live> and I<--current> flags may be given, but I<--current> is +exclusive. If no flag is specified, behavior is different depending +on hypervisor. + For QEMU/KVM, the parameters are applied to the QEMU process as a whole. Thus, when counting them, one needs to add up guest RAM, guest video RAM, and some memory overhead of QEMU itself. The last piece is hard to determine so -- 1.7.3.1

--- src/qemu/qemu_driver.c | 100 ++++++++++++++++++++++++++++++++++++++++------- tools/virsh.c | 2 +- 2 files changed, 86 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fdb3b30..1e133ae 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4918,23 +4918,22 @@ cleanup: static int qemuDomainGetMemoryParameters(virDomainPtr dom, virMemoryParameterPtr params, int *nparams, - unsigned int flags ATTRIBUTE_UNUSED) + unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; int i; virCgroupPtr group = NULL; virDomainObjPtr vm = NULL; + virDomainDefPtr persistentDef = NULL; unsigned long long val; int ret = -1; int rc; + bool isActive; - qemuDriverLock(driver); + virCheckFlags(VIR_DOMAIN_MEMORY_PARAM_LIVE | + VIR_DOMAIN_MEMORY_PARAM_CONFIG, -1); - if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_MEMORY)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cgroup memory controller is not mounted")); - goto cleanup; - } + qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -4944,10 +4943,43 @@ static int qemuDomainGetMemoryParameters(virDomainPtr dom, goto cleanup; } - if (!virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); - goto cleanup; + isActive = virDomainObjIsActive(vm); + + if (flags == VIR_DOMAIN_MEMORY_PARAM_CURRENT) { + if (isActive) + flags = VIR_DOMAIN_MEMORY_PARAM_LIVE; + else + flags = VIR_DOMAIN_MEMORY_PARAM_CONFIG; + } + + if (flags & VIR_DOMAIN_MEMORY_PARAM_LIVE) { + if (!isActive) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + + if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_MEMORY)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cgroup memory controller is not mounted")); + goto cleanup; + } + + if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find cgroup for domain %s"), vm->def->name); + goto cleanup; + } + } + + if (flags & VIR_DOMAIN_MEMORY_PARAM_CONFIG) { + if (!vm->persistent) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot change persistent config of a transient domain")); + goto cleanup; + } + if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm))) + goto cleanup; } if ((*nparams) == 0) { @@ -4963,10 +4995,47 @@ static int qemuDomainGetMemoryParameters(virDomainPtr dom, goto cleanup; } - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), vm->def->name); - goto cleanup; + if (flags & VIR_DOMAIN_MEMORY_PARAM_CONFIG) { + for (i = 0; i < *nparams; i++) { + virMemoryParameterPtr param = ¶ms[i]; + val = 0; + param->value.ul = 0; + param->type = VIR_DOMAIN_MEMORY_PARAM_ULLONG; + + switch (i) { + case 0: /* fill memory hard limit here */ + if (virStrcpyStatic(param->field, VIR_DOMAIN_MEMORY_HARD_LIMIT) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field memory hard limit too long for destination")); + goto cleanup; + } + param->value.ul = persistentDef->mem.hard_limit; + break; + + case 1: /* fill memory soft limit here */ + if (virStrcpyStatic(param->field, VIR_DOMAIN_MEMORY_SOFT_LIMIT) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field memory soft limit too long for destination")); + goto cleanup; + } + param->value.ul = persistentDef->mem.soft_limit; + break; + + case 2: /* fill swap hard limit here */ + if (virStrcpyStatic(param->field, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field swap hard limit too long for destination")); + goto cleanup; + } + param->value.ul = persistentDef->mem.swap_hard_limit; + break; + + default: + break; + /* should not hit here */ + } + } + goto out; } for (i = 0; i < *nparams; i++) { @@ -5027,6 +5096,7 @@ static int qemuDomainGetMemoryParameters(virDomainPtr dom, } } +out: ret = 0; cleanup: diff --git a/tools/virsh.c b/tools/virsh.c index b2a5a8d..5594389 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -3357,7 +3357,7 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd) if (nparams == 0) { /* get the number of memory parameters */ - if (virDomainGetMemoryParameters(dom, NULL, &nparams, 0) != 0) { + if (virDomainGetMemoryParameters(dom, NULL, &nparams, flags) != 0) { vshError(ctl, "%s", _("Unable to get number of memory parameters")); goto cleanup; -- 1.7.3.1

--- src/qemu/qemu_driver.c | 106 +++++++++++++++++++++++++++++++++++------------- 1 files changed, 77 insertions(+), 29 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1e133ae..4c62dfe 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4812,20 +4812,20 @@ cleanup: static int qemuDomainSetMemoryParameters(virDomainPtr dom, virMemoryParameterPtr params, int nparams, - unsigned int flags ATTRIBUTE_UNUSED) + unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; int i; + virDomainDefPtr persistentDef = NULL; virCgroupPtr group = NULL; virDomainObjPtr vm = NULL; int ret = -1; + bool isActive; + + virCheckFlags(VIR_DOMAIN_MEMORY_PARAM_LIVE | + VIR_DOMAIN_MEMORY_PARAM_CONFIG, -1); qemuDriverLock(driver); - if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_MEMORY)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cgroup memory controller is not mounted")); - goto cleanup; - } vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -4835,16 +4835,43 @@ static int qemuDomainSetMemoryParameters(virDomainPtr dom, goto cleanup; } - if (!virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); - goto cleanup; + isActive = virDomainObjIsActive(vm); + + if (flags == VIR_DOMAIN_MEMORY_PARAM_CURRENT) { + if (isActive) + flags = VIR_DOMAIN_MEMORY_PARAM_LIVE; + else + flags = VIR_DOMAIN_MEMORY_PARAM_CONFIG; } - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), vm->def->name); - goto cleanup; + if (flags & VIR_DOMAIN_MEMORY_PARAM_LIVE) { + if (!isActive) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + + if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_MEMORY)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cgroup memory controller is not mounted")); + goto cleanup; + } + + if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find cgroup for domain %s"), vm->def->name); + goto cleanup; + } + } + + if (flags & VIR_DOMAIN_MEMORY_PARAM_CONFIG) { + if (!vm->persistent) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot change persistent config of a transient domain")); + goto cleanup; + } + if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm))) + goto cleanup; } ret = 0; @@ -4860,11 +4887,17 @@ static int qemuDomainSetMemoryParameters(virDomainPtr dom, continue; } - rc = virCgroupSetMemoryHardLimit(group, params[i].value.ul); - if (rc != 0) { - virReportSystemError(-rc, "%s", - _("unable to set memory hard_limit tunable")); - ret = -1; + if (flags & VIR_DOMAIN_MEMORY_PARAM_LIVE) { + rc = virCgroupSetMemoryHardLimit(group, params[i].value.ul); + if (rc != 0) { + virReportSystemError(-rc, "%s", + _("unable to set memory hard_limit tunable")); + ret = -1; + } + } + + if (flags & VIR_DOMAIN_MEMORY_PARAM_CONFIG) { + persistentDef->mem.hard_limit = params[i].value.ul; } } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_SOFT_LIMIT)) { int rc; @@ -4875,11 +4908,17 @@ static int qemuDomainSetMemoryParameters(virDomainPtr dom, continue; } - rc = virCgroupSetMemorySoftLimit(group, params[i].value.ul); - if (rc != 0) { - virReportSystemError(-rc, "%s", - _("unable to set memory soft_limit tunable")); - ret = -1; + if (flags & VIR_DOMAIN_MEMORY_PARAM_LIVE) { + rc = virCgroupSetMemorySoftLimit(group, params[i].value.ul); + if (rc != 0) { + virReportSystemError(-rc, "%s", + _("unable to set memory soft_limit tunable")); + ret = -1; + } + } + + if (flags & VIR_DOMAIN_MEMORY_PARAM_CONFIG) { + persistentDef->mem.soft_limit = params[i].value.ul; } } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT)) { int rc; @@ -4890,11 +4929,16 @@ static int qemuDomainSetMemoryParameters(virDomainPtr dom, continue; } - rc = virCgroupSetMemSwapHardLimit(group, params[i].value.ul); - if (rc != 0) { - virReportSystemError(-rc, "%s", - _("unable to set swap_hard_limit tunable")); - ret = -1; + if (flags & VIR_DOMAIN_MEMORY_PARAM_LIVE) { + rc = virCgroupSetMemSwapHardLimit(group, params[i].value.ul); + if (rc != 0) { + virReportSystemError(-rc, "%s", + _("unable to set swap_hard_limit tunable")); + ret = -1; + } + } + if (flags & VIR_DOMAIN_MEMORY_PARAM_CONFIG) { + persistentDef->mem.swap_hard_limit = params[i].value.ul; } } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_MIN_GUARANTEE)) { qemuReportError(VIR_ERR_INVALID_ARG, @@ -4907,6 +4951,10 @@ static int qemuDomainSetMemoryParameters(virDomainPtr dom, } } + if (flags & VIR_DOMAIN_MEMORY_PARAM_CONFIG) { + ret = virDomainSaveConfig(driver->configDir, persistentDef); + } + cleanup: virCgroupFree(&group); if (vm) -- 1.7.3.1

This series enables user to change memory parameters for inactive domains from virsh command line. This series is rebased on the latest git tree since patch 2 of the previous sent series is applied wrongly by git on the latest git tree. Hu Tao (3): Add new flags for setting memory parameters update qemuDomainGetMemoryParameters to use flags update qemuDomainSetMemoryParameters to use flags include/libvirt/libvirt.h.in | 7 ++ src/qemu/qemu_driver.c | 206 +++++++++++++++++++++++++++++++++--------- tools/virsh.c | 26 +++++- tools/virsh.pod | 7 ++ 4 files changed, 199 insertions(+), 47 deletions(-) -- 1.7.3.1

This enables users to modify memory parameters for inactive domains --- include/libvirt/libvirt.h.in | 7 +++++++ tools/virsh.c | 26 +++++++++++++++++++++++--- tools/virsh.pod | 7 +++++++ 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 7cd6e13..0058d17 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -871,6 +871,13 @@ typedef enum { VIR_DOMAIN_MEMORY_PARAM_BOOLEAN = VIR_TYPED_PARAM_BOOLEAN, } virMemoryParameterType; +/* flags for setting memory parameters */ +typedef enum { + VIR_DOMAIN_MEMORY_PARAM_CURRENT = 0, /* affect current domain state */ + VIR_DOMAIN_MEMORY_PARAM_LIVE = (1 << 0), /* affect active domain */ + VIR_DOMAIN_MEMORY_PARAM_CONFIG = (1 << 1) /* affect next boot */ +} virMemoryParamFlags; + /** * VIR_DOMAIN_MEMORY_FIELD_LENGTH: * diff --git a/tools/virsh.c b/tools/virsh.c index de49489..ea8fd13 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -3307,6 +3307,9 @@ static const vshCmdOptDef opts_memtune[] = { N_("Max memory plus swap in kilobytes")}, {"min-guarantee", VSH_OT_INT, VSH_OFLAG_NONE, N_("Min guaranteed memory in kilobytes")}, + {"config", VSH_OT_BOOL, 0, N_("affect next boot")}, + {"live", VSH_OT_BOOL, 0, N_("affect running domain")}, + {"current", VSH_OT_BOOL, 0, N_("affect current domain")}, {NULL, 0, 0, NULL} }; @@ -3320,6 +3323,23 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd) unsigned int i = 0; virMemoryParameterPtr params = NULL, temp = NULL; bool ret = false; + unsigned int flags = 0; + int current = vshCommandOptBool(cmd, "current"); + int config = vshCommandOptBool(cmd, "config"); + int live = vshCommandOptBool(cmd, "live"); + + if (current) { + if (live || config) { + vshError(ctl, "%s", _("--current must be specified exclusively")); + return false; + } + flags = VIR_DOMAIN_MEMORY_PARAM_CURRENT; + } else { + if (config) + flags |= VIR_DOMAIN_MEMORY_PARAM_CONFIG; + if (live) + flags |= VIR_DOMAIN_MEMORY_PARAM_LIVE; + } if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -3350,7 +3370,7 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd) if (nparams == 0) { /* get the number of memory parameters */ - if (virDomainGetMemoryParameters(dom, NULL, &nparams, 0) != 0) { + if (virDomainGetMemoryParameters(dom, NULL, &nparams, flags) != 0) { vshError(ctl, "%s", _("Unable to get number of memory parameters")); goto cleanup; @@ -3364,7 +3384,7 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd) /* now go get all the memory parameters */ params = vshCalloc(ctl, nparams, sizeof(*params)); - if (virDomainGetMemoryParameters(dom, params, &nparams, 0) != 0) { + if (virDomainGetMemoryParameters(dom, params, &nparams, flags) != 0) { vshError(ctl, "%s", _("Unable to get memory parameters")); goto cleanup; } @@ -3444,7 +3464,7 @@ cmdMemtune(vshControl * ctl, const vshCmd * cmd) if (temp->value.ul == -1) temp->value.ul = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; } - if (virDomainSetMemoryParameters(dom, params, nparams, 0) != 0) + if (virDomainSetMemoryParameters(dom, params, nparams, flags) != 0) vshError(ctl, "%s", _("Unable to change memory parameters")); else ret = true; diff --git a/tools/virsh.pod b/tools/virsh.pod index ef01f41..9251db6 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -644,6 +644,13 @@ flags, the current settings are displayed; with a flag, the appropriate limit is adjusted if supported by the hypervisor. LXC and QEMU/KVM support I<--hard-limit>, I<--soft-limit>, and I<--swap-hard-limit>. +If I<--live> is specified, affect a running guest. +If I<--config> is specified, affect the next boot of a persistent guest. +If I<--current> is specified, affect the current guest state. +Both I<--live> and I<--current> flags may be given, but I<--current> is +exclusive. If no flag is specified, behavior is different depending +on hypervisor. + For QEMU/KVM, the parameters are applied to the QEMU process as a whole. Thus, when counting them, one needs to add up guest RAM, guest video RAM, and some memory overhead of QEMU itself. The last piece is hard to determine so -- 1.7.3.1

On 05/25/2011 02:54 AM, Hu Tao wrote:
This enables users to modify memory parameters for inactive domains --- include/libvirt/libvirt.h.in | 7 +++++++ tools/virsh.c | 26 +++++++++++++++++++++++--- tools/virsh.pod | 7 +++++++ 3 files changed, 37 insertions(+), 3 deletions(-)
+/* flags for setting memory parameters */ +typedef enum { + VIR_DOMAIN_MEMORY_PARAM_CURRENT = 0, /* affect current domain state */ + VIR_DOMAIN_MEMORY_PARAM_LIVE = (1 << 0), /* affect active domain */ + VIR_DOMAIN_MEMORY_PARAM_CONFIG = (1 << 1) /* affect next boot */ +} virMemoryParamFlags;
This commit is already pushed, but our general style has been to include the trailing comma in enum lists (this requires a C99 compiler, but we require C99 for other reasons). It makes maintenance easier, because then adding a new value does not require modifying the former last line to add the comma, for a smaller diff. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

--- src/qemu/qemu_driver.c | 100 ++++++++++++++++++++++++++++++++++++++++------- 1 files changed, 85 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 691965d..f04e443 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4918,23 +4918,22 @@ cleanup: static int qemuDomainGetMemoryParameters(virDomainPtr dom, virMemoryParameterPtr params, int *nparams, - unsigned int flags ATTRIBUTE_UNUSED) + unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; int i; virCgroupPtr group = NULL; virDomainObjPtr vm = NULL; + virDomainDefPtr persistentDef = NULL; unsigned long long val; int ret = -1; int rc; + bool isActive; - qemuDriverLock(driver); + virCheckFlags(VIR_DOMAIN_MEMORY_PARAM_LIVE | + VIR_DOMAIN_MEMORY_PARAM_CONFIG, -1); - if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_MEMORY)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cgroup memory controller is not mounted")); - goto cleanup; - } + qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -4944,10 +4943,43 @@ static int qemuDomainGetMemoryParameters(virDomainPtr dom, goto cleanup; } - if (!virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); - goto cleanup; + isActive = virDomainObjIsActive(vm); + + if (flags == VIR_DOMAIN_MEMORY_PARAM_CURRENT) { + if (isActive) + flags = VIR_DOMAIN_MEMORY_PARAM_LIVE; + else + flags = VIR_DOMAIN_MEMORY_PARAM_CONFIG; + } + + if (flags & VIR_DOMAIN_MEMORY_PARAM_LIVE) { + if (!isActive) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + + if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_MEMORY)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cgroup memory controller is not mounted")); + goto cleanup; + } + + if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find cgroup for domain %s"), vm->def->name); + goto cleanup; + } + } + + if (flags & VIR_DOMAIN_MEMORY_PARAM_CONFIG) { + if (!vm->persistent) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot change persistent config of a transient domain")); + goto cleanup; + } + if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm))) + goto cleanup; } if ((*nparams) == 0) { @@ -4963,10 +4995,47 @@ static int qemuDomainGetMemoryParameters(virDomainPtr dom, goto cleanup; } - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), vm->def->name); - goto cleanup; + if (flags & VIR_DOMAIN_MEMORY_PARAM_CONFIG) { + for (i = 0; i < *nparams; i++) { + virMemoryParameterPtr param = ¶ms[i]; + val = 0; + param->value.ul = 0; + param->type = VIR_DOMAIN_MEMORY_PARAM_ULLONG; + + switch (i) { + case 0: /* fill memory hard limit here */ + if (virStrcpyStatic(param->field, VIR_DOMAIN_MEMORY_HARD_LIMIT) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field memory hard limit too long for destination")); + goto cleanup; + } + param->value.ul = persistentDef->mem.hard_limit; + break; + + case 1: /* fill memory soft limit here */ + if (virStrcpyStatic(param->field, VIR_DOMAIN_MEMORY_SOFT_LIMIT) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field memory soft limit too long for destination")); + goto cleanup; + } + param->value.ul = persistentDef->mem.soft_limit; + break; + + case 2: /* fill swap hard limit here */ + if (virStrcpyStatic(param->field, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Field swap hard limit too long for destination")); + goto cleanup; + } + param->value.ul = persistentDef->mem.swap_hard_limit; + break; + + default: + break; + /* should not hit here */ + } + } + goto out; } for (i = 0; i < QEMU_NB_MEM_PARAM; i++) { @@ -5027,6 +5096,7 @@ static int qemuDomainGetMemoryParameters(virDomainPtr dom, } } +out: *nparams = QEMU_NB_MEM_PARAM; ret = 0; -- 1.7.3.1

--- src/qemu/qemu_driver.c | 106 +++++++++++++++++++++++++++++++++++------------- 1 files changed, 77 insertions(+), 29 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f04e443..9057a83 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4812,20 +4812,20 @@ cleanup: static int qemuDomainSetMemoryParameters(virDomainPtr dom, virMemoryParameterPtr params, int nparams, - unsigned int flags ATTRIBUTE_UNUSED) + unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; int i; + virDomainDefPtr persistentDef = NULL; virCgroupPtr group = NULL; virDomainObjPtr vm = NULL; int ret = -1; + bool isActive; + + virCheckFlags(VIR_DOMAIN_MEMORY_PARAM_LIVE | + VIR_DOMAIN_MEMORY_PARAM_CONFIG, -1); qemuDriverLock(driver); - if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_MEMORY)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cgroup memory controller is not mounted")); - goto cleanup; - } vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -4835,16 +4835,43 @@ static int qemuDomainSetMemoryParameters(virDomainPtr dom, goto cleanup; } - if (!virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); - goto cleanup; + isActive = virDomainObjIsActive(vm); + + if (flags == VIR_DOMAIN_MEMORY_PARAM_CURRENT) { + if (isActive) + flags = VIR_DOMAIN_MEMORY_PARAM_LIVE; + else + flags = VIR_DOMAIN_MEMORY_PARAM_CONFIG; } - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), vm->def->name); - goto cleanup; + if (flags & VIR_DOMAIN_MEMORY_PARAM_LIVE) { + if (!isActive) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + + if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_MEMORY)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cgroup memory controller is not mounted")); + goto cleanup; + } + + if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find cgroup for domain %s"), vm->def->name); + goto cleanup; + } + } + + if (flags & VIR_DOMAIN_MEMORY_PARAM_CONFIG) { + if (!vm->persistent) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot change persistent config of a transient domain")); + goto cleanup; + } + if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm))) + goto cleanup; } ret = 0; @@ -4860,11 +4887,17 @@ static int qemuDomainSetMemoryParameters(virDomainPtr dom, continue; } - rc = virCgroupSetMemoryHardLimit(group, params[i].value.ul); - if (rc != 0) { - virReportSystemError(-rc, "%s", - _("unable to set memory hard_limit tunable")); - ret = -1; + if (flags & VIR_DOMAIN_MEMORY_PARAM_LIVE) { + rc = virCgroupSetMemoryHardLimit(group, params[i].value.ul); + if (rc != 0) { + virReportSystemError(-rc, "%s", + _("unable to set memory hard_limit tunable")); + ret = -1; + } + } + + if (flags & VIR_DOMAIN_MEMORY_PARAM_CONFIG) { + persistentDef->mem.hard_limit = params[i].value.ul; } } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_SOFT_LIMIT)) { int rc; @@ -4875,11 +4908,17 @@ static int qemuDomainSetMemoryParameters(virDomainPtr dom, continue; } - rc = virCgroupSetMemorySoftLimit(group, params[i].value.ul); - if (rc != 0) { - virReportSystemError(-rc, "%s", - _("unable to set memory soft_limit tunable")); - ret = -1; + if (flags & VIR_DOMAIN_MEMORY_PARAM_LIVE) { + rc = virCgroupSetMemorySoftLimit(group, params[i].value.ul); + if (rc != 0) { + virReportSystemError(-rc, "%s", + _("unable to set memory soft_limit tunable")); + ret = -1; + } + } + + if (flags & VIR_DOMAIN_MEMORY_PARAM_CONFIG) { + persistentDef->mem.soft_limit = params[i].value.ul; } } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_SWAP_HARD_LIMIT)) { int rc; @@ -4890,11 +4929,16 @@ static int qemuDomainSetMemoryParameters(virDomainPtr dom, continue; } - rc = virCgroupSetMemSwapHardLimit(group, params[i].value.ul); - if (rc != 0) { - virReportSystemError(-rc, "%s", - _("unable to set swap_hard_limit tunable")); - ret = -1; + if (flags & VIR_DOMAIN_MEMORY_PARAM_LIVE) { + rc = virCgroupSetMemSwapHardLimit(group, params[i].value.ul); + if (rc != 0) { + virReportSystemError(-rc, "%s", + _("unable to set swap_hard_limit tunable")); + ret = -1; + } + } + if (flags & VIR_DOMAIN_MEMORY_PARAM_CONFIG) { + persistentDef->mem.swap_hard_limit = params[i].value.ul; } } else if (STREQ(param->field, VIR_DOMAIN_MEMORY_MIN_GUARANTEE)) { qemuReportError(VIR_ERR_INVALID_ARG, @@ -4907,6 +4951,10 @@ static int qemuDomainSetMemoryParameters(virDomainPtr dom, } } + if (flags & VIR_DOMAIN_MEMORY_PARAM_CONFIG) { + ret = virDomainSaveConfig(driver->configDir, persistentDef); + } + cleanup: virCgroupFree(&group); if (vm) -- 1.7.3.1

On Wed, May 25, 2011 at 04:54:43PM +0800, Hu Tao wrote:
This series enables user to change memory parameters for inactive domains from virsh command line.
This series is rebased on the latest git tree since patch 2 of the previous sent series is applied wrongly by git on the latest git tree.
Hu Tao (3): Add new flags for setting memory parameters update qemuDomainGetMemoryParameters to use flags update qemuDomainSetMemoryParameters to use flags
include/libvirt/libvirt.h.in | 7 ++ src/qemu/qemu_driver.c | 206 +++++++++++++++++++++++++++++++++--------- tools/virsh.c | 26 +++++- tools/virsh.pod | 7 ++ 4 files changed, 199 insertions(+), 47 deletions(-)
Okay, the code it relatively simple, and looks fine to me, ACK, I wonder if somehow we should not unify the PARAM_CURRENT/PARAM_LIVE/PARAM_CONFIG flags already present in a couple of place to avoid duplicating the enums and making sure we have coherent values for all the APIs. But it's an incremental improvement which can be added as a separate step, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (3)
-
Daniel Veillard
-
Eric Blake
-
Hu Tao