[Qemu-devel] [PATCH 00/11] monitor: New commands netdev_add, netdev_del

[Qemu-devel] [PATCH 00/11] monitor: New commands netdev_add, netdev_del

[Markus Armbruster]

Markus Armbruster (11):
  error: Put error definitions back in alphabetical order
  error: New QERR_DUPLICATE_ID
  error: Convert qemu_opts_create() to QError
  error: New QERR_INVALID_PARAMETER_VALUE
  error: Convert qemu_opts_set() to QError
  error: Drop extra messages after qemu_opts_set() and
    qemu_opts_parse()
  error: Use QERR_INVALID_PARAMETER_VALUE instead of
    QERR_INVALID_PARAMETER
  error: Convert qemu_opts_validate() to QError
  error: Convert net_client_init() to QError
  error: New QERR_DEVICE_IN_USE
  monitor: New commands netdev_add, netdev_del

 hw/pci-hotplug.c |    2 -
 hw/qdev.c        |    2 +-
 monitor.c        |    6 ++-
 net.c            |   95 +++++++++++++++++++++++++++++++++++++++++------------
 net.h            |    2 +
 qemu-config.c    |    1 -
 qemu-monitor.hx  |   30 +++++++++++++++++
 qemu-option.c    |   24 +++++--------
 qerror.c         |   20 ++++++++++-
 qerror.h         |   17 ++++++++--
 vl.c             |    5 ---
 11 files changed, 151 insertions(+), 53 deletions(-)

[Qemu-devel] [PATCH 01/11] error: Put error definitions back in alphabetical order

[Markus Armbruster]

