virsh: schedinfo --set invalid=value would simply ignore the option

For example, virsh -c test:///default schedinfo 1 --set P=k would
mistakenly exit successfully, giving no indication that it had failed
to set the scheduling parameter "P".
* tools/virsh.c (cmdSchedinfo): Diagnose an invalid --set j=k option,
rather than silently ignoring it.
* tests/virsh-schedinfo: New test for the above.
* tests/Makefile.am (test_scripts): Add it.
Reported by Jintao Yang in http://bugzilla.redhat.com/586632

diff --git a/tests/Makefile.am b/tests/Makefile.am
index ef12386cdd162fde2911b404ba3dca48dfc1fe24..b5e09e35b95abed6dbb75bdb6217287c2367b0fb 100644 (file)
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -139,6 +139,7 @@ test_scripts +=                             \
        undefine                        \
        vcpupin                         \
        virsh-all                       \
+       virsh-schedinfo                 \
        virsh-synopsis
 endif
 

diff --git a/tests/virsh-schedinfo b/tests/virsh-schedinfo
new file mode 100755 (executable)
index 0000000..b276a2e
--- /dev/null
+++ b/tests/virsh-schedinfo
@@ -0,0 +1,49 @@
+#!/bin/sh
+# Ensure that virsh schedinfo --set invalid=val fails
+
+# Copyright (C) 2010 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
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+: ${srcdir=$(pwd)}
+: ${abs_top_srcdir=$(pwd)/..}
+: ${abs_top_builddir=$(pwd)/..}
+
+# If $abs_top_builddir/tools/virsh is not early in $PATH, put it there,
+# so that we can safely invoke "virsh" simply with its name.
+case $PATH in
+  $abs_top_builddir/tools/src:$abs_top_builddir/tools/virsh:*) ;;
+  $abs_top_builddir/tools/virsh:*) ;;
+  *) PATH=$abs_top_builddir/tools/virsh:$PATH; export PATH ;;
+esac
+
+if test "$VERBOSE" = yes; then
+  set -x
+  $abs_top_builddir/tools/virsh --version
+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
+
+fail=0
+
+test_url=test:///default
+
+virsh -c $test_url schedinfo 1 --set j=k >out 2>err && fail=1
+compare out exp-out || fail=1
+compare err exp-err || fail=1
+
+(exit $fail); exit $fail

diff --git a/tools/virsh.c b/tools/virsh.c
index b9bf06d0abab8a8655d6da39ecbfef7b3143299e..21325c3e54cee843f6990fa9c912e92df8f575fd 100644 (file)
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1594,6 +1594,16 @@ cmdSchedinfo(vshControl *ctl, const vshCmd *cmd)
             ret = virDomainGetSchedulerParameters(dom, params, &nparams);
             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.  */
+            char *var_value_pair = vshCommandOptString(cmd, "set", NULL);
+            if (var_value_pair) {
+                vshError(ctl, _("invalid scheduler option: %s"),
+                         var_value_pair);
+                goto cleanup;
+            }
         }
 
         ret_val = TRUE;