Re: [libvirt] [PATCH v2] Allow multiple parameters for schedinfo

virsh schedinfo was able to set only one parameter at a time (not counting the deprecated options), but it is useful to set more at once, so this patch adds the possibility to do stuff like this: virsh schedinfo <domain> cpu_shares=0 vcpu_period=0 vcpu_quota=0 \ emulator_period=0 emulator_quota=0 Invalid scheduler options are reported as well. These were previously reported only if the command hadn't updated any values (when cmdSchedInfoUpdate returned 0). Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=810078 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=919372 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=919375 Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; --- v2: - correctly report unsupported options - man page updated tests/virsh-schedinfo | 4 +- tools/virsh-domain.c | 119 ++++++++++++++++++++++++++++---------------------- tools/virsh.pod | 4 +- 3 files changed, 72 insertions(+), 55 deletions(-) diff --git a/tests/virsh-schedinfo b/tests/virsh-schedinfo index 4f462f8..37f7bd3 100755 --- a/tests/virsh-schedinfo +++ b/tests/virsh-schedinfo @@ -1,7 +1,7 @@ #!/bin/sh # Ensure that virsh schedinfo --set invalid=val fails -# Copyright (C) 2010-2011 Red Hat, Inc. +# Copyright (C) 2010-2011, 2013 Red Hat, Inc. # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -37,7 +37,7 @@ fi . "$srcdir/test-lib.sh" printf 'Scheduler : fair\n\n' > exp-out || framework_failure -printf 'error: invalid scheduler option: j=k\n' > exp-err || framework_failure +printf 'error: invalid scheduler option: j\n' > exp-err || framework_failure fail=0 diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 128e516..cc2eddc 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -3918,16 +3918,14 @@ static const vshCmdOptDef opts_schedinfo[] = { .flags = VSH_OFLAG_REQ, .help = N_("domain name, id or uuid") }, - {.name = "set", - .type = VSH_OT_STRING, - .help = N_("parameter=value") - }, {.name = "weight", .type = VSH_OT_INT, + .flags = VSH_OFLAG_REQ_OPT, .help = N_("weight for XEN_CREDIT") }, {.name = "cap", .type = VSH_OT_INT, + .flags = VSH_OFLAG_REQ_OPT, .help = N_("cap for XEN_CREDIT") }, {.name = "current", @@ -3942,72 +3940,100 @@ static const vshCmdOptDef opts_schedinfo[] = { .type = VSH_OT_BOOL, .help = N_("get/set value from running domain") }, + {.name = "set", + .type = VSH_OT_ARGV, + .flags = VSH_OFLAG_NONE, + .help = N_("parameter=value") + }, {.name = NULL} }; static int +cmdSchedInfoUpdateOne(vshControl *ctl, + virTypedParameterPtr src_params, int nsrc_params, + virTypedParameterPtr *params, + int *nparams, int *maxparams, + const char *field, const char *value) +{ + virTypedParameterPtr param; + int ret = -1; + int i; + + for (i = 0; i < nsrc_params; i++) { + param = &(src_params[i]); + + if (STRNEQ(field, param->field)) + continue; + + if (virTypedParamsAddFromString(params, nparams, maxparams, + field, param->type, + value) < 0) { + vshSaveLibvirtError(); + goto cleanup; + } + ret = 0; + break; + } + + if (ret < 0) + vshError(ctl, _("invalid scheduler option: %s"), field); + + cleanup: + return ret; +} + +static int cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd, virTypedParameterPtr src_params, int nsrc_params, virTypedParameterPtr *update_params) { - const char *set_arg; char *set_field = NULL; char *set_val = NULL; - virTypedParameterPtr param; + const char *val = NULL; + const vshCmdOpt *opt = NULL; virTypedParameterPtr params = NULL; int nparams = 0; int maxparams = 0; int ret = -1; int rv; - int val; - int i; - if (vshCommandOptString(cmd, "set", &set_arg) > 0) { - set_field = vshStrdup(ctl, set_arg); + while ((opt = vshCommandOptArgv(cmd, opt))) { + set_field = vshStrdup(ctl, opt->data); if (!(set_val = strchr(set_field, '='))) { - vshError(ctl, "%s", _("Invalid syntax for --set, expecting name=value")); + vshError(ctl, "%s", _("Invalid syntax for --set, " + "expecting name=value")); goto cleanup; } *set_val = '\0'; set_val++; - } - for (i = 0; i < nsrc_params; i++) { - param = &(src_params[i]); - - /* Legacy 'weight' and 'cap' parameter */ - if (param->type == VIR_TYPED_PARAM_UINT && - (STREQ(param->field, "weight") || STREQ(param->field, "cap")) && - (rv = vshCommandOptInt(cmd, param->field, &val)) != 0) { - if (rv < 0) { - vshError(ctl, _("Invalid value of %s"), param->field); - goto cleanup; - } - - if (virTypedParamsAddUInt(&params, &nparams, &maxparams, - param->field, val) < 0) { - vshSaveLibvirtError(); - goto cleanup; - } + if (cmdSchedInfoUpdateOne(ctl, src_params, nsrc_params, + &params, &nparams, &maxparams, + set_field, set_val) < 0) + goto cleanup; - continue; - } + VIR_FREE(set_field); + } - if (set_field && STREQ(set_field, param->field)) { - if (virTypedParamsAddFromString(&params, &nparams, &maxparams, - set_field, param->type, - set_val) < 0) { - vshSaveLibvirtError(); - goto cleanup; - } + rv = vshCommandOptStringReq(ctl, cmd, "cap", &val); + if (rv < 0 || + (val && + cmdSchedInfoUpdateOne(ctl, src_params, nsrc_params, + &params, &nparams, &maxparams, + "cap", val) < 0)) + goto cleanup; - continue; - } - } + rv = vshCommandOptStringReq(ctl, cmd, "weight", &val); + if (rv < 0 || + (val && + cmdSchedInfoUpdateOne(ctl, src_params, nsrc_params, + &params, &nparams, &maxparams, + "weight", val) < 0)) + goto cleanup; - *update_params = params; ret = nparams; + *update_params = params; params = NULL; cleanup: @@ -4102,15 +4128,6 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd) if (ret == -1) goto cleanup; } else { - /* See if we've tried to --set var=val. If so, the fact that - we reach this point (with update == 0) means that "var" did - not match any of the settable parameters. Report the error. */ - const char *var_value_pair = NULL; - if (vshCommandOptString(cmd, "set", &var_value_pair) > 0) { - vshError(ctl, _("invalid scheduler option: %s"), - var_value_pair); - goto cleanup; - } /* When not doing --set, --live and --config do not mix. */ if (live && config) { vshError(ctl, "%s", diff --git a/tools/virsh.pod b/tools/virsh.pod index b5e632e..6111e58 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1281,8 +1281,8 @@ except that it does some error checking. The editor used can be supplied by the C<$VISUAL> or C<$EDITOR> environment variables, and defaults to C<vi>. -=item B<schedinfo> [I<--set> B<parameter=value>] I<domain> [[I<--config>] -[I<--live>] | [I<--current>]] +=item B<schedinfo> I<domain> [[I<--config>] [I<--live>] | [I<--current>]] +[[I<--set>] B<parameter=value>]... =item B<schedinfo> [I<--weight> B<number>] [I<--cap> B<number>] I<domain> -- 1.8.1.5 -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list