Add suitable comments to help keerp them in order.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qerror.c |   12 ++++++++----
 qerror.h |    8 +++++---
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/qerror.c b/qerror.c
index eaa1deb..4520b0d 100644
--- a/qerror.c
+++ b/qerror.c
@@ -38,6 +38,10 @@ static const QType qerror_type = {
  * for example:
  *
  * "running out of foo: %(foo)%%"
+ *
+ * Please keep the entries in alphabetical order.
+ * Use "sed -n '/^static.*qerror_table\[\]/,/^};/s/QERR_/&/gp' qerror.c | sort -c"
+ * to check.
  */
 static const QErrorStringTable qerror_table[] = {
     {
@@ -65,10 +69,6 @@ static const QErrorStringTable qerror_table[] = {
         .desc      = "Device '%(device)' could not be initialized",
     },
     {
-        .error_fmt = QERR_DEVICE_NOT_ENCRYPTED,
-        .desc      = "Device '%(device)' is not encrypted",
-    },
-    {
         .error_fmt = QERR_DEVICE_LOCKED,
         .desc      = "Device '%(device)' is locked",
     },
@@ -81,6 +81,10 @@ static const QErrorStringTable qerror_table[] = {
         .desc      = "Device '%(device)' has not been activated by the guest",
     },
     {
+        .error_fmt = QERR_DEVICE_NOT_ENCRYPTED,
+        .desc      = "Device '%(device)' is not encrypted",
+    },
+    {
         .error_fmt = QERR_DEVICE_NOT_FOUND,
         .desc      = "Device '%(device)' not found",
     },
diff --git a/qerror.h b/qerror.h
index dd298d4..a2664ab 100644
--- a/qerror.h
+++ b/qerror.h
@@ -46,6 +46,8 @@ QError *qobject_to_qerror(const QObject *obj);
 
 /*
  * QError class list
+ * Please keep the definitions in alphabetical order.
+ * Use "grep '^#define QERR_' qerror.h | sort -c" to check.
  */
 #define QERR_BAD_BUS_FOR_DEVICE \
     "{ 'class': 'BadBusForDevice', 'data': { 'device': %s, 'bad_bus_type': %s } }"
@@ -62,9 +64,6 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_DEVICE_ENCRYPTED \
     "{ 'class': 'DeviceEncrypted', 'data': { 'device': %s } }"
 
-#define QERR_DEVICE_NOT_ENCRYPTED \
-    "{ 'class': 'DeviceNotEncrypted', 'data': { 'device': %s } }"
-
 #define QERR_DEVICE_INIT_FAILED \
     "{ 'class': 'DeviceInitFailed', 'data': { 'device': %s } }"
 
@@ -77,6 +76,9 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_DEVICE_NOT_ACTIVE \
     "{ 'class': 'DeviceNotActive', 'data': { 'device': %s } }"
 
+#define QERR_DEVICE_NOT_ENCRYPTED \
+    "{ 'class': 'DeviceNotEncrypted', 'data': { 'device': %s } }"
+
 #define QERR_DEVICE_NOT_FOUND \
     "{ 'class': 'DeviceNotFound', 'data': { 'device': %s } }"
 
-- 
1.6.6.1

[Qemu-devel] [PATCH 02/11] error: New QERR_DUPLICATE_ID

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qerror.c |    4 ++++
 qerror.h |    3 +++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/qerror.c b/qerror.c
index 4520b0d..9fb817e 100644
--- a/qerror.c
+++ b/qerror.c
@@ -97,6 +97,10 @@ static const QErrorStringTable qerror_table[] = {
         .desc      = "Device '%(device)' has no child bus",
     },
     {
+        .error_fmt = QERR_DUPLICATE_ID,
+        .desc      = "Duplicate ID '%(id)' for %(object)",
+    },
+    {
         .error_fmt = QERR_FD_NOT_FOUND,
         .desc      = "File descriptor named '%(name)' not found",
     },
diff --git a/qerror.h b/qerror.h
index a2664ab..870cdc3 100644
--- a/qerror.h
+++ b/qerror.h
@@ -88,6 +88,9 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_DEVICE_NO_BUS \
     "{ 'class': 'DeviceNoBus', 'data': { 'device': %s } }"
 
+#define QERR_DUPLICATE_ID \
+    "{ 'class': 'DuplicateId', 'data': { 'id': %s, 'object': %s } }"
+
 #define QERR_FD_NOT_FOUND \
     "{ 'class': 'FdNotFound', 'data': { 'name': %s } }"
 
-- 
1.6.6.1

[Qemu-devel] [PATCH 03/11] error: Convert qemu_opts_create() to QError

[Markus Armbruster]

Fixes device_add to report duplicate ID properly in QMP, as
DuplicateId instead of UndefinedError.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qemu-option.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index dc340b8..e3916be 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -30,6 +30,7 @@
 #include "qemu-error.h"
 #include "qemu-objects.h"
 #include "qemu-option.h"
+#include "qerror.h"
 
 /*
  * Extracts the name of an option from the parameter string (p points at the
@@ -643,8 +644,7 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id, int fail_if_exist
         opts = qemu_opts_find(list, id);
         if (opts != NULL) {
             if (fail_if_exists) {
-                fprintf(stderr, "tried to create id \"%s\" twice for \"%s\"\n",
-                        id, list->name);
+                qerror_report(QERR_DUPLICATE_ID, id, list->name);
                 return NULL;
             } else {
                 return opts;
-- 
1.6.6.1

[Qemu-devel] [PATCH 04/11] error: New QERR_INVALID_PARAMETER_VALUE

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qerror.c |    4 ++++
 qerror.h |    3 +++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/qerror.c b/qerror.c
index 9fb817e..97e8d4a 100644
--- a/qerror.c
+++ b/qerror.c
@@ -121,6 +121,10 @@ static const QErrorStringTable qerror_table[] = {
         .desc      = "Invalid parameter type, expected: %(expected)",
     },
     {
+        .error_fmt = QERR_INVALID_PARAMETER_VALUE,
+        .desc      = "Parameter '%(name)' expects %(expected)",
+    },
+    {
         .error_fmt = QERR_INVALID_PASSWORD,
         .desc      = "Password incorrect",
     },
diff --git a/qerror.h b/qerror.h
index 870cdc3..5625d54 100644
--- a/qerror.h
+++ b/qerror.h
@@ -106,6 +106,9 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_INVALID_PARAMETER_TYPE \
     "{ 'class': 'InvalidParameterType', 'data': { 'name': %s,'expected': %s } }"
 
+#define QERR_INVALID_PARAMETER_VALUE \
+    "{ 'class': 'InvalidParameterValue', 'data': { 'name': %s, 'expected': %s } }"
+
 #define QERR_INVALID_PASSWORD \
     "{ 'class': 'InvalidPassword', 'data': {} }"
 
-- 
1.6.6.1

[Qemu-devel] [PATCH 05/11] error: Convert qemu_opts_set() to QError

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qemu-option.c |   17 ++++++-----------
 1 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index e3916be..5c39666 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -176,7 +176,7 @@ static int parse_option_bool(const char *name, const char *value, int *ret)
         } else if (!strcmp(value, "off")) {
             *ret = 0;
         } else {
-            fprintf(stderr, "Option '%s': Use 'on' or 'off'\n", name);
+            qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "'on' or 'off'");
             return -1;
         }
     } else {
@@ -193,12 +193,12 @@ static int parse_option_number(const char *name, const char *value, uint64_t *re
     if (value != NULL) {
         number = strtoull(value, &postfix, 0);
         if (*postfix != '\0') {
-            fprintf(stderr, "Option '%s' needs a number as parameter\n", name);
+            qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a number");
             return -1;
         }
         *ret = number;
     } else {
-        fprintf(stderr, "Option '%s' needs a parameter\n", name);
+        qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a number");
         return -1;
     }
     return 0;
@@ -226,13 +226,11 @@ static int parse_option_size(const char *name, const char *value, uint64_t *ret)
             *ret = (uint64_t) sizef;
             break;
         default:
-            fprintf(stderr, "Option '%s' needs size as parameter\n", name);
-            fprintf(stderr, "You may use k, M, G or T suffixes for "
-                    "kilobytes, megabytes, gigabytes and terabytes.\n");
+            qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a size");
             return -1;
         }
     } else {
-        fprintf(stderr, "Option '%s' needs a parameter\n", name);
+        qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a size");
         return -1;
     }
     return 0;
@@ -581,8 +579,7 @@ int qemu_opt_set(QemuOpts *opts, const char *name, const char *value)
         if (i == 0) {
             /* empty list -> allow any */;
         } else {
-            fprintf(stderr, "option \"%s\" is not valid for %s\n",
-                    name, opts->list->name);
+            qerror_report(QERR_INVALID_PARAMETER, name);
             return -1;
         }
     }
@@ -598,8 +595,6 @@ int qemu_opt_set(QemuOpts *opts, const char *name, const char *value)
         opt->str = qemu_strdup(value);
     }
     if (qemu_opt_parse(opt) < 0) {
-        fprintf(stderr, "Failed to parse \"%s\" for \"%s.%s\"\n", opt->str,
-                opts->list->name, opt->name);
         qemu_opt_del(opt);
         return -1;
     }
