* [Qemu-devel] [PULL 01/14] vl: add missing transition debug->finish_migrate
2014-01-06 22:03 [Qemu-devel] [PULL 00/14] QMP queue Luiz Capitulino
@ 2014-01-06 22:03 ` Luiz Capitulino
2014-01-06 22:03 ` [Qemu-devel] [PULL 02/14] qemu-monitor: HMP cpu-add wrapper Luiz Capitulino
` (14 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Luiz Capitulino @ 2014-01-06 22:03 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
From: Paolo Bonzini <pbonzini@redhat.com>
This fixes an abort if you invoke the "migrate" command while the
guest is being debugged.
Cc: qemu-stable@nongnu.org
Cc: lcapitulino@redhat.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
vl.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/vl.c b/vl.c
index 7511e70..0f67545 100644
--- a/vl.c
+++ b/vl.c
@@ -591,6 +591,7 @@ typedef struct {
static const RunStateTransition runstate_transitions_def[] = {
/* from -> to */
{ RUN_STATE_DEBUG, RUN_STATE_RUNNING },
+ { RUN_STATE_DEBUG, RUN_STATE_FINISH_MIGRATE },
{ RUN_STATE_INMIGRATE, RUN_STATE_RUNNING },
{ RUN_STATE_INMIGRATE, RUN_STATE_PAUSED },
--
1.8.1.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL 02/14] qemu-monitor: HMP cpu-add wrapper
2014-01-06 22:03 [Qemu-devel] [PULL 00/14] QMP queue Luiz Capitulino
2014-01-06 22:03 ` [Qemu-devel] [PULL 01/14] vl: add missing transition debug->finish_migrate Luiz Capitulino
@ 2014-01-06 22:03 ` Luiz Capitulino
2014-01-06 22:03 ` [Qemu-devel] [PULL 03/14] rng: initialize file descriptor to -1 Luiz Capitulino
` (13 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Luiz Capitulino @ 2014-01-06 22:03 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
From: "Jason J. Herne" <jjherne@us.ibm.com>
Add HMP cpu-add wrapper to allow cpu hot plugging via monitor.
Signed-off-by: Jason J. Herne <jjherne@us.ibm.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
hmp-commands.hx | 13 +++++++++++++
hmp.c | 10 ++++++++++
hmp.h | 1 +
3 files changed, 24 insertions(+)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index ebe8e78..929550d 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1620,6 +1620,19 @@ Executes a qemu-io command on the given block device.
ETEXI
{
+ .name = "cpu-add",
+ .args_type = "id:i",
+ .params = "id",
+ .help = "add cpu",
+ .mhandler.cmd = hmp_cpu_add,
+ },
+
+STEXI
+@item cpu-add @var{id}
+Add CPU with id @var{id}
+ETEXI
+
+ {
.name = "info",
.args_type = "item:s?",
.params = "[subcommand]",
diff --git a/hmp.c b/hmp.c
index 32ee285..c513f9b 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1525,6 +1525,16 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, &errp);
}
+void hmp_cpu_add(Monitor *mon, const QDict *qdict)
+{
+ int cpuid;
+ Error *err = NULL;
+
+ cpuid = qdict_get_int(qdict, "id");
+ qmp_cpu_add(cpuid, &err);
+ hmp_handle_error(mon, &err);
+}
+
void hmp_chardev_add(Monitor *mon, const QDict *qdict)
{
const char *args = qdict_get_str(qdict, "args");
diff --git a/hmp.h b/hmp.h
index 54cf71f..f92fc89 100644
--- a/hmp.h
+++ b/hmp.h
@@ -89,5 +89,6 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict);
void hmp_chardev_add(Monitor *mon, const QDict *qdict);
void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
void hmp_qemu_io(Monitor *mon, const QDict *qdict);
+void hmp_cpu_add(Monitor *mon, const QDict *qdict);
#endif
--
1.8.1.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL 03/14] rng: initialize file descriptor to -1
2014-01-06 22:03 [Qemu-devel] [PULL 00/14] QMP queue Luiz Capitulino
2014-01-06 22:03 ` [Qemu-devel] [PULL 01/14] vl: add missing transition debug->finish_migrate Luiz Capitulino
2014-01-06 22:03 ` [Qemu-devel] [PULL 02/14] qemu-monitor: HMP cpu-add wrapper Luiz Capitulino
@ 2014-01-06 22:03 ` Luiz Capitulino
2014-01-06 22:03 ` [Qemu-devel] [PULL 04/14] qom: fix leak for objects created with -object Luiz Capitulino
` (12 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Luiz Capitulino @ 2014-01-06 22:03 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
From: Paolo Bonzini <pbonzini@redhat.com>
The file descriptor is never initialized to -1, which makes rng-random
close stdin if an object is created and immediately destroyed. If we
change it to -1, we also need to protect qemu_set_fd_handler from
receiving a bogus file descriptor.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Tested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
backends/rng-random.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/backends/rng-random.c b/backends/rng-random.c
index 68dfc8a..136499d 100644
--- a/backends/rng-random.c
+++ b/backends/rng-random.c
@@ -123,15 +123,15 @@ static void rng_random_init(Object *obj)
NULL);
s->filename = g_strdup("/dev/random");
+ s->fd = -1;
}
static void rng_random_finalize(Object *obj)
{
RndRandom *s = RNG_RANDOM(obj);
- qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
-
if (s->fd != -1) {
+ qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
qemu_close(s->fd);
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL 04/14] qom: fix leak for objects created with -object
2014-01-06 22:03 [Qemu-devel] [PULL 00/14] QMP queue Luiz Capitulino
` (2 preceding siblings ...)
2014-01-06 22:03 ` [Qemu-devel] [PULL 03/14] rng: initialize file descriptor to -1 Luiz Capitulino
@ 2014-01-06 22:03 ` Luiz Capitulino
2014-01-06 22:03 ` [Qemu-devel] [PULL 05/14] qom: catch errors in object_property_add_child Luiz Capitulino
` (11 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Luiz Capitulino @ 2014-01-06 22:03 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
From: Paolo Bonzini <pbonzini@redhat.com>
The object must be unref-ed when its variable goes out of scope.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Tested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
vl.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/vl.c b/vl.c
index 0f67545..2170a5e 100644
--- a/vl.c
+++ b/vl.c
@@ -2810,12 +2810,13 @@ static int object_create(QemuOpts *opts, void *opaque)
obj = object_new(type);
if (qemu_opt_foreach(opts, object_set_property, obj, 1) < 0) {
+ object_unref(obj);
return -1;
}
object_property_add_child(container_get(object_get_root(), "/objects"),
id, obj, NULL);
-
+ object_unref(obj);
return 0;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL 05/14] qom: catch errors in object_property_add_child
2014-01-06 22:03 [Qemu-devel] [PULL 00/14] QMP queue Luiz Capitulino
` (3 preceding siblings ...)
2014-01-06 22:03 ` [Qemu-devel] [PULL 04/14] qom: fix leak for objects created with -object Luiz Capitulino
@ 2014-01-06 22:03 ` Luiz Capitulino
2014-01-06 22:03 ` [Qemu-devel] [PULL 06/14] monitor: add object-del (QMP) and object_del (HMP) command Luiz Capitulino
` (10 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Luiz Capitulino @ 2014-01-06 22:03 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
From: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Tested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qom/object.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/qom/object.c b/qom/object.c
index fc19cf6..4dee02b 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -988,17 +988,22 @@ static void object_finalize_child_property(Object *obj, const char *name,
void object_property_add_child(Object *obj, const char *name,
Object *child, Error **errp)
{
+ Error *local_err = NULL;
gchar *type;
type = g_strdup_printf("child<%s>", object_get_typename(OBJECT(child)));
- object_property_add(obj, name, type, object_get_child_property,
- NULL, object_finalize_child_property, child, errp);
-
+ object_property_add(obj, name, type, object_get_child_property, NULL,
+ object_finalize_child_property, child, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ goto out;
+ }
object_ref(child);
g_assert(child->parent == NULL);
child->parent = obj;
+out:
g_free(type);
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL 06/14] monitor: add object-del (QMP) and object_del (HMP) command
2014-01-06 22:03 [Qemu-devel] [PULL 00/14] QMP queue Luiz Capitulino
` (4 preceding siblings ...)
2014-01-06 22:03 ` [Qemu-devel] [PULL 05/14] qom: catch errors in object_property_add_child Luiz Capitulino
@ 2014-01-06 22:03 ` Luiz Capitulino
2014-01-06 22:03 ` [Qemu-devel] [PULL 07/14] monitor: add object-add (QMP) and object_add " Luiz Capitulino
` (9 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Luiz Capitulino @ 2014-01-06 22:03 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
From: Paolo Bonzini <pbonzini@redhat.com>
These two commands invoke the "unparent" method of Object.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Tested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
hmp-commands.hx | 14 ++++++++++++++
hmp.c | 9 +++++++++
hmp.h | 1 +
qapi-schema.json | 14 ++++++++++++++
qmp-commands.hx | 25 +++++++++++++++++++++++++
qmp.c | 14 ++++++++++++++
6 files changed, 77 insertions(+)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 929550d..4d5e2b5 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1243,6 +1243,20 @@ STEXI
Remove host network device.
ETEXI
+ {
+ .name = "object_del",
+ .args_type = "id:s",
+ .params = "id",
+ .help = "destroy QOM object",
+ .mhandler.cmd = hmp_object_del,
+ },
+
+STEXI
+@item object_del
+@findex object_del
+Destroy QOM object.
+ETEXI
+
#ifdef CONFIG_SLIRP
{
.name = "hostfwd_add",
diff --git a/hmp.c b/hmp.c
index c513f9b..fe05c6b 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1574,3 +1574,12 @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, &err);
}
+
+void hmp_object_del(Monitor *mon, const QDict *qdict)
+{
+ const char *id = qdict_get_str(qdict, "id");
+ Error *err = NULL;
+
+ qmp_object_del(id, &err);
+ hmp_handle_error(mon, &err);
+}
diff --git a/hmp.h b/hmp.h
index f92fc89..7a11f68 100644
--- a/hmp.h
+++ b/hmp.h
@@ -90,5 +90,6 @@ void hmp_chardev_add(Monitor *mon, const QDict *qdict);
void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
void hmp_qemu_io(Monitor *mon, const QDict *qdict);
void hmp_cpu_add(Monitor *mon, const QDict *qdict);
+void hmp_object_del(Monitor *mon, const QDict *qdict);
#endif
diff --git a/qapi-schema.json b/qapi-schema.json
index c3c939c..af3a83b 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2759,6 +2759,20 @@
{ 'command': 'netdev_del', 'data': {'id': 'str'} }
##
+# @object-del:
+#
+# Remove a QOM object.
+#
+# @id: the name of the QOM object to remove
+#
+# Returns: Nothing on success
+# Error if @id is not a valid id for a QOM object
+#
+# Since: 2.0
+##
+{ 'command': 'object-del', 'data': {'id': 'str'} }
+
+##
# @NetdevNoneOptions
#
# Use it alone to have zero network devices.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index fba15cd..71422cd 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -879,6 +879,31 @@ Example:
EQMP
{
+ .name = "object-del",
+ .args_type = "id:s",
+ .mhandler.cmd_new = qmp_marshal_input_object_del,
+ },
+
+SQMP
+object-del
+----------
+
+Remove QOM object.
+
+Arguments:
+
+- "id": the object's ID (json-string)
+
+Example:
+
+-> { "execute": "object-del", "arguments": { "id": "rng1" } }
+<- { "return": {} }
+
+
+EQMP
+
+
+ {
.name = "block_resize",
.args_type = "device:B,size:o",
.mhandler.cmd_new = qmp_marshal_input_block_resize,
diff --git a/qmp.c b/qmp.c
index 1d7a04d..73aab58 100644
--- a/qmp.c
+++ b/qmp.c
@@ -529,3 +529,17 @@ void qmp_add_client(const char *protocol, const char *fdname,
error_setg(errp, "protocol '%s' is invalid", protocol);
close(fd);
}
+
+void qmp_object_del(const char *id, Error **errp)
+{
+ Object *container;
+ Object *obj;
+
+ container = container_get(object_get_root(), "/objects");
+ obj = object_resolve_path_component(container, id);
+ if (!obj) {
+ error_setg(errp, "object id not found");
+ return;
+ }
+ object_unparent(obj);
+}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL 07/14] monitor: add object-add (QMP) and object_add (HMP) command
2014-01-06 22:03 [Qemu-devel] [PULL 00/14] QMP queue Luiz Capitulino
` (5 preceding siblings ...)
2014-01-06 22:03 ` [Qemu-devel] [PULL 06/14] monitor: add object-del (QMP) and object_del (HMP) command Luiz Capitulino
@ 2014-01-06 22:03 ` Luiz Capitulino
2014-01-06 22:03 ` [Qemu-devel] [PULL 08/14] error: Add error_abort Luiz Capitulino
` (8 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Luiz Capitulino @ 2014-01-06 22:03 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
From: Paolo Bonzini <pbonzini@redhat.com>
Add two commands that are the monitor counterparts of -object. The commands
have the same Visitor-based implementation, but use different kinds of
visitors so that the HMP command has a DWIM string-based syntax, while
the QMP variant accepts a stricter JSON-based properties dictionary.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Tested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
hmp-commands.hx | 14 +++++++++++
hmp.c | 58 ++++++++++++++++++++++++++++++++++++++++++++
hmp.h | 1 +
include/monitor/monitor.h | 3 +++
include/qapi/visitor.h | 3 +--
include/qemu/typedefs.h | 2 ++
qapi-schema.json | 20 +++++++++++++++
qmp-commands.hx | 26 ++++++++++++++++++++
qmp.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++
9 files changed, 187 insertions(+), 2 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 4d5e2b5..feca084 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1244,6 +1244,20 @@ Remove host network device.
ETEXI
{
+ .name = "object_add",
+ .args_type = "object:O",
+ .params = "[qom-type=]type,id=str[,prop=value][,...]",
+ .help = "create QOM object",
+ .mhandler.cmd = hmp_object_add,
+ },
+
+STEXI
+@item object_add
+@findex object_add
+Create QOM object.
+ETEXI
+
+ {
.name = "object_del",
.args_type = "id:s",
.params = "id",
diff --git a/hmp.c b/hmp.c
index fe05c6b..79f9c7d 100644
--- a/hmp.c
+++ b/hmp.c
@@ -21,6 +21,7 @@
#include "qmp-commands.h"
#include "qemu/sockets.h"
#include "monitor/monitor.h"
+#include "qapi/opts-visitor.h"
#include "ui/console.h"
#include "block/qapi.h"
#include "qemu-io.h"
@@ -1354,6 +1355,63 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, &err);
}
+void hmp_object_add(Monitor *mon, const QDict *qdict)
+{
+ Error *err = NULL;
+ QemuOpts *opts;
+ char *type = NULL;
+ char *id = NULL;
+ void *dummy = NULL;
+ OptsVisitor *ov;
+ QDict *pdict;
+
+ opts = qemu_opts_from_qdict(qemu_find_opts("object"), qdict, &err);
+ if (err) {
+ goto out;
+ }
+
+ ov = opts_visitor_new(opts);
+ pdict = qdict_clone_shallow(qdict);
+
+ visit_start_struct(opts_get_visitor(ov), &dummy, NULL, NULL, 0, &err);
+ if (err) {
+ goto out_clean;
+ }
+
+ qdict_del(pdict, "qom-type");
+ visit_type_str(opts_get_visitor(ov), &type, "qom-type", &err);
+ if (err) {
+ goto out_clean;
+ }
+
+ qdict_del(pdict, "id");
+ visit_type_str(opts_get_visitor(ov), &id, "id", &err);
+ if (err) {
+ goto out_clean;
+ }
+
+ object_add(type, id, pdict, opts_get_visitor(ov), &err);
+ if (err) {
+ goto out_clean;
+ }
+ visit_end_struct(opts_get_visitor(ov), &err);
+ if (err) {
+ qmp_object_del(id, NULL);
+ }
+
+out_clean:
+ opts_visitor_cleanup(ov);
+
+ QDECREF(pdict);
+ qemu_opts_del(opts);
+ g_free(id);
+ g_free(type);
+ g_free(dummy);
+
+out:
+ hmp_handle_error(mon, &err);
+}
+
void hmp_getfd(Monitor *mon, const QDict *qdict)
{
const char *fdname = qdict_get_str(qdict, "fdname");
diff --git a/hmp.h b/hmp.h
index 7a11f68..ed58f0e 100644
--- a/hmp.h
+++ b/hmp.h
@@ -90,6 +90,7 @@ void hmp_chardev_add(Monitor *mon, const QDict *qdict);
void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
void hmp_qemu_io(Monitor *mon, const QDict *qdict);
void hmp_cpu_add(Monitor *mon, const QDict *qdict);
+void hmp_object_add(Monitor *mon, const QDict *qdict);
void hmp_object_del(Monitor *mon, const QDict *qdict);
#endif
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 10fa0e3..22d8b8f 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -93,6 +93,9 @@ int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
int qmp_qom_set(Monitor *mon, const QDict *qdict, QObject **ret);
int qmp_qom_get(Monitor *mon, const QDict *qdict, QObject **ret);
+int qmp_object_add(Monitor *mon, const QDict *qdict, QObject **ret);
+void object_add(const char *type, const char *id, const QDict *qdict,
+ Visitor *v, Error **errp);
AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
bool has_opaque, const char *opaque,
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 48a2a2e..29da211 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -13,6 +13,7 @@
#ifndef QAPI_VISITOR_CORE_H
#define QAPI_VISITOR_CORE_H
+#include "qemu/typedefs.h"
#include "qapi/qmp/qobject.h"
#include "qapi/error.h"
#include <stdlib.h>
@@ -26,8 +27,6 @@ typedef struct GenericList
struct GenericList *next;
} GenericList;
-typedef struct Visitor Visitor;
-
void visit_start_handle(Visitor *v, void **obj, const char *kind,
const char *name, Error **errp);
void visit_end_handle(Visitor *v, Error **errp);
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index a4c1b84..4524496 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -10,6 +10,8 @@ typedef struct QEMUBH QEMUBH;
typedef struct AioContext AioContext;
+typedef struct Visitor Visitor;
+
struct Monitor;
typedef struct Monitor Monitor;
typedef struct MigrationParams MigrationParams;
diff --git a/qapi-schema.json b/qapi-schema.json
index af3a83b..4abbf36 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2759,6 +2759,26 @@
{ 'command': 'netdev_del', 'data': {'id': 'str'} }
##
+# @object-add:
+#
+# Create a QOM object.
+#
+# @qom-type: the class name for the object to be created
+#
+# @id: the name of the new object
+#
+# @props: #optional a dictionary of properties to be passed to the backend
+#
+# Returns: Nothing on success
+# Error if @qom-type is not a valid class name
+#
+# Since: 2.0
+##
+{ 'command': 'object-add',
+ 'data': {'qom-type': 'str', 'id': 'str', '*props': 'dict'},
+ 'gen': 'no' }
+
+##
# @object-del:
#
# Remove a QOM object.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 71422cd..02cc815 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -879,6 +879,32 @@ Example:
EQMP
{
+ .name = "object-add",
+ .args_type = "qom-type:s,id:s,props:q?",
+ .mhandler.cmd_new = qmp_object_add,
+ },
+
+SQMP
+object-add
+----------
+
+Create QOM object.
+
+Arguments:
+
+- "qom-type": the object's QOM type, i.e. the class name (json-string)
+- "id": the object's ID, must be unique (json-string)
+- "props": a dictionary of object property values (optional, json-dict)
+
+Example:
+
+-> { "execute": "object-add", "arguments": { "qom-type": "rng-random", "id": "rng1",
+ "props": { "filename": "/dev/hwrng" } } }
+<- { "return": {} }
+
+EQMP
+
+ {
.name = "object-del",
.args_type = "id:s",
.mhandler.cmd_new = qmp_marshal_input_object_del,
diff --git a/qmp.c b/qmp.c
index 73aab58..0f46171 100644
--- a/qmp.c
+++ b/qmp.c
@@ -24,6 +24,8 @@
#include "hw/qdev.h"
#include "sysemu/blockdev.h"
#include "qom/qom-qobject.h"
+#include "qapi/qmp/qobject.h"
+#include "qapi/qmp-input-visitor.h"
#include "hw/boards.h"
NameInfo *qmp_query_name(Error **errp)
@@ -530,6 +532,66 @@ void qmp_add_client(const char *protocol, const char *fdname,
close(fd);
}
+void object_add(const char *type, const char *id, const QDict *qdict,
+ Visitor *v, Error **errp)
+{
+ Object *obj;
+ const QDictEntry *e;
+ Error *local_err = NULL;
+
+ if (!object_class_by_name(type)) {
+ error_setg(errp, "invalid class name");
+ return;
+ }
+
+ obj = object_new(type);
+ if (qdict) {
+ for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
+ object_property_set(obj, v, e->key, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ object_unref(obj);
+ return;
+ }
+ }
+ }
+
+ object_property_add_child(container_get(object_get_root(), "/objects"),
+ id, obj, errp);
+ object_unref(obj);
+}
+
+int qmp_object_add(Monitor *mon, const QDict *qdict, QObject **ret)
+{
+ const char *type = qdict_get_str(qdict, "qom-type");
+ const char *id = qdict_get_str(qdict, "id");
+ QObject *props = qdict_get(qdict, "props");
+ const QDict *pdict = NULL;
+ Error *local_err = NULL;
+ QmpInputVisitor *qiv;
+
+ if (props) {
+ pdict = qobject_to_qdict(props);
+ if (!pdict) {
+ error_set(&local_err, QERR_INVALID_PARAMETER_TYPE, "props", "dict");
+ goto out;
+ }
+ }
+
+ qiv = qmp_input_visitor_new(props);
+ object_add(type, id, pdict, qmp_input_get_visitor(qiv), &local_err);
+ qmp_input_visitor_cleanup(qiv);
+
+out:
+ if (local_err) {
+ qerror_report_err(local_err);
+ error_free(local_err);
+ return -1;
+ }
+
+ return 0;
+}
+
void qmp_object_del(const char *id, Error **errp)
{
Object *container;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL 08/14] error: Add error_abort
2014-01-06 22:03 [Qemu-devel] [PULL 00/14] QMP queue Luiz Capitulino
` (6 preceding siblings ...)
2014-01-06 22:03 ` [Qemu-devel] [PULL 07/14] monitor: add object-add (QMP) and object_add " Luiz Capitulino
@ 2014-01-06 22:03 ` Luiz Capitulino
2014-01-06 22:03 ` [Qemu-devel] [PULL 09/14] qdev: Delete dead code Luiz Capitulino
` (7 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Luiz Capitulino @ 2014-01-06 22:03 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Add a special Error * that can be passed to error handling APIs to
signal that any errors are fatal and should abort QEMU. There are two
advantages to this:
- allows for brevity when wishing to assert success of Error **
accepting APIs. No need for this pattern:
Error * local_err = NULL;
api_call(foo, bar, &local_err);
assert_no_error(local_err);
This also removes the need for _nofail variants of APIs with
asserting call sites now reduced to 1LOC.
- SIGABRT happens from within the offending API. When a fatal error
occurs in an API call (when the caller is asserting sucess) failure
often means the API itself is broken. With the abort happening in the
API call now, the stack frames into the call are available at debug
time. In the assert_no_error scheme the abort happens after the fact.
The exact semantic is that when an error is raised, if the argument
Error ** matches &error_abort, then the abort occurs immediately. The
error messaged is reported.
For error_propagate, if the destination error is &error_abort, then
the abort happens at propagation time.
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
include/qapi/error.h | 6 ++++++
util/error.c | 22 +++++++++++++++++++++-
2 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/include/qapi/error.h b/include/qapi/error.h
index 7d4c696..c0f0c3b 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -95,4 +95,10 @@ void error_propagate(Error **dst_err, Error *local_err);
*/
void error_free(Error *err);
+/**
+ * If passed to error_set and friends, abort().
+ */
+
+extern Error *error_abort;
+
#endif
diff --git a/util/error.c b/util/error.c
index 3ee362a..f11f1d5 100644
--- a/util/error.c
+++ b/util/error.c
@@ -23,6 +23,8 @@ struct Error
ErrorClass err_class;
};
+Error *error_abort;
+
void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
{
Error *err;
@@ -41,6 +43,11 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
va_end(ap);
err->err_class = err_class;
+ if (errp == &error_abort) {
+ error_report("%s", error_get_pretty(err));
+ abort();
+ }
+
*errp = err;
errno = saved_errno;
@@ -72,6 +79,11 @@ void error_set_errno(Error **errp, int os_errno, ErrorClass err_class,
va_end(ap);
err->err_class = err_class;
+ if (errp == &error_abort) {
+ error_report("%s", error_get_pretty(err));
+ abort();
+ }
+
*errp = err;
errno = saved_errno;
@@ -112,6 +124,11 @@ void error_set_win32(Error **errp, int win32_err, ErrorClass err_class,
va_end(ap);
err->err_class = err_class;
+ if (errp == &error_abort) {
+ error_report("%s", error_get_pretty(err));
+ abort();
+ }
+
*errp = err;
}
@@ -153,7 +170,10 @@ void error_free(Error *err)
void error_propagate(Error **dst_err, Error *local_err)
{
- if (dst_err && !*dst_err) {
+ if (local_err && dst_err == &error_abort) {
+ error_report("%s", error_get_pretty(local_err));
+ abort();
+ } else if (dst_err && !*dst_err) {
*dst_err = local_err;
} else if (local_err) {
error_free(local_err);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL 09/14] qdev: Delete dead code
2014-01-06 22:03 [Qemu-devel] [PULL 00/14] QMP queue Luiz Capitulino
` (7 preceding siblings ...)
2014-01-06 22:03 ` [Qemu-devel] [PULL 08/14] error: Add error_abort Luiz Capitulino
@ 2014-01-06 22:03 ` Luiz Capitulino
2014-01-06 22:03 ` [Qemu-devel] [PULL 10/14] hw: Remove assert_no_error usages Luiz Capitulino
` (6 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Luiz Capitulino @ 2014-01-06 22:03 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
This is unreachable code, as it's already asserted that no errors have
occurred. Delete.
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
hw/core/qdev.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index e374a93..adbff18 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -746,11 +746,6 @@ static void device_initfn(Object *obj)
}
class = object_class_get_parent(class);
} while (class != object_class_by_name(TYPE_DEVICE));
- if (err != NULL) {
- qerror_report_err(err);
- error_free(err);
- exit(1);
- }
object_property_add_link(OBJECT(dev), "parent_bus", TYPE_BUS,
(Object **)&dev->parent_bus, &err);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL 10/14] hw: Remove assert_no_error usages
2014-01-06 22:03 [Qemu-devel] [PULL 00/14] QMP queue Luiz Capitulino
` (8 preceding siblings ...)
2014-01-06 22:03 ` [Qemu-devel] [PULL 09/14] qdev: Delete dead code Luiz Capitulino
@ 2014-01-06 22:03 ` Luiz Capitulino
2014-01-06 22:03 ` [Qemu-devel] [PULL 11/14] target-i386: Remove assert_no_error usage Luiz Capitulino
` (5 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Luiz Capitulino @ 2014-01-06 22:03 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Replace assert_no_error() usages with the error_abort system.
&error_abort is passed into API calls to signal to the Error sub-system
that any errors are fatal. Removes need for caller assertions.
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
hw/core/qdev-properties-system.c | 8 ++------
hw/core/qdev-properties.c | 40 ++++++++++------------------------------
hw/core/qdev.c | 23 +++++++----------------
hw/dma/xilinx_axidma.c | 13 ++++---------
hw/net/xilinx_axienet.c | 13 ++++---------
include/hw/xilinx.h | 14 ++++----------
target-arm/cpu.c | 7 ++-----
7 files changed, 33 insertions(+), 85 deletions(-)
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 729efa8..3f29b49 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -352,21 +352,17 @@ void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name,
void qdev_prop_set_chr(DeviceState *dev, const char *name,
CharDriverState *value)
{
- Error *errp = NULL;
assert(!value || value->label);
object_property_set_str(OBJECT(dev),
- value ? value->label : "", name, &errp);
- assert_no_error(errp);
+ value ? value->label : "", name, &error_abort);
}
void qdev_prop_set_netdev(DeviceState *dev, const char *name,
NetClientState *value)
{
- Error *errp = NULL;
assert(!value || value->name);
object_property_set_str(OBJECT(dev),
- value ? value->name : "", name, &errp);
- assert_no_error(errp);
+ value ? value->name : "", name, &error_abort);
}
void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index dc8ae69..b949f0e 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1003,73 +1003,55 @@ void qdev_prop_parse(DeviceState *dev, const char *name, const char *value,
void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value)
{
- Error *errp = NULL;
- object_property_set_bool(OBJECT(dev), value, name, &errp);
- assert_no_error(errp);
+ object_property_set_bool(OBJECT(dev), value, name, &error_abort);
}
void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value)
{
- Error *errp = NULL;
- object_property_set_int(OBJECT(dev), value, name, &errp);
- assert_no_error(errp);
+ object_property_set_int(OBJECT(dev), value, name, &error_abort);
}
void qdev_prop_set_uint16(DeviceState *dev, const char *name, uint16_t value)
{
- Error *errp = NULL;
- object_property_set_int(OBJECT(dev), value, name, &errp);
- assert_no_error(errp);
+ object_property_set_int(OBJECT(dev), value, name, &error_abort);
}
void qdev_prop_set_uint32(DeviceState *dev, const char *name, uint32_t value)
{
- Error *errp = NULL;
- object_property_set_int(OBJECT(dev), value, name, &errp);
- assert_no_error(errp);
+ object_property_set_int(OBJECT(dev), value, name, &error_abort);
}
void qdev_prop_set_int32(DeviceState *dev, const char *name, int32_t value)
{
- Error *errp = NULL;
- object_property_set_int(OBJECT(dev), value, name, &errp);
- assert_no_error(errp);
+ object_property_set_int(OBJECT(dev), value, name, &error_abort);
}
void qdev_prop_set_uint64(DeviceState *dev, const char *name, uint64_t value)
{
- Error *errp = NULL;
- object_property_set_int(OBJECT(dev), value, name, &errp);
- assert_no_error(errp);
+ object_property_set_int(OBJECT(dev), value, name, &error_abort);
}
void qdev_prop_set_string(DeviceState *dev, const char *name, const char *value)
{
- Error *errp = NULL;
- object_property_set_str(OBJECT(dev), value, name, &errp);
- assert_no_error(errp);
+ object_property_set_str(OBJECT(dev), value, name, &error_abort);
}
void qdev_prop_set_macaddr(DeviceState *dev, const char *name, uint8_t *value)
{
- Error *errp = NULL;
char str[2 * 6 + 5 + 1];
snprintf(str, sizeof(str), "%02x:%02x:%02x:%02x:%02x:%02x",
value[0], value[1], value[2], value[3], value[4], value[5]);
- object_property_set_str(OBJECT(dev), str, name, &errp);
- assert_no_error(errp);
+ object_property_set_str(OBJECT(dev), str, name, &error_abort);
}
void qdev_prop_set_enum(DeviceState *dev, const char *name, int value)
{
Property *prop;
- Error *errp = NULL;
prop = qdev_prop_find(dev, name);
object_property_set_str(OBJECT(dev), prop->info->enum_table[value],
- name, &errp);
- assert_no_error(errp);
+ name, &error_abort);
}
void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value)
@@ -1161,12 +1143,10 @@ static void set_size(Object *obj, Visitor *v, void *opaque,
static int parse_size(DeviceState *dev, Property *prop, const char *str)
{
uint64_t *ptr = qdev_get_prop_ptr(dev, prop);
- Error *errp = NULL;
if (str != NULL) {
- parse_option_size(prop->name, str, ptr, &errp);
+ parse_option_size(prop->name, str, ptr, &error_abort);
}
- assert_no_error(errp);
return 0;
}
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index adbff18..7d869fc 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -656,14 +656,13 @@ void qdev_property_add_static(DeviceState *dev, Property *prop,
}
if (prop->qtype == QTYPE_QBOOL) {
- object_property_set_bool(obj, prop->defval, prop->name, &local_err);
+ object_property_set_bool(obj, prop->defval, prop->name, &error_abort);
} else if (prop->info->enum_table) {
object_property_set_str(obj, prop->info->enum_table[prop->defval],
- prop->name, &local_err);
+ prop->name, &error_abort);
} else if (prop->qtype == QTYPE_QINT) {
- object_property_set_int(obj, prop->defval, prop->name, &local_err);
+ object_property_set_int(obj, prop->defval, prop->name, &error_abort);
}
- assert_no_error(local_err);
}
static bool device_get_realized(Object *obj, Error **err)
@@ -723,7 +722,6 @@ static void device_initfn(Object *obj)
DeviceState *dev = DEVICE(obj);
ObjectClass *class;
Property *prop;
- Error *err = NULL;
if (qdev_hotplug) {
dev->hotplugged = 1;
@@ -739,26 +737,19 @@ static void device_initfn(Object *obj)
class = object_get_class(OBJECT(dev));
do {
for (prop = DEVICE_CLASS(class)->props; prop && prop->name; prop++) {
- qdev_property_add_legacy(dev, prop, &err);
- assert_no_error(err);
- qdev_property_add_static(dev, prop, &err);
- assert_no_error(err);
+ qdev_property_add_legacy(dev, prop, &error_abort);
+ qdev_property_add_static(dev, prop, &error_abort);
}
class = object_class_get_parent(class);
} while (class != object_class_by_name(TYPE_DEVICE));
object_property_add_link(OBJECT(dev), "parent_bus", TYPE_BUS,
- (Object **)&dev->parent_bus, &err);
- assert_no_error(err);
+ (Object **)&dev->parent_bus, &error_abort);
}
static void device_post_init(Object *obj)
{
- DeviceState *dev = DEVICE(obj);
- Error *err = NULL;
-
- qdev_prop_set_globals(dev, &err);
- assert_no_error(err);
+ qdev_prop_set_globals(DEVICE(obj), &error_abort);
}
/* Unlink device from bus and free the structure. */
diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
index d67c5f1..19f07b3 100644
--- a/hw/dma/xilinx_axidma.c
+++ b/hw/dma/xilinx_axidma.c
@@ -569,26 +569,21 @@ static void xilinx_axidma_init(Object *obj)
{
XilinxAXIDMA *s = XILINX_AXI_DMA(obj);
SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
- Error *errp = NULL;
object_property_add_link(obj, "axistream-connected", TYPE_STREAM_SLAVE,
- (Object **) &s->tx_data_dev, &errp);
- assert_no_error(errp);
+ (Object **)&s->tx_data_dev, &error_abort);
object_property_add_link(obj, "axistream-control-connected",
TYPE_STREAM_SLAVE,
- (Object **) &s->tx_control_dev, &errp);
- assert_no_error(errp);
+ (Object **)&s->tx_control_dev, &error_abort);
object_initialize(&s->rx_data_dev, sizeof(s->rx_data_dev),
TYPE_XILINX_AXI_DMA_DATA_STREAM);
object_initialize(&s->rx_control_dev, sizeof(s->rx_control_dev),
TYPE_XILINX_AXI_DMA_CONTROL_STREAM);
object_property_add_child(OBJECT(s), "axistream-connected-target",
- (Object *)&s->rx_data_dev, &errp);
- assert_no_error(errp);
+ (Object *)&s->rx_data_dev, &error_abort);
object_property_add_child(OBJECT(s), "axistream-control-connected-target",
- (Object *)&s->rx_control_dev, &errp);
- assert_no_error(errp);
+ (Object *)&s->rx_control_dev, &error_abort);
sysbus_init_irq(sbd, &s->streams[0].irq);
sysbus_init_irq(sbd, &s->streams[1].irq);
diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
index 3eb7715..0bd5eda 100644
--- a/hw/net/xilinx_axienet.c
+++ b/hw/net/xilinx_axienet.c
@@ -980,26 +980,21 @@ static void xilinx_enet_init(Object *obj)
{
XilinxAXIEnet *s = XILINX_AXI_ENET(obj);
SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
- Error *errp = NULL;
object_property_add_link(obj, "axistream-connected", TYPE_STREAM_SLAVE,
- (Object **) &s->tx_data_dev, &errp);
- assert_no_error(errp);
+ (Object **) &s->tx_data_dev, &error_abort);
object_property_add_link(obj, "axistream-control-connected",
TYPE_STREAM_SLAVE,
- (Object **) &s->tx_control_dev, &errp);
- assert_no_error(errp);
+ (Object **) &s->tx_control_dev, &error_abort);
object_initialize(&s->rx_data_dev, sizeof(s->rx_data_dev),
TYPE_XILINX_AXI_ENET_DATA_STREAM);
object_initialize(&s->rx_control_dev, sizeof(s->rx_control_dev),
TYPE_XILINX_AXI_ENET_CONTROL_STREAM);
object_property_add_child(OBJECT(s), "axistream-connected-target",
- (Object *)&s->rx_data_dev, &errp);
- assert_no_error(errp);
+ (Object *)&s->rx_data_dev, &error_abort);
object_property_add_child(OBJECT(s), "axistream-control-connected-target",
- (Object *)&s->rx_control_dev, &errp);
- assert_no_error(errp);
+ (Object *)&s->rx_control_dev, &error_abort);
sysbus_init_irq(sbd, &s->irq);
diff --git a/include/hw/xilinx.h b/include/hw/xilinx.h
index 0c0251a..9d6debe 100644
--- a/include/hw/xilinx.h
+++ b/include/hw/xilinx.h
@@ -59,16 +59,13 @@ xilinx_axiethernet_init(DeviceState *dev, NICInfo *nd, StreamSlave *ds,
StreamSlave *cs, hwaddr base, qemu_irq irq, int txmem,
int rxmem)
{
- Error *errp = NULL;
-
qdev_set_nic_properties(dev, nd);
qdev_prop_set_uint32(dev, "rxmem", rxmem);
qdev_prop_set_uint32(dev, "txmem", txmem);
object_property_set_link(OBJECT(dev), OBJECT(ds),
- "axistream-connected", &errp);
+ "axistream-connected", &error_abort);
object_property_set_link(OBJECT(dev), OBJECT(cs),
- "axistream-control-connected", &errp);
- assert_no_error(errp);
+ "axistream-control-connected", &error_abort);
qdev_init_nofail(dev);
sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq);
@@ -78,14 +75,11 @@ static inline void
xilinx_axidma_init(DeviceState *dev, StreamSlave *ds, StreamSlave *cs,
hwaddr base, qemu_irq irq, qemu_irq irq2, int freqhz)
{
- Error *errp = NULL;
-
qdev_prop_set_uint32(dev, "freqhz", freqhz);
object_property_set_link(OBJECT(dev), OBJECT(ds),
- "axistream-connected", &errp);
+ "axistream-connected", &error_abort);
object_property_set_link(OBJECT(dev), OBJECT(cs),
- "axistream-control-connected", &errp);
- assert_no_error(errp);
+ "axistream-control-connected", &error_abort);
qdev_init_nofail(dev);
sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 408d207..e4801a8 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -252,18 +252,15 @@ static Property arm_cpu_reset_hivecs_property =
static void arm_cpu_post_init(Object *obj)
{
ARMCPU *cpu = ARM_CPU(obj);
- Error *err = NULL;
if (arm_feature(&cpu->env, ARM_FEATURE_CBAR)) {
qdev_property_add_static(DEVICE(obj), &arm_cpu_reset_cbar_property,
- &err);
- assert_no_error(err);
+ &error_abort);
}
if (!arm_feature(&cpu->env, ARM_FEATURE_M)) {
qdev_property_add_static(DEVICE(obj), &arm_cpu_reset_hivecs_property,
- &err);
- assert_no_error(err);
+ &error_abort);
}
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL 11/14] target-i386: Remove assert_no_error usage
2014-01-06 22:03 [Qemu-devel] [PULL 00/14] QMP queue Luiz Capitulino
` (9 preceding siblings ...)
2014-01-06 22:03 ` [Qemu-devel] [PULL 10/14] hw: Remove assert_no_error usages Luiz Capitulino
@ 2014-01-06 22:03 ` Luiz Capitulino
2014-01-06 22:03 ` [Qemu-devel] [PULL 12/14] qemu-option: Remove qemu_opts_create_nofail Luiz Capitulino
` (4 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Luiz Capitulino @ 2014-01-06 22:03 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Replace an assert_no_error() usage with the error_abort system.
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
target-i386/cpu.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index bb98f6d..6b7b1a9 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1600,7 +1600,6 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def,
const char *name)
{
x86_def_t *def;
- Error *err = NULL;
int i;
if (name == NULL) {
@@ -1608,8 +1607,7 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def,
}
if (kvm_enabled() && strcmp(name, "host") == 0) {
kvm_cpu_fill_host(x86_cpu_def);
- object_property_set_bool(OBJECT(cpu), true, "pmu", &err);
- assert_no_error(err);
+ object_property_set_bool(OBJECT(cpu), true, "pmu", &error_abort);
return 0;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL 12/14] qemu-option: Remove qemu_opts_create_nofail
2014-01-06 22:03 [Qemu-devel] [PULL 00/14] QMP queue Luiz Capitulino
` (10 preceding siblings ...)
2014-01-06 22:03 ` [Qemu-devel] [PULL 11/14] target-i386: Remove assert_no_error usage Luiz Capitulino
@ 2014-01-06 22:03 ` Luiz Capitulino
2014-01-06 22:03 ` [Qemu-devel] [PULL 13/14] qerror: Remove assert_no_error() Luiz Capitulino
` (3 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Luiz Capitulino @ 2014-01-06 22:03 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
This is a boiler-plate _nofail variant of qemu_opts_create. Remove and
use error_abort in call sites.
null/0 arguments needs to be added for the id and fail_if_exists fields
in affected callsites due to argument inconsistency between the normal and
no_fail variants.
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
block/blkdebug.c | 2 +-
block/blkverify.c | 2 +-
block/curl.c | 2 +-
block/gluster.c | 2 +-
block/iscsi.c | 2 +-
block/nbd.c | 3 ++-
block/qcow2.c | 2 +-
block/raw-posix.c | 2 +-
block/raw-win32.c | 5 +++--
block/rbd.c | 2 +-
block/sheepdog.c | 2 +-
block/vvfat.c | 2 +-
blockdev.c | 6 ++++--
hw/watchdog/watchdog.c | 3 ++-
include/qemu/option.h | 1 -
qdev-monitor.c | 2 +-
qemu-img.c | 2 +-
util/qemu-config.c | 2 +-
util/qemu-option.c | 9 ---------
util/qemu-sockets.c | 18 +++++++++---------
vl.c | 15 +++++++++------
21 files changed, 42 insertions(+), 44 deletions(-)
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 957be2c..ebc5f13 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -359,7 +359,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
const char *filename, *config;
int ret;
- opts = qemu_opts_create_nofail(&runtime_opts);
+ opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
qemu_opts_absorb_qdict(opts, options, &local_err);
if (error_is_set(&local_err)) {
error_propagate(errp, local_err);
diff --git a/block/blkverify.c b/block/blkverify.c
index 3c63528..1c1637f 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -125,7 +125,7 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
const char *filename, *raw;
int ret;
- opts = qemu_opts_create_nofail(&runtime_opts);
+ opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
qemu_opts_absorb_qdict(opts, options, &local_err);
if (error_is_set(&local_err)) {
error_propagate(errp, local_err);
diff --git a/block/curl.c b/block/curl.c
index 5a46f97..a603936 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -413,7 +413,7 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
return -EROFS;
}
- opts = qemu_opts_create_nofail(&runtime_opts);
+ opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
qemu_opts_absorb_qdict(opts, options, &local_err);
if (error_is_set(&local_err)) {
qerror_report_err(local_err);
diff --git a/block/gluster.c b/block/gluster.c
index 877686a..563d497 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -298,7 +298,7 @@ static int qemu_gluster_open(BlockDriverState *bs, QDict *options,
Error *local_err = NULL;
const char *filename;
- opts = qemu_opts_create_nofail(&runtime_opts);
+ opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
qemu_opts_absorb_qdict(opts, options, &local_err);
if (error_is_set(&local_err)) {
qerror_report_err(local_err);
diff --git a/block/iscsi.c b/block/iscsi.c
index fa69408..02eba5d 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1109,7 +1109,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
return -EINVAL;
}
- opts = qemu_opts_create_nofail(&runtime_opts);
+ opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
qemu_opts_absorb_qdict(opts, options, &local_err);
if (error_is_set(&local_err)) {
qerror_report_err(local_err);
diff --git a/block/nbd.c b/block/nbd.c
index 4455a13..327e913 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -205,7 +205,8 @@ static int nbd_config(BDRVNBDState *s, QDict *options, char **export)
return -EINVAL;
}
- s->socket_opts = qemu_opts_create_nofail(&socket_optslist);
+ s->socket_opts = qemu_opts_create(&socket_optslist, NULL, 0,
+ &error_abort);
qemu_opts_absorb_qdict(s->socket_opts, options, &local_err);
if (error_is_set(&local_err)) {
diff --git a/block/qcow2.c b/block/qcow2.c
index f29aa88..8ec9db1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -669,7 +669,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
}
/* Enable lazy_refcounts according to image and command line options */
- opts = qemu_opts_create_nofail(&qcow2_runtime_opts);
+ opts = qemu_opts_create(&qcow2_runtime_opts, NULL, 0, &error_abort);
qemu_opts_absorb_qdict(opts, options, &local_err);
if (error_is_set(&local_err)) {
error_propagate(errp, local_err);
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 10c6b34..0676037 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -287,7 +287,7 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
int fd, ret;
struct stat st;
- opts = qemu_opts_create_nofail(&raw_runtime_opts);
+ opts = qemu_opts_create(&raw_runtime_opts, NULL, 0, &error_abort);
qemu_opts_absorb_qdict(opts, options, &local_err);
if (error_is_set(&local_err)) {
error_propagate(errp, local_err);
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 2bad5a3..ce314fd 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -248,7 +248,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,
s->type = FTYPE_FILE;
- opts = qemu_opts_create_nofail(&raw_runtime_opts);
+ opts = qemu_opts_create(&raw_runtime_opts, NULL, 0, &error_abort);
qemu_opts_absorb_qdict(opts, options, &local_err);
if (error_is_set(&local_err)) {
error_propagate(errp, local_err);
@@ -550,7 +550,8 @@ static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
Error *local_err = NULL;
const char *filename;
- QemuOpts *opts = qemu_opts_create_nofail(&raw_runtime_opts);
+ QemuOpts *opts = qemu_opts_create(&raw_runtime_opts, NULL, 0,
+ &error_abort);
qemu_opts_absorb_qdict(opts, options, &local_err);
if (error_is_set(&local_err)) {
error_propagate(errp, local_err);
diff --git a/block/rbd.c b/block/rbd.c
index 4a1ea5b..f453f04 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -461,7 +461,7 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
const char *filename;
int r;
- opts = qemu_opts_create_nofail(&runtime_opts);
+ opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
qemu_opts_absorb_qdict(opts, options, &local_err);
if (error_is_set(&local_err)) {
qerror_report_err(local_err);
diff --git a/block/sheepdog.c b/block/sheepdog.c
index d1c812d..5ce0658 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1383,7 +1383,7 @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags,
s->bs = bs;
- opts = qemu_opts_create_nofail(&runtime_opts);
+ opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
qemu_opts_absorb_qdict(opts, options, &local_err);
if (error_is_set(&local_err)) {
qerror_report_err(local_err);
diff --git a/block/vvfat.c b/block/vvfat.c
index 1abb8ad..664941c 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1083,7 +1083,7 @@ DLOG(if (stderr == NULL) {
setbuf(stderr, NULL);
})
- opts = qemu_opts_create_nofail(&runtime_opts);
+ opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
qemu_opts_absorb_qdict(opts, options, &local_err);
if (error_is_set(&local_err)) {
qerror_report_err(local_err);
diff --git a/blockdev.c b/blockdev.c
index 6a85961..1cb6f4c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -682,7 +682,8 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
bs_opts = qdict_new();
qemu_opts_to_qdict(all_opts, bs_opts);
- legacy_opts = qemu_opts_create_nofail(&qemu_legacy_drive_opts);
+ legacy_opts = qemu_opts_create(&qemu_legacy_drive_opts, NULL, 0,
+ &error_abort);
qemu_opts_absorb_qdict(legacy_opts, bs_opts, &local_err);
if (error_is_set(&local_err)) {
qerror_report_err(local_err);
@@ -853,7 +854,8 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
if (type == IF_VIRTIO) {
QemuOpts *devopts;
- devopts = qemu_opts_create_nofail(qemu_find_opts("device"));
+ devopts = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
+ &error_abort);
if (arch_type == QEMU_ARCH_S390X) {
qemu_opt_set(devopts, "driver", "virtio-blk-s390");
} else {
diff --git a/hw/watchdog/watchdog.c b/hw/watchdog/watchdog.c
index 387962e..f28161b 100644
--- a/hw/watchdog/watchdog.c
+++ b/hw/watchdog/watchdog.c
@@ -66,7 +66,8 @@ int select_watchdog(const char *p)
QLIST_FOREACH(model, &watchdog_list, entry) {
if (strcasecmp(model->wdt_name, p) == 0) {
/* add the device */
- opts = qemu_opts_create_nofail(qemu_find_opts("device"));
+ opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
+ &error_abort);
qemu_opt_set(opts, "driver", p);
return 0;
}
diff --git a/include/qemu/option.h b/include/qemu/option.h
index 5c0c6dd..3ea871a 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -136,7 +136,6 @@ int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
QemuOpts *qemu_opts_find(QemuOptsList *list, const char *id);
QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id,
int fail_if_exists, Error **errp);
-QemuOpts *qemu_opts_create_nofail(QemuOptsList *list);
void qemu_opts_reset(QemuOptsList *list);
void qemu_opts_loc_restore(QemuOpts *opts);
int qemu_opts_set(QemuOptsList *list, const char *id,
diff --git a/qdev-monitor.c b/qdev-monitor.c
index dc37a43..6280771 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -730,7 +730,7 @@ int qemu_global_option(const char *str)
return -1;
}
- opts = qemu_opts_create_nofail(&qemu_global_opts);
+ opts = qemu_opts_create(&qemu_global_opts, NULL, 0, &error_abort);
qemu_opt_set(opts, "driver", driver);
qemu_opt_set(opts, "property", property);
qemu_opt_set(opts, "value", str+offset+1);
diff --git a/qemu-img.c b/qemu-img.c
index a4b3931..c989850 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2564,7 +2564,7 @@ static int img_resize(int argc, char **argv)
}
/* Parse size */
- param = qemu_opts_create_nofail(&resize_options);
+ param = qemu_opts_create(&resize_options, NULL, 0, &error_abort);
if (qemu_opt_set(param, BLOCK_OPT_SIZE, size)) {
/* Error message already printed when size parsing fails */
ret = -1;
diff --git a/util/qemu-config.c b/util/qemu-config.c
index 04da942..7973659 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -311,7 +311,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname)
error_free(local_err);
goto out;
}
- opts = qemu_opts_create_nofail(list);
+ opts = qemu_opts_create(list, NULL, 0, &error_abort);
continue;
}
if (sscanf(line, " %63s = \"%1023[^\"]\"", arg, value) == 2) {
diff --git a/util/qemu-option.c b/util/qemu-option.c
index efcb5dc..668e5d9 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -791,15 +791,6 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id,
return opts;
}
-QemuOpts *qemu_opts_create_nofail(QemuOptsList *list)
-{
- QemuOpts *opts;
- Error *errp = NULL;
- opts = qemu_opts_create(list, NULL, 0, &errp);
- assert_no_error(errp);
- return opts;
-}
-
void qemu_opts_reset(QemuOptsList *list)
{
QemuOpts *opts, *next_opts;
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 6b97dc1..8818d7c 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -578,7 +578,7 @@ int inet_listen(const char *str, char *ostr, int olen,
addr = inet_parse(str, errp);
if (addr != NULL) {
- opts = qemu_opts_create_nofail(&socket_optslist);
+ opts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort);
inet_addr_to_opts(opts, addr);
qapi_free_InetSocketAddress(addr);
sock = inet_listen_opts(opts, port_offset, errp);
@@ -617,7 +617,7 @@ int inet_connect(const char *str, Error **errp)
addr = inet_parse(str, errp);
if (addr != NULL) {
- opts = qemu_opts_create_nofail(&socket_optslist);
+ opts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort);
inet_addr_to_opts(opts, addr);
qapi_free_InetSocketAddress(addr);
sock = inet_connect_opts(opts, errp, NULL, NULL);
@@ -651,7 +651,7 @@ int inet_nonblocking_connect(const char *str,
addr = inet_parse(str, errp);
if (addr != NULL) {
- opts = qemu_opts_create_nofail(&socket_optslist);
+ opts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort);
inet_addr_to_opts(opts, addr);
qapi_free_InetSocketAddress(addr);
sock = inet_connect_opts(opts, errp, callback, opaque);
@@ -794,7 +794,7 @@ int unix_listen(const char *str, char *ostr, int olen, Error **errp)
char *path, *optstr;
int sock, len;
- opts = qemu_opts_create_nofail(&socket_optslist);
+ opts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort);
optstr = strchr(str, ',');
if (optstr) {
@@ -822,7 +822,7 @@ int unix_connect(const char *path, Error **errp)
QemuOpts *opts;
int sock;
- opts = qemu_opts_create_nofail(&socket_optslist);
+ opts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort);
qemu_opt_set(opts, "path", path);
sock = unix_connect_opts(opts, errp, NULL, NULL);
qemu_opts_del(opts);
@@ -839,7 +839,7 @@ int unix_nonblocking_connect(const char *path,
g_assert(callback != NULL);
- opts = qemu_opts_create_nofail(&socket_optslist);
+ opts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort);
qemu_opt_set(opts, "path", path);
sock = unix_connect_opts(opts, errp, callback, opaque);
qemu_opts_del(opts);
@@ -889,7 +889,7 @@ int socket_connect(SocketAddress *addr, Error **errp,
QemuOpts *opts;
int fd;
- opts = qemu_opts_create_nofail(&socket_optslist);
+ opts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort);
switch (addr->kind) {
case SOCKET_ADDRESS_KIND_INET:
inet_addr_to_opts(opts, addr->inet);
@@ -921,7 +921,7 @@ int socket_listen(SocketAddress *addr, Error **errp)
QemuOpts *opts;
int fd;
- opts = qemu_opts_create_nofail(&socket_optslist);
+ opts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort);
switch (addr->kind) {
case SOCKET_ADDRESS_KIND_INET:
inet_addr_to_opts(opts, addr->inet);
@@ -949,7 +949,7 @@ int socket_dgram(SocketAddress *remote, SocketAddress *local, Error **errp)
QemuOpts *opts;
int fd;
- opts = qemu_opts_create_nofail(&socket_optslist);
+ opts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort);
switch (remote->kind) {
case SOCKET_ADDRESS_KIND_INET:
qemu_opt_set(opts, "host", remote->inet->host);
diff --git a/vl.c b/vl.c
index 2170a5e..b14032a 100644
--- a/vl.c
+++ b/vl.c
@@ -545,7 +545,7 @@ QemuOpts *qemu_get_machine_opts(void)
assert(list);
opts = qemu_opts_find(list, NULL);
if (!opts) {
- opts = qemu_opts_create_nofail(list);
+ opts = qemu_opts_create(list, NULL, 0, &error_abort);
}
return opts;
}
@@ -2255,7 +2255,8 @@ static int balloon_parse(const char *arg)
return -1;
} else {
/* create empty opts */
- opts = qemu_opts_create_nofail(qemu_find_opts("device"));
+ opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
+ &error_abort);
}
qemu_opt_set(opts, "driver", "virtio-balloon");
return 0;
@@ -2515,14 +2516,14 @@ static int virtcon_parse(const char *devname)
exit(1);
}
- bus_opts = qemu_opts_create_nofail(device);
+ bus_opts = qemu_opts_create(device, NULL, 0, &error_abort);
if (arch_type == QEMU_ARCH_S390X) {
qemu_opt_set(bus_opts, "driver", "virtio-serial-s390");
} else {
qemu_opt_set(bus_opts, "driver", "virtio-serial-pci");
}
- dev_opts = qemu_opts_create_nofail(device);
+ dev_opts = qemu_opts_create(device, NULL, 0, &error_abort);
qemu_opt_set(dev_opts, "driver", "virtconsole");
snprintf(label, sizeof(label), "virtcon%d", index);
@@ -3382,7 +3383,8 @@ int main(int argc, char **argv, char **envp)
qemu_opt_set_bool(fsdev, "readonly",
qemu_opt_get_bool(opts, "readonly", 0));
- device = qemu_opts_create_nofail(qemu_find_opts("device"));
+ device = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
+ &error_abort);
qemu_opt_set(device, "driver", "virtio-9p-pci");
qemu_opt_set(device, "fsdev",
qemu_opt_get(opts, "mount_tag"));
@@ -3402,7 +3404,8 @@ int main(int argc, char **argv, char **envp)
}
qemu_opt_set(fsdev, "fsdriver", "synth");
- device = qemu_opts_create_nofail(qemu_find_opts("device"));
+ device = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
+ &error_abort);
qemu_opt_set(device, "driver", "virtio-9p-pci");
qemu_opt_set(device, "fsdev", "v_synth");
qemu_opt_set(device, "mount_tag", "v_synth");
--
1.8.1.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL 13/14] qerror: Remove assert_no_error()
2014-01-06 22:03 [Qemu-devel] [PULL 00/14] QMP queue Luiz Capitulino
` (11 preceding siblings ...)
2014-01-06 22:03 ` [Qemu-devel] [PULL 12/14] qemu-option: Remove qemu_opts_create_nofail Luiz Capitulino
@ 2014-01-06 22:03 ` Luiz Capitulino
2014-01-06 22:03 ` [Qemu-devel] [PULL 14/14] migration: qmp_migrate(): keep working after syntax error Luiz Capitulino
` (2 subsequent siblings)
15 siblings, 0 replies; 25+ messages in thread
From: Luiz Capitulino @ 2014-01-06 22:03 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
This is no longer needed, and is obsoleted by error_abort. Remove.
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
include/qapi/qmp/qerror.h | 1 -
qobject/qerror.c | 8 --------
2 files changed, 9 deletions(-)
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index c30c2f6..73c67b7 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -29,7 +29,6 @@ typedef struct QError {
QString *qerror_human(const QError *qerror);
void qerror_report(ErrorClass err_class, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
void qerror_report_err(Error *err);
-void assert_no_error(Error *err);
/*
* QError class list
diff --git a/qobject/qerror.c b/qobject/qerror.c
index fc8331a..e3608e2 100644
--- a/qobject/qerror.c
+++ b/qobject/qerror.c
@@ -121,14 +121,6 @@ void qerror_report_err(Error *err)
}
}
-void assert_no_error(Error *err)
-{
- if (err) {
- qerror_report_err(err);
- abort();
- }
-}
-
/**
* qobject_to_qerror(): Convert a QObject into a QError
*/
--
1.8.1.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Qemu-devel] [PULL 14/14] migration: qmp_migrate(): keep working after syntax error
2014-01-06 22:03 [Qemu-devel] [PULL 00/14] QMP queue Luiz Capitulino
` (12 preceding siblings ...)
2014-01-06 22:03 ` [Qemu-devel] [PULL 13/14] qerror: Remove assert_no_error() Luiz Capitulino
@ 2014-01-06 22:03 ` Luiz Capitulino
2014-01-06 22:41 ` [Qemu-devel] [PULL 00/14] QMP queue Peter Maydell
2014-01-13 23:27 ` Peter Crosthwaite
15 siblings, 0 replies; 25+ messages in thread
From: Luiz Capitulino @ 2014-01-06 22:03 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
If a user or QMP client enter a bad syntax for the migrate
command in QMP/HMP, then the migrate command will never succeed
from that point on.
For example, if you enter:
(qemu) migrate tcp;0:4444
migrate: Parameter 'uri' expects a valid migration protocol
Then the migrate command will always fail from now on:
(qemu) migrate tcp:0:4444
migrate: There's a migration process in progress
The problem is that qmp_migrate() sets the migration status to
MIG_STATE_SETUP and doesn't reset it on syntax error. This bug
was introduced by commit 29ae8a4133082e16970c9d4be09f4b6a15034617.
Reviewed-by: Michael R. Hines <mrhines@us.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
migration.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/migration.c b/migration.c
index 2b1ab20..557195a 100644
--- a/migration.c
+++ b/migration.c
@@ -437,6 +437,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
#endif
} else {
error_set(errp, QERR_INVALID_PARAMETER_VALUE, "uri", "a valid migration protocol");
+ s->state = MIG_STATE_ERROR;
return;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PULL 00/14] QMP queue
2014-01-06 22:03 [Qemu-devel] [PULL 00/14] QMP queue Luiz Capitulino
` (13 preceding siblings ...)
2014-01-06 22:03 ` [Qemu-devel] [PULL 14/14] migration: qmp_migrate(): keep working after syntax error Luiz Capitulino
@ 2014-01-06 22:41 ` Peter Maydell
2014-01-13 23:27 ` Peter Crosthwaite
15 siblings, 0 replies; 25+ messages in thread
From: Peter Maydell @ 2014-01-06 22:41 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: QEMU Developers, Anthony Liguori
On 6 January 2014 22:03, Luiz Capitulino <lcapitulino@redhat.com> wrote:
> Peter Crosthwaite (6):
> error: Add error_abort
> qdev: Delete dead code
> hw: Remove assert_no_error usages
> target-i386: Remove assert_no_error usage
> qemu-option: Remove qemu_opts_create_nofail
> qerror: Remove assert_no_error()
>
> target-arm/cpu.c | 7 ++--
> target-i386/cpu.c | 4 +--
It took me a while to figure out where the target-arm/ change in this
pullreq was, because it's been stuffed into the "hw:" patch (whereas
target-i386 got its own patch). However, the change itself is OK so
it's fine.
thanks
-- PMM
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PULL 00/14] QMP queue
2014-01-06 22:03 [Qemu-devel] [PULL 00/14] QMP queue Luiz Capitulino
` (14 preceding siblings ...)
2014-01-06 22:41 ` [Qemu-devel] [PULL 00/14] QMP queue Peter Maydell
@ 2014-01-13 23:27 ` Peter Crosthwaite
2014-01-14 3:38 ` Edgar E. Iglesias
15 siblings, 1 reply; 25+ messages in thread
From: Peter Crosthwaite @ 2014-01-13 23:27 UTC (permalink / raw)
To: Luiz Capitulino, Edgar Iglesias, Anthony Liguori
Cc: qemu-devel@nongnu.org Developers, Anthony Liguori
Ping,
Has this one been forgotten or are there issues? PMM had a small
comment, but he waived it AFAICT.
Regards,
Peter
On Tue, Jan 7, 2014 at 8:03 AM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
> The following changes since commit f976b09ea249cccc3fd41c98aaf6512908db0bae:
>
> PPC: Fix compilation with TCG debug (2013-12-22 19:15:55 +0100)
>
> are available in the git repository at:
>
> git://repo.or.cz/qemu/qmp-unstable.git queue/qmp
>
> for you to fetch changes up to c950114286ea358a93ce632db0421945e1008395:
>
> migration: qmp_migrate(): keep working after syntax error (2014-01-06 15:02:30 -0500)
>
> ----------------------------------------------------------------
> Jason J. Herne (1):
> qemu-monitor: HMP cpu-add wrapper
>
> Luiz Capitulino (1):
> migration: qmp_migrate(): keep working after syntax error
>
> Paolo Bonzini (6):
> vl: add missing transition debug->finish_migrate
> rng: initialize file descriptor to -1
> qom: fix leak for objects created with -object
> qom: catch errors in object_property_add_child
> monitor: add object-del (QMP) and object_del (HMP) command
> monitor: add object-add (QMP) and object_add (HMP) command
>
> Peter Crosthwaite (6):
> error: Add error_abort
> qdev: Delete dead code
> hw: Remove assert_no_error usages
> target-i386: Remove assert_no_error usage
> qemu-option: Remove qemu_opts_create_nofail
> qerror: Remove assert_no_error()
>
> backends/rng-random.c | 4 +--
> block/blkdebug.c | 2 +-
> block/blkverify.c | 2 +-
> block/curl.c | 2 +-
> block/gluster.c | 2 +-
> block/iscsi.c | 2 +-
> block/nbd.c | 3 +-
> block/qcow2.c | 2 +-
> block/raw-posix.c | 2 +-
> block/raw-win32.c | 5 +--
> block/rbd.c | 2 +-
> block/sheepdog.c | 2 +-
> block/vvfat.c | 2 +-
> blockdev.c | 6 ++--
> hmp-commands.hx | 41 +++++++++++++++++++++
> hmp.c | 77 ++++++++++++++++++++++++++++++++++++++++
> hmp.h | 3 ++
> hw/core/qdev-properties-system.c | 8 ++---
> hw/core/qdev-properties.c | 40 ++++++---------------
> hw/core/qdev.c | 28 ++++-----------
> hw/dma/xilinx_axidma.c | 13 +++----
> hw/net/xilinx_axienet.c | 13 +++----
> hw/watchdog/watchdog.c | 3 +-
> include/hw/xilinx.h | 14 +++-----
> include/monitor/monitor.h | 3 ++
> include/qapi/error.h | 6 ++++
> include/qapi/qmp/qerror.h | 1 -
> include/qapi/visitor.h | 3 +-
> include/qemu/option.h | 1 -
> include/qemu/typedefs.h | 2 ++
> migration.c | 1 +
> qapi-schema.json | 34 ++++++++++++++++++
> qdev-monitor.c | 2 +-
> qemu-img.c | 2 +-
> qmp-commands.hx | 51 ++++++++++++++++++++++++++
> qmp.c | 76 +++++++++++++++++++++++++++++++++++++++
> qobject/qerror.c | 8 -----
> qom/object.c | 11 ++++--
> target-arm/cpu.c | 7 ++--
> target-i386/cpu.c | 4 +--
> util/error.c | 22 +++++++++++-
> util/qemu-config.c | 2 +-
> util/qemu-option.c | 9 -----
> util/qemu-sockets.c | 18 +++++-----
> vl.c | 19 ++++++----
> 45 files changed, 405 insertions(+), 155 deletions(-)
>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] [PULL 00/14] QMP queue
2014-01-13 23:27 ` Peter Crosthwaite
@ 2014-01-14 3:38 ` Edgar E. Iglesias
2014-01-14 16:44 ` Kevin Wolf
0 siblings, 1 reply; 25+ messages in thread
From: Edgar E. Iglesias @ 2014-01-14 3:38 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: Anthony Liguori, qemu-devel@nongnu.org Developers,
Anthony Liguori, Luiz Capitulino
On Tue, Jan 14, 2014 at 09:27:10AM +1000, Peter Crosthwaite wrote:
> Ping,
>
> Has this one been forgotten or are there issues? PMM had a small
> comment, but he waived it AFAICT.
Pong,
I've merged it now, thanks!
Cheers,
Edgar
>
> Regards,
> Peter
>
> On Tue, Jan 7, 2014 at 8:03 AM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
> > The following changes since commit f976b09ea249cccc3fd41c98aaf6512908db0bae:
> >
> > PPC: Fix compilation with TCG debug (2013-12-22 19:15:55 +0100)
> >
> > are available in the git repository at:
> >
> > git://repo.or.cz/qemu/qmp-unstable.git queue/qmp
> >
> > for you to fetch changes up to c950114286ea358a93ce632db0421945e1008395:
> >
> > migration: qmp_migrate(): keep working after syntax error (2014-01-06 15:02:30 -0500)
> >
> > ----------------------------------------------------------------
> > Jason J. Herne (1):
> > qemu-monitor: HMP cpu-add wrapper
> >
> > Luiz Capitulino (1):
> > migration: qmp_migrate(): keep working after syntax error
> >
> > Paolo Bonzini (6):
> > vl: add missing transition debug->finish_migrate
> > rng: initialize file descriptor to -1
> > qom: fix leak for objects created with -object
> > qom: catch errors in object_property_add_child
> > monitor: add object-del (QMP) and object_del (HMP) command
> > monitor: add object-add (QMP) and object_add (HMP) command
> >
> > Peter Crosthwaite (6):
> > error: Add error_abort
> > qdev: Delete dead code
> > hw: Remove assert_no_error usages
> > target-i386: Remove assert_no_error usage
> > qemu-option: Remove qemu_opts_create_nofail
> > qerror: Remove assert_no_error()
> >
> > backends/rng-random.c | 4 +--
> > block/blkdebug.c | 2 +-
> > block/blkverify.c | 2 +-
> > block/curl.c | 2 +-
> > block/gluster.c | 2 +-
> > block/iscsi.c | 2 +-
> > block/nbd.c | 3 +-
> > block/qcow2.c | 2 +-
> > block/raw-posix.c | 2 +-
> > block/raw-win32.c | 5 +--
> > block/rbd.c | 2 +-
> > block/sheepdog.c | 2 +-
> > block/vvfat.c | 2 +-
> > blockdev.c | 6 ++--
> > hmp-commands.hx | 41 +++++++++++++++++++++
> > hmp.c | 77 ++++++++++++++++++++++++++++++++++++++++
> > hmp.h | 3 ++
> > hw/core/qdev-properties-system.c | 8 ++---
> > hw/core/qdev-properties.c | 40 ++++++---------------
> > hw/core/qdev.c | 28 ++++-----------
> > hw/dma/xilinx_axidma.c | 13 +++----
> > hw/net/xilinx_axienet.c | 13 +++----
> > hw/watchdog/watchdog.c | 3 +-
> > include/hw/xilinx.h | 14 +++-----
> > include/monitor/monitor.h | 3 ++
> > include/qapi/error.h | 6 ++++
> > include/qapi/qmp/qerror.h | 1 -
> > include/qapi/visitor.h | 3 +-
> > include/qemu/option.h | 1 -
> > include/qemu/typedefs.h | 2 ++
> > migration.c | 1 +
> > qapi-schema.json | 34 ++++++++++++++++++
> > qdev-monitor.c | 2 +-
> > qemu-img.c | 2 +-
> > qmp-commands.hx | 51 ++++++++++++++++++++++++++
> > qmp.c | 76 +++++++++++++++++++++++++++++++++++++++
> > qobject/qerror.c | 8 -----
> > qom/object.c | 11 ++++--
> > target-arm/cpu.c | 7 ++--
> > target-i386/cpu.c | 4 +--
> > util/error.c | 22 +++++++++++-
> > util/qemu-config.c | 2 +-
> > util/qemu-option.c | 9 -----
> > util/qemu-sockets.c | 18 +++++-----
> > vl.c | 19 ++++++----
> > 45 files changed, 405 insertions(+), 155 deletions(-)
> >
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PULL 00/14] QMP queue
2014-01-14 3:38 ` Edgar E. Iglesias
@ 2014-01-14 16:44 ` Kevin Wolf
2014-01-14 18:22 ` [Qemu-devel] Fix make check breakage (was [PULL 00/14] QMP queue) Luiz Capitulino
0 siblings, 1 reply; 25+ messages in thread
From: Kevin Wolf @ 2014-01-14 16:44 UTC (permalink / raw)
To: Edgar E. Iglesias
Cc: Peter Crosthwaite, Luiz Capitulino,
qemu-devel@nongnu.org Developers, Anthony Liguori,
Anthony Liguori
Am 14.01.2014 um 04:38 hat Edgar E. Iglesias geschrieben:
> On Tue, Jan 14, 2014 at 09:27:10AM +1000, Peter Crosthwaite wrote:
> > Ping,
> >
> > Has this one been forgotten or are there issues? PMM had a small
> > comment, but he waived it AFAICT.
>
> Pong,
>
> I've merged it now, thanks!
I believe it's something in this pull requests that breaks make check.
Kevin
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] Fix make check breakage (was [PULL 00/14] QMP queue)
2014-01-14 16:44 ` Kevin Wolf
@ 2014-01-14 18:22 ` Luiz Capitulino
2014-01-14 22:26 ` Peter Crosthwaite
2014-01-15 9:55 ` Markus Armbruster
0 siblings, 2 replies; 25+ messages in thread
From: Luiz Capitulino @ 2014-01-14 18:22 UTC (permalink / raw)
To: Kevin Wolf
Cc: Edgar E. Iglesias, Peter Crosthwaite,
qemu-devel@nongnu.org Developers, Anthony Liguori,
Anthony Liguori
On Tue, 14 Jan 2014 17:44:51 +0100
Kevin Wolf <kwolf@redhat.com> wrote:
> Am 14.01.2014 um 04:38 hat Edgar E. Iglesias geschrieben:
> > On Tue, Jan 14, 2014 at 09:27:10AM +1000, Peter Crosthwaite wrote:
> > > Ping,
> > >
> > > Has this one been forgotten or are there issues? PMM had a small
> > > comment, but he waived it AFAICT.
> >
> > Pong,
> >
> > I've merged it now, thanks!
>
> I believe it's something in this pull requests that breaks make check.
And you're right. But first, let me confirm that we're talking about the
same breakage. This is what I'm getting:
make tests/check-qom-interface
libqemuutil.a(qemu-error.o): In function `error_vprintf':
/home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:23: undefined reference to `cur_mon'
/home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:24: undefined reference to `cur_mon'
/home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:24: undefined reference to `monitor_vprintf'
libqemuutil.a(qemu-error.o): In function `error_printf_unless_qmp':
/home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:47: undefined reference to `monitor_cur_is_qmp'
libqemuutil.a(qemu-error.o): In function `error_print_loc':
/home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:174: undefined reference to `cur_mon'
collect2: error: ld returned 1 exit status
make: *** [tests/check-qom-interface] Error 1
I tried bisecting it, but git bisect weren't capable of finding the
culprit. So debugged it, and the problem was introduced by:
commit 594278718323ca7bffaab0fb7fc6c82fa2c1cd5f
Author: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Date: Wed Jan 1 18:49:52 2014 -0800
qerror: Remove assert_no_error()
There isn't nothing really wrong with this commit. The problem seems to
be that the tests link against libqemuutil.a and this library pulls in
everything from util/. The commit above changed util/error.c to call
error_report(), which depends on 'cur_mon', which is only made available
by monitor.o.
I don't think we want to mess up with including monitor.o on libqemuutil.a.
Besides, I now find it a bit weird to call error_report() from an error
reporting function. So it's better to just call fprintf(stderr,) instead.
Peter, Markus, are you ok with this patch?
PS: I do run make check before sending a pull request, and did run this
time too. Not sure how I didn't catch this. Thanks for the report
Kevin!
diff --git a/util/error.c b/util/error.c
index f11f1d5..7c7650c 100644
--- a/util/error.c
+++ b/util/error.c
@@ -44,7 +44,7 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
err->err_class = err_class;
if (errp == &error_abort) {
- error_report("%s", error_get_pretty(err));
+ fprintf(stderr, "%s", error_get_pretty(err));
abort();
}
@@ -80,7 +80,7 @@ void error_set_errno(Error **errp, int os_errno, ErrorClass err_class,
err->err_class = err_class;
if (errp == &error_abort) {
- error_report("%s", error_get_pretty(err));
+ fprintf(stderr, "%s", error_get_pretty(err));
abort();
}
@@ -125,7 +125,7 @@ void error_set_win32(Error **errp, int win32_err, ErrorClass err_class,
err->err_class = err_class;
if (errp == &error_abort) {
- error_report("%s", error_get_pretty(err));
+ fprintf(stderr, "%s", error_get_pretty(err));
abort();
}
@@ -171,7 +171,7 @@ void error_free(Error *err)
void error_propagate(Error **dst_err, Error *local_err)
{
if (local_err && dst_err == &error_abort) {
- error_report("%s", error_get_pretty(local_err));
+ fprintf(stderr, "%s", error_get_pretty(local_err));
abort();
} else if (dst_err && !*dst_err) {
*dst_err = local_err;
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Qemu-devel] Fix make check breakage (was [PULL 00/14] QMP queue)
2014-01-14 18:22 ` [Qemu-devel] Fix make check breakage (was [PULL 00/14] QMP queue) Luiz Capitulino
@ 2014-01-14 22:26 ` Peter Crosthwaite
2014-01-15 0:30 ` Peter Crosthwaite
2014-01-15 9:55 ` Markus Armbruster
1 sibling, 1 reply; 25+ messages in thread
From: Peter Crosthwaite @ 2014-01-14 22:26 UTC (permalink / raw)
To: Luiz Capitulino
Cc: Kevin Wolf, Edgar E. Iglesias, qemu-devel@nongnu.org Developers,
Anthony Liguori, Anthony Liguori
On Wed, Jan 15, 2014 at 4:22 AM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
> On Tue, 14 Jan 2014 17:44:51 +0100
> Kevin Wolf <kwolf@redhat.com> wrote:
>
>> Am 14.01.2014 um 04:38 hat Edgar E. Iglesias geschrieben:
>> > On Tue, Jan 14, 2014 at 09:27:10AM +1000, Peter Crosthwaite wrote:
>> > > Ping,
>> > >
>> > > Has this one been forgotten or are there issues? PMM had a small
>> > > comment, but he waived it AFAICT.
>> >
>> > Pong,
>> >
>> > I've merged it now, thanks!
>>
>> I believe it's something in this pull requests that breaks make check.
>
> And you're right. But first, let me confirm that we're talking about the
> same breakage. This is what I'm getting:
>
> make tests/check-qom-interface
> libqemuutil.a(qemu-error.o): In function `error_vprintf':
> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:23: undefined reference to `cur_mon'
> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:24: undefined reference to `cur_mon'
> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:24: undefined reference to `monitor_vprintf'
> libqemuutil.a(qemu-error.o): In function `error_printf_unless_qmp':
> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:47: undefined reference to `monitor_cur_is_qmp'
> libqemuutil.a(qemu-error.o): In function `error_print_loc':
> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:174: undefined reference to `cur_mon'
> collect2: error: ld returned 1 exit status
> make: *** [tests/check-qom-interface] Error 1
>
> I tried bisecting it, but git bisect weren't capable of finding the
> culprit. So debugged it, and the problem was introduced by:
>
> commit 594278718323ca7bffaab0fb7fc6c82fa2c1cd5f
> Author: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> Date: Wed Jan 1 18:49:52 2014 -0800
>
> qerror: Remove assert_no_error()
>
> There isn't nothing really wrong with this commit. The problem seems to
> be that the tests link against libqemuutil.a and this library pulls in
> everything from util/. The commit above changed util/error.c to call
> error_report(), which depends on 'cur_mon', which is only made available
> by monitor.o.
>
> I don't think we want to mess up with including monitor.o on libqemuutil.a.
> Besides, I now find it a bit weird to call error_report() from an error
> reporting function. So it's better to just call fprintf(stderr,) instead.
>
> Peter, Markus, are you ok with this patch?
Patch is good.
Acked-by: Peter Crosthwiate <peter.crosthwaite@xilinx.com>
>
> PS: I do run make check before sending a pull request, and did run this
> time too. Not sure how I didn't catch this. Thanks for the report
> Kevin!
>
I ran make check before sending out the patches too. Not sure what
happened since.
Regards,
Peter
> diff --git a/util/error.c b/util/error.c
> index f11f1d5..7c7650c 100644
> --- a/util/error.c
> +++ b/util/error.c
> @@ -44,7 +44,7 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
> err->err_class = err_class;
>
> if (errp == &error_abort) {
> - error_report("%s", error_get_pretty(err));
> + fprintf(stderr, "%s", error_get_pretty(err));
> abort();
> }
>
> @@ -80,7 +80,7 @@ void error_set_errno(Error **errp, int os_errno, ErrorClass err_class,
> err->err_class = err_class;
>
> if (errp == &error_abort) {
> - error_report("%s", error_get_pretty(err));
> + fprintf(stderr, "%s", error_get_pretty(err));
> abort();
> }
>
> @@ -125,7 +125,7 @@ void error_set_win32(Error **errp, int win32_err, ErrorClass err_class,
> err->err_class = err_class;
>
> if (errp == &error_abort) {
> - error_report("%s", error_get_pretty(err));
> + fprintf(stderr, "%s", error_get_pretty(err));
> abort();
> }
>
> @@ -171,7 +171,7 @@ void error_free(Error *err)
> void error_propagate(Error **dst_err, Error *local_err)
> {
> if (local_err && dst_err == &error_abort) {
> - error_report("%s", error_get_pretty(local_err));
> + fprintf(stderr, "%s", error_get_pretty(local_err));
> abort();
> } else if (dst_err && !*dst_err) {
> *dst_err = local_err;
>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] Fix make check breakage (was [PULL 00/14] QMP queue)
2014-01-14 22:26 ` Peter Crosthwaite
@ 2014-01-15 0:30 ` Peter Crosthwaite
0 siblings, 0 replies; 25+ messages in thread
From: Peter Crosthwaite @ 2014-01-15 0:30 UTC (permalink / raw)
To: Luiz Capitulino
Cc: Kevin Wolf, Edgar E. Iglesias, qemu-devel@nongnu.org Developers,
Anthony Liguori, Anthony Liguori
On Wed, Jan 15, 2014 at 8:26 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Wed, Jan 15, 2014 at 4:22 AM, Luiz Capitulino <lcapitulino@redhat.com> wrote:
>> On Tue, 14 Jan 2014 17:44:51 +0100
>> Kevin Wolf <kwolf@redhat.com> wrote:
>>
>>> Am 14.01.2014 um 04:38 hat Edgar E. Iglesias geschrieben:
>>> > On Tue, Jan 14, 2014 at 09:27:10AM +1000, Peter Crosthwaite wrote:
>>> > > Ping,
>>> > >
>>> > > Has this one been forgotten or are there issues? PMM had a small
>>> > > comment, but he waived it AFAICT.
>>> >
>>> > Pong,
>>> >
>>> > I've merged it now, thanks!
>>>
>>> I believe it's something in this pull requests that breaks make check.
>>
>> And you're right. But first, let me confirm that we're talking about the
>> same breakage. This is what I'm getting:
>>
>> make tests/check-qom-interface
>> libqemuutil.a(qemu-error.o): In function `error_vprintf':
>> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:23: undefined reference to `cur_mon'
>> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:24: undefined reference to `cur_mon'
>> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:24: undefined reference to `monitor_vprintf'
>> libqemuutil.a(qemu-error.o): In function `error_printf_unless_qmp':
>> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:47: undefined reference to `monitor_cur_is_qmp'
>> libqemuutil.a(qemu-error.o): In function `error_print_loc':
>> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:174: undefined reference to `cur_mon'
>> collect2: error: ld returned 1 exit status
>> make: *** [tests/check-qom-interface] Error 1
>>
>> I tried bisecting it, but git bisect weren't capable of finding the
>> culprit. So debugged it, and the problem was introduced by:
>>
>> commit 594278718323ca7bffaab0fb7fc6c82fa2c1cd5f
>> Author: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> Date: Wed Jan 1 18:49:52 2014 -0800
>>
>> qerror: Remove assert_no_error()
>>
>> There isn't nothing really wrong with this commit. The problem seems to
>> be that the tests link against libqemuutil.a and this library pulls in
>> everything from util/. The commit above changed util/error.c to call
>> error_report(), which depends on 'cur_mon', which is only made available
>> by monitor.o.
>>
>> I don't think we want to mess up with including monitor.o on libqemuutil.a.
>> Besides, I now find it a bit weird to call error_report() from an error
>> reporting function. So it's better to just call fprintf(stderr,) instead.
>>
>> Peter, Markus, are you ok with this patch?
>
> Patch is good.
>
> Acked-by: Peter Crosthwiate <peter.crosthwaite@xilinx.com>
>
Do you want to spin this as a patch or should I?
>>
>> PS: I do run make check before sending a pull request, and did run this
>> time too. Not sure how I didn't catch this. Thanks for the report
>> Kevin!
>>
>
> I ran make check before sending out the patches too. Not sure what
> happened since.
>
I tested the tip of the merged branch and it is ok:
commit c950114286ea358a93ce632db0421945e1008395
Author: Luiz Capitulino <lcapitulino@redhat.com>
Date: Sun Dec 29 22:39:58 2013 -0500
migration: qmp_migrate(): keep working after syntax error
If a user or QMP client enter a bad syntax for the migrate
command in QMP/HMP, then the migrate command will never succeed
from that point on.
Something merged since has had an effect, so bisection is impossible
as we would need to reorder the history to figure out what it was and
I don't think it's worth it.
Regards,
Peter
> Regards,
> Peter
>
>> diff --git a/util/error.c b/util/error.c
>> index f11f1d5..7c7650c 100644
>> --- a/util/error.c
>> +++ b/util/error.c
>> @@ -44,7 +44,7 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
>> err->err_class = err_class;
>>
>> if (errp == &error_abort) {
>> - error_report("%s", error_get_pretty(err));
>> + fprintf(stderr, "%s", error_get_pretty(err));
>> abort();
>> }
>>
>> @@ -80,7 +80,7 @@ void error_set_errno(Error **errp, int os_errno, ErrorClass err_class,
>> err->err_class = err_class;
>>
>> if (errp == &error_abort) {
>> - error_report("%s", error_get_pretty(err));
>> + fprintf(stderr, "%s", error_get_pretty(err));
>> abort();
>> }
>>
>> @@ -125,7 +125,7 @@ void error_set_win32(Error **errp, int win32_err, ErrorClass err_class,
>> err->err_class = err_class;
>>
>> if (errp == &error_abort) {
>> - error_report("%s", error_get_pretty(err));
>> + fprintf(stderr, "%s", error_get_pretty(err));
>> abort();
>> }
>>
>> @@ -171,7 +171,7 @@ void error_free(Error *err)
>> void error_propagate(Error **dst_err, Error *local_err)
>> {
>> if (local_err && dst_err == &error_abort) {
>> - error_report("%s", error_get_pretty(local_err));
>> + fprintf(stderr, "%s", error_get_pretty(local_err));
>> abort();
>> } else if (dst_err && !*dst_err) {
>> *dst_err = local_err;
>>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] Fix make check breakage (was [PULL 00/14] QMP queue)
2014-01-14 18:22 ` [Qemu-devel] Fix make check breakage (was [PULL 00/14] QMP queue) Luiz Capitulino
2014-01-14 22:26 ` Peter Crosthwaite
@ 2014-01-15 9:55 ` Markus Armbruster
2014-01-15 12:27 ` Peter Crosthwaite
1 sibling, 1 reply; 25+ messages in thread
From: Markus Armbruster @ 2014-01-15 9:55 UTC (permalink / raw)
To: Luiz Capitulino
Cc: Kevin Wolf, Edgar E. Iglesias, Peter Crosthwaite, Anthony Liguori,
qemu-devel@nongnu.org Developers, Anthony Liguori
Luiz Capitulino <lcapitulino@redhat.com> writes:
> On Tue, 14 Jan 2014 17:44:51 +0100
> Kevin Wolf <kwolf@redhat.com> wrote:
>
>> Am 14.01.2014 um 04:38 hat Edgar E. Iglesias geschrieben:
>> > On Tue, Jan 14, 2014 at 09:27:10AM +1000, Peter Crosthwaite wrote:
>> > > Ping,
>> > >
>> > > Has this one been forgotten or are there issues? PMM had a small
>> > > comment, but he waived it AFAICT.
>> >
>> > Pong,
>> >
>> > I've merged it now, thanks!
>>
>> I believe it's something in this pull requests that breaks make check.
>
> And you're right. But first, let me confirm that we're talking about the
> same breakage. This is what I'm getting:
>
> make tests/check-qom-interface
> libqemuutil.a(qemu-error.o): In function `error_vprintf':
> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:23: undefined reference to `cur_mon'
> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:24: undefined reference to `cur_mon'
> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:24: undefined reference to `monitor_vprintf'
> libqemuutil.a(qemu-error.o): In function `error_printf_unless_qmp':
> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:47: undefined reference to `monitor_cur_is_qmp'
> libqemuutil.a(qemu-error.o): In function `error_print_loc':
> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:174: undefined reference to `cur_mon'
> collect2: error: ld returned 1 exit status
> make: *** [tests/check-qom-interface] Error 1
>
> I tried bisecting it, but git bisect weren't capable of finding the
> culprit. So debugged it, and the problem was introduced by:
>
> commit 594278718323ca7bffaab0fb7fc6c82fa2c1cd5f
> Author: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> Date: Wed Jan 1 18:49:52 2014 -0800
>
> qerror: Remove assert_no_error()
>
Are you sure you don't mean commit 5d24ee7 "error: Add error_abort"?
> There isn't nothing really wrong with this commit. The problem seems to
> be that the tests link against libqemuutil.a and this library pulls in
> everything from util/. The commit above changed util/error.c to call
> error_report(),
Yes, 5d24ee7 does that.
> which depends on 'cur_mon', which is only made available
> by monitor.o.
And stubs/mon-set-error.o
> I don't think we want to mess up with including monitor.o on libqemuutil.a.
> Besides, I now find it a bit weird to call error_report() from an error
> reporting function. So it's better to just call fprintf(stderr,) instead.
It's not weird at all. Higher layer calls lower layer.
> Peter, Markus, are you ok with this patch?
>
> PS: I do run make check before sending a pull request, and did run this
> time too. Not sure how I didn't catch this. Thanks for the report
> Kevin!
>
> diff --git a/util/error.c b/util/error.c
> index f11f1d5..7c7650c 100644
> --- a/util/error.c
> +++ b/util/error.c
> @@ -44,7 +44,7 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
> err->err_class = err_class;
>
> if (errp == &error_abort) {
> - error_report("%s", error_get_pretty(err));
> + fprintf(stderr, "%s", error_get_pretty(err));
> abort();
> }
>
> @@ -80,7 +80,7 @@ void error_set_errno(Error **errp, int os_errno, ErrorClass err_class,
> err->err_class = err_class;
>
> if (errp == &error_abort) {
> - error_report("%s", error_get_pretty(err));
> + fprintf(stderr, "%s", error_get_pretty(err));
> abort();
> }
>
> @@ -125,7 +125,7 @@ void error_set_win32(Error **errp, int win32_err, ErrorClass err_class,
> err->err_class = err_class;
>
> if (errp == &error_abort) {
> - error_report("%s", error_get_pretty(err));
> + fprintf(stderr, "%s", error_get_pretty(err));
> abort();
> }
>
> @@ -171,7 +171,7 @@ void error_free(Error *err)
> void error_propagate(Error **dst_err, Error *local_err)
> {
> if (local_err && dst_err == &error_abort) {
> - error_report("%s", error_get_pretty(local_err));
> + fprintf(stderr, "%s", error_get_pretty(local_err));
> abort();
> } else if (dst_err && !*dst_err) {
> *dst_err = local_err;
Error message screwed up!
Before:
$ qemu -msg timestamp=on -global i440FX-pcihost.foo=bar
2014-01-15T09:50:18.066725Z upstream-qemu: Property '.foo' not found
Aborted (core dumped)
After:
Property '.foo' not foundAborted (core dumped)
Note the loss of timestamp, name of executable, location, and the final
newline. Please fix.
Amazing super-secret trick to get error messages fit for human
consumption: reproduce them before you commit! ;-P
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] Fix make check breakage (was [PULL 00/14] QMP queue)
2014-01-15 9:55 ` Markus Armbruster
@ 2014-01-15 12:27 ` Peter Crosthwaite
2014-01-15 13:52 ` Markus Armbruster
0 siblings, 1 reply; 25+ messages in thread
From: Peter Crosthwaite @ 2014-01-15 12:27 UTC (permalink / raw)
To: Markus Armbruster
Cc: Kevin Wolf, Edgar E. Iglesias, Anthony Liguori,
qemu-devel@nongnu.org Developers, Luiz Capitulino,
Anthony Liguori
On Wed, Jan 15, 2014 at 7:55 PM, Markus Armbruster <armbru@redhat.com> wrote:
> Luiz Capitulino <lcapitulino@redhat.com> writes:
>
>> On Tue, 14 Jan 2014 17:44:51 +0100
>> Kevin Wolf <kwolf@redhat.com> wrote:
>>
>>> Am 14.01.2014 um 04:38 hat Edgar E. Iglesias geschrieben:
>>> > On Tue, Jan 14, 2014 at 09:27:10AM +1000, Peter Crosthwaite wrote:
>>> > > Ping,
>>> > >
>>> > > Has this one been forgotten or are there issues? PMM had a small
>>> > > comment, but he waived it AFAICT.
>>> >
>>> > Pong,
>>> >
>>> > I've merged it now, thanks!
>>>
>>> I believe it's something in this pull requests that breaks make check.
>>
>> And you're right. But first, let me confirm that we're talking about the
>> same breakage. This is what I'm getting:
>>
>> make tests/check-qom-interface
>> libqemuutil.a(qemu-error.o): In function `error_vprintf':
>> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:23: undefined reference to `cur_mon'
>> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:24: undefined reference to `cur_mon'
>> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:24: undefined reference to `monitor_vprintf'
>> libqemuutil.a(qemu-error.o): In function `error_printf_unless_qmp':
>> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:47: undefined reference to `monitor_cur_is_qmp'
>> libqemuutil.a(qemu-error.o): In function `error_print_loc':
>> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:174: undefined reference to `cur_mon'
>> collect2: error: ld returned 1 exit status
>> make: *** [tests/check-qom-interface] Error 1
>>
>> I tried bisecting it, but git bisect weren't capable of finding the
>> culprit. So debugged it, and the problem was introduced by:
>>
>> commit 594278718323ca7bffaab0fb7fc6c82fa2c1cd5f
>> Author: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> Date: Wed Jan 1 18:49:52 2014 -0800
>>
>> qerror: Remove assert_no_error()
>>
>
> Are you sure you don't mean commit 5d24ee7 "error: Add error_abort"?
>
>> There isn't nothing really wrong with this commit. The problem seems to
>> be that the tests link against libqemuutil.a and this library pulls in
>> everything from util/. The commit above changed util/error.c to call
>> error_report(),
>
> Yes, 5d24ee7 does that.
>
>> which depends on 'cur_mon', which is only made available
>> by monitor.o.
>
> And stubs/mon-set-error.o
>
>> I don't think we want to mess up with including monitor.o on libqemuutil.a.
>> Besides, I now find it a bit weird to call error_report() from an error
>> reporting function. So it's better to just call fprintf(stderr,) instead.
>
> It's not weird at all. Higher layer calls lower layer.
>
>> Peter, Markus, are you ok with this patch?
>>
>> PS: I do run make check before sending a pull request, and did run this
>> time too. Not sure how I didn't catch this. Thanks for the report
>> Kevin!
>>
>> diff --git a/util/error.c b/util/error.c
>> index f11f1d5..7c7650c 100644
>> --- a/util/error.c
>> +++ b/util/error.c
>> @@ -44,7 +44,7 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
>> err->err_class = err_class;
>>
>> if (errp == &error_abort) {
>> - error_report("%s", error_get_pretty(err));
>> + fprintf(stderr, "%s", error_get_pretty(err));
>> abort();
>> }
>>
>> @@ -80,7 +80,7 @@ void error_set_errno(Error **errp, int os_errno, ErrorClass err_class,
>> err->err_class = err_class;
>>
>> if (errp == &error_abort) {
>> - error_report("%s", error_get_pretty(err));
>> + fprintf(stderr, "%s", error_get_pretty(err));
>> abort();
>> }
>>
>> @@ -125,7 +125,7 @@ void error_set_win32(Error **errp, int win32_err, ErrorClass err_class,
>> err->err_class = err_class;
>>
>> if (errp == &error_abort) {
>> - error_report("%s", error_get_pretty(err));
>> + fprintf(stderr, "%s", error_get_pretty(err));
>> abort();
>> }
>>
>> @@ -171,7 +171,7 @@ void error_free(Error *err)
>> void error_propagate(Error **dst_err, Error *local_err)
>> {
>> if (local_err && dst_err == &error_abort) {
>> - error_report("%s", error_get_pretty(local_err));
>> + fprintf(stderr, "%s", error_get_pretty(local_err));
>> abort();
>> } else if (dst_err && !*dst_err) {
>> *dst_err = local_err;
>
> Error message screwed up!
>
> Before:
>
> $ qemu -msg timestamp=on -global i440FX-pcihost.foo=bar
> 2014-01-15T09:50:18.066725Z upstream-qemu: Property '.foo' not found
> Aborted (core dumped)
>
curious - should the user be able to cause an abort just on command
line misuse like that? My understanding is that assert (and therefore
assert_no_error and error_abort) should be limited to fatal errors
indicating qemu source bugs. Is it ok to report-and-abort a non
recoverable error like this one? If so, theres significant cleanup we
can do as many of us have been using the verbose error-report ->
exit(1) for situations much like this.
> After:
>
> Property '.foo' not foundAborted (core dumped)
>
> Note the loss of timestamp, name of executable, location, and the final
> newline. Please fix.
>
> Amazing super-secret trick to get error messages fit for human
> consumption: reproduce them before you commit! ;-P
>
Short series on list that straightens it all out based on your stub
recommendations.
Regards,
Peter
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [Qemu-devel] Fix make check breakage (was [PULL 00/14] QMP queue)
2014-01-15 12:27 ` Peter Crosthwaite
@ 2014-01-15 13:52 ` Markus Armbruster
0 siblings, 0 replies; 25+ messages in thread
From: Markus Armbruster @ 2014-01-15 13:52 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: Kevin Wolf, Edgar E. Iglesias, Anthony Liguori,
qemu-devel@nongnu.org Developers, Luiz Capitulino,
Anthony Liguori
Peter Crosthwaite <peter.crosthwaite@xilinx.com> writes:
> On Wed, Jan 15, 2014 at 7:55 PM, Markus Armbruster <armbru@redhat.com> wrote:
>> Luiz Capitulino <lcapitulino@redhat.com> writes:
>>
>>> On Tue, 14 Jan 2014 17:44:51 +0100
>>> Kevin Wolf <kwolf@redhat.com> wrote:
>>>
>>>> Am 14.01.2014 um 04:38 hat Edgar E. Iglesias geschrieben:
>>>> > On Tue, Jan 14, 2014 at 09:27:10AM +1000, Peter Crosthwaite wrote:
>>>> > > Ping,
>>>> > >
>>>> > > Has this one been forgotten or are there issues? PMM had a small
>>>> > > comment, but he waived it AFAICT.
>>>> >
>>>> > Pong,
>>>> >
>>>> > I've merged it now, thanks!
>>>>
>>>> I believe it's something in this pull requests that breaks make check.
>>>
>>> And you're right. But first, let me confirm that we're talking about the
>>> same breakage. This is what I'm getting:
>>>
>>> make tests/check-qom-interface
>>> libqemuutil.a(qemu-error.o): In function `error_vprintf':
>>> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:23: undefined reference to `cur_mon'
>>> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:24: undefined reference to `cur_mon'
>>> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:24: undefined reference to `monitor_vprintf'
>>> libqemuutil.a(qemu-error.o): In function `error_printf_unless_qmp':
>>> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:47: undefined reference to `monitor_cur_is_qmp'
>>> libqemuutil.a(qemu-error.o): In function `error_print_loc':
>>> /home/lcapitulino/work/src/upstream/qmp-unstable/util/qemu-error.c:174: undefined reference to `cur_mon'
>>> collect2: error: ld returned 1 exit status
>>> make: *** [tests/check-qom-interface] Error 1
>>>
>>> I tried bisecting it, but git bisect weren't capable of finding the
>>> culprit. So debugged it, and the problem was introduced by:
>>>
>>> commit 594278718323ca7bffaab0fb7fc6c82fa2c1cd5f
>>> Author: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>> Date: Wed Jan 1 18:49:52 2014 -0800
>>>
>>> qerror: Remove assert_no_error()
>>>
>>
>> Are you sure you don't mean commit 5d24ee7 "error: Add error_abort"?
>>
>>> There isn't nothing really wrong with this commit. The problem seems to
>>> be that the tests link against libqemuutil.a and this library pulls in
>>> everything from util/. The commit above changed util/error.c to call
>>> error_report(),
>>
>> Yes, 5d24ee7 does that.
>>
>>> which depends on 'cur_mon', which is only made available
>>> by monitor.o.
>>
>> And stubs/mon-set-error.o
>>
>>> I don't think we want to mess up with including monitor.o on libqemuutil.a.
>>> Besides, I now find it a bit weird to call error_report() from an error
>>> reporting function. So it's better to just call fprintf(stderr,) instead.
>>
>> It's not weird at all. Higher layer calls lower layer.
>>
>>> Peter, Markus, are you ok with this patch?
>>>
>>> PS: I do run make check before sending a pull request, and did run this
>>> time too. Not sure how I didn't catch this. Thanks for the report
>>> Kevin!
>>>
>>> diff --git a/util/error.c b/util/error.c
>>> index f11f1d5..7c7650c 100644
>>> --- a/util/error.c
>>> +++ b/util/error.c
>>> @@ -44,7 +44,7 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
>>> err->err_class = err_class;
>>>
>>> if (errp == &error_abort) {
>>> - error_report("%s", error_get_pretty(err));
>>> + fprintf(stderr, "%s", error_get_pretty(err));
>>> abort();
>>> }
>>>
>>> @@ -80,7 +80,7 @@ void error_set_errno(Error **errp, int os_errno, ErrorClass err_class,
>>> err->err_class = err_class;
>>>
>>> if (errp == &error_abort) {
>>> - error_report("%s", error_get_pretty(err));
>>> + fprintf(stderr, "%s", error_get_pretty(err));
>>> abort();
>>> }
>>>
>>> @@ -125,7 +125,7 @@ void error_set_win32(Error **errp, int win32_err, ErrorClass err_class,
>>> err->err_class = err_class;
>>>
>>> if (errp == &error_abort) {
>>> - error_report("%s", error_get_pretty(err));
>>> + fprintf(stderr, "%s", error_get_pretty(err));
>>> abort();
>>> }
>>>
>>> @@ -171,7 +171,7 @@ void error_free(Error *err)
>>> void error_propagate(Error **dst_err, Error *local_err)
>>> {
>>> if (local_err && dst_err == &error_abort) {
>>> - error_report("%s", error_get_pretty(local_err));
>>> + fprintf(stderr, "%s", error_get_pretty(local_err));
>>> abort();
>>> } else if (dst_err && !*dst_err) {
>>> *dst_err = local_err;
>>
>> Error message screwed up!
>>
>> Before:
>>
>> $ qemu -msg timestamp=on -global i440FX-pcihost.foo=bar
>> 2014-01-15T09:50:18.066725Z upstream-qemu: Property '.foo' not found
>> Aborted (core dumped)
>>
>
> curious - should the user be able to cause an abort just on command
> line misuse like that? My understanding is that assert (and therefore
> assert_no_error and error_abort) should be limited to fatal errors
> indicating qemu source bugs. Is it ok to report-and-abort a non
> recoverable error like this one? If so, theres significant cleanup we
> can do as many of us have been using the verbose error-report ->
> exit(1) for situations much like this.
Your understanding is correct: assertions are not an acceptable error
reporting mechanism. The code is clearly abusing error_abort here.
error_report() produces consistently formatted error messages. Less
important for genuine "this cannot happen" reports; the part I need
there a message that leads me to the point of failure in the code, and a
core dump. Straight assert() or fprintf(); abort() can do that. But as
long as error_abort is used cavalierly, as my test case demonstrates, we
better stick to error_report().
>> After:
>>
>> Property '.foo' not foundAborted (core dumped)
>>
>> Note the loss of timestamp, name of executable, location, and the final
>> newline. Please fix.
>>
>> Amazing super-secret trick to get error messages fit for human
>> consumption: reproduce them before you commit! ;-P
>>
>
> Short series on list that straightens it all out based on your stub
> recommendations.
Thanks!
^ permalink raw reply [flat|nested] 25+ messages in thread