[Qemu-devel] [PATCH v2 0/4] monitor: Convert do_set_link() to QObject, QError

[Qemu-devel] [PATCH v2 0/4] monitor: Convert do_set_link() to QObject, QError

[Markus Armbruster]

PATCH 3/4 changes syntax of set_link's second argument from up|down to
on|off.  I feel that the argument needs to be boolean in QMP, and this
is the simplest way to get it.  Luiz likes this approach.  The change
doesn't affect libvirt, because it doesn't use set_link, yet.

Alternatives I could try if the syntax change is unwanted:

* Use the old string argument in QMP.  Easy.

* Don't convert set_link, create a new command with a boolean
  argument.

* Create a argument parser for up|down.

v2: Fix parsing of 'on' and 'off' in PATCH 2/4.

Markus Armbruster (4):
  monitor: Rename argument type 'b' to 'f'
  monitor: New argument type 'b'
  monitor: Use argument type 'b' for set_link
  monitor: Convert do_set_link() to QObject, QError

 monitor.c       |   39 +++++++++++++++++++++++++++++++++++----
 net.c           |   17 ++++++-----------
 net.h           |    2 +-
 qemu-monitor.hx |   13 +++++++------
 4 files changed, 49 insertions(+), 22 deletions(-)

[Qemu-devel] [PATCH v2 1/4] monitor: Rename argument type 'b' to 'f'

[Markus Armbruster]

To make 'b' available for boolean argument.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 monitor.c       |    8 ++++----
 qemu-monitor.hx |    2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/monitor.c b/monitor.c
index 35cbce7..3ce9a4e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -74,7 +74,7 @@
  * 'l'          target long (32 or 64 bit)
  * 'M'          just like 'l', except in user mode the value is
  *              multiplied by 2^20 (think Mebibyte)
- * 'b'          double
+ * 'f'          double
  *              user mode accepts an optional G, g, M, m, K, k suffix,
  *              which multiplies the value by 2^30 for suffixes G and
  *              g, 2^20 for M and m, 2^10 for K and k
@@ -3798,7 +3798,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                 qdict_put(qdict, key, qint_from_int(val));
             }
             break;