-- 
1.6.6.1

[Qemu-devel] [PATCH 06/11] error: Drop extra messages after qemu_opts_set() and qemu_opts_parse()

[Markus Armbruster]

Both functions report errors nicely enough now, no need for additional
messages.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/pci-hotplug.c |    2 --
 net.c            |    2 --
 qemu-config.c    |    1 -
 vl.c             |    5 -----
 4 files changed, 0 insertions(+), 10 deletions(-)

diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index eb3701b..343fd17 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -56,8 +56,6 @@ static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
 
     opts = qemu_opts_parse(&qemu_net_opts, opts_str ? opts_str : "", 0);
     if (!opts) {
-        monitor_printf(mon, "parsing network options '%s' failed\n",
-                       opts_str ? opts_str : "");
         return NULL;
     }
 
diff --git a/net.c b/net.c
index 1092bdc..9338f35 100644
--- a/net.c
+++ b/net.c
@@ -1161,8 +1161,6 @@ void net_host_device_add(Monitor *mon, const QDict *qdict)
 
     opts = qemu_opts_parse(&qemu_net_opts, opts_str ? opts_str : "", 0);
     if (!opts) {
-        monitor_printf(mon, "parsing network options '%s' failed\n",
-                       opts_str ? opts_str : "");
         return;
     }
 
diff --git a/qemu-config.c b/qemu-config.c
index 150157c..d4a2f43 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -472,7 +472,6 @@ int qemu_config_parse(FILE *fp, const char *fname)
                 goto out;
             }
             if (qemu_opt_set(opts, arg, value) != 0) {
-                error_report("failed to set \"%s\" for %s", arg, group);
                 goto out;
             }
             continue;
diff --git a/vl.c b/vl.c
index 01dc1aa..328b4cd 100644
--- a/vl.c
+++ b/vl.c
@@ -1815,8 +1815,6 @@ QemuOpts *drive_add(const char *file, const char *fmt, ...)
 
     opts = qemu_opts_parse(&qemu_drive_opts, optstr, 0);
     if (!opts) {
-        fprintf(stderr, "%s: huh? duplicate? (%s)\n",
-                __FUNCTION__, optstr);
         return NULL;
     }
     if (file)
@@ -5367,7 +5365,6 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_mon:
                 opts = qemu_opts_parse(&qemu_mon_opts, optarg, 1);
                 if (!opts) {
-                    fprintf(stderr, "parse error: %s\n", optarg);
                     exit(1);
                 }
                 default_monitor = 0;
@@ -5375,7 +5372,6 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_chardev:
                 opts = qemu_opts_parse(&qemu_chardev_opts, optarg, 1);
                 if (!opts) {
-                    fprintf(stderr, "parse error: %s\n", optarg);
                     exit(1);
                 }
                 break;
@@ -5587,7 +5583,6 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_rtc:
                 opts = qemu_opts_parse(&qemu_rtc_opts, optarg, 0);
                 if (!opts) {
-                    fprintf(stderr, "parse error: %s\n", optarg);
                     exit(1);
                 }
                 configure_rtc(opts);
-- 
1.6.6.1

[Qemu-devel] [PATCH 07/11] error: Use QERR_INVALID_PARAMETER_VALUE instead of QERR_INVALID_PARAMETER

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/qdev.c |    2 +-
 monitor.c |    6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 35460eb..c3a00d2 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -207,7 +207,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
     /* find driver */
     info = qdev_find_info(NULL, driver);
     if (!info || info->no_user) {
-        qerror_report(QERR_INVALID_PARAMETER, "driver");
+        qerror_report(QERR_INVALID_PARAMETER_VALUE, "driver", "a driver name");
         error_printf_unless_qmp("Try with argument '?' for a list.\n");
         return NULL;
     }
diff --git a/monitor.c b/monitor.c
index 822dc27..35cbce7 100644
--- a/monitor.c
+++ b/monitor.c
@@ -970,7 +970,8 @@ static int do_cpu_set(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     int index = qdict_get_int(qdict, "index");
     if (mon_set_cpu(index) < 0) {
-        qerror_report(QERR_INVALID_PARAMETER, "index");
+        qerror_report(QERR_INVALID_PARAMETER_VALUE, "index",
+                      "a CPU number");
         return -1;
     }
     return 0;
@@ -2489,7 +2490,8 @@ static int do_getfd(Monitor *mon, const QDict *qdict, QObject **ret_data)
     }
 
     if (qemu_isdigit(fdname[0])) {
-        qerror_report(QERR_INVALID_PARAMETER, "fdname");
+        qerror_report(QERR_INVALID_PARAMETER_VALUE, "fdname",
+                      "a name not starting with a digit");
         return -1;
     }
 
-- 
1.6.6.1

