* [Qemu-devel] [PATCH 00/11] monitor: New commands netdev_add, netdev_del
@ 2010-03-18 16:33 Markus Armbruster
2010-03-18 16:33 ` [Qemu-devel] [PATCH 01/11] error: Put error definitions back in alphabetical order Markus Armbruster
` (11 more replies)
0 siblings, 12 replies; 19+ messages in thread
From: Markus Armbruster @ 2010-03-18 16:33 UTC (permalink / raw)
To: qemu-devel; +Cc: lcapitulino
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(-)
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 01/11] error: Put error definitions back in alphabetical order
2010-03-18 16:33 [Qemu-devel] [PATCH 00/11] monitor: New commands netdev_add, netdev_del Markus Armbruster
@ 2010-03-18 16:33 ` Markus Armbruster
2010-03-18 16:33 ` [Qemu-devel] [PATCH 02/11] error: New QERR_DUPLICATE_ID Markus Armbruster
` (10 subsequent siblings)
11 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2010-03-18 16:33 UTC (permalink / raw)
To: qemu-devel; +Cc: lcapitulino
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 02/11] error: New QERR_DUPLICATE_ID
2010-03-18 16:33 [Qemu-devel] [PATCH 00/11] monitor: New commands netdev_add, netdev_del Markus Armbruster
2010-03-18 16:33 ` [Qemu-devel] [PATCH 01/11] error: Put error definitions back in alphabetical order Markus Armbruster
@ 2010-03-18 16:33 ` Markus Armbruster
2010-03-18 16:33 ` [Qemu-devel] [PATCH 03/11] error: Convert qemu_opts_create() to QError Markus Armbruster
` (9 subsequent siblings)
11 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2010-03-18 16:33 UTC (permalink / raw)
To: qemu-devel; +Cc: lcapitulino
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 03/11] error: Convert qemu_opts_create() to QError
2010-03-18 16:33 [Qemu-devel] [PATCH 00/11] monitor: New commands netdev_add, netdev_del Markus Armbruster
2010-03-18 16:33 ` [Qemu-devel] [PATCH 01/11] error: Put error definitions back in alphabetical order Markus Armbruster
2010-03-18 16:33 ` [Qemu-devel] [PATCH 02/11] error: New QERR_DUPLICATE_ID Markus Armbruster
@ 2010-03-18 16:33 ` Markus Armbruster
2010-03-18 16:33 ` [Qemu-devel] [PATCH 04/11] error: New QERR_INVALID_PARAMETER_VALUE Markus Armbruster
` (8 subsequent siblings)
11 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2010-03-18 16:33 UTC (permalink / raw)
To: qemu-devel; +Cc: lcapitulino
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 04/11] error: New QERR_INVALID_PARAMETER_VALUE
2010-03-18 16:33 [Qemu-devel] [PATCH 00/11] monitor: New commands netdev_add, netdev_del Markus Armbruster
` (2 preceding siblings ...)
2010-03-18 16:33 ` [Qemu-devel] [PATCH 03/11] error: Convert qemu_opts_create() to QError Markus Armbruster
@ 2010-03-18 16:33 ` Markus Armbruster
2010-03-19 19:49 ` [Qemu-devel] " Luiz Capitulino
2010-03-18 16:33 ` [Qemu-devel] [PATCH 05/11] error: Convert qemu_opts_set() to QError Markus Armbruster
` (7 subsequent siblings)
11 siblings, 1 reply; 19+ messages in thread
From: Markus Armbruster @ 2010-03-18 16:33 UTC (permalink / raw)
To: qemu-devel; +Cc: lcapitulino
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 05/11] error: Convert qemu_opts_set() to QError
2010-03-18 16:33 [Qemu-devel] [PATCH 00/11] monitor: New commands netdev_add, netdev_del Markus Armbruster
` (3 preceding siblings ...)
2010-03-18 16:33 ` [Qemu-devel] [PATCH 04/11] error: New QERR_INVALID_PARAMETER_VALUE Markus Armbruster
@ 2010-03-18 16:33 ` Markus Armbruster
2010-03-19 19:50 ` [Qemu-devel] " Luiz Capitulino
2010-03-18 16:33 ` [Qemu-devel] [PATCH 06/11] error: Drop extra messages after qemu_opts_set() and qemu_opts_parse() Markus Armbruster
` (6 subsequent siblings)
11 siblings, 1 reply; 19+ messages in thread
From: Markus Armbruster @ 2010-03-18 16:33 UTC (permalink / raw)
To: qemu-devel; +Cc: lcapitulino
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 06/11] error: Drop extra messages after qemu_opts_set() and qemu_opts_parse()
2010-03-18 16:33 [Qemu-devel] [PATCH 00/11] monitor: New commands netdev_add, netdev_del Markus Armbruster
` (4 preceding siblings ...)
2010-03-18 16:33 ` [Qemu-devel] [PATCH 05/11] error: Convert qemu_opts_set() to QError Markus Armbruster
@ 2010-03-18 16:33 ` Markus Armbruster
2010-03-18 16:33 ` [Qemu-devel] [PATCH 07/11] error: Use QERR_INVALID_PARAMETER_VALUE instead of QERR_INVALID_PARAMETER Markus Armbruster
` (5 subsequent siblings)
11 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2010-03-18 16:33 UTC (permalink / raw)
To: qemu-devel; +Cc: lcapitulino
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 07/11] error: Use QERR_INVALID_PARAMETER_VALUE instead of QERR_INVALID_PARAMETER
2010-03-18 16:33 [Qemu-devel] [PATCH 00/11] monitor: New commands netdev_add, netdev_del Markus Armbruster
` (5 preceding siblings ...)
2010-03-18 16:33 ` [Qemu-devel] [PATCH 06/11] error: Drop extra messages after qemu_opts_set() and qemu_opts_parse() Markus Armbruster
@ 2010-03-18 16:33 ` Markus Armbruster
2010-03-18 16:33 ` [Qemu-devel] [PATCH 08/11] error: Convert qemu_opts_validate() to QError Markus Armbruster
` (4 subsequent siblings)
11 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2010-03-18 16:33 UTC (permalink / raw)
To: qemu-devel; +Cc: lcapitulino
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 08/11] error: Convert qemu_opts_validate() to QError
2010-03-18 16:33 [Qemu-devel] [PATCH 00/11] monitor: New commands netdev_add, netdev_del Markus Armbruster
` (6 preceding siblings ...)
2010-03-18 16:33 ` [Qemu-devel] [PATCH 07/11] error: Use QERR_INVALID_PARAMETER_VALUE instead of QERR_INVALID_PARAMETER Markus Armbruster
@ 2010-03-18 16:33 ` Markus Armbruster
2010-03-18 16:33 ` [Qemu-devel] [PATCH 09/11] error: Convert net_client_init() " Markus Armbruster
` (3 subsequent siblings)
11 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2010-03-18 16:33 UTC (permalink / raw)
To: qemu-devel; +Cc: lcapitulino
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 09/11] error: Convert net_client_init() to QError
2010-03-18 16:33 [Qemu-devel] [PATCH 00/11] monitor: New commands netdev_add, netdev_del Markus Armbruster
` (7 preceding siblings ...)
2010-03-18 16:33 ` [Qemu-devel] [PATCH 08/11] error: Convert qemu_opts_validate() to QError Markus Armbruster
@ 2010-03-18 16:33 ` Markus Armbruster
2010-03-18 16:35 ` [Qemu-devel] [PATCH 10/11] error: New QERR_DEVICE_IN_USE Markus Armbruster
` (2 subsequent siblings)
11 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2010-03-18 16:33 UTC (permalink / raw)
To: qemu-devel; +Cc: lcapitulino
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 10/11] error: New QERR_DEVICE_IN_USE
2010-03-18 16:33 [Qemu-devel] [PATCH 00/11] monitor: New commands netdev_add, netdev_del Markus Armbruster
` (8 preceding siblings ...)
2010-03-18 16:33 ` [Qemu-devel] [PATCH 09/11] error: Convert net_client_init() " Markus Armbruster
@ 2010-03-18 16:35 ` Markus Armbruster
2010-03-18 16:35 ` [Qemu-devel] [PATCH 11/11] monitor: New commands netdev_add, netdev_del Markus Armbruster
2010-03-19 19:48 ` [Qemu-devel] Re: [PATCH 00/11] " Luiz Capitulino
11 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2010-03-18 16:35 UTC (permalink / raw)
To: qemu-devel; +Cc: lcapitulino
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 11/11] monitor: New commands netdev_add, netdev_del
2010-03-18 16:33 [Qemu-devel] [PATCH 00/11] monitor: New commands netdev_add, netdev_del Markus Armbruster
` (9 preceding siblings ...)
2010-03-18 16:35 ` [Qemu-devel] [PATCH 10/11] error: New QERR_DEVICE_IN_USE Markus Armbruster
@ 2010-03-18 16:35 ` Markus Armbruster
2010-03-19 19:48 ` [Qemu-devel] Re: [PATCH 00/11] " Luiz Capitulino
11 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2010-03-18 16:35 UTC (permalink / raw)
To: qemu-devel; +Cc: lcapitulino
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] Re: [PATCH 00/11] monitor: New commands netdev_add, netdev_del
2010-03-18 16:33 [Qemu-devel] [PATCH 00/11] monitor: New commands netdev_add, netdev_del Markus Armbruster
` (10 preceding siblings ...)
2010-03-18 16:35 ` [Qemu-devel] [PATCH 11/11] monitor: New commands netdev_add, netdev_del Markus Armbruster
@ 2010-03-19 19:48 ` Luiz Capitulino
2010-03-19 20:55 ` Markus Armbruster
11 siblings, 1 reply; 19+ messages in thread
From: Luiz Capitulino @ 2010-03-19 19:48 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel
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(-)
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] Re: [PATCH 04/11] error: New QERR_INVALID_PARAMETER_VALUE
2010-03-18 16:33 ` [Qemu-devel] [PATCH 04/11] error: New QERR_INVALID_PARAMETER_VALUE Markus Armbruster
@ 2010-03-19 19:49 ` Luiz Capitulino
2010-03-19 21:05 ` Markus Armbruster
0 siblings, 1 reply; 19+ messages in thread
From: Luiz Capitulino @ 2010-03-19 19:49 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel
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': {} }"
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] Re: [PATCH 05/11] error: Convert qemu_opts_set() to QError
2010-03-18 16:33 ` [Qemu-devel] [PATCH 05/11] error: Convert qemu_opts_set() to QError Markus Armbruster
@ 2010-03-19 19:50 ` Luiz Capitulino
2010-03-19 20:58 ` Markus Armbruster
0 siblings, 1 reply; 19+ messages in thread
From: Luiz Capitulino @ 2010-03-19 19:50 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel
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?
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] Re: [PATCH 00/11] monitor: New commands netdev_add, netdev_del
2010-03-19 19:48 ` [Qemu-devel] Re: [PATCH 00/11] " Luiz Capitulino
@ 2010-03-19 20:55 ` Markus Armbruster
0 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2010-03-19 20:55 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: qemu-devel
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.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] Re: [PATCH 05/11] error: Convert qemu_opts_set() to QError
2010-03-19 19:50 ` [Qemu-devel] " Luiz Capitulino
@ 2010-03-19 20:58 ` Markus Armbruster
0 siblings, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2010-03-19 20:58 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: qemu-devel
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.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] Re: [PATCH 04/11] error: New QERR_INVALID_PARAMETER_VALUE
2010-03-19 19:49 ` [Qemu-devel] " Luiz Capitulino
@ 2010-03-19 21:05 ` Markus Armbruster
2010-03-22 0:34 ` Luiz Capitulino
0 siblings, 1 reply; 19+ messages in thread
From: Markus Armbruster @ 2010-03-19 21:05 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: qemu-devel
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': {} }"
>>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] Re: [PATCH 04/11] error: New QERR_INVALID_PARAMETER_VALUE
2010-03-19 21:05 ` Markus Armbruster
@ 2010-03-22 0:34 ` Luiz Capitulino
0 siblings, 0 replies; 19+ messages in thread
From: Luiz Capitulino @ 2010-03-22 0:34 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel
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': {} }"
> >>
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2010-03-22 0:34 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-03-18 16:33 [Qemu-devel] [PATCH 00/11] monitor: New commands netdev_add, netdev_del Markus Armbruster
2010-03-18 16:33 ` [Qemu-devel] [PATCH 01/11] error: Put error definitions back in alphabetical order Markus Armbruster
2010-03-18 16:33 ` [Qemu-devel] [PATCH 02/11] error: New QERR_DUPLICATE_ID Markus Armbruster
2010-03-18 16:33 ` [Qemu-devel] [PATCH 03/11] error: Convert qemu_opts_create() to QError Markus Armbruster
2010-03-18 16:33 ` [Qemu-devel] [PATCH 04/11] error: New QERR_INVALID_PARAMETER_VALUE Markus Armbruster
2010-03-19 19:49 ` [Qemu-devel] " Luiz Capitulino
2010-03-19 21:05 ` Markus Armbruster
2010-03-22 0:34 ` Luiz Capitulino
2010-03-18 16:33 ` [Qemu-devel] [PATCH 05/11] error: Convert qemu_opts_set() to QError Markus Armbruster
2010-03-19 19:50 ` [Qemu-devel] " Luiz Capitulino
2010-03-19 20:58 ` Markus Armbruster
2010-03-18 16:33 ` [Qemu-devel] [PATCH 06/11] error: Drop extra messages after qemu_opts_set() and qemu_opts_parse() Markus Armbruster
2010-03-18 16:33 ` [Qemu-devel] [PATCH 07/11] error: Use QERR_INVALID_PARAMETER_VALUE instead of QERR_INVALID_PARAMETER Markus Armbruster
2010-03-18 16:33 ` [Qemu-devel] [PATCH 08/11] error: Convert qemu_opts_validate() to QError Markus Armbruster
2010-03-18 16:33 ` [Qemu-devel] [PATCH 09/11] error: Convert net_client_init() " Markus Armbruster
2010-03-18 16:35 ` [Qemu-devel] [PATCH 10/11] error: New QERR_DEVICE_IN_USE Markus Armbruster
2010-03-18 16:35 ` [Qemu-devel] [PATCH 11/11] monitor: New commands netdev_add, netdev_del Markus Armbruster
2010-03-19 19:48 ` [Qemu-devel] Re: [PATCH 00/11] " Luiz Capitulino
2010-03-19 20:55 ` Markus Armbruster
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.