Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2015-02-05' into staging

qmp hmp balloon: Cleanups around error reporting

# gpg: Signature made Thu 05 Feb 2015 07:15:11 GMT using RSA key ID EB918653
# gpg: Good signature from "Markus Armbruster <armbru@redhat.com>"
# gpg:                 aka "Markus Armbruster <armbru@pond.sub.org>"

* remotes/armbru/tags/pull-error-2015-02-05:
  balloon: Eliminate silly QERR_ macros
  balloon: Factor out common "is balloon active" test
  balloon: Inline qemu_balloon(), qemu_balloon_status()
  qmp: Eliminate silly QERR_COMMAND_NOT_FOUND macro
  qmp: Simplify recognition of capability negotiation command
  qmp: Clean up qmp_query_spice() #ifndef !CONFIG_SPICE dummy
  hmp: Compile hmp_info_spice() only with CONFIG_SPICE
  qmp hmp: Improve error messages when SPICE is not in use
  qmp hmp: Factor out common "using spice" test

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

7 files changed, 58 insertions, 71 deletions

diff --git a/balloon.c b/balloon.c
index b70da4fa29..dea19a470a 100644
--- a/balloon.c
+++ b/balloon.c
@@ -36,6 +36,21 @@ static QEMUBalloonEvent *balloon_event_fn;
 static QEMUBalloonStatus *balloon_stat_fn;
 static void *balloon_opaque;
 
+static bool have_ballon(Error **errp)
+{
+    if (kvm_enabled() && !kvm_has_sync_mmu()) {
+        error_set(errp, ERROR_CLASS_KVM_MISSING_CAP,
+                  "Using KVM without synchronous MMU, balloon unavailable");
+        return false;
+    }
+    if (!balloon_event_fn) {
+        error_set(errp, ERROR_CLASS_DEVICE_NOT_ACTIVE,
+                  "No balloon device has been activated");
+        return false;
+    }
+    return true;
+}
+
 int qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
                              QEMUBalloonStatus *stat_func, void *opaque)
 {
@@ -62,58 +77,30 @@ void qemu_remove_balloon_handler(void *opaque)
     balloon_opaque = NULL;
 }
 