[Qemu-devel] [PATCH 08/11] error: Convert qemu_opts_validate() to QError

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qemu-option.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index 5c39666..8b45068 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -874,8 +874,7 @@ int qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc)
             }
         }
         if (desc[i].name == NULL) {
-            fprintf(stderr, "option \"%s\" is not valid for %s\n",
-                    opt->name, opts->list->name);
+            qerror_report(QERR_INVALID_PARAMETER, opt->name);
             return -1;
         }
 
-- 
1.6.6.1

[Qemu-devel] [PATCH 09/11] error: Convert net_client_init() to QError

[Markus Armbruster]

The conversion is shallow: client type init() methods aren't
converted.  Converting them is a big job for relatively little
practical benefit, so leave it for later.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 net.c |   38 ++++++++++++++++++--------------------
 1 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/net.c b/net.c
index 9338f35..1f3c39c 100644
--- a/net.c
+++ b/net.c
@@ -1057,18 +1057,12 @@ int net_client_init(Monitor *mon, QemuOpts *opts, int is_netdev)
     int i;
 
     type = qemu_opt_get(opts, "type");
+    if (!type) {
+        qerror_report(QERR_MISSING_PARAMETER, "type");
+        return -1;
+    }
 
-    if (!is_netdev) {
-        if (!type) {
-            error_report("No type specified for -net");
-            return -1;
-        }
-    } else {
-        if (!type) {
-            error_report("No type specified for -netdev");
-            return -1;
-        }
-
+    if (is_netdev) {
         if (strcmp(type, "tap") != 0 &&
 #ifdef CONFIG_SLIRP
             strcmp(type, "user") != 0 &&
@@ -1077,21 +1071,21 @@ int net_client_init(Monitor *mon, QemuOpts *opts, int is_netdev)
             strcmp(type, "vde") != 0 &&
 #endif
             strcmp(type, "socket") != 0) {
-            error_report("The '%s' network backend type is not valid with -netdev",
-                         type);
+            qerror_report(QERR_INVALID_PARAMETER_VALUE, "type",
+                          "a netdev backend type");
             return -1;
         }
 
         if (qemu_opt_get(opts, "vlan")) {
-            error_report("The 'vlan' parameter is not valid with -netdev");
+            qerror_report(QERR_INVALID_PARAMETER, "vlan");
             return -1;
         }
         if (qemu_opt_get(opts, "name")) {
-            error_report("The 'name' parameter is not valid with -netdev");
+            qerror_report(QERR_INVALID_PARAMETER, "name");
             return -1;
         }
         if (!qemu_opts_id(opts)) {
-            error_report("The id= parameter is required with -netdev");
+            qerror_report(QERR_MISSING_PARAMETER, "id");
             return -1;
         }
     }
@@ -1117,14 +1111,18 @@ int net_client_init(Monitor *mon, QemuOpts *opts, int is_netdev)
             }
 
             if (net_client_types[i].init) {
-                return net_client_types[i].init(opts, mon, name, vlan);
-            } else {
-                return 0;
+                if (net_client_types[i].init(opts, mon, name, vlan) < 0) {
+                    /* TODO push error reporting into init() methods */
+                    qerror_report(QERR_DEVICE_INIT_FAILED, type);
+                    return -1;
+                }
             }
+            return 0;
         }
     }
 
-    error_report("Invalid -net type '%s'", type);
+    qerror_report(QERR_INVALID_PARAMETER_VALUE, "type",
+                  "a network backend type");
     return -1;
 }
 
-- 
1.6.6.1

[Qemu-devel] [PATCH 10/11] error: New QERR_DEVICE_IN_USE

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qerror.c |    4 ++++
 qerror.h |    3 +++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/qerror.c b/qerror.c
index 97e8d4a..8d885cd 100644
--- a/qerror.c
+++ b/qerror.c
@@ -69,6 +69,10 @@ static const QErrorStringTable qerror_table[] = {
         .desc      = "Device '%(device)' could not be initialized",
     },
     {
+        .error_fmt = QERR_DEVICE_IN_USE,
+        .desc      = "Device '%(device)' is in use",
+    },
+    {
         .error_fmt = QERR_DEVICE_LOCKED,
         .desc      = "Device '%(device)' is locked",
     },
diff --git a/qerror.h b/qerror.h
index 5625d54..bae08c0 100644
--- a/qerror.h
+++ b/qerror.h
@@ -67,6 +67,9 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_DEVICE_INIT_FAILED \
     "{ 'class': 'DeviceInitFailed', 'data': { 'device': %s } }"
 
+#define QERR_DEVICE_IN_USE \
+    "{ 'class': 'DeviceInUse', 'data': { 'device': %s } }"
+
 #define QERR_DEVICE_LOCKED \
     "{ 'class': 'DeviceLocked', 'data': { 'device': %s } }"
 
-- 
1.6.6.1

[Qemu-devel] [PATCH 11/11] monitor: New commands netdev_add, netdev_del

[Markus Armbruster]

Monitor commands to go with -netdev.

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

diff --git a/net.c b/net.c
index 1f3c39c..80e9025 100644
--- a/net.c
+++ b/net.c
@@ -1122,7 +1122,7 @@ int net_client_init(Monitor *mon, QemuOpts *opts, int is_netdev)
     }
 
     qerror_report(QERR_INVALID_PARAMETER_VALUE, "type",
-                  "a network backend type");
+                  "a network client type");
     return -1;
 }
 