-        case 'b':
+        case 'f':
         case 'T':
             {
                 double val;
@@ -3814,7 +3814,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                 if (get_double(mon, &val, &p) < 0) {
                     goto fail;
                 }
-                if (c == 'b' && *p) {
+                if (c == 'f' && *p) {
                     switch (*p) {
                     case 'K': case 'k':
                         val *= 1 << 10; p++; break;
@@ -4315,7 +4315,7 @@ static int check_arg(const CmdArgs *cmd_args, QDict *args)
                 return -1;
             }
             break;
-        case 'b':
+        case 'f':
         case 'T':
             if (qobject_type(value) != QTYPE_QINT && qobject_type(value) != QTYPE_QFLOAT) {
                 qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "number");
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 31087bd..8c9a41c 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -804,7 +804,7 @@ ETEXI
 
     {
         .name       = "migrate_set_speed",
-        .args_type  = "value:b",
+        .args_type  = "value:f",
         .params     = "value",
         .help       = "set maximum speed (in bytes) for migrations",
         .user_print = monitor_user_noop,
-- 
1.6.6.1

[Qemu-devel] [PATCH v2 2/4] monitor: New argument type 'b'

[Markus Armbruster]

This is a boolean value.  Human monitor accepts "on" or "off".
Consistent with option parsing (see parse_option_bool()).

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 monitor.c |   31 +++++++++++++++++++++++++++++++
 1 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/monitor.c b/monitor.c
index 3ce9a4e..389485d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -85,6 +85,8 @@
  *
  * '?'          optional type (for all types, except '/')
  * '.'          other form of optional type (for 'i' and 'l')
+ * 'b'          boolean
+ *              user mode accepts "on" or "off"
  * '-'          optional parameter (eg. '-f')
  *
  */
@@ -3841,6 +3843,29 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                 qdict_put(qdict, key, qfloat_from_double(val));
             }
             break;
+        case 'b':
+            {
+                const char *beg;
+                int val;
+
+                while (qemu_isspace(*p)) {
+                    p++;
+                }
+                beg = p;
+                while (qemu_isgraph(*p)) {
+                    p++;
+                }
+                if (p - beg == 2 && !memcmp(beg, "on", p - beg)) {
+                    val = 1;
+                } else if (p - beg == 3 && !memcmp(beg, "off", p - beg)) {
+                    val = 0;
+                } else {
+                    monitor_printf(mon, "Expected 'on' or 'off'\n");
+                    goto fail;
+                }
+                qdict_put(qdict, key, qbool_from_int(val));
+            }
+            break;
         case '-':
             {
                 const char *tmp = p;
@@ -4322,6 +4347,12 @@ static int check_arg(const CmdArgs *cmd_args, QDict *args)
                 return -1;
             }
             break;
+        case 'b':
+            if (qobject_type(value) != QTYPE_QBOOL) {
+                qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "bool");
+                return -1;
+            }
+            break;
         case '-':
             if (qobject_type(value) != QTYPE_QINT &&
                 qobject_type(value) != QTYPE_QBOOL) {
-- 
1.6.6.1

[Qemu-devel] [PATCH v2 3/4] monitor: Use argument type 'b' for set_link

[Markus Armbruster]

Second argument is now "on" or "off" instead of "up" or "down".

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 net.c           |   10 ++--------
 qemu-monitor.hx |    8 ++++----
 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/net.c b/net.c
index ae971ff..830021d 100644
--- a/net.c
+++ b/net.c
@@ -1288,7 +1288,7 @@ void do_set_link(Monitor *mon, const QDict *qdict)
     VLANState *vlan;
     VLANClientState *vc = NULL;
     const char *name = qdict_get_str(qdict, "name");
-    const char *up_or_down = qdict_get_str(qdict, "up_or_down");
+    int up = qdict_get_bool(qdict, "up");
 
     QTAILQ_FOREACH(vlan, &vlans, next) {
         QTAILQ_FOREACH(vc, &vlan->clients, next) {
@@ -1305,13 +1305,7 @@ done:
         return;
     }
 
-    if (strcmp(up_or_down, "up") == 0)
-        vc->link_down = 0;
-    else if (strcmp(up_or_down, "down") == 0)
-        vc->link_down = 1;
-    else
-        monitor_printf(mon, "invalid link status '%s'; only 'up' or 'down' "
-                       "valid\n", up_or_down);
+    vc->link_down = !up;
 
     if (vc->info->link_status_changed) {
         vc->info->link_status_changed(vc);
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 8c9a41c..7b7dcf5 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -986,16 +986,16 @@ ETEXI
 
     {
         .name       = "set_link",
-        .args_type  = "name:s,up_or_down:s",
-        .params     = "name up|down",
+        .args_type  = "name:s,up:b",
+        .params     = "name on|off",
         .help       = "change the link status of a network adapter",
         .mhandler.cmd = do_set_link,
     },
 
 STEXI
-@item set_link @var{name} [up|down]
+@item set_link @var{name} [on|off]
 @findex set_link
-Set link @var{name} up or down.
+Switch link @var{name} on (i.e. up) or off (i.e. down).
 ETEXI
 
     {
-- 
1.6.6.1

[Qemu-devel] [PATCH v2 4/4] monitor: Convert do_set_link() to QObject, QError

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 net.c           |    7 ++++---
 net.h           |    2 +-
 qemu-monitor.hx |    3 ++-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/net.c b/net.c
index 830021d..2559c07 100644
--- a/net.c
+++ b/net.c
@@ -1283,7 +1283,7 @@ void do_info_network(Monitor *mon)
     }
 }
 
-void do_set_link(Monitor *mon, const QDict *qdict)
+int do_set_link(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     VLANState *vlan;
     VLANClientState *vc = NULL;
@@ -1301,8 +1301,8 @@ void do_set_link(Monitor *mon, const QDict *qdict)
 done:
 
     if (!vc) {
-        monitor_printf(mon, "could not find network device '%s'\n", name);
-        return;
+        qerror_report(QERR_DEVICE_NOT_FOUND, name);
+        return -1;
     }
 
     vc->link_down = !up;
@@ -1310,6 +1310,7 @@ done:
     if (vc->info->link_status_changed) {
         vc->info->link_status_changed(vc);
     }
+    return 0;
 }
 
 void net_cleanup(void)
diff --git a/net.h b/net.h
index ce9e2c6..c7a3a1b 100644
--- a/net.h
+++ b/net.h
@@ -118,7 +118,7 @@ int qemu_find_nic_model(NICInfo *nd, const char * const *models,
                         const char *default_model);
 
 void do_info_network(Monitor *mon);
-void do_set_link(Monitor *mon, const QDict *qdict);
+int do_set_link(Monitor *mon, const QDict *qdict, QObject **ret_data);
 
 /* NIC info */
 
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 7b7dcf5..62fa346 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -989,7 +989,8 @@ ETEXI
         .args_type  = "name:s,up:b",
         .params     = "name on|off",
         .help       = "change the link status of a network adapter",
-        .mhandler.cmd = do_set_link,
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_set_link,
     },
 
 STEXI
-- 
1.6.6.1

[Qemu-devel] Re: [PATCH v2 0/4] monitor: Convert do_set_link() to QObject, QError

[Luiz Capitulino]

On Fri, 26 Mar 2010 09:07:07 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> PATCH 3/4 changes syntax of set_link's second argument from up|down to
> on|off.  I feel that the argument needs to be boolean in QMP, and this
> is the simplest way to get it.  Luiz likes this approach.  The change
> doesn't affect libvirt, because it doesn't use set_link, yet.

 Looks good to me.