-static int qemu_balloon(ram_addr_t target)
-{
-    if (!balloon_event_fn) {
-        return 0;
-    }
-    trace_balloon_event(balloon_opaque, target);
-    balloon_event_fn(balloon_opaque, target);
-    return 1;
-}
-
-static int qemu_balloon_status(BalloonInfo *info)
-{
-    if (!balloon_stat_fn) {
-        return 0;
-    }
-    balloon_stat_fn(balloon_opaque, info);
-    return 1;
-}
-
 BalloonInfo *qmp_query_balloon(Error **errp)
 {
     BalloonInfo *info;
 
-    if (kvm_enabled() && !kvm_has_sync_mmu()) {
-        error_set(errp, QERR_KVM_MISSING_CAP, "synchronous MMU", "balloon");
+    if (!have_ballon(errp)) {
         return NULL;
     }
 
     info = g_malloc0(sizeof(*info));
-
-    if (qemu_balloon_status(info) == 0) {
-        error_set(errp, QERR_DEVICE_NOT_ACTIVE, "balloon");
-        qapi_free_BalloonInfo(info);
-        return NULL;
-    }
-
+    balloon_stat_fn(balloon_opaque, info);
     return info;
 }
 
-void qmp_balloon(int64_t value, Error **errp)
+void qmp_balloon(int64_t target, Error **errp)
 {
-    if (kvm_enabled() && !kvm_has_sync_mmu()) {
-        error_set(errp, QERR_KVM_MISSING_CAP, "synchronous MMU", "balloon");
+    if (!have_ballon(errp)) {
         return;
     }
 
-    if (value <= 0) {
+    if (target <= 0) {
         error_set(errp, QERR_INVALID_PARAMETER_VALUE, "target", "a size");
         return;
     }
-    
-    if (qemu_balloon(value) == 0) {
-        error_set(errp, QERR_DEVICE_NOT_ACTIVE, "balloon");
-    }
+
+    trace_balloon_event(balloon_opaque, target);
+    balloon_event_fn(balloon_opaque, target);
 }
diff --git a/hmp.c b/hmp.c
index 481be8019b..a42c5c07be 100644
--- a/hmp.c
+++ b/hmp.c
@@ -535,6 +535,7 @@ out:
     qapi_free_VncInfo(info);
 }
 
+#ifdef CONFIG_SPICE
 void hmp_info_spice(Monitor *mon, const QDict *qdict)
 {
     SpiceChannelList *chan;
@@ -581,6 +582,7 @@ void hmp_info_spice(Monitor *mon, const QDict *qdict)
 out:
     qapi_free_SpiceInfo(info);
 }
+#endif
 
 void hmp_info_balloon(Monitor *mon, const QDict *qdict)
 {
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 0ca6cbd0e6..eeaf0cb981 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -52,9 +52,6 @@ void qerror_report_err(Error *err);
 #define QERR_BUS_NOT_FOUND \
     ERROR_CLASS_GENERIC_ERROR, "Bus '%s' not found"
 
-#define QERR_COMMAND_NOT_FOUND \
-    ERROR_CLASS_COMMAND_NOT_FOUND, "The command %s has not been found"
-
 #define QERR_DEVICE_ENCRYPTED \
     ERROR_CLASS_DEVICE_ENCRYPTED, "'%s' (%s) is encrypted"
 
@@ -73,9 +70,6 @@ void qerror_report_err(Error *err);
 #define QERR_DEVICE_NO_HOTPLUG \
     ERROR_CLASS_GENERIC_ERROR, "Device '%s' does not support hotplugging"
 
-#define QERR_DEVICE_NOT_ACTIVE \
-    ERROR_CLASS_DEVICE_NOT_ACTIVE, "No %s device has been activated"
-
 #define QERR_DEVICE_NOT_ENCRYPTED \
     ERROR_CLASS_GENERIC_ERROR, "Device '%s' is not encrypted"
 
@@ -112,9 +106,6 @@ void qerror_report_err(Error *err);
 #define QERR_JSON_PARSING \
     ERROR_CLASS_GENERIC_ERROR, "Invalid JSON syntax"
 
-#define QERR_KVM_MISSING_CAP \
-    ERROR_CLASS_KVM_MISSING_CAP, "Using KVM without %s, %s unavailable"
-
 #define QERR_MIGRATION_ACTIVE \
     ERROR_CLASS_GENERIC_ERROR, "There's a migration process in progress"
 
diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
index a93b4b2572..762e063125 100644
--- a/include/ui/qemu-spice.h
+++ b/include/ui/qemu-spice.h
@@ -88,4 +88,14 @@ static inline int qemu_spice_display_add_client(int csock, int skipauth,
 
 #endif /* CONFIG_SPICE */
 
+static inline bool qemu_using_spice(Error **errp)
+{
+    if (!using_spice) {
+        error_set(errp, ERROR_CLASS_DEVICE_NOT_ACTIVE,
+                  "SPICE is not in use");
+        return false;
+    }
+    return true;
+}
+
 #endif /* QEMU_SPICE_H */
diff --git a/monitor.c b/monitor.c
index 7e4f605e6d..2e2b0e5b37 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1095,11 +1095,12 @@ static int client_migrate_info(Monitor *mon, const QDict *qdict,
     const char *subject  = qdict_get_try_str(qdict, "cert-subject");
     int port             = qdict_get_try_int(qdict, "port", -1);
     int tls_port         = qdict_get_try_int(qdict, "tls-port", -1);
+    Error *err;
     int ret;
 
     if (strcmp(protocol, "spice") == 0) {
-        if (!using_spice) {
-            qerror_report(QERR_DEVICE_NOT_ACTIVE, "spice");
+        if (!qemu_using_spice(&err)) {
+            qerror_report_err(err);
             return -1;
         }
 
@@ -4782,9 +4783,9 @@ static int monitor_can_read(void *opaque)
     return (mon->suspend_cnt == 0) ? 1 : 0;
 }
 
-static int invalid_qmp_mode(const Monitor *mon, const char *cmd_name)
+static int invalid_qmp_mode(const Monitor *mon, const mon_cmd_t *cmd)
 {
-    int is_cap = compare_cmd(cmd_name, "qmp_capabilities");
+    int is_cap = cmd->mhandler.cmd_new == do_qmp_capabilities;
     return (qmp_cmd_mode(mon) ? is_cap : !is_cap);
 }
 
@@ -5078,14 +5079,10 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
 
     cmd_name = qdict_get_str(input, "execute");
     trace_handle_qmp_command(mon, cmd_name);
-    if (invalid_qmp_mode(mon, cmd_name)) {
-        qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
-        goto err_out;
-    }
-
     cmd = qmp_find_cmd(cmd_name);
-    if (!cmd) {
-        qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
+    if (!cmd || invalid_qmp_mode(mon, cmd)) {
+        qerror_report(ERROR_CLASS_COMMAND_NOT_FOUND,
+                      "The command %s has not been found", cmd_name);
         goto err_out;
     }
 
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 168b083c87..222742013f 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -76,7 +76,8 @@ static QObject *do_qmp_dispatch(QObject *request, Error **errp)
     command = qdict_get_str(dict, "execute");
     cmd = qmp_find_command(command);
     if (cmd == NULL) {
-        error_set(errp, QERR_COMMAND_NOT_FOUND, command);
+        error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
+                  "The command %s has not been found", command);
         return NULL;
     }
     if (!cmd->enabled) {
diff --git a/qmp.c b/qmp.c
index 963305c269..7f2d25a492 100644
--- a/qmp.c
+++ b/qmp.c
@@ -137,14 +137,18 @@ VncInfo *qmp_query_vnc(Error **errp)
 #endif
 
 #ifndef CONFIG_SPICE
-/* If SPICE support is enabled, the "true" query-spice command is
-   defined in the SPICE subsystem. Also note that we use a small
-   trick to maintain query-spice's original behavior, which is not
-   to be available in the namespace if SPICE is not compiled in */
+/*
+ * qmp-commands.hx ensures that QMP command query-spice exists only
+ * #ifdef CONFIG_SPICE.  Necessary for an accurate query-commands
+ * result.  However, the QAPI schema is blissfully unaware of that,
+ * and the QAPI code generator happily generates a dead
+ * qmp_marshal_input_query_spice() that calls qmp_query_spice().
+ * Provide it one, or else linking fails.
+ * FIXME Educate the QAPI schema on CONFIG_SPICE.
+ */
 SpiceInfo *qmp_query_spice(Error **errp)
 {
-    error_set(errp, QERR_COMMAND_NOT_FOUND, "query-spice");
-    return NULL;
+    abort();
 };
 #endif
 
@@ -287,9 +291,7 @@ void qmp_set_password(const char *protocol, const char *password,
     }
 
     if (strcmp(protocol, "spice") == 0) {
-        if (!using_spice) {
-            /* correct one? spice isn't a device ,,, */
-            error_set(errp, QERR_DEVICE_NOT_ACTIVE, "spice");
+        if (!qemu_using_spice(errp)) {
             return;
         }
         rc = qemu_spice_set_passwd(password, fail_if_connected,
@@ -335,9 +337,7 @@ void qmp_expire_password(const char *protocol, const char *whenstr,
     }
 
     if (strcmp(protocol, "spice") == 0) {
-        if (!using_spice) {
-            /* correct one? spice isn't a device ,,, */
-            error_set(errp, QERR_DEVICE_NOT_ACTIVE, "spice");
+        if (!qemu_using_spice(errp)) {
             return;
         }
         rc = qemu_spice_set_pw_expire(when);
@@ -575,8 +575,7 @@ void qmp_add_client(const char *protocol, const char *fdname,
     }
 
     if (strcmp(protocol, "spice") == 0) {
-        if (!using_spice) {
-            error_set(errp, QERR_DEVICE_NOT_ACTIVE, "spice");
+        if (!qemu_using_spice(errp)) {
             close(fd);
             return;
         }