@@ -1186,6 +1186,61 @@ void net_host_device_remove(Monitor *mon, const QDict *qdict)
     qemu_del_vlan_client(vc);
 }
 
+/**
+ * do_netdev_add(): Add a host network device
+ *
+ * Argument qdict contains
+ * - "type": the device type, "tap", "user", ...
+ * - "id": the device's ID (must be unique)
+ * - device options
+ *
+ * Example:
+ *
+ * { "type": "user", "id": "netdev1", "hostname": "a-guest" }
+ */
+int do_netdev_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+    QemuOpts *opts;
+    int res;
+
+    opts = qemu_opts_from_qdict(&qemu_netdev_opts, qdict);
+    if (!opts) {
+        return -1;
+    }
+
+    res = net_client_init(mon, opts, 1);
+    qemu_opts_del(opts);
+    return res;
+}
+
+/**
+ * do_netdev_del(): Delete a host network device
+ *
+ * Argument qdict contains
+ * - "id": the device's ID
+ *
+ * Example:
+ *
+ * { "id": "netdev1" }
+ */
+int do_netdev_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+    const char *id = qdict_get_str(qdict, "id");
+    VLANClientState *vc;
+
+    vc = qemu_find_netdev(id);
+    if (!vc || vc->info->type == NET_CLIENT_TYPE_NIC) {
+        qerror_report(QERR_DEVICE_NOT_FOUND, id);
+        return -1;
+    }
+    if (vc->peer) {
+        qerror_report(QERR_DEVICE_IN_USE, id);
+        return -1;
+    }
+    qemu_del_vlan_client(vc);
+    return 0;
+}
+
 void net_set_boot_mask(int net_boot_mask)
 {
     int i;
diff --git a/net.h b/net.h
index 16f19c5..ce9e2c6 100644
--- a/net.h
+++ b/net.h
@@ -166,6 +166,8 @@ void net_cleanup(void);
 void net_set_boot_mask(int boot_mask);
 void net_host_device_add(Monitor *mon, const QDict *qdict);
 void net_host_device_remove(Monitor *mon, const QDict *qdict);
+int do_netdev_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
+int do_netdev_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
 
 #define DEFAULT_NETWORK_SCRIPT "/etc/qemu-ifup"
 #define DEFAULT_NETWORK_DOWN_SCRIPT "/etc/qemu-ifdown"
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index d290b4b..31087bd 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -914,6 +914,36 @@ STEXI
 Remove host VLAN client.
 ETEXI
 
+    {
+        .name       = "netdev_add",
+        .args_type  = "netdev:O",
+        .params     = "[user|tap|socket],id=str[,prop=value][,...]",
+        .help       = "add host network device",
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_netdev_add,
+    },
+
+STEXI
+@item netdev_add
+@findex netdev_add
+Add host network device.
+ETEXI
+
+    {
+        .name       = "netdev_del",
+        .args_type  = "id:s",
+        .params     = "id",
+        .help       = "remove host network device",
+        .user_print = monitor_user_noop,
+        .mhandler.cmd_new = do_netdev_del,
+    },
+
+STEXI
+@item netdev_del
+@findex netdev_del
+Remove host network device.
+ETEXI
+
 #ifdef CONFIG_SLIRP
     {
         .name       = "hostfwd_add",
-- 
1.6.6.1

[Qemu-devel] Re: [PATCH 00/11] monitor: New commands netdev_add, netdev_del

[Luiz Capitulino]

On Thu, 18 Mar 2010 17:33:07 +0100
Markus Armbruster <armbru@redhat.com> wrote:

 I'm getting the following build error:

"""
qemu-option.o: In function `parse_option_bool':
/home/lcapitulino/src/qmp-unstable/qemu-option.c:179: undefined reference to `qerror_report_internal'
qemu-option.o: In function `parse_option_number':
/home/lcapitulino/src/qmp-unstable/qemu-option.c:196: undefined reference to `qerror_report_internal'
/home/lcapitulino/src/qmp-unstable/qemu-option.c:201: undefined reference to `qerror_report_internal'
qemu-option.o: In function `parse_option_size':
/home/lcapitulino/src/qmp-unstable/qemu-option.c:229: undefined reference to `qerror_report_internal'
/home/lcapitulino/src/qmp-unstable/qemu-option.c:233: undefined reference to `qerror_report_internal'
qemu-option.o:/home/lcapitulino/src/qmp-unstable/qemu-option.c:582: more undefined references to `qerror_report_internal' follow
collect2: ld returned 1 exit status
make: *** [qemu-nbd] Error 1
"""

 Adding a qerror_report_internal() entry in qemu-tool.c fixes it.


> Markus Armbruster (11):
>   error: Put error definitions back in alphabetical order
>   error: New QERR_DUPLICATE_ID
>   error: Convert qemu_opts_create() to QError
>   error: New QERR_INVALID_PARAMETER_VALUE
>   error: Convert qemu_opts_set() to QError
>   error: Drop extra messages after qemu_opts_set() and
>     qemu_opts_parse()
>   error: Use QERR_INVALID_PARAMETER_VALUE instead of
>     QERR_INVALID_PARAMETER
>   error: Convert qemu_opts_validate() to QError
>   error: Convert net_client_init() to QError
>   error: New QERR_DEVICE_IN_USE
>   monitor: New commands netdev_add, netdev_del
> 
>  hw/pci-hotplug.c |    2 -
>  hw/qdev.c        |    2 +-
>  monitor.c        |    6 ++-
>  net.c            |   95 +++++++++++++++++++++++++++++++++++++++++------------
>  net.h            |    2 +
>  qemu-config.c    |    1 -
>  qemu-monitor.hx  |   30 +++++++++++++++++
>  qemu-option.c    |   24 +++++--------
>  qerror.c         |   20 ++++++++++-
>  qerror.h         |   17 ++++++++--
>  vl.c             |    5 ---
>  11 files changed, 151 insertions(+), 53 deletions(-)
> 

[Qemu-devel] Re: [PATCH 04/11] error: New QERR_INVALID_PARAMETER_VALUE

[Luiz Capitulino]

On Thu, 18 Mar 2010 17:33:11 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qerror.c |    4 ++++
>  qerror.h |    3 +++
>  2 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/qerror.c b/qerror.c
> index 9fb817e..97e8d4a 100644
> --- a/qerror.c
> +++ b/qerror.c
> @@ -121,6 +121,10 @@ static const QErrorStringTable qerror_table[] = {
>          .desc      = "Invalid parameter type, expected: %(expected)",
>      },
>      {
> +        .error_fmt = QERR_INVALID_PARAMETER_VALUE,
> +        .desc      = "Parameter '%(name)' expects %(expected)",
> +    },

 Would we need this error if we improve QERR_INVALID_PARAMETER to
accept an optional 'expects' parameter?

> +    {
>          .error_fmt = QERR_INVALID_PASSWORD,
>          .desc      = "Password incorrect",
>      },
> diff --git a/qerror.h b/qerror.h
> index 870cdc3..5625d54 100644
> --- a/qerror.h
> +++ b/qerror.h
> @@ -106,6 +106,9 @@ QError *qobject_to_qerror(const QObject *obj);
>  #define QERR_INVALID_PARAMETER_TYPE \
>      "{ 'class': 'InvalidParameterType', 'data': { 'name': %s,'expected': %s } }"
>  
> +#define QERR_INVALID_PARAMETER_VALUE \
> +    "{ 'class': 'InvalidParameterValue', 'data': { 'name': %s, 'expected': %s } }"
> +
>  #define QERR_INVALID_PASSWORD \
>      "{ 'class': 'InvalidPassword', 'data': {} }"
>  

[Qemu-devel] Re: [PATCH 05/11] error: Convert qemu_opts_set() to QError

[Luiz Capitulino]

On Thu, 18 Mar 2010 17:33:12 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qemu-option.c |   17 ++++++-----------
>  1 files changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/qemu-option.c b/qemu-option.c
> index e3916be..5c39666 100644
> --- a/qemu-option.c
> +++ b/qemu-option.c
> @@ -176,7 +176,7 @@ static int parse_option_bool(const char *name, const char *value, int *ret)
>          } else if (!strcmp(value, "off")) {
>              *ret = 0;
>          } else {
> -            fprintf(stderr, "Option '%s': Use 'on' or 'off'\n", name);
> +            qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "'on' or 'off'");
>              return -1;
>          }
>      } else {
> @@ -193,12 +193,12 @@ static int parse_option_number(const char *name, const char *value, uint64_t *re
>      if (value != NULL) {
>          number = strtoull(value, &postfix, 0);
>          if (*postfix != '\0') {
> -            fprintf(stderr, "Option '%s' needs a number as parameter\n", name);
> +            qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a number");
>              return -1;
>          }
>          *ret = number;
>      } else {
> -        fprintf(stderr, "Option '%s' needs a parameter\n", name);
> +        qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a number");
>          return -1;
>      }
>      return 0;
> @@ -226,13 +226,11 @@ static int parse_option_size(const char *name, const char *value, uint64_t *ret)
>              *ret = (uint64_t) sizef;
>              break;
>          default:
> -            fprintf(stderr, "Option '%s' needs size as parameter\n", name);
> -            fprintf(stderr, "You may use k, M, G or T suffixes for "
> -                    "kilobytes, megabytes, gigabytes and terabytes.\n");
> +            qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a size");

 What about using error_printf_unless_qmp() to improve the error message?
Although I don't like it very much it documents the code and if we feel like
we're abusing it we'll have an excuse to do something more complicated
like error message override.

>              return -1;
>          }
>      } else {
> -        fprintf(stderr, "Option '%s' needs a parameter\n", name);
> +        qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a size");
>          return -1;
>      }
>      return 0;
> @@ -581,8 +579,7 @@ int qemu_opt_set(QemuOpts *opts, const char *name, const char *value)
>          if (i == 0) {
>              /* empty list -> allow any */;
>          } else {
> -            fprintf(stderr, "option \"%s\" is not valid for %s\n",
> -                    name, opts->list->name);
> +            qerror_report(QERR_INVALID_PARAMETER, name);
>              return -1;
>          }
>      }
> @@ -598,8 +595,6 @@ int qemu_opt_set(QemuOpts *opts, const char *name, const char *value)
>          opt->str = qemu_strdup(value);
>      }
>      if (qemu_opt_parse(opt) < 0) {
> -        fprintf(stderr, "Failed to parse \"%s\" for \"%s.%s\"\n", opt->str,
> -                opts->list->name, opt->name);
>          qemu_opt_del(opt);
>          return -1;
>      }

 Not generating an error on purpose here?

