[libvirt] [PATCH v3 0/4] support for changing cpu.shares for inactive domains from virsh cmd
Currently cpu.shares can only be configured by editing domains' xmls. this series enables us to change cpu.shares from virsh cmd schedinfo even when domain is inactive. changes: v3: - add parameters --config, --live and --current. v2: - since v1 patches that delete all generated RPC files(by Daniel) and that refactor remote generator(by Matthias) have gone into master branch, which affects the series heavily. So rebase the series on the latest code. Hu Tao (4): introduce virDomainSetSchedulerParametersFlags qemu: introduce qemuSetSchedulerParametersFlags remote: introduce remoteSetSchedulerParametersFlags virsh: add parameters --live, --config and --current to cmd schedinfo daemon/remote.c | 71 +++++++++++++++++++++++++++++++ include/libvirt/libvirt.h.in | 14 ++++++ python/generator.py | 1 + src/driver.h | 8 ++++ src/libvirt.c | 64 ++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + src/qemu/qemu_driver.c | 94 +++++++++++++++++++++++++++++++---------- src/remote/remote_driver.c | 69 ++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 9 ++++- tools/virsh.c | 25 +++++++++++- tools/virsh.pod | 6 ++- 11 files changed, 336 insertions(+), 26 deletions(-) -- 1.7.3.1
This new function allows aditional flags to be passed into from the virsh command line. --- include/libvirt/libvirt.h.in | 14 +++++++++ python/generator.py | 1 + src/driver.h | 8 +++++ src/libvirt.c | 64 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 5 files changed, 88 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index f4d0b40..ec32b4b 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -295,6 +295,12 @@ typedef enum { VIR_DOMAIN_SCHED_FIELD_BOOLEAN = 6 /* boolean(character) case */ } virSchedParameterType; +typedef enum { + VIR_DOMAIN_SCHEDPARAM_CURRENT = (1 << 0), /* affect current domain state */ + VIR_DOMAIN_SCHEDPARAM_LIVE = (1 << 1), /* Affect active domain */ + VIR_DOMAIN_SCHEDPARAM_CONFIG = (1 << 2), /* Affect next boot */ +} virDomainSchedParameterFlags; + /** * VIR_DOMAIN_SCHED_FIELD_LENGTH: * @@ -346,6 +352,14 @@ int virDomainSetSchedulerParameters (virDomainPtr domain, virSchedParameterPtr params, int nparams); +/* + * Change scheduler parameters + */ +int virDomainSetSchedulerParametersFlags (virDomainPtr domain, + virSchedParameterPtr params, + int nparams, + unsigned int flags); + /** * virDomainBlockStats: * diff --git a/python/generator.py b/python/generator.py index b395caf..9f9deb2 100755 --- a/python/generator.py +++ b/python/generator.py @@ -312,6 +312,7 @@ skip_impl = ( 'virDomainGetSchedulerType', 'virDomainGetSchedulerParameters', 'virDomainSetSchedulerParameters', + 'virDomainSetSchedulerParametersFlags', 'virDomainSetBlkioParameters', 'virDomainGetBlkioParameters', 'virDomainSetMemoryParameters', diff --git a/src/driver.h b/src/driver.h index 006e0bb..450dd53 100644 --- a/src/driver.h +++ b/src/driver.h @@ -286,6 +286,13 @@ typedef int int nparams); typedef int + (*virDrvDomainSetSchedulerParametersFlags) + (virDomainPtr domain, + virSchedParameterPtr params, + int nparams, + unsigned int flags); + +typedef int (*virDrvDomainBlockStats) (virDomainPtr domain, const char *path, @@ -677,6 +684,7 @@ struct _virDriver { virDrvDomainGetSchedulerType domainGetSchedulerType; virDrvDomainGetSchedulerParameters domainGetSchedulerParameters; virDrvDomainSetSchedulerParameters domainSetSchedulerParameters; + virDrvDomainSetSchedulerParametersFlags domainSetSchedulerParametersFlags; virDrvDomainMigratePrepare domainMigratePrepare; virDrvDomainMigratePerform domainMigratePerform; virDrvDomainMigrateFinish domainMigrateFinish; diff --git a/src/libvirt.c b/src/libvirt.c index 5a5439d..9684b83 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -5104,6 +5104,70 @@ error: /** + * virDomainSetSchedulerParametersFlags: + * @domain: pointer to domain object + * @params: pointer to scheduler parameter objects + * @nparams: number of scheduler parameter + * (this value should be same or less than the returned value + * nparams of virDomainGetSchedulerType) + * @flags: virDomainSchedParameterFlags + * + * Change the scheduler parameters + * + * Returns -1 in case of error, 0 in case of success. + */ +int +virDomainSetSchedulerParametersFlags(virDomainPtr domain, + virSchedParameterPtr params, + int nparams, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%d, flags=%u", + params, nparams, flags); + + virResetLastError(); + + if (!(flags & (VIR_DOMAIN_SCHEDPARAM_LIVE | + VIR_DOMAIN_SCHEDPARAM_CONFIG | + VIR_DOMAIN_SCHEDPARAM_CURRENT))) { + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (!VIR_IS_CONNECTED_DOMAIN(domain)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + if (domain->conn->flags & VIR_CONNECT_RO) { + virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + conn = domain->conn; + + if (conn->driver->domainSetSchedulerParametersFlags) { + int ret; + ret = conn->driver->domainSetSchedulerParametersFlags(domain, + params, + nparams, + flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(domain->conn); + return -1; +} + + +/** * virDomainBlockStats: * @dom: pointer to the domain object * @path: path to the block device diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 1444b55..0590535 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -441,6 +441,7 @@ LIBVIRT_0.9.2 { virDomainGetState; virDomainInjectNMI; virDomainScreenshot; + virDomainSetSchedulerParametersFlags; } LIBVIRT_0.9.0; # .... define new API here using predicted next version number .... -- 1.7.3.1
On 05/17/2011 12:20 AM, Hu Tao wrote:
This new function allows aditional flags to be passed into from
s/aditional/additional/
the virsh command line. --- include/libvirt/libvirt.h.in | 14 +++++++++ python/generator.py | 1 + src/driver.h | 8 +++++ src/libvirt.c | 64 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 5 files changed, 88 insertions(+), 0 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index f4d0b40..ec32b4b 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -295,6 +295,12 @@ typedef enum { VIR_DOMAIN_SCHED_FIELD_BOOLEAN = 6 /* boolean(character) case */ } virSchedParameterType;
+typedef enum { + VIR_DOMAIN_SCHEDPARAM_CURRENT = (1 << 0), /* affect current domain state */
This should be 0,
+ VIR_DOMAIN_SCHEDPARAM_LIVE = (1 << 1), /* Affect active domain */ + VIR_DOMAIN_SCHEDPARAM_CONFIG = (1 << 2), /* Affect next boot */
and these 1 and 2 (1 << 0, 1 << 2).
+++ b/src/libvirt.c @@ -5104,6 +5104,70 @@ error:
/** + * virDomainSetSchedulerParametersFlags: + * @domain: pointer to domain object + * @params: pointer to scheduler parameter objects + * @nparams: number of scheduler parameter + * (this value should be same or less than the returned value + * nparams of virDomainGetSchedulerType)
Can @nparams be 0, and if so, can @params be NULL and use this as a way to query the correct nparams setting (rather than having to call virDomainGetSchedulerType)? Related to my comment on Matthias's libvirt.c cleanup. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On 05/17/2011 09:48 AM, Eric Blake wrote:
On 05/17/2011 12:20 AM, Hu Tao wrote:
This new function allows aditional flags to be passed into from
s/aditional/additional/
+++ b/include/libvirt/libvirt.h.in @@ -295,6 +295,12 @@ typedef enum { VIR_DOMAIN_SCHED_FIELD_BOOLEAN = 6 /* boolean(character) case */ } virSchedParameterType;
+typedef enum { + VIR_DOMAIN_SCHEDPARAM_CURRENT = (1 << 0), /* affect current domain state */
This should be 0,
+ VIR_DOMAIN_SCHEDPARAM_LIVE = (1 << 1), /* Affect active domain */ + VIR_DOMAIN_SCHEDPARAM_CONFIG = (1 << 2), /* Affect next boot */
and these 1 and 2 (1 << 0, 1 << 2).
Aargh. I accidentally pushed this commit before adding my changes (I have enough latency connecting to libvirt.git that my 'git push' in one window had not completed by the time my 'git am' in another window had, and 'git push' uses the current HEAD at the time of the remote connection, rather than at the time the git command started). diff --git i/include/libvirt/libvirt.h.in w/include/libvirt/libvirt.h.in index ec32b4b..a174201 100644 --- i/include/libvirt/libvirt.h.in +++ w/include/libvirt/libvirt.h.in @@ -296,9 +296,9 @@ typedef enum { } virSchedParameterType; typedef enum { - VIR_DOMAIN_SCHEDPARAM_CURRENT = (1 << 0), /* affect current domain state */ - VIR_DOMAIN_SCHEDPARAM_LIVE = (1 << 1), /* Affect active domain */ - VIR_DOMAIN_SCHEDPARAM_CONFIG = (1 << 2), /* Affect next boot */ + VIR_DOMAIN_SCHEDPARAM_CURRENT = 0, /* affect current domain state */ + VIR_DOMAIN_SCHEDPARAM_LIVE = (1 << 0), /* Affect active domain */ + VIR_DOMAIN_SCHEDPARAM_CONFIG = (1 << 1), /* Affect next boot */ } virDomainSchedParameterFlags; /** -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Tue, May 17, 2011 at 09:48:06AM -0600, Eric Blake wrote:
On 05/17/2011 12:20 AM, Hu Tao wrote:
This new function allows aditional flags to be passed into from
s/aditional/additional/
the virsh command line. --- include/libvirt/libvirt.h.in | 14 +++++++++ python/generator.py | 1 + src/driver.h | 8 +++++ src/libvirt.c | 64 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 5 files changed, 88 insertions(+), 0 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index f4d0b40..ec32b4b 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -295,6 +295,12 @@ typedef enum { VIR_DOMAIN_SCHED_FIELD_BOOLEAN = 6 /* boolean(character) case */ } virSchedParameterType;
+typedef enum { + VIR_DOMAIN_SCHEDPARAM_CURRENT = (1 << 0), /* affect current domain state */
This should be 0,
This causes problem when setting parameters for an inactive domain, with --current. In the case, the flags is 0, which fails flag-check in virDomainSetSchedulerParametersFlags. Maybe we should not check flags at this stage, and leave it for drivers. The following patch removes the check: 9eaceed112b3d8578f1cb0ae76eb47510f7342a Mon Sep 17 00:00:00 2001 From: Hu Tao <hutao@cn.fujitsu.com> Date: Wed, 18 May 2011 10:29:41 +0800 Subject: [PATCH] don't check flags in virDomainSetSchedulerParametersFlags drivers implementing domainSetSchedulerParametersFlags should check flags themselves. --- src/libvirt.c | 8 -------- 1 files changed, 0 insertions(+), 8 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 56b1257..eb2b0a0 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -5132,14 +5132,6 @@ virDomainSetSchedulerParametersFlags(virDomainPtr domain, virResetLastError(); - if (!(flags & (VIR_DOMAIN_SCHEDPARAM_LIVE | - VIR_DOMAIN_SCHEDPARAM_CONFIG | - VIR_DOMAIN_SCHEDPARAM_CURRENT))) { - virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__); - virDispatchError(NULL); - return -1; - } - if (!VIR_IS_CONNECTED_DOMAIN(domain)) { virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); virDispatchError(NULL); -- 1.7.3.1
On 05/17/2011 08:34 PM, Hu Tao wrote:
This causes problem when setting parameters for an inactive domain, with --current. In the case, the flags is 0, which fails flag-check in virDomainSetSchedulerParametersFlags. Maybe we should not check flags at this stage, and leave it for drivers.
The following patch removes the check:
9eaceed112b3d8578f1cb0ae76eb47510f7342a Mon Sep 17 00:00:00 2001 From: Hu Tao <hutao@cn.fujitsu.com> Date: Wed, 18 May 2011 10:29:41 +0800 Subject: [PATCH] don't check flags in virDomainSetSchedulerParametersFlags
drivers implementing domainSetSchedulerParametersFlags should check flags themselves. --- src/libvirt.c | 8 -------- 1 files changed, 0 insertions(+), 8 deletions(-)
ACK and pushed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Support for virDomainSetSchedulerParametersFlags of qemu driver. --- src/qemu/qemu_driver.c | 94 ++++++++++++++++++++++++++++++++++++------------ 1 files changed, 71 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ccaae66..d3b832d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4835,7 +4835,7 @@ static int qemuDomainSetMemoryParameters(virDomainPtr dom, goto cleanup; } - if (!virDomainObjIsActive(vm)) { + if (!virDomainObjIsActive(vm) && (flags & VIR_DOMAIN_SCHEDPARAM_LIVE)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); goto cleanup; @@ -5038,22 +5038,24 @@ cleanup: return ret; } -static int qemuSetSchedulerParameters(virDomainPtr dom, - virSchedParameterPtr params, - int nparams) +static int qemuSetSchedulerParametersFlags(virDomainPtr dom, + virSchedParameterPtr params, + int nparams, + unsigned int flags) { struct qemud_driver *driver = dom->conn->privateData; int i; virCgroupPtr group = NULL; virDomainObjPtr vm = NULL; + virDomainDefPtr persistentDef = NULL; int ret = -1; + bool isActive; + + virCheckFlags(VIR_DOMAIN_SCHEDPARAM_LIVE | + VIR_DOMAIN_SCHEDPARAM_CONFIG | + VIR_DOMAIN_SCHEDPARAM_CURRENT, -1); qemuDriverLock(driver); - if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("cgroup CPU controller is not mounted")); - goto cleanup; - } vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -5063,16 +5065,32 @@ static int qemuSetSchedulerParameters(virDomainPtr dom, goto cleanup; } - if (!virDomainObjIsActive(vm)) { + isActive = virDomainObjIsActive(vm); + + if (flags == VIR_DOMAIN_SCHEDPARAM_CURRENT) { + if (isActive) + flags = VIR_DOMAIN_SCHEDPARAM_LIVE; + else + flags = VIR_DOMAIN_SCHEDPARAM_CONFIG; + } + + if (!isActive && (flags & VIR_DOMAIN_SCHEDPARAM_LIVE)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); 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_SCHEDPARAM_LIVE) { + if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("cgroup CPU 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; + } } for (i = 0; i < nparams; i++) { @@ -5086,20 +5104,39 @@ static int qemuSetSchedulerParameters(virDomainPtr dom, goto cleanup; } - rc = virCgroupSetCpuShares(group, params[i].value.ul); - if (rc != 0) { - virReportSystemError(-rc, "%s", - _("unable to set cpu shares tunable")); - goto cleanup; + if (flags & VIR_DOMAIN_SCHEDPARAM_LIVE) { + rc = virCgroupSetCpuShares(group, params[i].value.ul); + if (rc != 0) { + virReportSystemError(-rc, "%s", + _("unable to set cpu shares tunable")); + goto cleanup; + } + + vm->def->cputune.shares = params[i].value.ul; } - vm->def->cputune.shares = params[i].value.ul; + if (flags & VIR_DOMAIN_SCHEDPARAM_CONFIG) { + persistentDef = virDomainObjGetPersistentDef(driver->caps, vm); + if (!persistentDef) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("can't get persistentDef")); + goto cleanup; + } + persistentDef->cputune.shares = params[i].value.ul; + rc = virDomainSaveConfig(driver->configDir, persistentDef); + if (rc) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("can't save config")); + goto cleanup; + } + } } else { qemuReportError(VIR_ERR_INVALID_ARG, _("Invalid parameter `%s'"), param->field); goto cleanup; } } + ret = 0; cleanup: @@ -5110,6 +5147,16 @@ cleanup: return ret; } +static int qemuSetSchedulerParameters(virDomainPtr dom, + virSchedParameterPtr params, + int nparams) +{ + return qemuSetSchedulerParametersFlags(dom, + params, + nparams, + VIR_DOMAIN_SCHEDPARAM_LIVE); +} + static int qemuGetSchedulerParameters(virDomainPtr dom, virSchedParameterPtr params, int *nparams) @@ -5143,9 +5190,8 @@ static int qemuGetSchedulerParameters(virDomainPtr dom, } if (!virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); - goto cleanup; + val = vm->def->cputune.shares; + goto out; } if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { @@ -5160,6 +5206,7 @@ static int qemuGetSchedulerParameters(virDomainPtr dom, _("unable to get cpu shares tunable")); goto cleanup; } +out: params[0].value.ul = val; params[0].type = VIR_DOMAIN_SCHED_FIELD_ULLONG; if (virStrcpyStatic(params[0].field, "cpu_shares") == NULL) { @@ -7675,6 +7722,7 @@ static virDriver qemuDriver = { .domainMigratePerform3 = qemuDomainMigratePerform3, /* 0.9.2 */ .domainMigrateFinish3 = qemuDomainMigrateFinish3, /* 0.9.2 */ .domainMigrateConfirm3 = qemuDomainMigrateConfirm3, /* 0.9.2 */ + .domainSetSchedulerParametersFlags = qemuSetSchedulerParametersFlags, /* 0.9.2 */ }; -- 1.7.3.1
On 05/17/2011 12:20 AM, Hu Tao wrote:
Support for virDomainSetSchedulerParametersFlags of qemu driver. --- src/qemu/qemu_driver.c | 94 ++++++++++++++++++++++++++++++++++++------------ 1 files changed, 71 insertions(+), 23 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ccaae66..d3b832d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4835,7 +4835,7 @@ static int qemuDomainSetMemoryParameters(virDomainPtr dom, goto cleanup; }
- if (!virDomainObjIsActive(vm)) { + if (!virDomainObjIsActive(vm) && (flags & VIR_DOMAIN_SCHEDPARAM_LIVE)) {
This hunk doesn't belong. SCHEDPARAM flags should not affect MemoryParameters.
+ bool isActive; + + virCheckFlags(VIR_DOMAIN_SCHEDPARAM_LIVE | + VIR_DOMAIN_SCHEDPARAM_CONFIG | + VIR_DOMAIN_SCHEDPARAM_CURRENT, -1);
Given that _CURRENT is 0, it is redundant in this check.
+ if (!isActive && (flags & VIR_DOMAIN_SCHEDPARAM_LIVE)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); goto cleanup; }
Missing a check that a transient domain can't use _CONFIG.
- vm->def->cputune.shares = params[i].value.ul; + if (flags & VIR_DOMAIN_SCHEDPARAM_CONFIG) { + persistentDef = virDomainObjGetPersistentDef(driver->caps, vm); + if (!persistentDef) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("can't get persistentDef")); + goto cleanup; + } + persistentDef->cputune.shares = params[i].value.ul; + rc = virDomainSaveConfig(driver->configDir, persistentDef);
Hmm, this is fetching and saving persistentDef every time through the loop. Then again, qemu only goes through the loop at most once, so it's not wasteful. But if we ever add another parameter, it would be worth refactoring things to grab persistentDef before the loop, and call virDomainSaveConfig only after the loop completes, rather than repeating it each iteration.
@@ -5143,9 +5190,8 @@ static int qemuGetSchedulerParameters(virDomainPtr dom, }
if (!virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); - goto cleanup; + val = vm->def->cputune.shares; + goto out;
Hmm, this really argues that we need qemuGetSchedulerParametersFlags as a counterpart to qemuSetSchedulerParametersFlags. But that can be a separate patch. ACK with this squashed in, so I'm pushing: diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c index d3b832d..fdb3b30 100644 --- i/src/qemu/qemu_driver.c +++ w/src/qemu/qemu_driver.c @@ -4835,7 +4835,7 @@ static int qemuDomainSetMemoryParameters(virDomainPtr dom, goto cleanup; } - if (!virDomainObjIsActive(vm) && (flags & VIR_DOMAIN_SCHEDPARAM_LIVE)) { + if (!virDomainObjIsActive(vm)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); goto cleanup; @@ -5052,8 +5052,7 @@ static int qemuSetSchedulerParametersFlags(virDomainPtr dom, bool isActive; virCheckFlags(VIR_DOMAIN_SCHEDPARAM_LIVE | - VIR_DOMAIN_SCHEDPARAM_CONFIG | - VIR_DOMAIN_SCHEDPARAM_CURRENT, -1); + VIR_DOMAIN_SCHEDPARAM_CONFIG, -1); qemuDriverLock(driver); @@ -5074,13 +5073,19 @@ static int qemuSetSchedulerParametersFlags(virDomainPtr dom, flags = VIR_DOMAIN_SCHEDPARAM_CONFIG; } - if (!isActive && (flags & VIR_DOMAIN_SCHEDPARAM_LIVE)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, - "%s", _("domain is not running")); + if ((flags & VIR_DOMAIN_MEM_CONFIG) && !vm->persistent) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("cannot change persistent config of a transient domain")); goto cleanup; } if (flags & VIR_DOMAIN_SCHEDPARAM_LIVE) { + if (!isActive) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cgroup CPU controller is not mounted")); @@ -5088,7 +5093,8 @@ static int qemuSetSchedulerParametersFlags(virDomainPtr dom, } if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot find cgroup for domain %s"), vm->def->name); + _("cannot find cgroup for domain %s"), + vm->def->name); goto cleanup; } } -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
support for virDomainSetSchedulerParametersFlags of remote driver. --- daemon/remote.c | 71 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 69 ++++++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 9 +++++- 3 files changed, 148 insertions(+), 1 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index ea36bf5..993813b 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -688,6 +688,77 @@ cleanup: } static int +remoteDispatchDomainSetSchedulerParametersFlags(struct qemud_server *server ATTRIBUTE_UNUSED, + struct qemud_client *client ATTRIBUTE_UNUSED, + virConnectPtr conn, + remote_message_header *hdr ATTRIBUTE_UNUSED, + remote_error *rerr, + remote_domain_set_scheduler_parameters_flags_args *args, + void *ret ATTRIBUTE_UNUSED) +{ + virDomainPtr dom = NULL; + virSchedParameterPtr params = NULL; + int i, nparams; + int rv = -1; + + if (!conn) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + nparams = args->params.params_len; + + if (nparams > REMOTE_DOMAIN_SCHEDULER_PARAMETERS_MAX) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); + goto cleanup; + } + if (VIR_ALLOC_N(params, nparams) < 0) { + virReportOOMError(); + goto cleanup; + } + + /* Deserialise parameters. */ + for (i = 0; i < nparams; ++i) { + if (virStrcpyStatic(params[i].field, args->params.params_val[i].field) == NULL) { + virNetError(VIR_ERR_INTERNAL_ERROR, _("Field %s too big for destination"), + args->params.params_val[i].field); + goto cleanup; + } + params[i].type = args->params.params_val[i].value.type; + switch (params[i].type) { + case VIR_DOMAIN_SCHED_FIELD_INT: + params[i].value.i = args->params.params_val[i].value.remote_sched_param_value_u.i; break; + case VIR_DOMAIN_SCHED_FIELD_UINT: + params[i].value.ui = args->params.params_val[i].value.remote_sched_param_value_u.ui; break; + case VIR_DOMAIN_SCHED_FIELD_LLONG: + params[i].value.l = args->params.params_val[i].value.remote_sched_param_value_u.l; break; + case VIR_DOMAIN_SCHED_FIELD_ULLONG: + params[i].value.ul = args->params.params_val[i].value.remote_sched_param_value_u.ul; break; + case VIR_DOMAIN_SCHED_FIELD_DOUBLE: + params[i].value.d = args->params.params_val[i].value.remote_sched_param_value_u.d; break; + case VIR_DOMAIN_SCHED_FIELD_BOOLEAN: + params[i].value.b = args->params.params_val[i].value.remote_sched_param_value_u.b; break; + } + } + + if (!(dom = get_nonnull_domain(conn, args->dom))) + goto cleanup; + + if (virDomainSetSchedulerParametersFlags(dom, params, nparams, args->flags) < 0) + goto cleanup; + + rv = 0; + +cleanup: + if (rv < 0) + remoteDispatchError(rerr); + if (dom) + virDomainFree(dom); + VIR_FREE(params); + return rv; +} + +static int remoteDispatchDomainMemoryStats(struct qemud_server *server ATTRIBUTE_UNUSED, struct qemud_client *client ATTRIBUTE_UNUSED, virConnectPtr conn, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 45d0f80..52ea6b9 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2692,6 +2692,74 @@ done: } static int +remoteDomainSetSchedulerParametersFlags(virDomainPtr domain, + virSchedParameterPtr params, + int nparams, + unsigned int flags) +{ + int rv = -1; + remote_domain_set_scheduler_parameters_flags_args args; + int i, do_error; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain (&args.dom, domain); + + /* Serialise the scheduler parameters. */ + args.params.params_len = nparams; + args.flags = flags; + if (VIR_ALLOC_N(args.params.params_val, nparams) < 0) { + virReportOOMError(); + goto done; + } + + do_error = 0; + for (i = 0; i < nparams; ++i) { + /* call() will free this: */ + args.params.params_val[i].field = strdup (params[i].field); + if (args.params.params_val[i].field == NULL) { + virReportOOMError(); + do_error = 1; + } + args.params.params_val[i].value.type = params[i].type; + switch (params[i].type) { + case VIR_DOMAIN_SCHED_FIELD_INT: + args.params.params_val[i].value.remote_sched_param_value_u.i = params[i].value.i; break; + case VIR_DOMAIN_SCHED_FIELD_UINT: + args.params.params_val[i].value.remote_sched_param_value_u.ui = params[i].value.ui; break; + case VIR_DOMAIN_SCHED_FIELD_LLONG: + args.params.params_val[i].value.remote_sched_param_value_u.l = params[i].value.l; break; + case VIR_DOMAIN_SCHED_FIELD_ULLONG: + args.params.params_val[i].value.remote_sched_param_value_u.ul = params[i].value.ul; break; + case VIR_DOMAIN_SCHED_FIELD_DOUBLE: + args.params.params_val[i].value.remote_sched_param_value_u.d = params[i].value.d; break; + case VIR_DOMAIN_SCHED_FIELD_BOOLEAN: + args.params.params_val[i].value.remote_sched_param_value_u.b = params[i].value.b; break; + default: + remoteError(VIR_ERR_RPC, "%s", _("unknown parameter type")); + do_error = 1; + } + } + + if (do_error) { + xdr_free ((xdrproc_t) xdr_remote_domain_set_scheduler_parameters_flags_args, (char *) &args); + goto done; + } + + if (call (domain->conn, priv, 0, REMOTE_PROC_DOMAIN_SET_SCHEDULER_PARAMETERS_FLAGS, + (xdrproc_t) xdr_remote_domain_set_scheduler_parameters_flags_args, (char *) &args, + (xdrproc_t) xdr_void, (char *) NULL) == -1) + goto done; + + rv = 0; + +done: + remoteDriverUnlock(priv); + return rv; +} + +static int remoteDomainMemoryStats (virDomainPtr domain, struct _virDomainMemoryStat *stats, unsigned int nr_stats) @@ -6880,6 +6948,7 @@ static virDriver remote_driver = { .domainMigratePerform3 = remoteDomainMigratePerform3, /* 0.9.2 */ .domainMigrateFinish3 = remoteDomainMigrateFinish3, /* 0.9.2 */ .domainMigrateConfirm3 = remoteDomainMigrateConfirm3, /* 0.9.2 */ + .domainSetSchedulerParametersFlags = remoteDomainSetSchedulerParametersFlags, /* 0.9.2 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index e204eb0..7bf023d 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -476,6 +476,12 @@ struct remote_domain_set_scheduler_parameters_args { remote_sched_param params<REMOTE_DOMAIN_SCHEDULER_PARAMETERS_MAX>; }; +struct remote_domain_set_scheduler_parameters_flags_args { + remote_nonnull_domain dom; + remote_sched_param params<REMOTE_DOMAIN_SCHEDULER_PARAMETERS_MAX>; + unsigned int flags; +}; + struct remote_domain_set_blkio_parameters_args { remote_nonnull_domain dom; remote_blkio_param params<REMOTE_DOMAIN_BLKIO_PARAMETERS_MAX>; @@ -2284,7 +2290,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_MIGRATE_PREPARE_TUNNEL3 = 215, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_MIGRATE_PERFORM3 = 216, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_MIGRATE_FINISH3 = 217, /* skipgen skipgen */ - REMOTE_PROC_DOMAIN_MIGRATE_CONFIRM3 = 218 /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_MIGRATE_CONFIRM3 = 218, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_SET_SCHEDULER_PARAMETERS_FLAGS = 219 /* skipgen skipgen */ /* * Notice how the entries are grouped in sets of 10 ? -- 1.7.3.1
On 05/17/2011 12:20 AM, Hu Tao wrote:
support for virDomainSetSchedulerParametersFlags of remote driver. --- daemon/remote.c | 71 ++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 69 ++++++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 9 +++++- 3 files changed, 148 insertions(+), 1 deletions(-)
Missing a patch to src/remote_protocol-structs (on Fedora, if you install 'dwarves', which includes the pdwtags program, them 'make check' will catch this). ACK with this squashed in, so I'm pushing: diff --git i/src/remote_protocol-structs w/src/remote_protocol-structs index 28496dd..5b43cb4 100644 --- i/src/remote_protocol-structs +++ w/src/remote_protocol-structs @@ -188,6 +188,14 @@ struct remote_domain_set_scheduler_parameters_args { remote_sched_param * params_val; } params; }; +struct remote_domain_set_scheduler_parameters_flags_args { + remote_nonnull_domain dom; + struct { + u_int params_len; + remote_sched_param * params_val; + } params; + u_int flags; +}; struct remote_domain_set_blkio_parameters_args { remote_nonnull_domain dom; struct { -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
This enables user to modify cpu.shares even when domain is inactive. --- tools/virsh.c | 25 ++++++++++++++++++++++++- tools/virsh.pod | 6 +++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index a38750f..0e0f8cf 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1596,6 +1596,9 @@ static const vshCmdOptDef opts_schedinfo[] = { {"set", VSH_OT_STRING, VSH_OFLAG_NONE, N_("parameter=value")}, {"weight", VSH_OT_INT, VSH_OFLAG_NONE, N_("weight for XEN_CREDIT")}, {"cap", VSH_OT_INT, VSH_OFLAG_NONE, N_("cap for XEN_CREDIT")}, + {"current", VSH_OT_BOOL, 0, N_("get current scheduler info")}, + {"config", VSH_OT_BOOL, 0, N_("get value to be used on next boot")}, + {"live", VSH_OT_BOOL, 0, N_("get value from running domain")}, {NULL, 0, 0, NULL} }; @@ -1703,6 +1706,23 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd) int update = 0; int i, ret; bool ret_val = 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_SCHEDPARAM_CURRENT; + } else { + if (config) + flags |= VIR_DOMAIN_SCHEDPARAM_CONFIG; + if (live) + flags |= VIR_DOMAIN_SCHEDPARAM_LIVE; + } if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -1741,7 +1761,10 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd) /* Update parameters & refresh data */ if (update) { - ret = virDomainSetSchedulerParameters(dom, params, nparams); + if (flags) + ret = virDomainSetSchedulerParametersFlags(dom, params, nparams, flags); + else + ret = virDomainSetSchedulerParameters(dom, params, nparams); if (ret == -1) goto cleanup; diff --git a/tools/virsh.pod b/tools/virsh.pod index d11a0e3..98e8580 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -570,7 +570,7 @@ This is roughly equivalent to doing a hibernate on a running computer, with all the same limitations. Open network connections may be severed upon restore, as TCP timeouts may have expired. -=item B<schedinfo> optional I<--set> B<parameter=value> I<domain-id> +=item B<schedinfo> optional I<--set> B<parameter=value> I<domain-id> I<--persistent> =item B<schedinfo> optional I<--weight> B<number> optional I<--cap> B<number> I<domain-id> @@ -582,6 +582,10 @@ Xen (credit scheduler): weight, cap ESX (allocation scheduler): reservation, limit, shares +If I<--live> is specified, set scheduler information of 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. + B<Note>: The cpu_shares parameter has a valid value range of 0-262144; Negative values are wrapped to positive, and larger values are capped at the maximum. Therefore, -1 is a useful shorthand for 262144. -- 1.7.3.1
On 05/17/2011 12:20 AM, Hu Tao wrote:
This enables user to modify cpu.shares even when domain is inactive. --- tools/virsh.c | 25 ++++++++++++++++++++++++- tools/virsh.pod | 6 +++++- 2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index a38750f..0e0f8cf 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1596,6 +1596,9 @@ static const vshCmdOptDef opts_schedinfo[] = { {"set", VSH_OT_STRING, VSH_OFLAG_NONE, N_("parameter=value")}, {"weight", VSH_OT_INT, VSH_OFLAG_NONE, N_("weight for XEN_CREDIT")}, {"cap", VSH_OT_INT, VSH_OFLAG_NONE, N_("cap for XEN_CREDIT")}, + {"current", VSH_OT_BOOL, 0, N_("get current scheduler info")}, + {"config", VSH_OT_BOOL, 0, N_("get value to be used on next boot")}, + {"live", VSH_OT_BOOL, 0, N_("get value from running domain")},
Hmm, this says get, but right now it only affects set. We really need to add virDomainGetSchedulerParametersFlags.
{NULL, 0, 0, NULL} };
@@ -1703,6 +1706,23 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd) int update = 0; int i, ret; bool ret_val = 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_SCHEDPARAM_CURRENT;
But this is 0, ...
+ } else { + if (config) + flags |= VIR_DOMAIN_SCHEDPARAM_CONFIG; + if (live) + flags |= VIR_DOMAIN_SCHEDPARAM_LIVE; + }
if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -1741,7 +1761,10 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd)
/* Update parameters & refresh data */ if (update) { - ret = virDomainSetSchedulerParameters(dom, params, nparams); + if (flags) + ret = virDomainSetSchedulerParametersFlags(dom, params, nparams, flags);
so this needs to be flags || current
+ else + ret = virDomainSetSchedulerParameters(dom, params, nparams); if (ret == -1) goto cleanup;
diff --git a/tools/virsh.pod b/tools/virsh.pod index d11a0e3..98e8580 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -570,7 +570,7 @@ This is roughly equivalent to doing a hibernate on a running computer, with all the same limitations. Open network connections may be severed upon restore, as TCP timeouts may have expired.
-=item B<schedinfo> optional I<--set> B<parameter=value> I<domain-id> +=item B<schedinfo> optional I<--set> B<parameter=value> I<domain-id> I<--persistent>
Not how we spelled it in virsh.c...
=item B<schedinfo> optional I<--weight> B<number> optional I<--cap> B<number> I<domain-id>
@@ -582,6 +582,10 @@ Xen (credit scheduler): weight, cap
ESX (allocation scheduler): reservation, limit, shares
+If I<--live> is specified, set scheduler information of 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. + B<Note>: The cpu_shares parameter has a valid value range of 0-262144; Negative values are wrapped to positive, and larger values are capped at the maximum. Therefore, -1 is a useful shorthand for 262144.
ACK with this squashed in, so I'm pushing: diff --git i/tools/virsh.c w/tools/virsh.c index 15d9c7a..77cadcb 100644 --- i/tools/virsh.c +++ w/tools/virsh.c @@ -1596,9 +1596,9 @@ static const vshCmdOptDef opts_schedinfo[] = { {"set", VSH_OT_STRING, VSH_OFLAG_NONE, N_("parameter=value")}, {"weight", VSH_OT_INT, VSH_OFLAG_NONE, N_("weight for XEN_CREDIT")}, {"cap", VSH_OT_INT, VSH_OFLAG_NONE, N_("cap for XEN_CREDIT")}, - {"current", VSH_OT_BOOL, 0, N_("get current scheduler info")}, - {"config", VSH_OT_BOOL, 0, N_("get value to be used on next boot")}, - {"live", VSH_OT_BOOL, 0, N_("get value from running domain")}, + {"current", VSH_OT_BOOL, 0, N_("get/set current scheduler info")}, + {"config", VSH_OT_BOOL, 0, N_("get/set value to be used on next boot")}, + {"live", VSH_OT_BOOL, 0, N_("get/set value from running domain")}, {NULL, 0, 0, NULL} }; @@ -1732,7 +1732,7 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd) /* Print SchedulerType */ schedulertype = virDomainGetSchedulerType(dom, &nparams); - if (schedulertype!= NULL){ + if (schedulertype != NULL){ vshPrint(ctl, "%-15s: %s\n", _("Scheduler"), schedulertype); VIR_FREE(schedulertype); @@ -1761,8 +1761,9 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd) /* Update parameters & refresh data */ if (update) { - if (flags) - ret = virDomainSetSchedulerParametersFlags(dom, params, nparams, flags); + if (flags || current) + ret = virDomainSetSchedulerParametersFlags(dom, params, + nparams, flags); else ret = virDomainSetSchedulerParameters(dom, params, nparams); if (ret == -1) diff --git i/tools/virsh.pod w/tools/virsh.pod index 98e8580..ef01f41 100644 --- i/tools/virsh.pod +++ w/tools/virsh.pod @@ -570,11 +570,14 @@ This is roughly equivalent to doing a hibernate on a running computer, with all the same limitations. Open network connections may be severed upon restore, as TCP timeouts may have expired. -=item B<schedinfo> optional I<--set> B<parameter=value> I<domain-id> I<--persistent> +=item B<schedinfo> optional I<--set> B<parameter=value> I<domain-id> I<--config> +I<--live> I<--current> -=item B<schedinfo> optional I<--weight> B<number> optional I<--cap> B<number> I<domain-id> +=item B<schedinfo> optional I<--weight> B<number> optional I<--cap> B<number> +I<domain-id> -Allows you to show (and set) the domain scheduler parameters. The parameters available for each hypervisor are: +Allows you to show (and set) the domain scheduler parameters. The parameters +available for each hypervisor are: LXC, QEMU/KVM (posix scheduler): cpu_shares -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Thanks for your changes! I'll be more careful next time. -- Thanks, Hu Tao
participants (2)
-
Eric Blake -
Hu Tao