[libvirt] [PATCH v2 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: 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 --persistent to cmd schedinfo daemon/remote.c | 71 ++++++++++++++++++++++++++++++++++++++++++ daemon/remote_generator.pl | 2 + include/libvirt/libvirt.h.in | 13 ++++++++ python/generator.py | 1 + src/driver.h | 8 +++++ src/esx/esx_driver.c | 1 + src/libvirt.c | 62 ++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++ src/libxl/libxl_driver.c | 1 + src/lxc/lxc_driver.c | 1 + src/openvz/openvz_driver.c | 1 + src/phyp/phyp_driver.c | 1 + src/qemu/qemu_driver.c | 66 ++++++++++++++++++++++++++++++--------- src/remote/remote_driver.c | 69 ++++++++++++++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 10 +++++- src/test/test_driver.c | 1 + src/uml/uml_driver.c | 1 + src/vbox/vbox_tmpl.c | 1 + src/vmware/vmware_driver.c | 1 + src/xen/xen_driver.c | 1 + src/xenapi/xenapi_driver.c | 1 + tools/virsh.c | 14 ++++++++- 22 files changed, 315 insertions(+), 17 deletions(-) -- 1.7.3.1 -- Thanks, Hu Tao

This new function allows aditional flags to be passed into from the virsh command line. --- include/libvirt/libvirt.h.in | 13 +++++++++ python/generator.py | 1 + src/driver.h | 8 +++++ src/esx/esx_driver.c | 1 + src/libvirt.c | 62 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++ src/libxl/libxl_driver.c | 1 + src/lxc/lxc_driver.c | 1 + src/openvz/openvz_driver.c | 1 + src/phyp/phyp_driver.c | 1 + src/qemu/qemu_driver.c | 1 + src/remote/remote_driver.c | 1 + src/test/test_driver.c | 1 + src/uml/uml_driver.c | 1 + src/vbox/vbox_tmpl.c | 1 + src/vmware/vmware_driver.c | 1 + src/xen/xen_driver.c | 1 + src/xenapi/xenapi_driver.c | 1 + 18 files changed, 102 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5783303..2c38118 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -243,6 +243,11 @@ typedef enum { VIR_DOMAIN_SCHED_FIELD_BOOLEAN = 6 /* boolean(character) case */ } virSchedParameterType; +typedef enum { + VIR_DOMAIN_SCHEDPARAM_LIVE = (1 << 0), /* Affect active domain */ + VIR_DOMAIN_SCHEDPARAM_CONFIG = (1 << 1), /* Affect next boot */ +} virDomainSchedParameterFlags; + /** * VIR_DOMAIN_SCHED_FIELD_LENGTH: * @@ -294,6 +299,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 4fa4f65..a44a4d0 100755 --- a/python/generator.py +++ b/python/generator.py @@ -311,6 +311,7 @@ skip_impl = ( 'virDomainGetSchedulerType', 'virDomainGetSchedulerParameters', 'virDomainSetSchedulerParameters', + 'virDomainSetSchedulerParametersFlags', 'virDomainSetBlkioParameters', 'virDomainGetBlkioParameters', 'virDomainSetMemoryParameters', diff --git a/src/driver.h b/src/driver.h index a8b79e6..3347a53 100644 --- a/src/driver.h +++ b/src/driver.h @@ -276,6 +276,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, @@ -593,6 +600,7 @@ struct _virDriver { virDrvDomainGetSchedulerType domainGetSchedulerType; virDrvDomainGetSchedulerParameters domainGetSchedulerParameters; virDrvDomainSetSchedulerParameters domainSetSchedulerParameters; + virDrvDomainSetSchedulerParametersFlags domainSetSchedulerParametersFlags; virDrvDomainMigratePrepare domainMigratePrepare; virDrvDomainMigratePerform domainMigratePerform; virDrvDomainMigrateFinish domainMigrateFinish; diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 7933f11..181d6aa 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -4653,6 +4653,7 @@ static virDriver esxDriver = { esxDomainGetSchedulerType, /* domainGetSchedulerType */ esxDomainGetSchedulerParameters, /* domainGetSchedulerParameters */ esxDomainSetSchedulerParameters, /* domainSetSchedulerParameters */ + NULL, /* domainSetSchedulerParametersFlags */ esxDomainMigratePrepare, /* domainMigratePrepare */ esxDomainMigratePerform, /* domainMigratePerform */ esxDomainMigrateFinish, /* domainMigrateFinish */ diff --git a/src/libvirt.c b/src/libvirt.c index e74e977..92e804e 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -4409,6 +4409,68 @@ 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))) { + 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 b4aed41..03d08f1 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -436,4 +436,9 @@ LIBVIRT_0.9.0 { virStorageVolUpload; } LIBVIRT_0.8.8; +LIBVIRT_0.9.1 { + global: + virDomainSetSchedulerParametersFlags; +} LIBVIRT_0.9.0; + # .... define new API here using predicted next version number .... diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index a2c8467..9488925 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2744,6 +2744,7 @@ static virDriver libxlDriver = { libxlDomainGetSchedulerType,/* domainGetSchedulerType */ libxlDomainGetSchedulerParameters,/* domainGetSchedulerParameters */ libxlDomainSetSchedulerParameters,/* domainSetSchedulerParameters */ + NULL, /* domainSetSchedulerParametersFlags */ NULL, /* domainMigratePrepare */ NULL, /* domainMigratePerform */ NULL, /* domainMigrateFinish */ diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index b94941d..e8afcd3 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2859,6 +2859,7 @@ static virDriver lxcDriver = { lxcGetSchedulerType, /* domainGetSchedulerType */ lxcGetSchedulerParameters, /* domainGetSchedulerParameters */ lxcSetSchedulerParameters, /* domainSetSchedulerParameters */ + NULL, /* domainSetSchedulerParametersFlags */ NULL, /* domainMigratePrepare */ NULL, /* domainMigratePerform */ NULL, /* domainMigrateFinish */ diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 0bd007a..1c11e73 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1621,6 +1621,7 @@ static virDriver openvzDriver = { NULL, /* domainGetSchedulerType */ NULL, /* domainGetSchedulerParameters */ NULL, /* domainSetSchedulerParameters */ + NULL, /* domainSetSchedulerParametersFlags */ NULL, /* domainMigratePrepare */ NULL, /* domainMigratePerform */ NULL, /* domainMigrateFinish */ diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 30d4adf..d2f30d2 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -3782,6 +3782,7 @@ static virDriver phypDriver = { NULL, /* domainGetSchedulerType */ NULL, /* domainGetSchedulerParameters */ NULL, /* domainSetSchedulerParameters */ + NULL, /* domainSetSchedulerParametersFlags */ NULL, /* domainMigratePrepare */ NULL, /* domainMigratePerform */ NULL, /* domainMigrateFinish */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0fd0f10..5f3167a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7146,6 +7146,7 @@ static virDriver qemuDriver = { qemuGetSchedulerType, /* domainGetSchedulerType */ qemuGetSchedulerParameters, /* domainGetSchedulerParameters */ qemuSetSchedulerParameters, /* domainSetSchedulerParameters */ + NULL, /* domainSetSchedulerParametersFlags */ NULL, /* domainMigratePrepare (v1) */ qemudDomainMigratePerform, /* domainMigratePerform */ NULL, /* domainMigrateFinish */ diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index d076a90..3e00880 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6447,6 +6447,7 @@ static virDriver remote_driver = { remoteDomainGetSchedulerType, /* domainGetSchedulerType */ remoteDomainGetSchedulerParameters, /* domainGetSchedulerParameters */ remoteDomainSetSchedulerParameters, /* domainSetSchedulerParameters */ + NULL, /* domainSetSchedulerParametersFlags */ remoteDomainMigratePrepare, /* domainMigratePrepare */ remoteDomainMigratePerform, /* domainMigratePerform */ remoteDomainMigrateFinish, /* domainMigrateFinish */ diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 0978214..abe4283 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5401,6 +5401,7 @@ static virDriver testDriver = { testDomainGetSchedulerType, /* domainGetSchedulerType */ testDomainGetSchedulerParams, /* domainGetSchedulerParameters */ testDomainSetSchedulerParams, /* domainSetSchedulerParameters */ + NULL, /* domainSetSchedulerParametersFlags */ NULL, /* domainMigratePrepare */ NULL, /* domainMigratePerform */ NULL, /* domainMigrateFinish */ diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 33849a0..852e066 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -2207,6 +2207,7 @@ static virDriver umlDriver = { NULL, /* domainGetSchedulerType */ NULL, /* domainGetSchedulerParameters */ NULL, /* domainSetSchedulerParameters */ + NULL, /* domainSetSchedulerParametersFlags */ NULL, /* domainMigratePrepare */ NULL, /* domainMigratePerform */ NULL, /* domainMigrateFinish */ diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 8241d34..6ec0f1a 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -8596,6 +8596,7 @@ virDriver NAME(Driver) = { NULL, /* domainGetSchedulerType */ NULL, /* domainGetSchedulerParameters */ NULL, /* domainSetSchedulerParameters */ + NULL, /* domainSetSchedulerParametersFlags */ NULL, /* domainMigratePrepare */ NULL, /* domainMigratePerform */ NULL, /* domainMigrateFinish */ diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index bbfb1a4..fc667fe 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -961,6 +961,7 @@ static virDriver vmwareDriver = { NULL, /* domainGetSchedulerType */ NULL, /* domainGetSchedulerParameters */ NULL, /* domainSetSchedulerParameters */ + NULL, /* domainSetSchedulerParametersFlags */ NULL, /* domainMigratePrepare */ NULL, /* domainMigratePerform */ NULL, /* domainMigrateFinish */ diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index dd94fbc..41b48ae 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -2162,6 +2162,7 @@ static virDriver xenUnifiedDriver = { xenUnifiedDomainGetSchedulerType, /* domainGetSchedulerType */ xenUnifiedDomainGetSchedulerParameters, /* domainGetSchedulerParameters */ xenUnifiedDomainSetSchedulerParameters, /* domainSetSchedulerParameters */ + NULL, /* domainSetSchedulerParametersFlags */ xenUnifiedDomainMigratePrepare, /* domainMigratePrepare */ xenUnifiedDomainMigratePerform, /* domainMigratePerform */ xenUnifiedDomainMigrateFinish, /* domainMigrateFinish */ diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index 3fbdcc6..a57c98f 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -1843,6 +1843,7 @@ static virDriver xenapiDriver = { xenapiDomainGetSchedulerType, /* domainGetSchedulerType */ NULL, /* domainGetSchedulerParameters */ NULL, /* domainSetSchedulerParameters */ + NULL, /* domainSetSchedulerParametersFlags */ NULL, /* domainMigratePrepare */ NULL, /* domainMigratePerform */ NULL, /* domainMigrateFinish */ -- 1.7.3.1 -- Thanks, Hu Tao

Support for virDomainSetSchedulerParametersFlags of qemu driver. --- src/qemu/qemu_driver.c | 67 ++++++++++++++++++++++++++++++++++++----------- 1 files changed, 51 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5f3167a..9ba1e56 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4770,7 +4770,7 @@ static int qemuDomainGetMemoryParameters(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; @@ -4864,16 +4864,21 @@ 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; + virCheckFlags(VIR_DOMAIN_SCHEDPARAM_LIVE | + VIR_DOMAIN_SCHEDPARAM_CONFIG, -1); + qemuDriverLock(driver); if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) { qemuReportError(VIR_ERR_OPERATION_INVALID, @@ -4889,13 +4894,14 @@ static int qemuSetSchedulerParameters(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; } - if (virCgroupForDomain(driver->cgroup, vm->def->name, &group, 0) != 0) { + if ((flags & VIR_DOMAIN_SCHEDPARAM_LIVE) && + 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; @@ -4912,20 +4918,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: @@ -4936,6 +4961,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) @@ -4969,9 +5004,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) { @@ -4986,6 +5020,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) { @@ -7146,7 +7181,7 @@ static virDriver qemuDriver = { qemuGetSchedulerType, /* domainGetSchedulerType */ qemuGetSchedulerParameters, /* domainGetSchedulerParameters */ qemuSetSchedulerParameters, /* domainSetSchedulerParameters */ - NULL, /* domainSetSchedulerParametersFlags */ + qemuSetSchedulerParametersFlags, /* domainSetSchedulerParametersFlags */ NULL, /* domainMigratePrepare (v1) */ qemudDomainMigratePerform, /* domainMigratePerform */ NULL, /* domainMigrateFinish */ -- 1.7.3.1 -- Thanks, Hu Tao

support for virDomainSetSchedulerParametersFlags of remote driver. --- daemon/remote.c | 71 ++++++++++++++++++++++++++++++++++++++++++ daemon/remote_generator.pl | 2 + src/remote/remote_driver.c | 70 ++++++++++++++++++++++++++++++++++++++++- src/remote/remote_protocol.x | 10 +++++- 4 files changed, 151 insertions(+), 2 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 2220655..b047f9f 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -687,6 +687,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/daemon/remote_generator.pl b/daemon/remote_generator.pl index 062ccc1..ccefbcc 100755 --- a/daemon/remote_generator.pl +++ b/daemon/remote_generator.pl @@ -249,6 +249,7 @@ elsif ($opt_b) { "DomainOpenConsole", "DomainPinVcpu", "DomainSetSchedulerParameters", + "DomainSetSchedulerParametersFlags", "DomainSetMemoryParameters", "DomainSetBlkioParameters", "Open", @@ -738,6 +739,7 @@ elsif ($opt_k) { "DomainMigratePrepareTunnel", "DomainOpenConsole", "DomainSetSchedulerParameters", + "DomainSetSchedulerParametersFlags", "DomainSetMemoryParameters", "DomainSetBlkioParameters", "Open", diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 3e00880..544c401 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2698,6 +2698,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) @@ -6447,7 +6515,7 @@ static virDriver remote_driver = { remoteDomainGetSchedulerType, /* domainGetSchedulerType */ remoteDomainGetSchedulerParameters, /* domainGetSchedulerParameters */ remoteDomainSetSchedulerParameters, /* domainSetSchedulerParameters */ - NULL, /* domainSetSchedulerParametersFlags */ + remoteDomainSetSchedulerParametersFlags, /* domainSetSchedulerParametersFlags */ remoteDomainMigratePrepare, /* domainMigratePrepare */ remoteDomainMigratePerform, /* domainMigratePerform */ remoteDomainMigrateFinish, /* domainMigrateFinish */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index c706c36..824605b 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>; @@ -2176,7 +2182,9 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_GET_BLKIO_PARAMETERS = 206, REMOTE_PROC_DOMAIN_MIGRATE_SET_MAX_SPEED = 207, REMOTE_PROC_STORAGE_VOL_UPLOAD = 208, - REMOTE_PROC_STORAGE_VOL_DOWNLOAD = 209 + REMOTE_PROC_STORAGE_VOL_DOWNLOAD = 209, + + REMOTE_PROC_DOMAIN_SET_SCHEDULER_PARAMETERS_FLAGS = 210 /* * Notice how the entries are grouped in sets of 10 ? -- 1.7.3.1 -- Thanks, Hu Tao

This enables user to modify cpu.shares even when domain is inactive. --- tools/virsh.c | 14 +++++++++++++- 1 files changed, 13 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 2b16714..58facc4 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1590,6 +1590,7 @@ 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")}, + {"persistent", VSH_OT_BOOL, 0, N_("persist VM on destination")}, {NULL, 0, 0, NULL} }; @@ -1697,6 +1698,7 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd) int update = 0; int i, ret; bool ret_val = false; + unsigned int flags = 0; if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -1704,6 +1706,16 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; + if (vshCommandOptBool(cmd, "persistent")) + flags |= VIR_DOMAIN_SCHEDPARAM_CONFIG; + if (virDomainIsActive(dom)) + flags |= VIR_DOMAIN_SCHEDPARAM_LIVE; + + if (!(flags & (VIR_DOMAIN_SCHEDPARAM_CONFIG | VIR_DOMAIN_SCHEDPARAM_LIVE))) { + vshError(ctl, "%s", _("Domain is not running and you didn't specify --persistent parameter.")); + goto cleanup; + } + /* Print SchedulerType */ schedulertype = virDomainGetSchedulerType(dom, &nparams); if (schedulertype!= NULL){ @@ -1735,7 +1747,7 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd) /* Update parameters & refresh data */ if (update) { - ret = virDomainSetSchedulerParameters(dom, params, nparams); + ret = virDomainSetSchedulerParametersFlags(dom, params, nparams, flags); if (ret == -1) goto cleanup; -- 1.7.3.1 -- Thanks, Hu Tao

于 2011年05月09日 16:31, Hu Tao 写道:
This enables user to modify cpu.shares even when domain is inactive. --- tools/virsh.c | 14 +++++++++++++- 1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 2b16714..58facc4 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1590,6 +1590,7 @@ 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")}, + {"persistent", VSH_OT_BOOL, 0, N_("persist VM on destination")}, {NULL, 0, 0, NULL} };
@@ -1697,6 +1698,7 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd) int update = 0; int i, ret; bool ret_val = false; + unsigned int flags = 0;
if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -1704,6 +1706,16 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false;
+ if (vshCommandOptBool(cmd, "persistent")) + flags |= VIR_DOMAIN_SCHEDPARAM_CONFIG; + if (virDomainIsActive(dom)) + flags |= VIR_DOMAIN_SCHEDPARAM_LIVE; + + if (!(flags& (VIR_DOMAIN_SCHEDPARAM_CONFIG | VIR_DOMAIN_SCHEDPARAM_LIVE))) { + vshError(ctl, "%s", _("Domain is not running and you didn't specify --persistent parameter.")); + goto cleanup; + } + /* Print SchedulerType */ schedulertype = virDomainGetSchedulerType(dom,&nparams);
Hi HuTao, AFAIK, virDomainGetSchedulerParameters won't work for inactive domain, cgroup for domain is removed when the domain is destroyed/shutdown'ed, (perhaps we will support persistent XML for domain cgroup, but currently it doesn't), means it will fail earlier, before could executing virDomainSetSchedulerParametersFlags. Actually I made similar patches about one month before: https://www.redhat.com/archives/libvir-list/2011-April/msg00806.html the difference with your patches is it only allows changing on running domain, options are "--live" and "--persistent", perhaps your principle is better though. :)
if (schedulertype!= NULL){ @@ -1735,7 +1747,7 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd)
/* Update parameters& refresh data */ if (update) { - ret = virDomainSetSchedulerParameters(dom, params, nparams); + ret = virDomainSetSchedulerParametersFlags(dom, params, nparams, flags); if (ret == -1) goto cleanup;
Regards Osier

On Mon, May 09, 2011 at 05:37:04PM +0800, Osier Yang wrote:
于 2011年05月09日 16:31, Hu Tao 写道:
This enables user to modify cpu.shares even when domain is inactive. --- tools/virsh.c | 14 +++++++++++++- 1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 2b16714..58facc4 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1590,6 +1590,7 @@ 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")}, + {"persistent", VSH_OT_BOOL, 0, N_("persist VM on destination")}, {NULL, 0, 0, NULL} };
@@ -1697,6 +1698,7 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd) int update = 0; int i, ret; bool ret_val = false; + unsigned int flags = 0;
if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -1704,6 +1706,16 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false;
+ if (vshCommandOptBool(cmd, "persistent")) + flags |= VIR_DOMAIN_SCHEDPARAM_CONFIG; + if (virDomainIsActive(dom)) + flags |= VIR_DOMAIN_SCHEDPARAM_LIVE; + + if (!(flags& (VIR_DOMAIN_SCHEDPARAM_CONFIG | VIR_DOMAIN_SCHEDPARAM_LIVE))) { + vshError(ctl, "%s", _("Domain is not running and you didn't specify --persistent parameter.")); + goto cleanup; + } + /* Print SchedulerType */ schedulertype = virDomainGetSchedulerType(dom,&nparams);
Hi HuTao,
AFAIK, virDomainGetSchedulerParameters won't work for inactive domain, cgroup for domain is removed when the domain is destroyed/shutdown'ed, (perhaps we will support persistent XML for domain cgroup, but currently it doesn't), means it will fail earlier, before could executing virDomainSetSchedulerParametersFlags.
Yes, virDomainGetSchedulerParameters tries to read cpu.shares value from domain's cgroup entry which doesn't exist when domain is off. But, IMHO, the configuration (cpu.shares) should not depend on whether domain is on or not. When domain is off, we can read/write the value from/to its XML(domain.cputune.shares is already for this). What's your opinion?
Actually I made similar patches about one month before:
https://www.redhat.com/archives/libvir-list/2011-April/msg00806.html
Thanks, I will take a look at it.
the difference with your patches is it only allows changing on running domain, options are "--live" and "--persistent", perhaps your principle is better though. :)

于 2011年05月10日 09:31, Hu Tao 写道:
On Mon, May 09, 2011 at 05:37:04PM +0800, Osier Yang wrote:
于 2011年05月09日 16:31, Hu Tao 写道:
This enables user to modify cpu.shares even when domain is inactive. --- tools/virsh.c | 14 +++++++++++++- 1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 2b16714..58facc4 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1590,6 +1590,7 @@ 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")}, + {"persistent", VSH_OT_BOOL, 0, N_("persist VM on destination")}, {NULL, 0, 0, NULL} };
@@ -1697,6 +1698,7 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd) int update = 0; int i, ret; bool ret_val = false; + unsigned int flags = 0;
if (!vshConnectionUsability(ctl, ctl->conn)) return false; @@ -1704,6 +1706,16 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false;
+ if (vshCommandOptBool(cmd, "persistent")) + flags |= VIR_DOMAIN_SCHEDPARAM_CONFIG; + if (virDomainIsActive(dom)) + flags |= VIR_DOMAIN_SCHEDPARAM_LIVE; + + if (!(flags& (VIR_DOMAIN_SCHEDPARAM_CONFIG | VIR_DOMAIN_SCHEDPARAM_LIVE))) { + vshError(ctl, "%s", _("Domain is not running and you didn't specify --persistent parameter.")); + goto cleanup; + } + /* Print SchedulerType */ schedulertype = virDomainGetSchedulerType(dom,&nparams);
Hi HuTao,
AFAIK, virDomainGetSchedulerParameters won't work for inactive domain, cgroup for domain is removed when the domain is destroyed/shutdown'ed, (perhaps we will support persistent XML for domain cgroup, but currently it doesn't), means it will fail earlier, before could executing virDomainSetSchedulerParametersFlags.
Yes, virDomainGetSchedulerParameters tries to read cpu.shares value from domain's cgroup entry which doesn't exist when domain is off. But, IMHO, the configuration (cpu.shares) should not depend on whether domain is on or not. When domain is off, we can read/write the value from/to its XML(domain.cputune.shares is already for this). What's your opinion?
Yes, we have persistent XML for cpu.shares now, we can change it even if domain is off, the problem is cmdSchedinfo mixes get/set schedParams together, it needs to use virDomainGetSchedulerParameters to get the schedParams the driver suppports, then looks up to see if there is something is set to be updated, (it's reasonable to do like so, as perhaps more driver will support scheduler setting, and more scheduler setting will be supported, though currently only cpu.shares is supported by only QEMU/LXC, we can't hardcode it). but virDomainGetSchedulerParameters can't work when domain is off. We can seperate get/set schedParams into two standalone functions, so that could form the params with anything the user provides, and simply pass it to internal driver function, it reports error if user provides wrong params feild. Then we can do changing on inactive domain. But I guess this won't be accepted, as "virsh schedinfo" exists for long time, changing the behaviour will affects existed scripts that based on virsh. That was my mainly concern.
Actually I made similar patches about one month before:
https://www.redhat.com/archives/libvir-list/2011-April/msg00806.html
Thanks, I will take a look at it.
the difference with your patches is it only allows changing on running domain, options are "--live" and "--persistent", perhaps your principle is better though. :)
participants (2)
-
Hu Tao
-
Osier Yang