[Qemu-devel] Re: [PATCH 00/11] monitor: New commands netdev_add, netdev_del

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Thu, 18 Mar 2010 17:33:07 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
>
>  I'm getting the following build error:
>
> """
> qemu-option.o: In function `parse_option_bool':
> /home/lcapitulino/src/qmp-unstable/qemu-option.c:179: undefined reference to `qerror_report_internal'
> qemu-option.o: In function `parse_option_number':
> /home/lcapitulino/src/qmp-unstable/qemu-option.c:196: undefined reference to `qerror_report_internal'
> /home/lcapitulino/src/qmp-unstable/qemu-option.c:201: undefined reference to `qerror_report_internal'
> qemu-option.o: In function `parse_option_size':
> /home/lcapitulino/src/qmp-unstable/qemu-option.c:229: undefined reference to `qerror_report_internal'
> /home/lcapitulino/src/qmp-unstable/qemu-option.c:233: undefined reference to `qerror_report_internal'
> qemu-option.o:/home/lcapitulino/src/qmp-unstable/qemu-option.c:582: more undefined references to `qerror_report_internal' follow
> collect2: ld returned 1 exit status
> make: *** [qemu-nbd] Error 1
> """
>
>  Adding a qerror_report_internal() entry in qemu-tool.c fixes it.

