[libvirt] [PATCH] virsh: schedinfo --set invalid=value would simply ignore the option

[Jim Meyering]

Currently, virsh does this:

    $ virsh -c test:///default schedinfo 1 --set j=k>/dev/null && echo bad
    bad

It should have diagnosed "j=k" as an invalid schedinfo --set argument.
With the patch below, it does:

    $ ./virsh -c test:///default schedinfo 1 --set j=k>/dev/null && echo bad
    error: invalid scheduler option: j=k
    [Exit 1]



>From cf56648333c4e5cf73ab3292ba727e33b0ae9708 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Tue, 11 May 2010 15:38:21 +0200
Subject: [PATCH] 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
---
 tests/Makefile.am     |    1 +
 tests/virsh-schedinfo |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
 tools/virsh.c         |   10 ++++++++++
 3 files changed, 60 insertions(+), 0 deletions(-)
 create mode 100755 tests/virsh-schedinfo

diff --git a/tests/Makefile.am b/tests/Makefile.am
index ef12386..b5e09e3 100644
--- 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
index 0000000..116014a
--- /dev/null
+++ b/tests/virsh-schedinfo
@@ -0,0 +1,49 @@
+#!/bin/sh
+# Ensure that virsh schedinfo --set invalid=val fails
+
+# Copyright (C) 2010 Free Software Foundation, 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 0a63f1b..99f383a 100644
--- 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;
--
1.7.1.189.g07419