diff mbox

[1/9] qmp hmp: Factor out common "using spice" test

Message ID 1421171432-8733-2-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster Jan. 13, 2015, 5:50 p.m. UTC
Into qemu_using_spice().  For want of a better place, put it next the
existing monitor command handler dummies in qemu-spice.h.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/ui/qemu-spice.h | 10 ++++++++++
 monitor.c               |  5 +++--
 qmp.c                   | 11 +++--------
 3 files changed, 16 insertions(+), 10 deletions(-)

Comments

Eric Blake Jan. 14, 2015, 1:10 p.m. UTC | #1
On 01/13/2015 10:50 AM, Markus Armbruster wrote:
> Into qemu_using_spice().  For want of a better place, put it next the
> existing monitor command handler dummies in qemu-spice.h.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/ui/qemu-spice.h | 10 ++++++++++
>  monitor.c               |  5 +++--
>  qmp.c                   | 11 +++--------
>  3 files changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
> index a93b4b2..b3f2679 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 int qemu_using_spice(Error **errp)

Why not s/int/bool/

> +{
> +    if (!using_spice) {
> +        /* correct one? spice isn't a device ,,, */
> +        error_set(errp, QERR_DEVICE_NOT_ACTIVE, "spice");
> +        return 0;
> +    }
> +    return 1;

and use true/false here? All callers use it only in a boolean context.

Either way,
Reviewed-by: Eric Blake <eblake@redhat.com>
Markus Armbruster Jan. 14, 2015, 1:53 p.m. UTC | #2
Eric Blake <eblake@redhat.com> writes:

> On 01/13/2015 10:50 AM, Markus Armbruster wrote:
>> Into qemu_using_spice().  For want of a better place, put it next the
>> existing monitor command handler dummies in qemu-spice.h.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  include/ui/qemu-spice.h | 10 ++++++++++
>>  monitor.c               |  5 +++--
>>  qmp.c                   | 11 +++--------
>>  3 files changed, 16 insertions(+), 10 deletions(-)
>> 
>> diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
>> index a93b4b2..b3f2679 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 int qemu_using_spice(Error **errp)
>
> Why not s/int/bool/
>
>> +{
>> +    if (!using_spice) {
>> +        /* correct one? spice isn't a device ,,, */
>> +        error_set(errp, QERR_DEVICE_NOT_ACTIVE, "spice");
>> +        return 0;
>> +    }
>> +    return 1;
>
> and use true/false here? All callers use it only in a boolean context.

I wrapped around using_spice, which is int, and didn't think of
switching to bool.  Old habits die hard...

Luiz, would you be willing to fix this up on commit?

> Either way,
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!
diff mbox

Patch

diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
index a93b4b2..b3f2679 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 int qemu_using_spice(Error **errp)
+{
+    if (!using_spice) {
+        /* correct one? spice isn't a device ,,, */
+        error_set(errp, QERR_DEVICE_NOT_ACTIVE, "spice");
+        return 0;
+    }
+    return 1;
+}
+
 #endif /* QEMU_SPICE_H */
diff --git a/monitor.c b/monitor.c
index 1808e41..cd81289 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;
         }
 
diff --git a/qmp.c b/qmp.c
index 0b4f131..f01ae7d 100644
--- a/qmp.c
+++ b/qmp.c
@@ -287,9 +287,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 +333,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);
@@ -562,8 +558,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;
         }