Looks like this depends on my "error: Clean up after recent changes"
series.

[Qemu-devel] Re: [PATCH 05/11] error: Convert qemu_opts_set() to QError

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Thu, 18 Mar 2010 17:33:12 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  qemu-option.c |   17 ++++++-----------
>>  1 files changed, 6 insertions(+), 11 deletions(-)
>> 
>> diff --git a/qemu-option.c b/qemu-option.c
>> index e3916be..5c39666 100644
>> --- a/qemu-option.c
>> +++ b/qemu-option.c
>> @@ -176,7 +176,7 @@ static int parse_option_bool(const char *name, const char *value, int *ret)
>>          } else if (!strcmp(value, "off")) {
>>              *ret = 0;
>>          } else {
>> -            fprintf(stderr, "Option '%s': Use 'on' or 'off'\n", name);
>> +            qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "'on' or 'off'");
>>              return -1;
>>          }
>>      } else {
>> @@ -193,12 +193,12 @@ static int parse_option_number(const char *name, const char *value, uint64_t *re
>>      if (value != NULL) {
>>          number = strtoull(value, &postfix, 0);
>>          if (*postfix != '\0') {
>> -            fprintf(stderr, "Option '%s' needs a number as parameter\n", name);
>> +            qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a number");
>>              return -1;
>>          }
>>          *ret = number;
>>      } else {
>> -        fprintf(stderr, "Option '%s' needs a parameter\n", name);
>> +        qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a number");
>>          return -1;
>>      }
>>      return 0;
>> @@ -226,13 +226,11 @@ static int parse_option_size(const char *name, const char *value, uint64_t *ret)
>>              *ret = (uint64_t) sizef;
>>              break;
>>          default:
>> -            fprintf(stderr, "Option '%s' needs size as parameter\n", name);
>> -            fprintf(stderr, "You may use k, M, G or T suffixes for "
>> -                    "kilobytes, megabytes, gigabytes and terabytes.\n");
>> +            qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a size");
>
>  What about using error_printf_unless_qmp() to improve the error message?
> Although I don't like it very much it documents the code and if we feel like
> we're abusing it we'll have an excuse to do something more complicated
> like error message override.

Sure, can do that.

>>              return -1;
>>          }
>>      } else {
>> -        fprintf(stderr, "Option '%s' needs a parameter\n", name);
>> +        qerror_report(QERR_INVALID_PARAMETER_VALUE, name, "a size");
>>          return -1;
>>      }
>>      return 0;
>> @@ -581,8 +579,7 @@ int qemu_opt_set(QemuOpts *opts, const char *name, const char *value)
>>          if (i == 0) {
>>              /* empty list -> allow any */;
>>          } else {
>> -            fprintf(stderr, "option \"%s\" is not valid for %s\n",
>> -                    name, opts->list->name);
>> +            qerror_report(QERR_INVALID_PARAMETER, name);
>>              return -1;
>>          }
>>      }
>> @@ -598,8 +595,6 @@ int qemu_opt_set(QemuOpts *opts, const char *name, const char *value)
>>          opt->str = qemu_strdup(value);
>>      }
>>      if (qemu_opt_parse(opt) < 0) {
>> -        fprintf(stderr, "Failed to parse \"%s\" for \"%s.%s\"\n", opt->str,
>> -                opts->list->name, opt->name);
>>          qemu_opt_del(opt);
>>          return -1;
>>      }
>
>  Not generating an error on purpose here?

Yes.  When qemu_opt_parse() returns failure, an error was already
emitted.  The first few patch hunks convert those errors.

