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

diff --git a/tests/virsh-schedinfo b/tests/virsh-schedinfo
index 4f462f87a0f945f2f89943ad3fe7ec27768cdd30..37f7bd3550e598fced012d1d6ca728ea1a69a6c7 100755 (executable)
--- 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 92796601a240d05d92db599a85d051335612d33f..c0884689f61e3594b2432750c34efe151104dabd 100644 (file)
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -3904,16 +3904,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",
@@ -3928,72 +3926,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:
@@ -4083,15 +4109,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 578b87808d3d962207421cb63267649f7a3ec63c..9ca4435a104625d2baf007e6f5fa09b59398515b 100644 (file)
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1307,8 +1307,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>