[Qemu-devel] Re: [PATCH 04/11] error: New QERR_INVALID_PARAMETER_VALUE

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Thu, 18 Mar 2010 17:33:11 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  qerror.c |    4 ++++
>>  qerror.h |    3 +++
>>  2 files changed, 7 insertions(+), 0 deletions(-)
>> 
>> diff --git a/qerror.c b/qerror.c
>> index 9fb817e..97e8d4a 100644
>> --- a/qerror.c
>> +++ b/qerror.c
>> @@ -121,6 +121,10 @@ static const QErrorStringTable qerror_table[] = {
>>          .desc      = "Invalid parameter type, expected: %(expected)",
>>      },
>>      {
>> +        .error_fmt = QERR_INVALID_PARAMETER_VALUE,
>> +        .desc      = "Parameter '%(name)' expects %(expected)",
>> +    },
>
>  Would we need this error if we improve QERR_INVALID_PARAMETER to
> accept an optional 'expects' parameter?

QERR_INVALID_PARAMETER_VALUE means "this value can't be accepted for
this parameter".

QERR_INVALID_PARAMETER means "this parameter can't be accepted,
regardless of value".  Pedantically speaking, that's a different error.
Pragmatically speaking, I doubt clients care much (but I'm the one who
doubts clients care much for differentiating errors in general, so I'm
biased).  Practically speaking, we can use a single error class
'InvalidParameter' if we don't care for the difference, but I figure we
still need separate QERR_ definitions, because we want different
human-readable error messages.

Want me to do anything here now?

>> +    {
>>          .error_fmt = QERR_INVALID_PASSWORD,
>>          .desc      = "Password incorrect",
>>      },
>> diff --git a/qerror.h b/qerror.h
>> index 870cdc3..5625d54 100644
>> --- a/qerror.h
>> +++ b/qerror.h
>> @@ -106,6 +106,9 @@ QError *qobject_to_qerror(const QObject *obj);
>>  #define QERR_INVALID_PARAMETER_TYPE \
>>      "{ 'class': 'InvalidParameterType', 'data': { 'name': %s,'expected': %s } }"
>>  
>> +#define QERR_INVALID_PARAMETER_VALUE \
>> +    "{ 'class': 'InvalidParameterValue', 'data': { 'name': %s, 'expected': %s } }"
>> +
>>  #define QERR_INVALID_PASSWORD \
>>      "{ 'class': 'InvalidPassword', 'data': {} }"
>>  

[Qemu-devel] Re: [PATCH 04/11] error: New QERR_INVALID_PARAMETER_VALUE

[Luiz Capitulino]

On Fri, 19 Mar 2010 22:05:53 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > On Thu, 18 Mar 2010 17:33:11 +0100
> > Markus Armbruster <armbru@redhat.com> wrote:
> >
> >> 
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> ---
> >>  qerror.c |    4 ++++
> >>  qerror.h |    3 +++
> >>  2 files changed, 7 insertions(+), 0 deletions(-)
> >> 
> >> diff --git a/qerror.c b/qerror.c
> >> index 9fb817e..97e8d4a 100644
> >> --- a/qerror.c
> >> +++ b/qerror.c
> >> @@ -121,6 +121,10 @@ static const QErrorStringTable qerror_table[] = {
> >>          .desc      = "Invalid parameter type, expected: %(expected)",
> >>      },
> >>      {
> >> +        .error_fmt = QERR_INVALID_PARAMETER_VALUE,
> >> +        .desc      = "Parameter '%(name)' expects %(expected)",
> >> +    },
> >
> >  Would we need this error if we improve QERR_INVALID_PARAMETER to
> > accept an optional 'expects' parameter?
> 
> QERR_INVALID_PARAMETER_VALUE means "this value can't be accepted for
> this parameter".
> 
> QERR_INVALID_PARAMETER means "this parameter can't be accepted,
> regardless of value".  Pedantically speaking, that's a different error.
> Pragmatically speaking, I doubt clients care much (but I'm the one who
> doubts clients care much for differentiating errors in general, so I'm
> biased).  Practically speaking, we can use a single error class
> 'InvalidParameter' if we don't care for the difference, but I figure we
> still need separate QERR_ definitions, because we want different
> human-readable error messages.
> 
> Want me to do anything here now?

 Well, I agree with the human-readable argument. I'm a bit in doubt
if adding new errors is the solution for that, but it's probably
good enough for the immediate term.

> 
> >> +    {
> >>          .error_fmt = QERR_INVALID_PASSWORD,
> >>          .desc      = "Password incorrect",
> >>      },
> >> diff --git a/qerror.h b/qerror.h
> >> index 870cdc3..5625d54 100644
> >> --- a/qerror.h
> >> +++ b/qerror.h
> >> @@ -106,6 +106,9 @@ QError *qobject_to_qerror(const QObject *obj);
> >>  #define QERR_INVALID_PARAMETER_TYPE \
> >>      "{ 'class': 'InvalidParameterType', 'data': { 'name': %s,'expected': %s } }"
> >>  
> >> +#define QERR_INVALID_PARAMETER_VALUE \
> >> +    "{ 'class': 'InvalidParameterValue', 'data': { 'name': %s, 'expected': %s } }"
> >> +
> >>  #define QERR_INVALID_PASSWORD \
> >>      "{ 'class': 'InvalidPassword', 'data': {} }"
> >>