* [Qemu-devel] [PATCH 01/11] qerror: rename QERR_SOCKET_* macros
2012-07-25 16:54 [Qemu-devel] [PATCH v2 00/11]: qapi: generate qerrors from qapi-schema-errors.json Luiz Capitulino
@ 2012-07-25 16:54 ` Luiz Capitulino
2012-07-25 16:54 ` [Qemu-devel] [PATCH 02/11] qerror: rename QERR_SOCK_CONNECT_IN_PROGRESS Luiz Capitulino
` (9 subsequent siblings)
10 siblings, 0 replies; 39+ messages in thread
From: Luiz Capitulino @ 2012-07-25 16:54 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, aliguori, armbru, mdroth
The socket error classes call a socket 'Sock', like in SockConnectFailed,
but the error macros call a socket SOCKET, like in QERR_SOCKET_CONNECT_FAILED.
This will cause problems when the error macros creation get automated,
because the macro name will be derived from the error class name.
Avoid that by renaming all QERR_SOCKET_* macros to QERR_SOCK_*.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
migration-tcp.c | 6 +++---
qemu-sockets.c | 22 +++++++++++-----------
qerror.c | 10 +++++-----
qerror.h | 10 +++++-----
4 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/migration-tcp.c b/migration-tcp.c
index 440804d..587fc70 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -90,13 +90,13 @@ int tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
if (!error_is_set(errp)) {
migrate_fd_connect(s);
- } else if (error_is_type(*errp, QERR_SOCKET_CONNECT_IN_PROGRESS)) {
+ } else if (error_is_type(*errp, QERR_SOCK_CONNECT_IN_PROGRESS)) {
DPRINTF("connect in progress\n");
qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
- } else if (error_is_type(*errp, QERR_SOCKET_CREATE_FAILED)) {
+ } else if (error_is_type(*errp, QERR_SOCK_CREATE_FAILED)) {
DPRINTF("connect failed\n");
return -1;
- } else if (error_is_type(*errp, QERR_SOCKET_CONNECT_FAILED)) {
+ } else if (error_is_type(*errp, QERR_SOCK_CONNECT_FAILED)) {
DPRINTF("connect failed\n");
migrate_fd_error(s);
return -1;
diff --git a/qemu-sockets.c b/qemu-sockets.c
index 2ae715d..1357ec0 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -120,7 +120,7 @@ int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp)
if ((qemu_opt_get(opts, "host") == NULL) ||
(qemu_opt_get(opts, "port") == NULL)) {
fprintf(stderr, "%s: host and/or port not specified\n", __FUNCTION__);
- error_set(errp, QERR_SOCKET_CREATE_FAILED);
+ error_set(errp, QERR_SOCK_CREATE_FAILED);
return -1;
}
pstrcpy(port, sizeof(port), qemu_opt_get(opts, "port"));
@@ -139,7 +139,7 @@ int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp)
if (rc != 0) {
fprintf(stderr,"getaddrinfo(%s,%s): %s\n", addr, port,
gai_strerror(rc));
- error_set(errp, QERR_SOCKET_CREATE_FAILED);
+ error_set(errp, QERR_SOCK_CREATE_FAILED);
return -1;
}
@@ -153,7 +153,7 @@ int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp)
fprintf(stderr,"%s: socket(%s): %s\n", __FUNCTION__,
inet_strfamily(e->ai_family), strerror(errno));
if (!e->ai_next) {
- error_set(errp, QERR_SOCKET_CREATE_FAILED);
+ error_set(errp, QERR_SOCK_CREATE_FAILED);
}
continue;
}
@@ -179,7 +179,7 @@ int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp)
inet_strfamily(e->ai_family), uaddr, inet_getport(e),
strerror(errno));
if (!e->ai_next) {
- error_set(errp, QERR_SOCKET_BIND_FAILED);
+ error_set(errp, QERR_SOCK_BIND_FAILED);
}
}
}
@@ -191,7 +191,7 @@ int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp)
listen:
if (listen(slisten,1) != 0) {
- error_set(errp, QERR_SOCKET_LISTEN_FAILED);
+ error_set(errp, QERR_SOCK_LISTEN_FAILED);
perror("listen");
closesocket(slisten);
freeaddrinfo(res);
@@ -226,7 +226,7 @@ int inet_connect_opts(QemuOpts *opts, Error **errp)
block = qemu_opt_get_bool(opts, "block", 0);
if (addr == NULL || port == NULL) {
fprintf(stderr, "inet_connect: host and/or port not specified\n");
- error_set(errp, QERR_SOCKET_CREATE_FAILED);
+ error_set(errp, QERR_SOCK_CREATE_FAILED);
return -1;
}
@@ -239,7 +239,7 @@ int inet_connect_opts(QemuOpts *opts, Error **errp)
if (0 != (rc = getaddrinfo(addr, port, &ai, &res))) {
fprintf(stderr,"getaddrinfo(%s,%s): %s\n", addr, port,
gai_strerror(rc));
- error_set(errp, QERR_SOCKET_CREATE_FAILED);
+ error_set(errp, QERR_SOCK_CREATE_FAILED);
return -1;
}
@@ -274,7 +274,7 @@ int inet_connect_opts(QemuOpts *opts, Error **errp)
#else
if (!block && (rc == -EINPROGRESS)) {
#endif
- error_set(errp, QERR_SOCKET_CONNECT_IN_PROGRESS);
+ error_set(errp, QERR_SOCK_CONNECT_IN_PROGRESS);
} else if (rc < 0) {
if (NULL == e->ai_next)
fprintf(stderr, "%s: connect(%s,%s,%s,%s): %s\n", __FUNCTION__,
@@ -287,7 +287,7 @@ int inet_connect_opts(QemuOpts *opts, Error **errp)
freeaddrinfo(res);
return sock;
}
- error_set(errp, QERR_SOCKET_CONNECT_FAILED);
+ error_set(errp, QERR_SOCK_CONNECT_FAILED);
freeaddrinfo(res);
return -1;
}
@@ -479,7 +479,7 @@ int inet_listen(const char *str, char *ostr, int olen,
}
}
} else {
- error_set(errp, QERR_SOCKET_CREATE_FAILED);
+ error_set(errp, QERR_SOCK_CREATE_FAILED);
}
qemu_opts_del(opts);
return sock;
@@ -497,7 +497,7 @@ int inet_connect(const char *str, bool block, Error **errp)
}
sock = inet_connect_opts(opts, errp);
} else {
- error_set(errp, QERR_SOCKET_CREATE_FAILED);
+ error_set(errp, QERR_SOCK_CREATE_FAILED);
}
qemu_opts_del(opts);
return sock;
diff --git a/qerror.c b/qerror.c
index 92c4eff..e988e36 100644
--- a/qerror.c
+++ b/qerror.c
@@ -309,23 +309,23 @@ static const QErrorStringTable qerror_table[] = {
.desc = "Could not start VNC server on %(target)",
},
{
- .error_fmt = QERR_SOCKET_CONNECT_IN_PROGRESS,
+ .error_fmt = QERR_SOCK_CONNECT_IN_PROGRESS,
.desc = "Connection can not be completed immediately",
},
{
- .error_fmt = QERR_SOCKET_CONNECT_FAILED,
+ .error_fmt = QERR_SOCK_CONNECT_FAILED,
.desc = "Failed to connect to socket",
},
{
- .error_fmt = QERR_SOCKET_LISTEN_FAILED,
+ .error_fmt = QERR_SOCK_LISTEN_FAILED,
.desc = "Failed to set socket to listening mode",
},
{
- .error_fmt = QERR_SOCKET_BIND_FAILED,
+ .error_fmt = QERR_SOCK_BIND_FAILED,
.desc = "Failed to bind socket",
},
{
- .error_fmt = QERR_SOCKET_CREATE_FAILED,
+ .error_fmt = QERR_SOCK_CREATE_FAILED,
.desc = "Failed to create socket",
},
{}
diff --git a/qerror.h b/qerror.h
index b4c8758..71b0496 100644
--- a/qerror.h
+++ b/qerror.h
@@ -251,19 +251,19 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_VNC_SERVER_FAILED \
"{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
-#define QERR_SOCKET_CONNECT_IN_PROGRESS \
+#define QERR_SOCK_CONNECT_IN_PROGRESS \
"{ 'class': 'SockConnectInprogress', 'data': {} }"
-#define QERR_SOCKET_CONNECT_FAILED \
+#define QERR_SOCK_CONNECT_FAILED \
"{ 'class': 'SockConnectFailed', 'data': {} }"
-#define QERR_SOCKET_LISTEN_FAILED \
+#define QERR_SOCK_LISTEN_FAILED \
"{ 'class': 'SockListenFailed', 'data': {} }"
-#define QERR_SOCKET_BIND_FAILED \
+#define QERR_SOCK_BIND_FAILED \
"{ 'class': 'SockBindFailed', 'data': {} }"
-#define QERR_SOCKET_CREATE_FAILED \
+#define QERR_SOCK_CREATE_FAILED \
"{ 'class': 'SockCreateFailed', 'data': {} }"
#endif /* QERROR_H */
--
1.7.11.2.249.g31c7954.dirty
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH 02/11] qerror: rename QERR_SOCK_CONNECT_IN_PROGRESS
2012-07-25 16:54 [Qemu-devel] [PATCH v2 00/11]: qapi: generate qerrors from qapi-schema-errors.json Luiz Capitulino
2012-07-25 16:54 ` [Qemu-devel] [PATCH 01/11] qerror: rename QERR_SOCKET_* macros Luiz Capitulino
@ 2012-07-25 16:54 ` Luiz Capitulino
2012-07-25 16:54 ` [Qemu-devel] [PATCH 03/11] qerror: rename QERR_QMP_EXTRA_MEMBER Luiz Capitulino
` (8 subsequent siblings)
10 siblings, 0 replies; 39+ messages in thread
From: Luiz Capitulino @ 2012-07-25 16:54 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, aliguori, armbru, mdroth
The error class name is SockConnectInprogress, not SockConnectInProgress.
Rename the QERR_SOCK_CONNECT_IN_PROGRESS macro to QERR_SOCK_CONNECT_INPROGRESS
to reflect that, so that future error macro generation generates the
expected macro name.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
migration-tcp.c | 2 +-
qemu-sockets.c | 2 +-
qerror.c | 2 +-
qerror.h | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/migration-tcp.c b/migration-tcp.c
index 587fc70..3cffa94 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -90,7 +90,7 @@ int tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
if (!error_is_set(errp)) {
migrate_fd_connect(s);
- } else if (error_is_type(*errp, QERR_SOCK_CONNECT_IN_PROGRESS)) {
+ } else if (error_is_type(*errp, QERR_SOCK_CONNECT_INPROGRESS)) {
DPRINTF("connect in progress\n");
qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
} else if (error_is_type(*errp, QERR_SOCK_CREATE_FAILED)) {
diff --git a/qemu-sockets.c b/qemu-sockets.c
index 1357ec0..20def3e 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -274,7 +274,7 @@ int inet_connect_opts(QemuOpts *opts, Error **errp)
#else
if (!block && (rc == -EINPROGRESS)) {
#endif
- error_set(errp, QERR_SOCK_CONNECT_IN_PROGRESS);
+ error_set(errp, QERR_SOCK_CONNECT_INPROGRESS);
} else if (rc < 0) {
if (NULL == e->ai_next)
fprintf(stderr, "%s: connect(%s,%s,%s,%s): %s\n", __FUNCTION__,
diff --git a/qerror.c b/qerror.c
index e988e36..a9d771b 100644
--- a/qerror.c
+++ b/qerror.c
@@ -309,7 +309,7 @@ static const QErrorStringTable qerror_table[] = {
.desc = "Could not start VNC server on %(target)",
},
{
- .error_fmt = QERR_SOCK_CONNECT_IN_PROGRESS,
+ .error_fmt = QERR_SOCK_CONNECT_INPROGRESS,
.desc = "Connection can not be completed immediately",
},
{
diff --git a/qerror.h b/qerror.h
index 71b0496..69d417d 100644
--- a/qerror.h
+++ b/qerror.h
@@ -251,7 +251,7 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_VNC_SERVER_FAILED \
"{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
-#define QERR_SOCK_CONNECT_IN_PROGRESS \
+#define QERR_SOCK_CONNECT_INPROGRESS \
"{ 'class': 'SockConnectInprogress', 'data': {} }"
#define QERR_SOCK_CONNECT_FAILED \
--
1.7.11.2.249.g31c7954.dirty
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH 03/11] qerror: rename QERR_QMP_EXTRA_MEMBER
2012-07-25 16:54 [Qemu-devel] [PATCH v2 00/11]: qapi: generate qerrors from qapi-schema-errors.json Luiz Capitulino
2012-07-25 16:54 ` [Qemu-devel] [PATCH 01/11] qerror: rename QERR_SOCKET_* macros Luiz Capitulino
2012-07-25 16:54 ` [Qemu-devel] [PATCH 02/11] qerror: rename QERR_SOCK_CONNECT_IN_PROGRESS Luiz Capitulino
@ 2012-07-25 16:54 ` Luiz Capitulino
2012-07-25 16:54 ` [Qemu-devel] [PATCH 04/11] qerror: rename QERR_PROPERTY_VALUE_NOT_POWER_OF_2 Luiz Capitulino
` (7 subsequent siblings)
10 siblings, 0 replies; 39+ messages in thread
From: Luiz Capitulino @ 2012-07-25 16:54 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, aliguori, armbru, mdroth
The error class name is QMPExtraInputObjectMember, not QMPExtraMember.
Rename the QERR_QMP_EXTRA_MEMBER macro to QERR_QMP_EXTRA_INPUT_OBJECT_MEMBER
to reflect that, so that future error macro generation generates the
expected macro name.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
monitor.c | 2 +-
qapi/qmp-dispatch.c | 2 +-
qapi/qmp-input-visitor.c | 2 +-
qerror.c | 2 +-
qerror.h | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/monitor.c b/monitor.c
index 09aa3cd..ec4e467 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4430,7 +4430,7 @@ static QDict *qmp_check_input_obj(QObject *input_obj)
} else if (!strcmp(arg_name, "id")) {
/* FIXME: check duplicated IDs for async commands */
} else {
- qerror_report(QERR_QMP_EXTRA_MEMBER, arg_name);
+ qerror_report(QERR_QMP_EXTRA_INPUT_OBJECT_MEMBER, arg_name);
return NULL;
}
}
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 122c1a2..29d6f30 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -47,7 +47,7 @@ static QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp)
}
has_exec_key = true;
} else if (strcmp(arg_name, "arguments")) {
- error_set(errp, QERR_QMP_EXTRA_MEMBER, arg_name);
+ error_set(errp, QERR_QMP_EXTRA_INPUT_OBJECT_MEMBER, arg_name);
return NULL;
}
}
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 107d8d3..a59d4f6 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -104,7 +104,7 @@ static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp)
if (g_hash_table_size(top_ht)) {
const char *key;
g_hash_table_find(top_ht, always_true, &key);
- error_set(errp, QERR_QMP_EXTRA_MEMBER, key);
+ error_set(errp, QERR_QMP_EXTRA_INPUT_OBJECT_MEMBER, key);
}
g_hash_table_unref(top_ht);
}
diff --git a/qerror.c b/qerror.c
index a9d771b..132ab2d 100644
--- a/qerror.c
+++ b/qerror.c
@@ -271,7 +271,7 @@ static const QErrorStringTable qerror_table[] = {
.desc = "QMP input object member '%(member)' expects '%(expected)'",
},
{
- .error_fmt = QERR_QMP_EXTRA_MEMBER,
+ .error_fmt = QERR_QMP_EXTRA_INPUT_OBJECT_MEMBER,
.desc = "QMP input object member '%(member)' is unexpected",
},
{
diff --git a/qerror.h b/qerror.h
index 69d417d..27ce395 100644
--- a/qerror.h
+++ b/qerror.h
@@ -224,7 +224,7 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_QMP_BAD_INPUT_OBJECT_MEMBER \
"{ 'class': 'QMPBadInputObjectMember', 'data': { 'member': %s, 'expected': %s } }"
-#define QERR_QMP_EXTRA_MEMBER \
+#define QERR_QMP_EXTRA_INPUT_OBJECT_MEMBER \
"{ 'class': 'QMPExtraInputObjectMember', 'data': { 'member': %s } }"
#define QERR_RESET_REQUIRED \
--
1.7.11.2.249.g31c7954.dirty
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH 04/11] qerror: rename QERR_PROPERTY_VALUE_NOT_POWER_OF_2
2012-07-25 16:54 [Qemu-devel] [PATCH v2 00/11]: qapi: generate qerrors from qapi-schema-errors.json Luiz Capitulino
` (2 preceding siblings ...)
2012-07-25 16:54 ` [Qemu-devel] [PATCH 03/11] qerror: rename QERR_QMP_EXTRA_MEMBER Luiz Capitulino
@ 2012-07-25 16:54 ` Luiz Capitulino
2012-07-25 16:54 ` [Qemu-devel] [PATCH 05/11] qerror: QERR_DEVICE_ENCRYPTED: add filename info to human msg Luiz Capitulino
` (6 subsequent siblings)
10 siblings, 0 replies; 39+ messages in thread
From: Luiz Capitulino @ 2012-07-25 16:54 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, aliguori, armbru, mdroth
The error class name is PropertyValueNotPowerOf2, not PropertyValueNotPowerOf_2.
Rename the QERR_PROPERTY_VALUE_NOT_POWER_OF_2 macro to
QERR_PROPERTY_VALUE_NOT_POWER_OF2 to reflect that, so that future error macro
generation generates the expected macro name.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
hw/qdev-properties.c | 2 +-
qerror.c | 2 +-
qerror.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 24b39e8..7677cfd 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -901,7 +901,7 @@ static void set_blocksize(Object *obj, Visitor *v, void *opaque,
/* We rely on power-of-2 blocksizes for bitmasks */
if ((value & (value - 1)) != 0) {
- error_set(errp, QERR_PROPERTY_VALUE_NOT_POWER_OF_2,
+ error_set(errp, QERR_PROPERTY_VALUE_NOT_POWER_OF2,
dev->id?:"", name, (int64_t)value);
return;
}
diff --git a/qerror.c b/qerror.c
index 132ab2d..e09c410 100644
--- a/qerror.c
+++ b/qerror.c
@@ -245,7 +245,7 @@ static const QErrorStringTable qerror_table[] = {
.desc = "Property '%(device).%(property)' can't find value '%(value)'",
},
{
- .error_fmt = QERR_PROPERTY_VALUE_NOT_POWER_OF_2,
+ .error_fmt = QERR_PROPERTY_VALUE_NOT_POWER_OF2,
.desc = "Property '%(device).%(property)' doesn't take "
"value '%(value)', it's not a power of 2",
},
diff --git a/qerror.h b/qerror.h
index 27ce395..58d0295 100644
--- a/qerror.h
+++ b/qerror.h
@@ -205,7 +205,7 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_PROPERTY_VALUE_NOT_FOUND \
"{ 'class': 'PropertyValueNotFound', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
-#define QERR_PROPERTY_VALUE_NOT_POWER_OF_2 \
+#define QERR_PROPERTY_VALUE_NOT_POWER_OF2 \
"{ 'class': 'PropertyValueNotPowerOf2', 'data': { " \
"'device': %s, 'property': %s, 'value': %"PRId64" } }"
--
1.7.11.2.249.g31c7954.dirty
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH 05/11] qerror: QERR_DEVICE_ENCRYPTED: add filename info to human msg
2012-07-25 16:54 [Qemu-devel] [PATCH v2 00/11]: qapi: generate qerrors from qapi-schema-errors.json Luiz Capitulino
` (3 preceding siblings ...)
2012-07-25 16:54 ` [Qemu-devel] [PATCH 04/11] qerror: rename QERR_PROPERTY_VALUE_NOT_POWER_OF_2 Luiz Capitulino
@ 2012-07-25 16:54 ` Luiz Capitulino
2012-07-25 16:54 ` [Qemu-devel] [PATCH 06/11] qerror: QERR_AMBIGUOUS_PATH: drop %(object) from " Luiz Capitulino
` (5 subsequent siblings)
10 siblings, 0 replies; 39+ messages in thread
From: Luiz Capitulino @ 2012-07-25 16:54 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, aliguori, armbru, mdroth
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qerror.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qerror.c b/qerror.c
index e09c410..3d8ba2a 100644
--- a/qerror.c
+++ b/qerror.c
@@ -81,7 +81,7 @@ static const QErrorStringTable qerror_table[] = {
},
{
.error_fmt = QERR_DEVICE_ENCRYPTED,
- .desc = "Device '%(device)' is encrypted",
+ .desc = "Device '%(device)' is encrypted (filename=%(filename))",
},
{
.error_fmt = QERR_DEVICE_FEATURE_BLOCKS_MIGRATION,
--
1.7.11.2.249.g31c7954.dirty
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH 06/11] qerror: QERR_AMBIGUOUS_PATH: drop %(object) from human msg
2012-07-25 16:54 [Qemu-devel] [PATCH v2 00/11]: qapi: generate qerrors from qapi-schema-errors.json Luiz Capitulino
` (4 preceding siblings ...)
2012-07-25 16:54 ` [Qemu-devel] [PATCH 05/11] qerror: QERR_DEVICE_ENCRYPTED: add filename info to human msg Luiz Capitulino
@ 2012-07-25 16:54 ` Luiz Capitulino
2012-07-26 11:12 ` Markus Armbruster
2012-07-25 16:54 ` [Qemu-devel] [PATCH 07/11] qapi: qapi.py: allow the "'" character be escaped Luiz Capitulino
` (4 subsequent siblings)
10 siblings, 1 reply; 39+ messages in thread
From: Luiz Capitulino @ 2012-07-25 16:54 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, aliguori, armbru, mdroth
Actually, renames it to 'object'. This must be what the original author
meant to write, as there's no 'object' in the error's data member.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qerror.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qerror.c b/qerror.c
index 3d8ba2a..01d2493 100644
--- a/qerror.c
+++ b/qerror.c
@@ -49,7 +49,7 @@ static const QErrorStringTable qerror_table[] = {
},
{
.error_fmt = QERR_AMBIGUOUS_PATH,
- .desc = "Path '%(path)' does not uniquely identify a %(object)"
+ .desc = "Path '%(path)' does not uniquely identify a object"
},
{
.error_fmt = QERR_BAD_BUS_FOR_DEVICE,
--
1.7.11.2.249.g31c7954.dirty
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH 06/11] qerror: QERR_AMBIGUOUS_PATH: drop %(object) from human msg
2012-07-25 16:54 ` [Qemu-devel] [PATCH 06/11] qerror: QERR_AMBIGUOUS_PATH: drop %(object) from " Luiz Capitulino
@ 2012-07-26 11:12 ` Markus Armbruster
2012-07-26 14:15 ` Luiz Capitulino
0 siblings, 1 reply; 39+ messages in thread
From: Markus Armbruster @ 2012-07-26 11:12 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: pbonzini, aliguori, qemu-devel, mdroth
Luiz Capitulino <lcapitulino@redhat.com> writes:
> Actually, renames it to 'object'. This must be what the original author
> meant to write, as there's no 'object' in the error's data member.
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
> qerror.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/qerror.c b/qerror.c
> index 3d8ba2a..01d2493 100644
> --- a/qerror.c
> +++ b/qerror.c
> @@ -49,7 +49,7 @@ static const QErrorStringTable qerror_table[] = {
> },
> {
> .error_fmt = QERR_AMBIGUOUS_PATH,
> - .desc = "Path '%(path)' does not uniquely identify a %(object)"
> + .desc = "Path '%(path)' does not uniquely identify a object"
> },
> {
> .error_fmt = QERR_BAD_BUS_FOR_DEVICE,
"an object", please.
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH 06/11] qerror: QERR_AMBIGUOUS_PATH: drop %(object) from human msg
2012-07-26 11:12 ` Markus Armbruster
@ 2012-07-26 14:15 ` Luiz Capitulino
0 siblings, 0 replies; 39+ messages in thread
From: Luiz Capitulino @ 2012-07-26 14:15 UTC (permalink / raw)
To: Markus Armbruster; +Cc: pbonzini, aliguori, qemu-devel, mdroth
On Thu, 26 Jul 2012 13:12:11 +0200
Markus Armbruster <armbru@redhat.com> wrote:
> Luiz Capitulino <lcapitulino@redhat.com> writes:
>
> > Actually, renames it to 'object'. This must be what the original author
> > meant to write, as there's no 'object' in the error's data member.
> >
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> > qerror.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/qerror.c b/qerror.c
> > index 3d8ba2a..01d2493 100644
> > --- a/qerror.c
> > +++ b/qerror.c
> > @@ -49,7 +49,7 @@ static const QErrorStringTable qerror_table[] = {
> > },
> > {
> > .error_fmt = QERR_AMBIGUOUS_PATH,
> > - .desc = "Path '%(path)' does not uniquely identify a %(object)"
> > + .desc = "Path '%(path)' does not uniquely identify a object"
> > },
> > {
> > .error_fmt = QERR_BAD_BUS_FOR_DEVICE,
>
> "an object", please.
Fixed for v3, thanks.
^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] [PATCH 07/11] qapi: qapi.py: allow the "'" character be escaped
2012-07-25 16:54 [Qemu-devel] [PATCH v2 00/11]: qapi: generate qerrors from qapi-schema-errors.json Luiz Capitulino
` (5 preceding siblings ...)
2012-07-25 16:54 ` [Qemu-devel] [PATCH 06/11] qerror: QERR_AMBIGUOUS_PATH: drop %(object) from " Luiz Capitulino
@ 2012-07-25 16:54 ` Luiz Capitulino
2012-07-25 17:45 ` Peter Maydell
2012-07-25 16:54 ` [Qemu-devel] [PATCH 08/11] qapi: add qapi-schema-errors.json Luiz Capitulino
` (3 subsequent siblings)
10 siblings, 1 reply; 39+ messages in thread
From: Luiz Capitulino @ 2012-07-25 16:54 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, aliguori, armbru, mdroth
A future commit will add a new qapi script which escapes that character.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
scripts/qapi.py | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index e062336..9aa518f 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -21,7 +21,9 @@ def tokenize(data):
elif data[0] == "'":
data = data[1:]
string = ''
- while data[0] != "'":
+ while True:
+ if data[0] == "'" and string[len(string)-1] != "\\":
+ break
string += data[0]
data = data[1:]
data = data[1:]
--
1.7.11.2.249.g31c7954.dirty
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH 07/11] qapi: qapi.py: allow the "'" character be escaped
2012-07-25 16:54 ` [Qemu-devel] [PATCH 07/11] qapi: qapi.py: allow the "'" character be escaped Luiz Capitulino
@ 2012-07-25 17:45 ` Peter Maydell
2012-07-25 19:18 ` Luiz Capitulino
0 siblings, 1 reply; 39+ messages in thread
From: Peter Maydell @ 2012-07-25 17:45 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: mdroth, pbonzini, aliguori, qemu-devel, armbru
On 25 July 2012 17:54, Luiz Capitulino <lcapitulino@redhat.com> wrote:
(Subject should be "to be", not "be".)
> A future commit will add a new qapi script which escapes that character.
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
> scripts/qapi.py | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index e062336..9aa518f 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -21,7 +21,9 @@ def tokenize(data):
> elif data[0] == "'":
> data = data[1:]
> string = ''
> - while data[0] != "'":
> + while True:
> + if data[0] == "'" and string[len(string)-1] != "\\":
> + break
> string += data[0]
> data = data[1:]
> data = data[1:]
Won't this cause us to look at string[-1] if
the input data has two ' characters in a row?
(also, maybe infinite loop if the input string has an
unterminated ' ?)
-- PMM
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Qemu-devel] [PATCH 07/11] qapi: qapi.py: allow the "'" character be escaped
2012-07-25 17:45 ` Peter Maydell
@ 2012-07-25 19:18 ` Luiz Capitulino
2012-07-25 19:47 ` Peter Maydell
0 siblings, 1 reply; 39+ messages in thread
From: Luiz Capitulino @ 2012-07-25 19:18 UTC (permalink / raw)
To: Peter Maydell; +Cc: mdroth, pbonzini, aliguori, qemu-devel, armbru
On Wed, 25 Jul 2012 18:45:21 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:
> On 25 July 2012 17:54, Luiz Capitulino <lcapitulino@redhat.com> wrote:
>
> (Subject should be "to be", not "be".)
>
> > A future commit will add a new qapi script which escapes that character.
> >
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> > scripts/qapi.py | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/scripts/qapi.py b/scripts/qapi.py
> > index e062336..9aa518f 100644
> > --- a/scripts/qapi.py
> > +++ b/scripts/qapi.py
> > @@ -21,7 +21,9 @@ def tokenize(data):
> > elif data[0] == "'":
> > data = data[1:]
> > string = ''
> > - while data[0] != "'":
> > + while True:
> > + if data[0] == "'" and string[len(string)-1] != "\\":
> > + break
> > string += data[0]
> > data = data[1:]
> > data = data[1:]
>
> Won't this cause us to look at string[-1] if
> the input data has two ' characters in a row?
Non escaped? If you meant '' that's a zero length string and should work, but
if you meant 'foo '' bar' that's illegal, because ' characters should be escaped.
I don't doubt bad things will happen in that case, but in general we don't do
much error detection in the qapi scripts and improving that is beyond this
series' scope.
> (also, maybe infinite loop if the input string has an
> unterminated ' ?)
Yes, that's how I found out I needed this patch :)
PS: Peter, I get claustrophobic when reading emails from you :)
PPS: I'm joking, don't get me wrong! :)
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Qemu-devel] [PATCH 07/11] qapi: qapi.py: allow the "'" character be escaped
2012-07-25 19:18 ` Luiz Capitulino
@ 2012-07-25 19:47 ` Peter Maydell
2012-07-26 11:22 ` Markus Armbruster
0 siblings, 1 reply; 39+ messages in thread
From: Peter Maydell @ 2012-07-25 19:47 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: mdroth, pbonzini, aliguori, qemu-devel, armbru
On 25 July 2012 20:18, Luiz Capitulino <lcapitulino@redhat.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 25 July 2012 17:54, Luiz Capitulino <lcapitulino@redhat.com> wrote:
>> > --- a/scripts/qapi.py
>> > +++ b/scripts/qapi.py
>> > @@ -21,7 +21,9 @@ def tokenize(data):
>> > elif data[0] == "'":
>> > data = data[1:]
>> > string = ''
>> > - while data[0] != "'":
>> > + while True:
>> > + if data[0] == "'" and string[len(string)-1] != "\\":
>> > + break
>> > string += data[0]
>> > data = data[1:]
>> > data = data[1:]
>>
>> Won't this cause us to look at string[-1] if
>> the input data has two ' characters in a row?
>
> Non escaped? If you meant '' that's a zero length string and should work, but
> if you meant 'foo '' bar' that's illegal, because ' characters should be escaped.
I meant the zero length string case. yes. We come in with data = "''",
strip the first ' and set string to empty. Then in the first time
in the while loop data[0] is "'" but len(string) is 0 and so we'll
do string[-1] which I think will throw an exception.
...and yep, quick test of a nobbbled qapi-schema.json confirms:
$ python /home/pm215/src/qemu/qemu/scripts/qapi-types.py -h -o "." <
/home/pm215/src/qemu/qemu/qapi-schema.json
Traceback (most recent call last):
File "/home/pm215/src/qemu/qemu/scripts/qapi-types.py", line 260, in <module>
exprs = parse_schema(sys.stdin)
File "/home/pm215/src/qemu/qemu/scripts/qapi.py", line 78, in parse_schema
expr_eval = evaluate(expr)
File "/home/pm215/src/qemu/qemu/scripts/qapi.py", line 64, in evaluate
return parse(map(lambda x: x, tokenize(string)))[0]
File "/home/pm215/src/qemu/qemu/scripts/qapi.py", line 25, in tokenize
if data[0] == "'" and string[len(string)-1] != "\\":
IndexError: string index out of range
Try this (very lightly tested but seems to work):
(feel free to do something nicer than raising an exception on
the syntax error, and sorry I'm feeling too lazy to make this
an actual patch email)
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -21,10 +21,16 @@ def tokenize(data):
elif data[0] == "'":
data = data[1:]
string = ''
- while data[0] != "'":
- string += data[0]
- data = data[1:]
- data = data[1:]
+ while True:
+ pos = data.find("'")
+ if pos == -1:
+ raise Exception("Mismatched quotes")
+ string += data[0:pos]
+ data = data[pos+1:]
+ if len(string) == 0 or string[-1] != "\\":
+ # found a ' and it wasn't escaped
+ break
+ string = string[0:-1] + "'"
yield string
def parse(tokens):
(if anybody wants to be able to use '\\' to escape escapes then
this approach is a bit stuffed, of course.)
> PS: Peter, I get claustrophobic when reading emails from you :)
I can add more blank lines if that helps? :-)
-- PMM
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH 07/11] qapi: qapi.py: allow the "'" character be escaped
2012-07-25 19:47 ` Peter Maydell
@ 2012-07-26 11:22 ` Markus Armbruster
2012-07-26 13:47 ` Luiz Capitulino
0 siblings, 1 reply; 39+ messages in thread
From: Markus Armbruster @ 2012-07-26 11:22 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-devel, pbonzini, aliguori, mdroth, Luiz Capitulino
Peter Maydell <peter.maydell@linaro.org> writes:
> On 25 July 2012 20:18, Luiz Capitulino <lcapitulino@redhat.com> wrote:
>> Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On 25 July 2012 17:54, Luiz Capitulino <lcapitulino@redhat.com> wrote:
>>> > --- a/scripts/qapi.py
>>> > +++ b/scripts/qapi.py
>>> > @@ -21,7 +21,9 @@ def tokenize(data):
>>> > elif data[0] == "'":
>>> > data = data[1:]
>>> > string = ''
>>> > - while data[0] != "'":
>>> > + while True:
>>> > + if data[0] == "'" and string[len(string)-1] != "\\":
>>> > + break
>>> > string += data[0]
>>> > data = data[1:]
>>> > data = data[1:]
>>>
>>> Won't this cause us to look at string[-1] if
>>> the input data has two ' characters in a row?
>>
>> Non escaped? If you meant '' that's a zero length string and should work, but
>> if you meant 'foo '' bar' that's illegal, because ' characters
>> should be escaped.
>
> I meant the zero length string case. yes. We come in with data = "''",
> strip the first ' and set string to empty. Then in the first time
> in the while loop data[0] is "'" but len(string) is 0 and so we'll
> do string[-1] which I think will throw an exception.
>
> ...and yep, quick test of a nobbbled qapi-schema.json confirms:
> $ python /home/pm215/src/qemu/qemu/scripts/qapi-types.py -h -o "." <
> /home/pm215/src/qemu/qemu/qapi-schema.json
> Traceback (most recent call last):
> File "/home/pm215/src/qemu/qemu/scripts/qapi-types.py", line 260, in <module>
> exprs = parse_schema(sys.stdin)
> File "/home/pm215/src/qemu/qemu/scripts/qapi.py", line 78, in parse_schema
> expr_eval = evaluate(expr)
> File "/home/pm215/src/qemu/qemu/scripts/qapi.py", line 64, in evaluate
> return parse(map(lambda x: x, tokenize(string)))[0]
> File "/home/pm215/src/qemu/qemu/scripts/qapi.py", line 25, in tokenize
> if data[0] == "'" and string[len(string)-1] != "\\":
> IndexError: string index out of range
>
> Try this (very lightly tested but seems to work):
> (feel free to do something nicer than raising an exception on
> the syntax error, and sorry I'm feeling too lazy to make this
> an actual patch email)
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -21,10 +21,16 @@ def tokenize(data):
> elif data[0] == "'":
> data = data[1:]
> string = ''
> - while data[0] != "'":
> - string += data[0]
> - data = data[1:]
> - data = data[1:]
> + while True:
> + pos = data.find("'")
> + if pos == -1:
> + raise Exception("Mismatched quotes")
> + string += data[0:pos]
> + data = data[pos+1:]
> + if len(string) == 0 or string[-1] != "\\":
> + # found a ' and it wasn't escaped
> + break
> + string = string[0:-1] + "'"
> yield string
>
> def parse(tokens):
>
> (if anybody wants to be able to use '\\' to escape escapes then
> this approach is a bit stuffed, of course.)
For what it's worth, the orthodox way to lexically analyze strings is a
finite automaton. Utterly untested sketch:
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 8082af3..a745e92 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -21,8 +21,17 @@ def tokenize(data):
elif data[0] == "'":
data = data[1:]
string = ''
- while data[0] != "'":
- string += data[0]
+ esc = False
+ while True:
+ if esc:
+ string += data[0]
+ esc = False
+ elif data[0] == "\\":
+ esc = True
+ elif data[0] == "'":
+ break
+ else
+ string += data[0]
data = data[1:]
data = data[1:]
yield string
Doesn't handle missing close quote gracefully; you may want to add that.
>> PS: Peter, I get claustrophobic when reading emails from you :)
>
> I can add more blank lines if that helps? :-)
>
> -- PMM
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH 07/11] qapi: qapi.py: allow the "'" character be escaped
2012-07-26 11:22 ` Markus Armbruster
@ 2012-07-26 13:47 ` Luiz Capitulino
2012-07-26 16:11 ` Markus Armbruster
0 siblings, 1 reply; 39+ messages in thread
From: Luiz Capitulino @ 2012-07-26 13:47 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, Peter Maydell, aliguori, mdroth, pbonzini
On Thu, 26 Jul 2012 13:22:00 +0200
Markus Armbruster <armbru@redhat.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > On 25 July 2012 20:18, Luiz Capitulino <lcapitulino@redhat.com> wrote:
> >> Peter Maydell <peter.maydell@linaro.org> wrote:
> >>> On 25 July 2012 17:54, Luiz Capitulino <lcapitulino@redhat.com> wrote:
> >>> > --- a/scripts/qapi.py
> >>> > +++ b/scripts/qapi.py
> >>> > @@ -21,7 +21,9 @@ def tokenize(data):
> >>> > elif data[0] == "'":
> >>> > data = data[1:]
> >>> > string = ''
> >>> > - while data[0] != "'":
> >>> > + while True:
> >>> > + if data[0] == "'" and string[len(string)-1] != "\\":
> >>> > + break
> >>> > string += data[0]
> >>> > data = data[1:]
> >>> > data = data[1:]
> >>>
> >>> Won't this cause us to look at string[-1] if
> >>> the input data has two ' characters in a row?
> >>
> >> Non escaped? If you meant '' that's a zero length string and should work, but
> >> if you meant 'foo '' bar' that's illegal, because ' characters
> >> should be escaped.
> >
> > I meant the zero length string case. yes. We come in with data = "''",
> > strip the first ' and set string to empty. Then in the first time
> > in the while loop data[0] is "'" but len(string) is 0 and so we'll
> > do string[-1] which I think will throw an exception.
> >
> > ...and yep, quick test of a nobbbled qapi-schema.json confirms:
> > $ python /home/pm215/src/qemu/qemu/scripts/qapi-types.py -h -o "." <
> > /home/pm215/src/qemu/qemu/qapi-schema.json
> > Traceback (most recent call last):
> > File "/home/pm215/src/qemu/qemu/scripts/qapi-types.py", line 260, in <module>
> > exprs = parse_schema(sys.stdin)
> > File "/home/pm215/src/qemu/qemu/scripts/qapi.py", line 78, in parse_schema
> > expr_eval = evaluate(expr)
> > File "/home/pm215/src/qemu/qemu/scripts/qapi.py", line 64, in evaluate
> > return parse(map(lambda x: x, tokenize(string)))[0]
> > File "/home/pm215/src/qemu/qemu/scripts/qapi.py", line 25, in tokenize
> > if data[0] == "'" and string[len(string)-1] != "\\":
> > IndexError: string index out of range
> >
> > Try this (very lightly tested but seems to work):
> > (feel free to do something nicer than raising an exception on
> > the syntax error, and sorry I'm feeling too lazy to make this
> > an actual patch email)
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Peter, I've replaced my original 07/11 patch with your patch below.
> >
> > --- a/scripts/qapi.py
> > +++ b/scripts/qapi.py
> > @@ -21,10 +21,16 @@ def tokenize(data):
> > elif data[0] == "'":
> > data = data[1:]
> > string = ''
> > - while data[0] != "'":
> > - string += data[0]
> > - data = data[1:]
> > - data = data[1:]
> > + while True:
> > + pos = data.find("'")
> > + if pos == -1:
> > + raise Exception("Mismatched quotes")
> > + string += data[0:pos]
> > + data = data[pos+1:]
> > + if len(string) == 0 or string[-1] != "\\":
> > + # found a ' and it wasn't escaped
> > + break
> > + string = string[0:-1] + "'"
> > yield string
> >
> > def parse(tokens):
> >
> > (if anybody wants to be able to use '\\' to escape escapes then
> > this approach is a bit stuffed, of course.)
>
> For what it's worth, the orthodox way to lexically analyze strings is a
> finite automaton. Utterly untested sketch:
Feel free to send a patch if you're strong about this.
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 8082af3..a745e92 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -21,8 +21,17 @@ def tokenize(data):
> elif data[0] == "'":
> data = data[1:]
> string = ''
> - while data[0] != "'":
> - string += data[0]
> + esc = False
> + while True:
> + if esc:
> + string += data[0]
> + esc = False
> + elif data[0] == "\\":
> + esc = True
> + elif data[0] == "'":
> + break
> + else
> + string += data[0]
> data = data[1:]
> data = data[1:]
> yield string
>
> Doesn't handle missing close quote gracefully; you may want to add that.
>
> >> PS: Peter, I get claustrophobic when reading emails from you :)
> >
> > I can add more blank lines if that helps? :-)
> >
> > -- PMM
>
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH 07/11] qapi: qapi.py: allow the "'" character be escaped
2012-07-26 13:47 ` Luiz Capitulino
@ 2012-07-26 16:11 ` Markus Armbruster
2012-07-26 17:09 ` [Qemu-devel] [PATCH] fixup! " Markus Armbruster
0 siblings, 1 reply; 39+ messages in thread
From: Markus Armbruster @ 2012-07-26 16:11 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: Peter Maydell, aliguori, pbonzini, qemu-devel, mdroth
Luiz Capitulino <lcapitulino@redhat.com> writes:
> On Thu, 26 Jul 2012 13:22:00 +0200
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>
>> > On 25 July 2012 20:18, Luiz Capitulino <lcapitulino@redhat.com> wrote:
>> >> Peter Maydell <peter.maydell@linaro.org> wrote:
>> >>> On 25 July 2012 17:54, Luiz Capitulino <lcapitulino@redhat.com> wrote:
>> >>> > --- a/scripts/qapi.py
>> >>> > +++ b/scripts/qapi.py
>> >>> > @@ -21,7 +21,9 @@ def tokenize(data):
>> >>> > elif data[0] == "'":
>> >>> > data = data[1:]
>> >>> > string = ''
>> >>> > - while data[0] != "'":
>> >>> > + while True:
>> >>> > + if data[0] == "'" and string[len(string)-1] != "\\":
>> >>> > + break
>> >>> > string += data[0]
>> >>> > data = data[1:]
>> >>> > data = data[1:]
>> >>>
>> >>> Won't this cause us to look at string[-1] if
>> >>> the input data has two ' characters in a row?
>> >>
>> >> Non escaped? If you meant '' that's a zero length string and
>> >> should work, but
>> >> if you meant 'foo '' bar' that's illegal, because ' characters
>> >> should be escaped.
>> >
>> > I meant the zero length string case. yes. We come in with data = "''",
>> > strip the first ' and set string to empty. Then in the first time
>> > in the while loop data[0] is "'" but len(string) is 0 and so we'll
>> > do string[-1] which I think will throw an exception.
>> >
>> > ...and yep, quick test of a nobbbled qapi-schema.json confirms:
>> > $ python /home/pm215/src/qemu/qemu/scripts/qapi-types.py -h -o "." <
>> > /home/pm215/src/qemu/qemu/qapi-schema.json
>> > Traceback (most recent call last):
>> > File "/home/pm215/src/qemu/qemu/scripts/qapi-types.py", line 260, in <module>
>> > exprs = parse_schema(sys.stdin)
>> > File "/home/pm215/src/qemu/qemu/scripts/qapi.py", line 78, in parse_schema
>> > expr_eval = evaluate(expr)
>> > File "/home/pm215/src/qemu/qemu/scripts/qapi.py", line 64, in evaluate
>> > return parse(map(lambda x: x, tokenize(string)))[0]
>> > File "/home/pm215/src/qemu/qemu/scripts/qapi.py", line 25, in tokenize
>> > if data[0] == "'" and string[len(string)-1] != "\\":
>> > IndexError: string index out of range
>> >
>> > Try this (very lightly tested but seems to work):
>> > (feel free to do something nicer than raising an exception on
>> > the syntax error, and sorry I'm feeling too lazy to make this
>> > an actual patch email)
>> >
>> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>
> Peter, I've replaced my original 07/11 patch with your patch below.
>
>> >
>> > --- a/scripts/qapi.py
>> > +++ b/scripts/qapi.py
>> > @@ -21,10 +21,16 @@ def tokenize(data):
>> > elif data[0] == "'":
>> > data = data[1:]
>> > string = ''
>> > - while data[0] != "'":
>> > - string += data[0]
>> > - data = data[1:]
>> > - data = data[1:]
>> > + while True:
>> > + pos = data.find("'")
>> > + if pos == -1:
>> > + raise Exception("Mismatched quotes")
>> > + string += data[0:pos]
>> > + data = data[pos+1:]
>> > + if len(string) == 0 or string[-1] != "\\":
>> > + # found a ' and it wasn't escaped
>> > + break
>> > + string = string[0:-1] + "'"
>> > yield string
>> >
>> > def parse(tokens):
>> >
>> > (if anybody wants to be able to use '\\' to escape escapes then
>> > this approach is a bit stuffed, of course.)
An escape mechanism that can't be escaped sucks :)
>> For what it's worth, the orthodox way to lexically analyze strings is a
>> finite automaton. Utterly untested sketch:
>
> Feel free to send a patch if you're strong about this.
I'll leave that to the poor guy who first needs to escape escapes.
[...]
^ permalink raw reply [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH] fixup! qapi: qapi.py: allow the "'" character be escaped
2012-07-26 16:11 ` Markus Armbruster
@ 2012-07-26 17:09 ` Markus Armbruster
2012-07-26 18:21 ` Peter Maydell
2012-07-27 14:29 ` Luiz Capitulino
0 siblings, 2 replies; 39+ messages in thread
From: Markus Armbruster @ 2012-07-26 17:09 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: Peter Maydell, aliguori, pbonzini, qemu-devel, mdroth
Support escaping the escape character, and make more robust (don't die
for '', handle ' without matching '.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
Luiz made me do this. Fixes up his 07/11.
Generates identical qmp-commands.h qapi-types.h qapi-visit.h
qapi-errors.h tests/test-qapi-types.h tests/test-qapi-visit.h
tests/test-qmp-commands.h qapi-generated/qga-qapi-types.h
qapi-generated/qga-qapi-visit.h qapi-generated/qga-qmp-commands.h
scripts/qapi.py | 31 ++++++++++++++++++++-----------
1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 9aa518f..169ea78 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -13,20 +13,29 @@ from ordereddict import OrderedDict
def tokenize(data):
while len(data):
- if data[0] in ['{', '}', ':', ',', '[', ']']:
- yield data[0]
- data = data[1:]
- elif data[0] in ' \n':
- data = data[1:]
- elif data[0] == "'":
- data = data[1:]
+ ch = data[0]
+ data = data[1:]
+ if ch in ['{', '}', ':', ',', '[', ']']:
+ yield ch
+ elif ch in ' \n':
+ None
+ elif ch == "'":
string = ''
+ esc = False
while True:
- if data[0] == "'" and string[len(string)-1] != "\\":
- break
- string += data[0]
+ if (data == ''):
+ raise Exception("Mismatched quotes")
+ ch = data[0]
data = data[1:]
- data = data[1:]
+ if esc:
+ string += ch
+ esc = False
+ elif ch == "\\":
+ esc = True
+ elif ch == "'":
+ break
+ else:
+ string += ch
yield string
def parse(tokens):
--
1.7.10.4
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH] fixup! qapi: qapi.py: allow the "'" character be escaped
2012-07-26 17:09 ` [Qemu-devel] [PATCH] fixup! " Markus Armbruster
@ 2012-07-26 18:21 ` Peter Maydell
2012-07-27 14:29 ` Luiz Capitulino
1 sibling, 0 replies; 39+ messages in thread
From: Peter Maydell @ 2012-07-26 18:21 UTC (permalink / raw)
To: Markus Armbruster; +Cc: mdroth, pbonzini, aliguori, qemu-devel, Luiz Capitulino
On 26 July 2012 18:09, Markus Armbruster <armbru@redhat.com> wrote:
> Support escaping the escape character, and make more robust (don't die
> for '', handle ' without matching '.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Yeah, let's do this the right way.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
-- PMM
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Qemu-devel] [PATCH] fixup! qapi: qapi.py: allow the "'" character be escaped
2012-07-26 17:09 ` [Qemu-devel] [PATCH] fixup! " Markus Armbruster
2012-07-26 18:21 ` Peter Maydell
@ 2012-07-27 14:29 ` Luiz Capitulino
2012-07-28 6:37 ` Markus Armbruster
1 sibling, 1 reply; 39+ messages in thread
From: Luiz Capitulino @ 2012-07-27 14:29 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Peter Maydell, aliguori, pbonzini, qemu-devel, mdroth
On Thu, 26 Jul 2012 19:09:13 +0200
Markus Armbruster <armbru@redhat.com> wrote:
> Support escaping the escape character, and make more robust (don't die
> for '', handle ' without matching '.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
Can you please do you it on top of master so that I can apply it on the qmp
branch?
> ---
> Luiz made me do this. Fixes up his 07/11.
>
> Generates identical qmp-commands.h qapi-types.h qapi-visit.h
> qapi-errors.h tests/test-qapi-types.h tests/test-qapi-visit.h
> tests/test-qmp-commands.h qapi-generated/qga-qapi-types.h
> qapi-generated/qga-qapi-visit.h qapi-generated/qga-qmp-commands.h
>
> scripts/qapi.py | 31 ++++++++++++++++++++-----------
> 1 file changed, 20 insertions(+), 11 deletions(-)
>
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 9aa518f..169ea78 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -13,20 +13,29 @@ from ordereddict import OrderedDict
>
> def tokenize(data):
> while len(data):
> - if data[0] in ['{', '}', ':', ',', '[', ']']:
> - yield data[0]
> - data = data[1:]
> - elif data[0] in ' \n':
> - data = data[1:]
> - elif data[0] == "'":
> - data = data[1:]
> + ch = data[0]
> + data = data[1:]
> + if ch in ['{', '}', ':', ',', '[', ']']:
> + yield ch
> + elif ch in ' \n':
> + None
> + elif ch == "'":
> string = ''
> + esc = False
> while True:
> - if data[0] == "'" and string[len(string)-1] != "\\":
> - break
> - string += data[0]
> + if (data == ''):
> + raise Exception("Mismatched quotes")
> + ch = data[0]
> data = data[1:]
> - data = data[1:]
> + if esc:
> + string += ch
> + esc = False
> + elif ch == "\\":
> + esc = True
> + elif ch == "'":
> + break
> + else:
> + string += ch
> yield string
>
> def parse(tokens):
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH] fixup! qapi: qapi.py: allow the "'" character be escaped
2012-07-27 14:29 ` Luiz Capitulino
@ 2012-07-28 6:37 ` Markus Armbruster
2012-07-30 13:09 ` Luiz Capitulino
0 siblings, 1 reply; 39+ messages in thread
From: Markus Armbruster @ 2012-07-28 6:37 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: mdroth, Peter Maydell, aliguori, qemu-devel, pbonzini
Luiz Capitulino <lcapitulino@redhat.com> writes:
> On Thu, 26 Jul 2012 19:09:13 +0200
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Support escaping the escape character, and make more robust (don't die
>> for '', handle ' without matching '.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
> Can you please do you it on top of master so that I can apply it on the qmp
> branch?
Have you tried squashing it into your commit? Apply on top of your
series, then git-rebase -i --autosquash master.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Qemu-devel] [PATCH] fixup! qapi: qapi.py: allow the "'" character be escaped
2012-07-28 6:37 ` Markus Armbruster
@ 2012-07-30 13:09 ` Luiz Capitulino
0 siblings, 0 replies; 39+ messages in thread
From: Luiz Capitulino @ 2012-07-30 13:09 UTC (permalink / raw)
To: Markus Armbruster; +Cc: mdroth, Peter Maydell, aliguori, qemu-devel, pbonzini
On Sat, 28 Jul 2012 08:37:03 +0200
Markus Armbruster <armbru@redhat.com> wrote:
> Luiz Capitulino <lcapitulino@redhat.com> writes:
>
> > On Thu, 26 Jul 2012 19:09:13 +0200
> > Markus Armbruster <armbru@redhat.com> wrote:
> >
> >> Support escaping the escape character, and make more robust (don't die
> >> for '', handle ' without matching '.
> >>
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >
> > Can you please do you it on top of master so that I can apply it on the qmp
> > branch?
>
> Have you tried squashing it into your commit?
Oh, I didn't think about doing that. It works, now. Thanks.
^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] [PATCH 08/11] qapi: add qapi-schema-errors.json
2012-07-25 16:54 [Qemu-devel] [PATCH v2 00/11]: qapi: generate qerrors from qapi-schema-errors.json Luiz Capitulino
` (6 preceding siblings ...)
2012-07-25 16:54 ` [Qemu-devel] [PATCH 07/11] qapi: qapi.py: allow the "'" character be escaped Luiz Capitulino
@ 2012-07-25 16:54 ` Luiz Capitulino
2012-07-25 16:54 ` [Qemu-devel] [PATCH 09/11] qapi: add qapi-errors.py Luiz Capitulino
` (2 subsequent siblings)
10 siblings, 0 replies; 39+ messages in thread
From: Luiz Capitulino @ 2012-07-25 16:54 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, aliguori, armbru, mdroth
This is the main error file, where all errors are defined and from
where error macros and whatnot will be automatically generated.
It contains all errors classes currently defined in qerror.[ch].
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qapi-schema-errors.json | 616 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 616 insertions(+)
create mode 100644 qapi-schema-errors.json
diff --git a/qapi-schema-errors.json b/qapi-schema-errors.json
new file mode 100644
index 0000000..bedbb2c
--- /dev/null
+++ b/qapi-schema-errors.json
@@ -0,0 +1,616 @@
+##
+# @AddClientFailed
+#
+# Since: 0.15
+##
+{ 'error': 'AddClientFailed',
+ 'description': 'Could not add client' }
+
+##
+# @AmbiguousPath
+#
+# Since: 1.1.0
+##
+{ 'error': 'AmbiguousPath',
+ 'description': 'Path \'%(path)\' does not uniquely identify an object',
+ 'data': {'path': 'str'} }
+
+##
+# @BadBusForDevice
+#
+# Since: 0.14
+##
+{ 'error': 'BadBusForDevice',
+ 'description': 'Device \'%(device)\' can\'t go on a %(bad_bus_type) bus',
+ 'data': {'device': 'str', 'bad_bus_type': 'str'} }
+
+##
+# @BaseNotFound
+#
+# Since: 1.1.0
+##
+{ 'error': 'BaseNotFound',
+ 'description': 'Base \'%(base)\' not found',
+ 'data': {'base': 'str'} }
+
+##
+# @BlockFormatFeatureNotSupported
+#
+# Since: 1.0
+##
+{ 'error': 'BlockFormatFeatureNotSupported',
+ 'description': 'Block format \'%(format)\' used by device \'%(name)\' does not support feature \'%(feature)\'',
+ 'data': {'format': 'str', 'name': 'str', 'feature': 'str'} }
+
+##
+# @BufferOverrun
+#
+# Since: 0.15
+##
+{ 'error': 'BufferOverrun',
+ 'description': 'An internal buffer overran' }
+
+##
+# @BusNoHotplug
+#
+# Since: 0.14
+##
+{ 'error': 'BusNoHotplug',
+ 'description': 'Bus \'%(bus)\' does not support hotplugging',
+ 'data': {'bus': 'str'} }
+
+##
+# @BusNotFound
+#
+# Since: 0.14
+##
+{ 'error': 'BusNotFound',
+ 'description': 'Bus \'%(bus)\' not found',
+ 'data': {'bus': 'str'} }
+
+##
+# @CommandDisabled
+#
+# Since: 1.1.0
+##
+{ 'error': 'CommandDisabled',
+ 'description': 'The command %(name) has been disabled for this instance',
+ 'data': {'name': 'str'} }
+
+##
+# @CommandNotFound
+#
+# Since: 0.14
+##
+{ 'error': 'CommandNotFound',
+ 'description': 'The command %(name) has not been found',
+ 'data': {'name': 'str'} }
+
+##
+# @DeviceEncrypted
+#
+# Since: 0.14
+##
+{ 'error': 'DeviceEncrypted',
+ 'description': 'Device \'%(device)\' is encrypted (filename=%(filename))',
+ 'data': {'device': 'str', 'filename': 'str'} }
+
+##
+# @DeviceFeatureBlocksMigration
+#
+# Since: 1.0
+##
+{ 'error': 'DeviceFeatureBlocksMigration',
+ 'description': 'Migration is disabled when using feature \'%(feature)\' in device \'%(device)\'',
+ 'data': {'device': 'str', 'feature': 'str'} }
+
+##
+# @DeviceHasNoMedium
+#
+# Since: 1.1.0
+##
+{ 'error': 'DeviceHasNoMedium',
+ 'description': 'Device \'%(device)\' has no medium',
+ 'data': {'device': 'str'} }
+
+##
+# @DeviceInitFailed
+#
+# Since: 0.14
+##
+{ 'error': 'DeviceInitFailed',
+ 'description': 'Device \'%(device)\' could not be initialized',
+ 'data': {'device': 'str'} }
+
+##
+# @DeviceInUse
+#
+# Since: 0.14
+##
+{ 'error': 'DeviceInUse',
+ 'description': 'Device \'%(device)\' is in use',
+ 'data': {'device': 'str'} }
+
+##
+# @DeviceIsReadOnly
+#
+# Since: 1.1.0
+##
+{ 'error': 'DeviceIsReadOnly',
+ 'description': 'Device \'%(device)\' is read only',
+ 'data': {'device': 'str'} }
+
+##
+# @DeviceLocked
+#
+# Since: 0.14
+##
+{ 'error': 'DeviceLocked',
+ 'description': 'Device \'%(device)\' is locked',
+ 'data': {'device': 'str'} }
+
+##
+# @DeviceMultipleBusses
+#
+# Since: 0.14
+##
+{ 'error': 'DeviceMultipleBusses',
+ 'description': 'Device \'%(device)\' has multiple child busses',
+ 'data': {'device': 'str'} }
+
+##
+# @DeviceNoBus
+#
+# Since: 0.14
+##
+{ 'error': 'DeviceNoBus',
+ 'description': 'Device \'%(device)\' has no child bus',
+ 'data': {'device': 'str'} }
+
+##
+# @DeviceNoHotplug
+#
+# Since: 0.14
+##
+{ 'error': 'DeviceNoHotplug',
+ 'description': 'Device \'%(device)\' does not support hotplugging',
+ 'data': {'device': 'str'} }
+
+##
+# @DeviceNotActive
+#
+# Since: 0.14
+##
+{ 'error': 'DeviceNotActive',
+ 'description': 'Device \'%(device)\' has not been activated',
+ 'data': {'device': 'str'} }
+
+##
+# @DeviceNotEncrypted
+#
+# Since: 0.14
+##
+{ 'error': 'DeviceNotEncrypted',
+ 'description': 'Device \'%(device)\' is not encrypted',
+ 'data': {'device': 'str'} }
+
+##
+# @DeviceNotFound
+#
+# Since: 0.14
+##
+{ 'error': 'DeviceNotFound',
+ 'description': 'Device \'%(device)\' not found',
+ 'data': {'device': 'str'} }
+
+##
+# @DeviceNotRemovable
+#
+# Since: 0.14
+##
+{ 'error': 'DeviceNotRemovable',
+ 'description': 'Device \'%(device)\' is not removable',
+ 'data': {'device': 'str'} }
+
+##
+# @DuplicateId
+#
+# Since: 0.14
+##
+{ 'error': 'DuplicateId',
+ 'description': 'Duplicate ID \'%(id)\' for %(object)',
+ 'data': {'id': 'str', 'object': 'str'} }
+
+##
+# @FdNotFound
+#
+# Since: 0.14
+##
+{ 'error': 'FdNotFound',
+ 'description': 'File descriptor named \'%(name)\' not found',
+ 'data': {'name': 'str'} }
+
+##
+# @FdNotSupplied
+#
+# Since: 0.14
+##
+{ 'error': 'FdNotSupplied',
+ 'description': 'No file descriptor supplied via SCM_RIGHTS' }
+
+##
+# @FeatureDisabled
+#
+# Since: 0.15
+##
+{ 'error': 'FeatureDisabled',
+ 'description': 'The feature \'%(name)\' is not enabled',
+ 'data': {'name': 'str'} }
+
+##
+# @InvalidBlockFormat
+#
+# Since: 0.14
+##
+{ 'error': 'InvalidBlockFormat',
+ 'description': 'Invalid block format \'%(name)\'',
+ 'data': {'name': 'str'} }
+
+##
+# @InvalidOptionGroup
+#
+# Since: 1.2.0
+##
+{ 'error': 'InvalidOptionGroup',
+ 'description': 'There is no option group \'%(group)\'',
+ 'data': {'group': 'str'} }
+
+##
+# @InvalidParameter
+#
+# Since: 0.14
+##
+{ 'error': 'InvalidParameter',
+ 'description': 'Invalid parameter \'%(name)\'',
+ 'data': {'name': 'str'} }
+
+##
+# @InvalidParameterCombination
+#
+# Since: 1.1.0
+##
+{ 'error': 'InvalidParameterCombination',
+ 'description': 'Invalid parameter combination' }
+
+##
+# @InvalidParameterType
+#
+# Since: 0.14
+##
+{ 'error': 'InvalidParameterType',
+ 'description': 'Invalid parameter type for \'%(name)\', expected: %(expected)',
+ 'data': {'name': 'str', 'expected': 'str'} }
+
+##
+# @InvalidParameterValue
+#
+# Since: 0.14
+##
+{ 'error': 'InvalidParameterValue',
+ 'description': 'Parameter \'%(name)\' expects %(expected)',
+ 'data': {'name': 'str', 'expected': 'str'} }
+
+##
+# @InvalidPassword
+#
+# Since: 0.14
+##
+{ 'error': 'InvalidPassword',
+ 'description': 'Password incorrect' }
+
+##
+# @IOError
+#
+# Since: 1.1.0
+##
+{ 'error': 'IOError',
+ 'description': 'An IO error has occurred' }
+
+##
+# @JSONParseError
+#
+# Since: 0.15
+##
+{ 'error': 'JSONParseError',
+ 'description': 'JSON parse error, %(message)',
+ 'data': {'message': 'str'} }
+
+##
+# @JSONParsing
+#
+# Since: 0.14
+##
+{ 'error': 'JSONParsing',
+ 'description': 'Invalid JSON syntax' }
+
+##
+# @KVMMissingCap
+#
+# Since: 0.14
+##
+{ 'error': 'KVMMissingCap',
+ 'description': 'Using KVM without %(capability), %(feature) unavailable',
+ 'data': {'capability': 'str', 'feature': 'str'} }
+
+##
+# @MigrationActive
+#
+# Since: 1.1.0
+##
+{ 'error': 'MigrationActive',
+ 'description': 'There\'s a migration process in progress' }
+
+##
+# @MigrationExpected
+#
+# Since: 0.14
+##
+{ 'error': 'MigrationExpected',
+ 'description': 'An incoming migration is expected before this command can be executed' }
+
+##
+# @MigrationNotSupported
+#
+# Since: 1.1.0
+##
+{ 'error': 'MigrationNotSupported',
+ 'description': 'State blocked by non-migratable device \'%(device)\'',
+ 'data': {'device': 'str'} }
+
+##
+# @MissingParameter
+#
+# Since: 0.14
+##
+{ 'error': 'MissingParameter',
+ 'description': 'Parameter \'%(name)\' is missing',
+ 'data': {'name': 'str'} }
+
+##
+# @NoBusForDevice
+#
+# Since: 0.14
+##
+{ 'error': 'NoBusForDevice',
+ 'description': 'No \'%(bus)\' bus found for device \'%(device)\'',
+ 'data': {'device': 'str', 'bus': 'str'} }
+
+##
+# @NotSupported
+#
+# Since: 1.0
+##
+{ 'error': 'NotSupported',
+ 'description': 'Not supported' }
+
+##
+# @OpenFileFailed
+#
+# Since: 0.14
+##
+{ 'error': 'OpenFileFailed',
+ 'description': 'Could not open \'%(filename)\'',
+ 'data': {'filename': 'str'} }
+
+##
+# @PermissionDenied
+#
+# Since: 1.1.0
+##
+{ 'error': 'PermissionDenied',
+ 'description': 'Insufficient permission to perform this operation' }
+
+##
+# @PropertyNotFound
+#
+# Since: 0.14
+##
+{ 'error': 'PropertyNotFound',
+ 'description': 'Property \'%(device).%(property)\' not found',
+ 'data': {'device': 'str', 'property': 'str'} }
+
+##
+# @PropertyValueBad
+#
+# Since: 0.14
+##
+{ 'error': 'PropertyValueBad',
+ 'description': 'Property \'%(device).%(property)\' doesn\'t take value \'%(value)\'',
+ 'data': {'device': 'str', 'property': 'str', 'value': 'str'} }
+
+##
+# @PropertyValueInUse
+#
+# Since: 0.14
+##
+{ 'error': 'PropertyValueInUse',
+ 'description': 'Property \'%(device).%(property)\' can\'t take value \'%(value)\', it\'s in use',
+ 'data': {'device': 'str', 'property': 'str', 'value': 'str'} }
+
+##
+# @PropertyValueNotFound
+#
+# Since: 0.14
+##
+{ 'error': 'PropertyValueNotFound',
+ 'description': 'Property \'%(device).%(property)\' can\'t find value \'%(value)\'',
+ 'data': {'device': 'str', 'property': 'str', 'value': 'str'} }
+
+##
+# @PropertyValueNotPowerOf2
+#
+# Since: 1.1.0
+##
+{ 'error': 'PropertyValueNotPowerOf2',
+ 'description': 'Property \'%(device).%(property)\' doesn\'t take value \'%(value)\', it\'s not a power of 2',
+ 'data': {'device': 'str', 'property': 'str', 'value': 'int'} }
+
+##
+# @PropertyValueOutOfRange
+#
+# Since: 1.1.0
+##
+{ 'error': 'PropertyValueOutOfRange',
+ 'description': 'Property \'%(device).%(property)\' doesn\'t take value %(value) (minimum: %(min), maximum: %(max))',
+ 'data': {'device': 'str', 'property': 'str', 'value': 'int', 'min': 'int', 'max': 'int'} }
+
+##
+# @QgaCommandFailed
+#
+# Since: 0.15
+##
+{ 'error': 'QgaCommandFailed',
+ 'description': 'Guest agent command failed, error was \'%(message)\'',
+ 'data': {'message': 'str'} }
+
+##
+# @QgaLoggingFailed
+#
+# Since: 0.15
+##
+{ 'error': 'QgaLoggingFailed',
+ 'description': 'Guest agent failed to log non-optional log statement' }
+
+##
+# @QMPBadInputObject
+#
+# Since: 0.14
+##
+{ 'error': 'QMPBadInputObject',
+ 'description': 'Expected \'%(expected)\' in QMP input',
+ 'data': {'expected': 'str'} }
+
+##
+# @QMPBadInputObjectMember
+#
+# Since: 0.14
+##
+{ 'error': 'QMPBadInputObjectMember',
+ 'description': 'QMP input object member \'%(member)\' expects \'%(expected)\'',
+ 'data': {'member': 'str', 'expected': 'str'} }
+
+##
+# @QMPExtraInputObjectMember
+#
+# Since: 0.14
+##
+{ 'error': 'QMPExtraInputObjectMember',
+ 'description': 'QMP input object member \'%(member)\' is unexpected',
+ 'data': {'member': 'str'} }
+
+##
+# @ResetRequired
+#
+# Since: 1.0
+##
+{ 'error': 'ResetRequired',
+ 'description': 'Resetting the Virtual Machine is required' }
+
+##
+# @SetPasswdFailed
+#
+# Since: 0.14
+##
+{ 'error': 'SetPasswdFailed',
+ 'description': 'Could not set password' }
+
+##
+# @SockBindFailed
+#
+# Since: 1.1.0
+##
+{ 'error': 'SockBindFailed',
+ 'description': 'Failed to bind socket' }
+
+##
+# @SockConnectFailed
+#
+# Since: 1.1.0
+##
+{ 'error': 'SockConnectFailed',
+ 'description': 'Failed to connect to socket' }
+
+##
+# @SockConnectInprogress
+#
+# Since: 1.1.0
+##
+{ 'error': 'SockConnectInprogress',
+ 'description': 'Connection can not be completed immediately' }
+
+##
+# @SockCreateFailed
+#
+# Since: 1.1.0
+##
+{ 'error': 'SockCreateFailed',
+ 'description': 'Failed to create socket' }
+
+##
+# @SockListenFailed
+#
+# Since: 1.1.0
+##
+{ 'error': 'SockListenFailed',
+ 'description': 'Failed to set socket to listening mode' }
+
+##
+# @TooManyFiles
+#
+# Since: 0.14
+##
+{ 'error': 'TooManyFiles',
+ 'description': 'Too many open files' }
+
+##
+# @UndefinedError
+#
+# Since: 0.14
+##
+{ 'error': 'UndefinedError',
+ 'description': 'An undefined error has occurred' }
+
+##
+# @UnknownBlockFormatFeature
+#
+# Since: 0.14
+##
+{ 'error': 'UnknownBlockFormatFeature',
+ 'description': '\'%(device)\' uses a %(format) feature which is not supported by this qemu version: %(feature)',
+ 'data': {'device': 'str', 'format': 'str', 'feature': 'str'} }
+
+##
+# @Unsupported
+#
+# Since: 0.15
+##
+{ 'error': 'Unsupported',
+ 'description': 'this feature or command is not currently supported' }
+
+##
+# @VirtFSFeatureBlocksMigration
+#
+# Since: 1.0.1
+##
+{ 'error': 'VirtFSFeatureBlocksMigration',
+ 'description': 'Migration is disabled when VirtFS export path \'%(path)\' is mounted in the guest using mount_tag \'%(tag)\'',
+ 'data': {'path': 'str', 'tag': 'str'} }
+
+##
+# @VNCServerFailed
+#
+# Since: 0.14
+##
+{ 'error': 'VNCServerFailed',
+ 'description': 'Could not start VNC server on %(target)',
+ 'data': {'target': 'str'} }
--
1.7.11.2.249.g31c7954.dirty
^ permalink raw reply related [flat|nested] 39+ messages in thread* [Qemu-devel] [PATCH 09/11] qapi: add qapi-errors.py
2012-07-25 16:54 [Qemu-devel] [PATCH v2 00/11]: qapi: generate qerrors from qapi-schema-errors.json Luiz Capitulino
` (7 preceding siblings ...)
2012-07-25 16:54 ` [Qemu-devel] [PATCH 08/11] qapi: add qapi-schema-errors.json Luiz Capitulino
@ 2012-07-25 16:54 ` Luiz Capitulino
2012-07-26 11:50 ` Markus Armbruster
2012-07-26 12:12 ` Markus Armbruster
2012-07-25 16:54 ` [Qemu-devel] [PATCH 10/11] qerror: switch to qapi generated error macros and table Luiz Capitulino
2012-07-25 16:54 ` [Qemu-devel] [PATCH 11/11] scripts: update check-qerror.sh Luiz Capitulino
10 siblings, 2 replies; 39+ messages in thread
From: Luiz Capitulino @ 2012-07-25 16:54 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, aliguori, armbru, mdroth
This script generates two files from qapi-schema-errors.json:
o qapi-errors.h: contains error macro definitions, eg. QERR_BASE_NOT_FOUND,
corresponds to most of today's qerror.h
o qapi-errors.c: contains the error table that currently exists in qerror.c
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
Makefile | 8 ++-
scripts/qapi-errors.py | 177 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 183 insertions(+), 2 deletions(-)
create mode 100644 scripts/qapi-errors.py
diff --git a/Makefile b/Makefile
index ab82ef3..2cdc732 100644
--- a/Makefile
+++ b/Makefile
@@ -22,8 +22,9 @@ GENERATED_HEADERS = config-host.h trace.h qemu-options.def
ifeq ($(TRACE_BACKEND),dtrace)
GENERATED_HEADERS += trace-dtrace.h
endif
-GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h
-GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c trace.c
+GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h qapi-errors.h
+GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-errors.c \
+ trace.c
# Don't try to regenerate Makefile or configure
# We don't generate any of them
@@ -200,6 +201,9 @@ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py
qmp-commands.h qmp-marshal.c :\
$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -m -o "." < $<, " GEN $@")
+qapi-errors.h qapi-errors.c :\
+$(SRC_PATH)/qapi-schema-errors.json $(SRC_PATH)/scripts/qapi-errors.py
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-errors.py -o "." < $<, " GEN $@")
QGALIB_OBJ=$(addprefix qapi-generated/, qga-qapi-types.o qga-qapi-visit.o qga-qmp-marshal.o)
QGALIB_GEN=$(addprefix qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-commands.h)
diff --git a/scripts/qapi-errors.py b/scripts/qapi-errors.py
new file mode 100644
index 0000000..59cf426
--- /dev/null
+++ b/scripts/qapi-errors.py
@@ -0,0 +1,177 @@
+#
+# QAPI errors generator
+#
+# Copyright (C) 2012 Red Hat, Inc.
+#
+# This work is licensed under the terms of the GNU GPLv2.
+# See the COPYING.LIB file in the top-level directory.
+
+from ordereddict import OrderedDict
+import getopt, sys, os, errno
+from qapi import *
+
+def gen_error_decl_prologue(header, guard, prefix=""):
+ ret = mcgen('''
+/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
+
+/*
+ * schema-defined QAPI Errors
+ *
+ * Copyright (C) 2012 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef %(guard)s
+#define %(guard)s
+
+''',
+ header=basename(header), guard=guardname(header), prefix=prefix)
+ return ret
+
+def gen_error_def_prologue(error_header, prefix=""):
+ ret = mcgen('''
+/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
+
+/*
+ * schema-defined QMP Error table
+ *
+ * Copyright (C) 2012 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "%(prefix)s%(error_header)s"
+
+''',
+ prefix=prefix, error_header=error_header)
+ return ret
+
+def gen_error_macro(err_domain):
+ string = ''
+ cur = err_domain[0]
+ for nxt in err_domain[1:]:
+ if string and cur.isupper() and nxt.islower():
+ string += '_'
+ string += cur
+ cur = nxt
+ string += cur
+ return 'QERR_' + string.upper()
+
+def gen_error_def_table(exprs):
+ ret = mcgen('''
+static const QErrorStringTable qerror_table[] = {
+''')
+
+ for err in exprs:
+ macro = gen_error_macro(err['error'])
+ desc = err['description']
+ ret += mcgen('''
+ {
+ .error_fmt = %(error_macro)s,
+ .desc = "%(error_desc)s",
+ },
+''',
+ error_macro=macro, error_desc=desc)
+
+ ret += mcgen('''
+ {}
+};
+''')
+
+ return ret
+
+def gen_error_macro_data_str(data):
+ colon = ''
+ data_str = ''
+ for k, v in data.items():
+ data_str += colon + "'%s': " % k
+ if v == 'str':
+ data_str += "%s"
+ elif v == 'int':
+ data_str += '%"PRId64"'
+ else:
+ sys.exit("unknown data type '%s' for error '%s'" % (v, name))
+ colon = ', '
+ return data_str
+
+def gen_error_decl_macros(exprs):
+ ret = ''
+ for err in exprs:
+ data = gen_error_macro_data_str({})
+ if err.has_key('data'):
+ data = gen_error_macro_data_str(err['data'])
+ macro = gen_error_macro(err['error'])
+ name = err['error']
+
+ ret += mcgen('''
+#define %(error_macro)s \\
+ "{ 'class': '%(error_class)s', 'data': { %(error_data)s } }"
+
+''',
+ error_macro=macro, error_class=name, error_data=data)
+ return ret
+
+def maybe_open(really, name, opt):
+ if really:
+ return open(name, opt)
+ else:
+ import StringIO
+ return StringIO.StringIO()
+
+if __name__ == '__main__':
+ try:
+ opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:o:",
+ ["prefix=", "output-dir="])
+ except getopt.GetoptError, err:
+ print str(err)
+ sys.exit(1)
+
+ prefix = ""
+ output_dir = ""
+ do_c = True
+ do_h = True
+ c_file = 'qapi-errors.c'
+ h_file = 'qapi-errors.h'
+
+ for o, a in opts:
+ if o in ("-p", "--prefix"):
+ prefix = a
+ elif o in ("-o", "--output-dir"):
+ output_dir = a + "/"
+
+ c_file = output_dir + prefix + c_file
+ h_file = output_dir + prefix + h_file
+
+ try:
+ os.makedirs(output_dir)
+ except os.error, e:
+ if e.errno != errno.EEXIST:
+ raise
+
+ exprs = parse_schema(sys.stdin)
+
+ fdecl = maybe_open(do_h, h_file, 'w')
+ ret = gen_error_decl_prologue(header=basename(h_file), guard=guardname(h_file), prefix=prefix)
+ fdecl.write(ret)
+
+ ret = gen_error_decl_macros(exprs)
+ fdecl.write(ret)
+
+ fdecl.write("#endif\n")
+ fdecl.flush()
+ fdecl.close()
+
+ fdef = maybe_open(do_c, c_file, 'w')
+ ret = gen_error_def_prologue(error_header=h_file, prefix=prefix)
+ fdef.write(ret)
+
+ ret = gen_error_def_table(exprs)
+ fdef.write(ret)
+
+ fdef.flush()
+ fdef.close()
--
1.7.11.2.249.g31c7954.dirty
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH 09/11] qapi: add qapi-errors.py
2012-07-25 16:54 ` [Qemu-devel] [PATCH 09/11] qapi: add qapi-errors.py Luiz Capitulino
@ 2012-07-26 11:50 ` Markus Armbruster
2012-07-26 11:55 ` Paolo Bonzini
2012-07-26 14:34 ` Luiz Capitulino
2012-07-26 12:12 ` Markus Armbruster
1 sibling, 2 replies; 39+ messages in thread
From: Markus Armbruster @ 2012-07-26 11:50 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: Paolo Bonzini, aliguori, Eric Blake, qemu-devel, mdroth
Paolo, Eric, I got a make puzzle for you below.
Luiz Capitulino <lcapitulino@redhat.com> writes:
> This script generates two files from qapi-schema-errors.json:
>
> o qapi-errors.h: contains error macro definitions, eg. QERR_BASE_NOT_FOUND,
> corresponds to most of today's qerror.h
>
> o qapi-errors.c: contains the error table that currently exists in qerror.c
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
> Makefile | 8 ++-
> scripts/qapi-errors.py | 177 +++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 183 insertions(+), 2 deletions(-)
> create mode 100644 scripts/qapi-errors.py
>
> diff --git a/Makefile b/Makefile
> index ab82ef3..2cdc732 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -22,8 +22,9 @@ GENERATED_HEADERS = config-host.h trace.h qemu-options.def
> ifeq ($(TRACE_BACKEND),dtrace)
> GENERATED_HEADERS += trace-dtrace.h
> endif
> -GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h
> -GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c trace.c
> +GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h qapi-errors.h
> +GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-errors.c \
> + trace.c
>
> # Don't try to regenerate Makefile or configure
> # We don't generate any of them
> @@ -200,6 +201,9 @@ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py
> qmp-commands.h qmp-marshal.c :\
> $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py
> $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -m -o "." < $<, " GEN $@")
> +qapi-errors.h qapi-errors.c :\
> +$(SRC_PATH)/qapi-schema-errors.json $(SRC_PATH)/scripts/qapi-errors.py
> + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-errors.py -o "." < $<, " GEN $@")
I'm afraid this isn't quite what you want. It's shorthand for two
separate rules with the same recipe[*]. Therefore, it's prone to run
the recipe twice, with make blissfully unaware that each of the two runs
clobbers the other file, too. Could conceivably lead to trouble with
parallel execution.
Paolo, Eric, maybe you can provide advice on how to best tell make that
a recipe generates multiple files.
> QGALIB_OBJ=$(addprefix qapi-generated/, qga-qapi-types.o qga-qapi-visit.o qga-qmp-marshal.o)
> QGALIB_GEN=$(addprefix qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-commands.h)
> diff --git a/scripts/qapi-errors.py b/scripts/qapi-errors.py
> new file mode 100644
> index 0000000..59cf426
> --- /dev/null
> +++ b/scripts/qapi-errors.py
> @@ -0,0 +1,177 @@
> +#
> +# QAPI errors generator
> +#
> +# Copyright (C) 2012 Red Hat, Inc.
> +#
> +# This work is licensed under the terms of the GNU GPLv2.
Sure you want v2 and not v2+?
> +# See the COPYING.LIB file in the top-level directory.
COPYING.LIB is for LGPL, use COPYING with GPL.
> +from ordereddict import OrderedDict
> +import getopt, sys, os, errno
> +from qapi import *
> +
> +def gen_error_decl_prologue(header, guard, prefix=""):
> + ret = mcgen('''
> +/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
> +
> +/*
> + * schema-defined QAPI Errors
> + *
> + * Copyright (C) 2012 Red Hat, Inc.
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#ifndef %(guard)s
> +#define %(guard)s
> +
> +''',
> + header=basename(header), guard=guardname(header), prefix=prefix)
> + return ret
> +
> +def gen_error_def_prologue(error_header, prefix=""):
> + ret = mcgen('''
> +/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
> +
> +/*
> + * schema-defined QMP Error table
> + *
> + * Copyright (C) 2012 Red Hat, Inc.
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#include "%(prefix)s%(error_header)s"
> +
> +''',
> + prefix=prefix, error_header=error_header)
> + return ret
> +
> +def gen_error_macro(err_domain):
> + string = ''
> + cur = err_domain[0]
> + for nxt in err_domain[1:]:
"err_domain" is just a fancy name for the error symbol, i.e. what
qerror.h calls 'class', isn't it?
Is it the same as error_class in gen_error_decl_macros() below?
> + if string and cur.isupper() and nxt.islower():
> + string += '_'
> + string += cur
> + cur = nxt
> + string += cur
> + return 'QERR_' + string.upper()
> +
> +def gen_error_def_table(exprs):
> + ret = mcgen('''
> +static const QErrorStringTable qerror_table[] = {
> +''')
> +
> + for err in exprs:
> + macro = gen_error_macro(err['error'])
> + desc = err['description']
> + ret += mcgen('''
> + {
> + .error_fmt = %(error_macro)s,
> + .desc = "%(error_desc)s",
> + },
> +''',
> + error_macro=macro, error_desc=desc)
> +
> + ret += mcgen('''
> + {}
> +};
> +''')
> +
> + return ret
> +
> +def gen_error_macro_data_str(data):
> + colon = ''
> + data_str = ''
> + for k, v in data.items():
> + data_str += colon + "'%s': " % k
> + if v == 'str':
> + data_str += "%s"
> + elif v == 'int':
> + data_str += '%"PRId64"'
PRId64 matches existing qerror.h practice. Requires the macro's users
to pass suitable argument, which can be bothersome, but at least the
compiler helps with it.
> + else:
> + sys.exit("unknown data type '%s' for error '%s'" % (v, name))
> + colon = ', '
colon is either empty or ', ', but never a colon. What about calling it
sep, for separator?
> + return data_str
> +
> +def gen_error_decl_macros(exprs):
> + ret = ''
> + for err in exprs:
> + data = gen_error_macro_data_str({})
> + if err.has_key('data'):
> + data = gen_error_macro_data_str(err['data'])
Wouldn't
if err.has_key('data'):
data = gen_error_macro_data_str(err['data'])
else:
data = gen_error_macro_data_str({})
be clearer?
> + macro = gen_error_macro(err['error'])
> + name = err['error']
> +
> + ret += mcgen('''
> +#define %(error_macro)s \\
> + "{ 'class': '%(error_class)s', 'data': { %(error_data)s } }"
> +
> +''',
> + error_macro=macro, error_class=name, error_data=data)
> + return ret
> +
> +def maybe_open(really, name, opt):
> + if really:
> + return open(name, opt)
> + else:
> + import StringIO
> + return StringIO.StringIO()
> +
> +if __name__ == '__main__':
> + try:
> + opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:o:",
> + ["prefix=", "output-dir="])
> + except getopt.GetoptError, err:
> + print str(err)
> + sys.exit(1)
> +
> + prefix = ""
> + output_dir = ""
> + do_c = True
> + do_h = True
Both are never false. Purpose?
> + c_file = 'qapi-errors.c'
> + h_file = 'qapi-errors.h'
> +
> + for o, a in opts:
> + if o in ("-p", "--prefix"):
> + prefix = a
> + elif o in ("-o", "--output-dir"):
> + output_dir = a + "/"
> +
> + c_file = output_dir + prefix + c_file
> + h_file = output_dir + prefix + h_file
> +
> + try:
> + os.makedirs(output_dir)
> + except os.error, e:
> + if e.errno != errno.EEXIST:
> + raise
> +
> + exprs = parse_schema(sys.stdin)
> +
> + fdecl = maybe_open(do_h, h_file, 'w')
> + ret = gen_error_decl_prologue(header=basename(h_file), guard=guardname(h_file), prefix=prefix)
> + fdecl.write(ret)
> +
> + ret = gen_error_decl_macros(exprs)
> + fdecl.write(ret)
> +
> + fdecl.write("#endif\n")
> + fdecl.flush()
> + fdecl.close()
Err, is flush before close really necessary?
> +
> + fdef = maybe_open(do_c, c_file, 'w')
> + ret = gen_error_def_prologue(error_header=h_file, prefix=prefix)
> + fdef.write(ret)
> +
> + ret = gen_error_def_table(exprs)
> + fdef.write(ret)
> +
> + fdef.flush()
> + fdef.close()
[*] http://www.gnu.org/software/make/manual/make.html#Multiple-Targets
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH 09/11] qapi: add qapi-errors.py
2012-07-26 11:50 ` Markus Armbruster
@ 2012-07-26 11:55 ` Paolo Bonzini
2012-07-26 12:38 ` Eric Blake
2012-07-26 14:45 ` Luiz Capitulino
2012-07-26 14:34 ` Luiz Capitulino
1 sibling, 2 replies; 39+ messages in thread
From: Paolo Bonzini @ 2012-07-26 11:55 UTC (permalink / raw)
To: Markus Armbruster
Cc: mdroth, aliguori, Eric Blake, qemu-devel, Luiz Capitulino
Il 26/07/2012 13:50, Markus Armbruster ha scritto:
>> > +qapi-errors.h qapi-errors.c :\
>> > +$(SRC_PATH)/qapi-schema-errors.json $(SRC_PATH)/scripts/qapi-errors.py
>> > + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-errors.py -o "." < $<, " GEN $@")
> I'm afraid this isn't quite what you want. It's shorthand for two
> separate rules with the same recipe[*]. Therefore, it's prone to run
> the recipe twice, with make blissfully unaware that each of the two runs
> clobbers the other file, too. Could conceivably lead to trouble with
> parallel execution.
>
> Paolo, Eric, maybe you can provide advice on how to best tell make that
> a recipe generates multiple files.
Hmm, I would just do
qapi-errors.h: qapi-errors.c
qapi-errors.c: $(SRC_PATH)/qapi-schema-errors.json $(SRC_PATH)/scripts/qapi-errors.py
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-errors.py -o "." < $<, " GEN $@")
I think that's what I usually saw for bison (which creates both .h and .c).
A perhaps cleaner alternative is to add a stamp file, and make both files
depend on it.
Paolo
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Qemu-devel] [PATCH 09/11] qapi: add qapi-errors.py
2012-07-26 11:55 ` Paolo Bonzini
@ 2012-07-26 12:38 ` Eric Blake
2012-07-26 12:42 ` Eric Blake
2012-07-26 14:45 ` Luiz Capitulino
1 sibling, 1 reply; 39+ messages in thread
From: Eric Blake @ 2012-07-26 12:38 UTC (permalink / raw)
To: Paolo Bonzini
Cc: qemu-devel, mdroth, aliguori, Markus Armbruster, Luiz Capitulino
[-- Attachment #1: Type: text/plain, Size: 1541 bytes --]
On 07/26/2012 05:55 AM, Paolo Bonzini wrote:
> Il 26/07/2012 13:50, Markus Armbruster ha scritto:
>>>> +qapi-errors.h qapi-errors.c :\
>>>> +$(SRC_PATH)/qapi-schema-errors.json $(SRC_PATH)/scripts/qapi-errors.py
>>>> + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-errors.py -o "." < $<, " GEN $@")
>> I'm afraid this isn't quite what you want. It's shorthand for two
>> separate rules with the same recipe[*]. Therefore, it's prone to run
>> the recipe twice, with make blissfully unaware that each of the two runs
>> clobbers the other file, too. Could conceivably lead to trouble with
>> parallel execution.
>>
>> Paolo, Eric, maybe you can provide advice on how to best tell make that
>> a recipe generates multiple files.
>
> Hmm, I would just do
>
> qapi-errors.h: qapi-errors.c
> qapi-errors.c: $(SRC_PATH)/qapi-schema-errors.json $(SRC_PATH)/scripts/qapi-errors.py
> $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-errors.py -o "." < $<, " GEN $@")
>
> I think that's what I usually saw for bison (which creates both .h and .c).
Indeed, per
https://www.gnu.org/software/automake/manual/automake.html#Multiple-Outputs,
that is an appropriate solution for a 2-file generation. It is only
when you have more than two files where...
>
> A perhaps cleaner alternative is to add a stamp file, and make both files
> depend on it.
a stamp file makes more sense.
--
Eric Blake eblake@redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Qemu-devel] [PATCH 09/11] qapi: add qapi-errors.py
2012-07-26 12:38 ` Eric Blake
@ 2012-07-26 12:42 ` Eric Blake
0 siblings, 0 replies; 39+ messages in thread
From: Eric Blake @ 2012-07-26 12:42 UTC (permalink / raw)
Cc: aliguori, Markus Armbruster, qemu-devel, mdroth, Paolo Bonzini,
Luiz Capitulino
[-- Attachment #1: Type: text/plain, Size: 1118 bytes --]
On 07/26/2012 06:38 AM, Eric Blake wrote:
>>> Paolo, Eric, maybe you can provide advice on how to best tell make that
>>> a recipe generates multiple files.
>>
>> Hmm, I would just do
>>
>> qapi-errors.h: qapi-errors.c
>> qapi-errors.c: $(SRC_PATH)/qapi-schema-errors.json $(SRC_PATH)/scripts/qapi-errors.py
>> $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-errors.py -o "." < $<, " GEN $@")
>>
>> I think that's what I usually saw for bison (which creates both .h and .c).
>
> Indeed, per
> https://www.gnu.org/software/automake/manual/automake.html#Multiple-Outputs,
> that is an appropriate solution for a 2-file generation.
Or, since we depend on GNU make, we can use pattern rules instead
(untested):
%-errors.h %-errors.c: \
%(SRC_PATH)/%-schema-errors.json $(SRC_PATH)/srcipts/%-errors.py
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-errors.py -o
"." < $<, " GEN $@")
https://www.gnu.org/software/automake/manual/make.html#Pattern-Examples
--
Eric Blake eblake@redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Qemu-devel] [PATCH 09/11] qapi: add qapi-errors.py
2012-07-26 11:55 ` Paolo Bonzini
2012-07-26 12:38 ` Eric Blake
@ 2012-07-26 14:45 ` Luiz Capitulino
1 sibling, 0 replies; 39+ messages in thread
From: Luiz Capitulino @ 2012-07-26 14:45 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: mdroth, aliguori, Eric Blake, Markus Armbruster, qemu-devel
On Thu, 26 Jul 2012 13:55:29 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 26/07/2012 13:50, Markus Armbruster ha scritto:
> >> > +qapi-errors.h qapi-errors.c :\
> >> > +$(SRC_PATH)/qapi-schema-errors.json $(SRC_PATH)/scripts/qapi-errors.py
> >> > + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-errors.py -o "." < $<, " GEN $@")
> > I'm afraid this isn't quite what you want. It's shorthand for two
> > separate rules with the same recipe[*]. Therefore, it's prone to run
> > the recipe twice, with make blissfully unaware that each of the two runs
> > clobbers the other file, too. Could conceivably lead to trouble with
> > parallel execution.
> >
> > Paolo, Eric, maybe you can provide advice on how to best tell make that
> > a recipe generates multiple files.
>
> Hmm, I would just do
>
> qapi-errors.h: qapi-errors.c
> qapi-errors.c: $(SRC_PATH)/qapi-schema-errors.json $(SRC_PATH)/scripts/qapi-errors.py
> $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-errors.py -o "." < $<, " GEN $@")
I've done this for v3, thanks for the suggestion.
>
> I think that's what I usually saw for bison (which creates both .h and .c).
>
> A perhaps cleaner alternative is to add a stamp file, and make both files
> depend on it.
>
> Paolo
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Qemu-devel] [PATCH 09/11] qapi: add qapi-errors.py
2012-07-26 11:50 ` Markus Armbruster
2012-07-26 11:55 ` Paolo Bonzini
@ 2012-07-26 14:34 ` Luiz Capitulino
2012-07-26 16:31 ` Markus Armbruster
1 sibling, 1 reply; 39+ messages in thread
From: Luiz Capitulino @ 2012-07-26 14:34 UTC (permalink / raw)
To: Markus Armbruster; +Cc: pbonzini, aliguori, Eric Blake, qemu-devel, mdroth
On Thu, 26 Jul 2012 13:50:24 +0200
Markus Armbruster <armbru@redhat.com> wrote:
> Paolo, Eric, I got a make puzzle for you below.
>
> Luiz Capitulino <lcapitulino@redhat.com> writes:
>
> > This script generates two files from qapi-schema-errors.json:
> >
> > o qapi-errors.h: contains error macro definitions, eg. QERR_BASE_NOT_FOUND,
> > corresponds to most of today's qerror.h
> >
> > o qapi-errors.c: contains the error table that currently exists in qerror.c
> >
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> > Makefile | 8 ++-
> > scripts/qapi-errors.py | 177 +++++++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 183 insertions(+), 2 deletions(-)
> > create mode 100644 scripts/qapi-errors.py
> >
> > diff --git a/Makefile b/Makefile
> > index ab82ef3..2cdc732 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -22,8 +22,9 @@ GENERATED_HEADERS = config-host.h trace.h qemu-options.def
> > ifeq ($(TRACE_BACKEND),dtrace)
> > GENERATED_HEADERS += trace-dtrace.h
> > endif
> > -GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h
> > -GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c trace.c
> > +GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h qapi-errors.h
> > +GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-errors.c \
> > + trace.c
> >
> > # Don't try to regenerate Makefile or configure
> > # We don't generate any of them
> > @@ -200,6 +201,9 @@ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py
> > qmp-commands.h qmp-marshal.c :\
> > $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py
> > $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -m -o "." < $<, " GEN $@")
> > +qapi-errors.h qapi-errors.c :\
> > +$(SRC_PATH)/qapi-schema-errors.json $(SRC_PATH)/scripts/qapi-errors.py
> > + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-errors.py -o "." < $<, " GEN $@")
>
> I'm afraid this isn't quite what you want. It's shorthand for two
> separate rules with the same recipe[*]. Therefore, it's prone to run
> the recipe twice, with make blissfully unaware that each of the two runs
> clobbers the other file, too. Could conceivably lead to trouble with
> parallel execution.
>
> Paolo, Eric, maybe you can provide advice on how to best tell make that
> a recipe generates multiple files.
>
> > QGALIB_OBJ=$(addprefix qapi-generated/, qga-qapi-types.o qga-qapi-visit.o qga-qmp-marshal.o)
> > QGALIB_GEN=$(addprefix qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-commands.h)
> > diff --git a/scripts/qapi-errors.py b/scripts/qapi-errors.py
> > new file mode 100644
> > index 0000000..59cf426
> > --- /dev/null
> > +++ b/scripts/qapi-errors.py
> > @@ -0,0 +1,177 @@
> > +#
> > +# QAPI errors generator
> > +#
> > +# Copyright (C) 2012 Red Hat, Inc.
> > +#
> > +# This work is licensed under the terms of the GNU GPLv2.
>
> Sure you want v2 and not v2+?
>
> > +# See the COPYING.LIB file in the top-level directory.
>
> COPYING.LIB is for LGPL, use COPYING with GPL.
I started this script as a copy from the others qapi scripts and didn't
notice that, thanks for spotting it (fixed in v3).
> > +from ordereddict import OrderedDict
> > +import getopt, sys, os, errno
> > +from qapi import *
> > +
> > +def gen_error_decl_prologue(header, guard, prefix=""):
> > + ret = mcgen('''
> > +/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
> > +
> > +/*
> > + * schema-defined QAPI Errors
> > + *
> > + * Copyright (C) 2012 Red Hat, Inc.
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> > + * See the COPYING.LIB file in the top-level directory.
> > + *
> > + */
> > +
> > +#ifndef %(guard)s
> > +#define %(guard)s
> > +
> > +''',
> > + header=basename(header), guard=guardname(header), prefix=prefix)
> > + return ret
> > +
> > +def gen_error_def_prologue(error_header, prefix=""):
> > + ret = mcgen('''
> > +/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
> > +
> > +/*
> > + * schema-defined QMP Error table
> > + *
> > + * Copyright (C) 2012 Red Hat, Inc.
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> > + * See the COPYING.LIB file in the top-level directory.
> > + *
> > + */
> > +
> > +#include "%(prefix)s%(error_header)s"
> > +
> > +''',
> > + prefix=prefix, error_header=error_header)
> > + return ret
> > +
> > +def gen_error_macro(err_domain):
> > + string = ''
> > + cur = err_domain[0]
> > + for nxt in err_domain[1:]:
>
> "err_domain" is just a fancy name for the error symbol, i.e. what
> qerror.h calls 'class', isn't it?
>
> Is it the same as error_class in gen_error_decl_macros() below?
Do you want to suggest a better name or are you just pointing out the lack of
consistence?
>
> > + if string and cur.isupper() and nxt.islower():
> > + string += '_'
> > + string += cur
> > + cur = nxt
> > + string += cur
> > + return 'QERR_' + string.upper()
> > +
> > +def gen_error_def_table(exprs):
> > + ret = mcgen('''
> > +static const QErrorStringTable qerror_table[] = {
> > +''')
> > +
> > + for err in exprs:
> > + macro = gen_error_macro(err['error'])
> > + desc = err['description']
> > + ret += mcgen('''
> > + {
> > + .error_fmt = %(error_macro)s,
> > + .desc = "%(error_desc)s",
> > + },
> > +''',
> > + error_macro=macro, error_desc=desc)
> > +
> > + ret += mcgen('''
> > + {}
> > +};
> > +''')
> > +
> > + return ret
> > +
> > +def gen_error_macro_data_str(data):
> > + colon = ''
> > + data_str = ''
> > + for k, v in data.items():
> > + data_str += colon + "'%s': " % k
> > + if v == 'str':
> > + data_str += "%s"
> > + elif v == 'int':
> > + data_str += '%"PRId64"'
>
> PRId64 matches existing qerror.h practice. Requires the macro's users
> to pass suitable argument, which can be bothersome, but at least the
> compiler helps with it.
>
> > + else:
> > + sys.exit("unknown data type '%s' for error '%s'" % (v, name))
> > + colon = ', '
>
> colon is either empty or ', ', but never a colon. What about calling it
> sep, for separator?
Done.
>
> > + return data_str
> > +
> > +def gen_error_decl_macros(exprs):
> > + ret = ''
> > + for err in exprs:
> > + data = gen_error_macro_data_str({})
> > + if err.has_key('data'):
> > + data = gen_error_macro_data_str(err['data'])
>
> Wouldn't
> if err.has_key('data'):
> data = gen_error_macro_data_str(err['data'])
> else:
> data = gen_error_macro_data_str({})
>
> be clearer?
Makes no difference for me, but I've changed to your suggestion.
>
> > + macro = gen_error_macro(err['error'])
> > + name = err['error']
> > +
> > + ret += mcgen('''
> > +#define %(error_macro)s \\
> > + "{ 'class': '%(error_class)s', 'data': { %(error_data)s } }"
> > +
> > +''',
> > + error_macro=macro, error_class=name, error_data=data)
> > + return ret
> > +
> > +def maybe_open(really, name, opt):
> > + if really:
> > + return open(name, opt)
> > + else:
> > + import StringIO
> > + return StringIO.StringIO()
> > +
> > +if __name__ == '__main__':
> > + try:
> > + opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:o:",
> > + ["prefix=", "output-dir="])
> > + except getopt.GetoptError, err:
> > + print str(err)
> > + sys.exit(1)
> > +
> > + prefix = ""
> > + output_dir = ""
> > + do_c = True
> > + do_h = True
>
> Both are never false. Purpose?
This was also copied from others qapi scripts, but qapi-errors.py always
creates both files so I've dropped this.
>
> > + c_file = 'qapi-errors.c'
> > + h_file = 'qapi-errors.h'
> > +
> > + for o, a in opts:
> > + if o in ("-p", "--prefix"):
> > + prefix = a
> > + elif o in ("-o", "--output-dir"):
> > + output_dir = a + "/"
> > +
> > + c_file = output_dir + prefix + c_file
> > + h_file = output_dir + prefix + h_file
> > +
> > + try:
> > + os.makedirs(output_dir)
> > + except os.error, e:
> > + if e.errno != errno.EEXIST:
> > + raise
> > +
> > + exprs = parse_schema(sys.stdin)
> > +
> > + fdecl = maybe_open(do_h, h_file, 'w')
> > + ret = gen_error_decl_prologue(header=basename(h_file), guard=guardname(h_file), prefix=prefix)
> > + fdecl.write(ret)
> > +
> > + ret = gen_error_decl_macros(exprs)
> > + fdecl.write(ret)
> > +
> > + fdecl.write("#endif\n")
> > + fdecl.flush()
> > + fdecl.close()
>
> Err, is flush before close really necessary?
I don't think so, but doesn't cause any harm either (and I'll maintain it as
the others qapi scripts do this too).
>
> > +
> > + fdef = maybe_open(do_c, c_file, 'w')
> > + ret = gen_error_def_prologue(error_header=h_file, prefix=prefix)
> > + fdef.write(ret)
> > +
> > + ret = gen_error_def_table(exprs)
> > + fdef.write(ret)
> > +
> > + fdef.flush()
> > + fdef.close()
>
>
> [*] http://www.gnu.org/software/make/manual/make.html#Multiple-Targets
>
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH 09/11] qapi: add qapi-errors.py
2012-07-26 14:34 ` Luiz Capitulino
@ 2012-07-26 16:31 ` Markus Armbruster
0 siblings, 0 replies; 39+ messages in thread
From: Markus Armbruster @ 2012-07-26 16:31 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: pbonzini, aliguori, Eric Blake, qemu-devel, mdroth
Luiz Capitulino <lcapitulino@redhat.com> writes:
> On Thu, 26 Jul 2012 13:50:24 +0200
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Paolo, Eric, I got a make puzzle for you below.
>>
>> Luiz Capitulino <lcapitulino@redhat.com> writes:
>>
>> > This script generates two files from qapi-schema-errors.json:
>> >
>> > o qapi-errors.h: contains error macro definitions, eg. QERR_BASE_NOT_FOUND,
>> > corresponds to most of today's qerror.h
>> >
>> > o qapi-errors.c: contains the error table that currently exists in qerror.c
>> >
>> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
>> > ---
>> > Makefile | 8 ++-
>> > scripts/qapi-errors.py | 177 +++++++++++++++++++++++++++++++++++++++++++++++++
>> > 2 files changed, 183 insertions(+), 2 deletions(-)
>> > create mode 100644 scripts/qapi-errors.py
>> >
>> > diff --git a/Makefile b/Makefile
>> > index ab82ef3..2cdc732 100644
>> > --- a/Makefile
>> > +++ b/Makefile
>> > @@ -22,8 +22,9 @@ GENERATED_HEADERS = config-host.h trace.h qemu-options.def
>> > ifeq ($(TRACE_BACKEND),dtrace)
>> > GENERATED_HEADERS += trace-dtrace.h
>> > endif
>> > -GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h
>> > -GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c trace.c
>> > +GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h qapi-errors.h
>> > +GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-errors.c \
>> > + trace.c
>> >
>> > # Don't try to regenerate Makefile or configure
>> > # We don't generate any of them
>> > @@ -200,6 +201,9 @@ $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py
>> > qmp-commands.h qmp-marshal.c :\
>> > $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py
>> > $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -m -o "." < $<, " GEN $@")
>> > +qapi-errors.h qapi-errors.c :\
>> > +$(SRC_PATH)/qapi-schema-errors.json $(SRC_PATH)/scripts/qapi-errors.py
>> > + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-errors.py -o "." < $<, " GEN $@")
>>
>> I'm afraid this isn't quite what you want. It's shorthand for two
>> separate rules with the same recipe[*]. Therefore, it's prone to run
>> the recipe twice, with make blissfully unaware that each of the two runs
>> clobbers the other file, too. Could conceivably lead to trouble with
>> parallel execution.
>>
>> Paolo, Eric, maybe you can provide advice on how to best tell make that
>> a recipe generates multiple files.
>>
>> > QGALIB_OBJ=$(addprefix qapi-generated/, qga-qapi-types.o
>> > qga-qapi-visit.o qga-qmp-marshal.o)
>> > QGALIB_GEN=$(addprefix qapi-generated/, qga-qapi-types.h
>> > qga-qapi-visit.h qga-qmp-commands.h)
>> > diff --git a/scripts/qapi-errors.py b/scripts/qapi-errors.py
>> > new file mode 100644
>> > index 0000000..59cf426
>> > --- /dev/null
>> > +++ b/scripts/qapi-errors.py
>> > @@ -0,0 +1,177 @@
>> > +#
>> > +# QAPI errors generator
>> > +#
>> > +# Copyright (C) 2012 Red Hat, Inc.
>> > +#
>> > +# This work is licensed under the terms of the GNU GPLv2.
>>
>> Sure you want v2 and not v2+?
>>
>> > +# See the COPYING.LIB file in the top-level directory.
>>
>> COPYING.LIB is for LGPL, use COPYING with GPL.
>
> I started this script as a copy from the others qapi scripts and didn't
> notice that, thanks for spotting it (fixed in v3).
The other qapi scripts need fixing then --- self-contradictory copyright
statements are not a good idea.
>> > +from ordereddict import OrderedDict
>> > +import getopt, sys, os, errno
>> > +from qapi import *
>> > +
>> > +def gen_error_decl_prologue(header, guard, prefix=""):
>> > + ret = mcgen('''
>> > +/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
>> > +
>> > +/*
>> > + * schema-defined QAPI Errors
>> > + *
>> > + * Copyright (C) 2012 Red Hat, Inc.
>> > + *
>> > + * This work is licensed under the terms of the GNU LGPL, version
>> > 2.1 or later.
>> > + * See the COPYING.LIB file in the top-level directory.
>> > + *
>> > + */
>> > +
>> > +#ifndef %(guard)s
>> > +#define %(guard)s
>> > +
>> > +''',
>> > + header=basename(header), guard=guardname(header), prefix=prefix)
>> > + return ret
>> > +
>> > +def gen_error_def_prologue(error_header, prefix=""):
>> > + ret = mcgen('''
>> > +/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
>> > +
>> > +/*
>> > + * schema-defined QMP Error table
>> > + *
>> > + * Copyright (C) 2012 Red Hat, Inc.
>> > + *
>> > + * This work is licensed under the terms of the GNU LGPL, version
>> > 2.1 or later.
>> > + * See the COPYING.LIB file in the top-level directory.
>> > + *
>> > + */
>> > +
>> > +#include "%(prefix)s%(error_header)s"
>> > +
>> > +''',
>> > + prefix=prefix, error_header=error_header)
>> > + return ret
>> > +
>> > +def gen_error_macro(err_domain):
>> > + string = ''
>> > + cur = err_domain[0]
>> > + for nxt in err_domain[1:]:
>>
>> "err_domain" is just a fancy name for the error symbol, i.e. what
>> qerror.h calls 'class', isn't it?
>>
>> Is it the same as error_class in gen_error_decl_macros() below?
>
> Do you want to suggest a better name or are you just pointing out the lack of
> consistence?
No, I'm just trying to understand what's going on, and asking you to
confirm my hypothesis.
But yes, same name for same thing would be nice.
>> > + if string and cur.isupper() and nxt.islower():
>> > + string += '_'
>> > + string += cur
>> > + cur = nxt
>> > + string += cur
>> > + return 'QERR_' + string.upper()
>> > +
>> > +def gen_error_def_table(exprs):
>> > + ret = mcgen('''
>> > +static const QErrorStringTable qerror_table[] = {
>> > +''')
>> > +
>> > + for err in exprs:
>> > + macro = gen_error_macro(err['error'])
>> > + desc = err['description']
>> > + ret += mcgen('''
>> > + {
>> > + .error_fmt = %(error_macro)s,
>> > + .desc = "%(error_desc)s",
>> > + },
>> > +''',
>> > + error_macro=macro, error_desc=desc)
>> > +
>> > + ret += mcgen('''
>> > + {}
>> > +};
>> > +''')
>> > +
>> > + return ret
>> > +
>> > +def gen_error_macro_data_str(data):
>> > + colon = ''
>> > + data_str = ''
>> > + for k, v in data.items():
>> > + data_str += colon + "'%s': " % k
>> > + if v == 'str':
>> > + data_str += "%s"
>> > + elif v == 'int':
>> > + data_str += '%"PRId64"'
>>
>> PRId64 matches existing qerror.h practice. Requires the macro's users
>> to pass suitable argument, which can be bothersome, but at least the
>> compiler helps with it.
>>
>> > + else:
>> > + sys.exit("unknown data type '%s' for error '%s'" % (v, name))
>> > + colon = ', '
>>
>> colon is either empty or ', ', but never a colon. What about calling it
>> sep, for separator?
>
> Done.
>
>>
>> > + return data_str
>> > +
>> > +def gen_error_decl_macros(exprs):
>> > + ret = ''
>> > + for err in exprs:
>> > + data = gen_error_macro_data_str({})
>> > + if err.has_key('data'):
>> > + data = gen_error_macro_data_str(err['data'])
>>
>> Wouldn't
>> if err.has_key('data'):
>> data = gen_error_macro_data_str(err['data'])
>> else:
>> data = gen_error_macro_data_str({})
>>
>> be clearer?
>
> Makes no difference for me, but I've changed to your suggestion.
>
>>
>> > + macro = gen_error_macro(err['error'])
>> > + name = err['error']
>> > +
>> > + ret += mcgen('''
>> > +#define %(error_macro)s \\
>> > + "{ 'class': '%(error_class)s', 'data': { %(error_data)s } }"
>> > +
>> > +''',
>> > + error_macro=macro, error_class=name, error_data=data)
>> > + return ret
>> > +
>> > +def maybe_open(really, name, opt):
>> > + if really:
>> > + return open(name, opt)
>> > + else:
>> > + import StringIO
>> > + return StringIO.StringIO()
>> > +
>> > +if __name__ == '__main__':
>> > + try:
>> > + opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:o:",
>> > + ["prefix=", "output-dir="])
>> > + except getopt.GetoptError, err:
>> > + print str(err)
>> > + sys.exit(1)
>> > +
>> > + prefix = ""
>> > + output_dir = ""
>> > + do_c = True
>> > + do_h = True
>>
>> Both are never false. Purpose?
>
> This was also copied from others qapi scripts, but qapi-errors.py always
> creates both files so I've dropped this.
>
>>
>> > + c_file = 'qapi-errors.c'
>> > + h_file = 'qapi-errors.h'
>> > +
>> > + for o, a in opts:
>> > + if o in ("-p", "--prefix"):
>> > + prefix = a
>> > + elif o in ("-o", "--output-dir"):
>> > + output_dir = a + "/"
>> > +
>> > + c_file = output_dir + prefix + c_file
>> > + h_file = output_dir + prefix + h_file
>> > +
>> > + try:
>> > + os.makedirs(output_dir)
>> > + except os.error, e:
>> > + if e.errno != errno.EEXIST:
>> > + raise
>> > +
>> > + exprs = parse_schema(sys.stdin)
>> > +
>> > + fdecl = maybe_open(do_h, h_file, 'w')
>> > + ret = gen_error_decl_prologue(header=basename(h_file),
>> > guard=guardname(h_file), prefix=prefix)
>> > + fdecl.write(ret)
>> > +
>> > + ret = gen_error_decl_macros(exprs)
>> > + fdecl.write(ret)
>> > +
>> > + fdecl.write("#endif\n")
>> > + fdecl.flush()
>> > + fdecl.close()
>>
>> Err, is flush before close really necessary?
>
> I don't think so, but doesn't cause any harm either (and I'll maintain it as
> the others qapi scripts do this too).
Looks like cargo cult programming to me :)
>> > +
>> > + fdef = maybe_open(do_c, c_file, 'w')
>> > + ret = gen_error_def_prologue(error_header=h_file, prefix=prefix)
>> > + fdef.write(ret)
>> > +
>> > + ret = gen_error_def_table(exprs)
>> > + fdef.write(ret)
>> > +
>> > + fdef.flush()
>> > + fdef.close()
>>
>>
>> [*] http://www.gnu.org/software/make/manual/make.html#Multiple-Targets
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Qemu-devel] [PATCH 09/11] qapi: add qapi-errors.py
2012-07-25 16:54 ` [Qemu-devel] [PATCH 09/11] qapi: add qapi-errors.py Luiz Capitulino
2012-07-26 11:50 ` Markus Armbruster
@ 2012-07-26 12:12 ` Markus Armbruster
2012-07-26 14:47 ` Luiz Capitulino
1 sibling, 1 reply; 39+ messages in thread
From: Markus Armbruster @ 2012-07-26 12:12 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: pbonzini, aliguori, qemu-devel, mdroth
Luiz Capitulino <lcapitulino@redhat.com> writes:
> This script generates two files from qapi-schema-errors.json:
>
> o qapi-errors.h: contains error macro definitions, eg. QERR_BASE_NOT_FOUND,
> corresponds to most of today's qerror.h
>
> o qapi-errors.c: contains the error table that currently exists in qerror.c
[...]
> diff --git a/scripts/qapi-errors.py b/scripts/qapi-errors.py
> new file mode 100644
> index 0000000..59cf426
> --- /dev/null
> +++ b/scripts/qapi-errors.py
[...]
> +def gen_error_def_table(exprs):
> + ret = mcgen('''
> +static const QErrorStringTable qerror_table[] = {
> +''')
> +
> + for err in exprs:
> + macro = gen_error_macro(err['error'])
> + desc = err['description']
> + ret += mcgen('''
> + {
> + .error_fmt = %(error_macro)s,
> + .desc = "%(error_desc)s",
> + },
> +''',
Trailing whitespace.
[...]
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH 09/11] qapi: add qapi-errors.py
2012-07-26 12:12 ` Markus Armbruster
@ 2012-07-26 14:47 ` Luiz Capitulino
0 siblings, 0 replies; 39+ messages in thread
From: Luiz Capitulino @ 2012-07-26 14:47 UTC (permalink / raw)
To: Markus Armbruster; +Cc: pbonzini, aliguori, qemu-devel, mdroth
On Thu, 26 Jul 2012 14:12:50 +0200
Markus Armbruster <armbru@redhat.com> wrote:
> Luiz Capitulino <lcapitulino@redhat.com> writes:
>
> > This script generates two files from qapi-schema-errors.json:
> >
> > o qapi-errors.h: contains error macro definitions, eg. QERR_BASE_NOT_FOUND,
> > corresponds to most of today's qerror.h
> >
> > o qapi-errors.c: contains the error table that currently exists in qerror.c
> [...]
> > diff --git a/scripts/qapi-errors.py b/scripts/qapi-errors.py
> > new file mode 100644
> > index 0000000..59cf426
> > --- /dev/null
> > +++ b/scripts/qapi-errors.py
> [...]
> > +def gen_error_def_table(exprs):
> > + ret = mcgen('''
> > +static const QErrorStringTable qerror_table[] = {
> > +''')
> > +
> > + for err in exprs:
> > + macro = gen_error_macro(err['error'])
> > + desc = err['description']
> > + ret += mcgen('''
> > + {
> > + .error_fmt = %(error_macro)s,
> > + .desc = "%(error_desc)s",
> > + },
> > +''',
>
> Trailing whitespace.
Fixed for v3.
^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] [PATCH 10/11] qerror: switch to qapi generated error macros and table
2012-07-25 16:54 [Qemu-devel] [PATCH v2 00/11]: qapi: generate qerrors from qapi-schema-errors.json Luiz Capitulino
` (8 preceding siblings ...)
2012-07-25 16:54 ` [Qemu-devel] [PATCH 09/11] qapi: add qapi-errors.py Luiz Capitulino
@ 2012-07-25 16:54 ` Luiz Capitulino
2012-07-26 11:56 ` Markus Armbruster
2012-07-25 16:54 ` [Qemu-devel] [PATCH 11/11] scripts: update check-qerror.sh Luiz Capitulino
10 siblings, 1 reply; 39+ messages in thread
From: Luiz Capitulino @ 2012-07-25 16:54 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, aliguori, armbru, mdroth
Previous commits added qapi infrastructure to automatically generate
qerror macros and the qerror table from qapi-schema-errors.json.
This commit drops the current error macros from qerror.h and the error
table from qerror.c and use the generated ones instead.
Please, note that qapi-error.c is actually _included_ by qerror.c.
This is hacky, but the alternative is to make the table private to
qapi-error.c and generate functions to return table entries. I think that
doesn't pay much off.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qerror.c | 310 +--------------------------------------------------------------
qerror.h | 220 +--------------------------------------------
2 files changed, 2 insertions(+), 528 deletions(-)
diff --git a/qerror.c b/qerror.c
index 01d2493..ec4ceb8 100644
--- a/qerror.c
+++ b/qerror.c
@@ -14,6 +14,7 @@
#include "qjson.h"
#include "qerror.h"
#include "qemu-common.h"
+#include "qapi-errors.c"
static void qerror_destroy_obj(QObject *obj);
@@ -23,315 +24,6 @@ static const QType qerror_type = {
};
/**
- * The 'desc' parameter is a printf-like string, the format of the format
- * string is:
- *
- * %(KEY)
- *
- * Where KEY is a QDict key, which has to be passed to qerror_from_info().
- *
- * Example:
- *
- * "foo error on device: %(device) slot: %(slot_nr)"
- *
- * A single percent sign can be printed if followed by a second one,
- * for example:
- *
- * "running out of foo: %(foo)%%"
- *
- * Please keep the entries in alphabetical order.
- * Use scripts/check-qerror.sh to check.
- */
-static const QErrorStringTable qerror_table[] = {
- {
- .error_fmt = QERR_ADD_CLIENT_FAILED,
- .desc = "Could not add client",
- },
- {
- .error_fmt = QERR_AMBIGUOUS_PATH,
- .desc = "Path '%(path)' does not uniquely identify a object"
- },
- {
- .error_fmt = QERR_BAD_BUS_FOR_DEVICE,
- .desc = "Device '%(device)' can't go on a %(bad_bus_type) bus",
- },
- {
- .error_fmt = QERR_BASE_NOT_FOUND,
- .desc = "Base '%(base)' not found",
- },
- {
- .error_fmt = QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- .desc = "Block format '%(format)' used by device '%(name)' does not support feature '%(feature)'",
- },
- {
- .error_fmt = QERR_BUS_NO_HOTPLUG,
- .desc = "Bus '%(bus)' does not support hotplugging",
- },
- {
- .error_fmt = QERR_BUS_NOT_FOUND,
- .desc = "Bus '%(bus)' not found",
- },
- {
- .error_fmt = QERR_COMMAND_DISABLED,
- .desc = "The command %(name) has been disabled for this instance",
- },
- {
- .error_fmt = QERR_COMMAND_NOT_FOUND,
- .desc = "The command %(name) has not been found",
- },
- {
- .error_fmt = QERR_DEVICE_ENCRYPTED,
- .desc = "Device '%(device)' is encrypted (filename=%(filename))",
- },
- {
- .error_fmt = QERR_DEVICE_FEATURE_BLOCKS_MIGRATION,
- .desc = "Migration is disabled when using feature '%(feature)' in device '%(device)'",
- },
- {
- .error_fmt = QERR_DEVICE_HAS_NO_MEDIUM,
- .desc = "Device '%(device)' has no medium",
- },
- {
- .error_fmt = QERR_DEVICE_INIT_FAILED,
- .desc = "Device '%(device)' could not be initialized",
- },
- {
- .error_fmt = QERR_DEVICE_IN_USE,
- .desc = "Device '%(device)' is in use",
- },
- {
- .error_fmt = QERR_DEVICE_IS_READ_ONLY,
- .desc = "Device '%(device)' is read only",
- },
- {
- .error_fmt = QERR_DEVICE_LOCKED,
- .desc = "Device '%(device)' is locked",
- },
- {
- .error_fmt = QERR_DEVICE_MULTIPLE_BUSSES,
- .desc = "Device '%(device)' has multiple child busses",
- },
- {
- .error_fmt = QERR_DEVICE_NO_BUS,
- .desc = "Device '%(device)' has no child bus",
- },
- {
- .error_fmt = QERR_DEVICE_NO_HOTPLUG,
- .desc = "Device '%(device)' does not support hotplugging",
- },
- {
- .error_fmt = QERR_DEVICE_NOT_ACTIVE,
- .desc = "Device '%(device)' has not been activated",
- },
- {
- .error_fmt = QERR_DEVICE_NOT_ENCRYPTED,
- .desc = "Device '%(device)' is not encrypted",
- },
- {
- .error_fmt = QERR_DEVICE_NOT_FOUND,
- .desc = "Device '%(device)' not found",
- },
- {
- .error_fmt = QERR_DEVICE_NOT_REMOVABLE,
- .desc = "Device '%(device)' is not removable",
- },
- {
- .error_fmt = QERR_DUPLICATE_ID,
- .desc = "Duplicate ID '%(id)' for %(object)",
- },
- {
- .error_fmt = QERR_FD_NOT_FOUND,
- .desc = "File descriptor named '%(name)' not found",
- },
- {
- .error_fmt = QERR_FD_NOT_SUPPLIED,
- .desc = "No file descriptor supplied via SCM_RIGHTS",
- },
- {
- .error_fmt = QERR_FEATURE_DISABLED,
- .desc = "The feature '%(name)' is not enabled",
- },
- {
- .error_fmt = QERR_INVALID_BLOCK_FORMAT,
- .desc = "Invalid block format '%(name)'",
- },
- {
- .error_fmt = QERR_INVALID_OPTION_GROUP,
- .desc = "There is no option group '%(group)'",
- },
- {
- .error_fmt = QERR_INVALID_PARAMETER,
- .desc = "Invalid parameter '%(name)'",
- },
- {
- .error_fmt = QERR_INVALID_PARAMETER_COMBINATION,
- .desc = "Invalid parameter combination",
- },
- {
- .error_fmt = QERR_INVALID_PARAMETER_TYPE,
- .desc = "Invalid parameter type for '%(name)', expected: %(expected)",
- },
- {
- .error_fmt = QERR_INVALID_PARAMETER_VALUE,
- .desc = "Parameter '%(name)' expects %(expected)",
- },
- {
- .error_fmt = QERR_INVALID_PASSWORD,
- .desc = "Password incorrect",
- },
- {
- .error_fmt = QERR_IO_ERROR,
- .desc = "An IO error has occurred",
- },
- {
- .error_fmt = QERR_JSON_PARSE_ERROR,
- .desc = "JSON parse error, %(message)",
-
- },
- {
- .error_fmt = QERR_JSON_PARSING,
- .desc = "Invalid JSON syntax",
- },
- {
- .error_fmt = QERR_KVM_MISSING_CAP,
- .desc = "Using KVM without %(capability), %(feature) unavailable",
- },
- {
- .error_fmt = QERR_MIGRATION_ACTIVE,
- .desc = "There's a migration process in progress",
- },
- {
- .error_fmt = QERR_MIGRATION_NOT_SUPPORTED,
- .desc = "State blocked by non-migratable device '%(device)'",
- },
- {
- .error_fmt = QERR_MIGRATION_EXPECTED,
- .desc = "An incoming migration is expected before this command can be executed",
- },
- {
- .error_fmt = QERR_MISSING_PARAMETER,
- .desc = "Parameter '%(name)' is missing",
- },
- {
- .error_fmt = QERR_NO_BUS_FOR_DEVICE,
- .desc = "No '%(bus)' bus found for device '%(device)'",
- },
- {
- .error_fmt = QERR_NOT_SUPPORTED,
- .desc = "Not supported",
- },
- {
- .error_fmt = QERR_OPEN_FILE_FAILED,
- .desc = "Could not open '%(filename)'",
- },
- {
- .error_fmt = QERR_PERMISSION_DENIED,
- .desc = "Insufficient permission to perform this operation",
- },
- {
- .error_fmt = QERR_PROPERTY_NOT_FOUND,
- .desc = "Property '%(device).%(property)' not found",
- },
- {
- .error_fmt = QERR_PROPERTY_VALUE_BAD,
- .desc = "Property '%(device).%(property)' doesn't take value '%(value)'",
- },
- {
- .error_fmt = QERR_PROPERTY_VALUE_IN_USE,
- .desc = "Property '%(device).%(property)' can't take value '%(value)', it's in use",
- },
- {
- .error_fmt = QERR_PROPERTY_VALUE_NOT_FOUND,
- .desc = "Property '%(device).%(property)' can't find value '%(value)'",
- },
- {
- .error_fmt = QERR_PROPERTY_VALUE_NOT_POWER_OF2,
- .desc = "Property '%(device).%(property)' doesn't take "
- "value '%(value)', it's not a power of 2",
- },
- {
- .error_fmt = QERR_PROPERTY_VALUE_OUT_OF_RANGE,
- .desc = "Property '%(device).%(property)' doesn't take "
- "value %(value) (minimum: %(min), maximum: %(max))",
- },
- {
- .error_fmt = QERR_QGA_COMMAND_FAILED,
- .desc = "Guest agent command failed, error was '%(message)'",
- },
- {
- .error_fmt = QERR_QGA_LOGGING_FAILED,
- .desc = "Guest agent failed to log non-optional log statement",
- },
- {
- .error_fmt = QERR_QMP_BAD_INPUT_OBJECT,
- .desc = "Expected '%(expected)' in QMP input",
- },
- {
- .error_fmt = QERR_QMP_BAD_INPUT_OBJECT_MEMBER,
- .desc = "QMP input object member '%(member)' expects '%(expected)'",
- },
- {
- .error_fmt = QERR_QMP_EXTRA_INPUT_OBJECT_MEMBER,
- .desc = "QMP input object member '%(member)' is unexpected",
- },
- {
- .error_fmt = QERR_RESET_REQUIRED,
- .desc = "Resetting the Virtual Machine is required",
- },
- {
- .error_fmt = QERR_SET_PASSWD_FAILED,
- .desc = "Could not set password",
- },
- {
- .error_fmt = QERR_TOO_MANY_FILES,
- .desc = "Too many open files",
- },
- {
- .error_fmt = QERR_UNDEFINED_ERROR,
- .desc = "An undefined error has occurred",
- },
- {
- .error_fmt = QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
- .desc = "'%(device)' uses a %(format) feature which is not "
- "supported by this qemu version: %(feature)",
- },
- {
- .error_fmt = QERR_UNSUPPORTED,
- .desc = "this feature or command is not currently supported",
- },
- {
- .error_fmt = QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION,
- .desc = "Migration is disabled when VirtFS export path '%(path)' "
- "is mounted in the guest using mount_tag '%(tag)'",
- },
- {
- .error_fmt = QERR_VNC_SERVER_FAILED,
- .desc = "Could not start VNC server on %(target)",
- },
- {
- .error_fmt = QERR_SOCK_CONNECT_INPROGRESS,
- .desc = "Connection can not be completed immediately",
- },
- {
- .error_fmt = QERR_SOCK_CONNECT_FAILED,
- .desc = "Failed to connect to socket",
- },
- {
- .error_fmt = QERR_SOCK_LISTEN_FAILED,
- .desc = "Failed to set socket to listening mode",
- },
- {
- .error_fmt = QERR_SOCK_BIND_FAILED,
- .desc = "Failed to bind socket",
- },
- {
- .error_fmt = QERR_SOCK_CREATE_FAILED,
- .desc = "Failed to create socket",
- },
- {}
-};
-
-/**
* qerror_new(): Create a new QError
*
* Return strong reference.
diff --git a/qerror.h b/qerror.h
index 58d0295..b22861f 100644
--- a/qerror.h
+++ b/qerror.h
@@ -15,6 +15,7 @@
#include "qdict.h"
#include "qstring.h"
#include "qemu-error.h"
+#include "qapi-errors.h"
#include "error.h"
#include <stdarg.h>
@@ -47,223 +48,4 @@ QString *qerror_format(const char *fmt, QDict *error);
qerror_report_internal(__FILE__, __LINE__, __func__, fmt, ## __VA_ARGS__)
QError *qobject_to_qerror(const QObject *obj);
-/*
- * QError class list
- * Please keep the definitions in alphabetical order.
- * Use scripts/check-qerror.sh to check.
- */
-#define QERR_ADD_CLIENT_FAILED \
- "{ 'class': 'AddClientFailed', 'data': {} }"
-
-#define QERR_AMBIGUOUS_PATH \
- "{ 'class': 'AmbiguousPath', 'data': { 'path': %s } }"
-
-#define QERR_BAD_BUS_FOR_DEVICE \
- "{ 'class': 'BadBusForDevice', 'data': { 'device': %s, 'bad_bus_type': %s } }"
-
-#define QERR_BASE_NOT_FOUND \
- "{ 'class': 'BaseNotFound', 'data': { 'base': %s } }"
-
-#define QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED \
- "{ 'class': 'BlockFormatFeatureNotSupported', 'data': { 'format': %s, 'name': %s, 'feature': %s } }"
-
-#define QERR_BUFFER_OVERRUN \
- "{ 'class': 'BufferOverrun', 'data': {} }"
-
-#define QERR_BUS_NO_HOTPLUG \
- "{ 'class': 'BusNoHotplug', 'data': { 'bus': %s } }"
-
-#define QERR_BUS_NOT_FOUND \
- "{ 'class': 'BusNotFound', 'data': { 'bus': %s } }"
-
-#define QERR_COMMAND_DISABLED \
- "{ 'class': 'CommandDisabled', 'data': { 'name': %s } }"
-
-#define QERR_COMMAND_NOT_FOUND \
- "{ 'class': 'CommandNotFound', 'data': { 'name': %s } }"
-
-#define QERR_DEVICE_ENCRYPTED \
- "{ 'class': 'DeviceEncrypted', 'data': { 'device': %s, 'filename': %s } }"
-
-#define QERR_DEVICE_FEATURE_BLOCKS_MIGRATION \
- "{ 'class': 'DeviceFeatureBlocksMigration', 'data': { 'device': %s, 'feature': %s } }"
-
-#define QERR_DEVICE_HAS_NO_MEDIUM \
- "{ 'class': 'DeviceHasNoMedium', 'data': { 'device': %s } }"
-
-#define QERR_DEVICE_INIT_FAILED \
- "{ 'class': 'DeviceInitFailed', 'data': { 'device': %s } }"
-
-#define QERR_DEVICE_IN_USE \
- "{ 'class': 'DeviceInUse', 'data': { 'device': %s } }"
-
-#define QERR_DEVICE_IS_READ_ONLY \
- "{ 'class': 'DeviceIsReadOnly', 'data': { 'device': %s } }"
-
-#define QERR_DEVICE_LOCKED \
- "{ 'class': 'DeviceLocked', 'data': { 'device': %s } }"
-
-#define QERR_DEVICE_MULTIPLE_BUSSES \
- "{ 'class': 'DeviceMultipleBusses', 'data': { 'device': %s } }"
-
-#define QERR_DEVICE_NO_BUS \
- "{ 'class': 'DeviceNoBus', 'data': { 'device': %s } }"
-
-#define QERR_DEVICE_NO_HOTPLUG \
- "{ 'class': 'DeviceNoHotplug', 'data': { 'device': %s } }"
-
-#define QERR_DEVICE_NOT_ACTIVE \
- "{ 'class': 'DeviceNotActive', 'data': { 'device': %s } }"
-
-#define QERR_DEVICE_NOT_ENCRYPTED \
- "{ 'class': 'DeviceNotEncrypted', 'data': { 'device': %s } }"
-
-#define QERR_DEVICE_NOT_FOUND \
- "{ 'class': 'DeviceNotFound', 'data': { 'device': %s } }"
-
-#define QERR_DEVICE_NOT_REMOVABLE \
- "{ 'class': 'DeviceNotRemovable', 'data': { 'device': %s } }"
-
-#define QERR_DUPLICATE_ID \
- "{ 'class': 'DuplicateId', 'data': { 'id': %s, 'object': %s } }"
-
-#define QERR_FD_NOT_FOUND \
- "{ 'class': 'FdNotFound', 'data': { 'name': %s } }"
-
-#define QERR_FD_NOT_SUPPLIED \
- "{ 'class': 'FdNotSupplied', 'data': {} }"
-
-#define QERR_FEATURE_DISABLED \
- "{ 'class': 'FeatureDisabled', 'data': { 'name': %s } }"
-
-#define QERR_INVALID_BLOCK_FORMAT \
- "{ 'class': 'InvalidBlockFormat', 'data': { 'name': %s } }"
-
-#define QERR_INVALID_OPTION_GROUP \
- "{ 'class': 'InvalidOptionGroup', 'data': { 'group': %s } }"
-
-#define QERR_INVALID_PARAMETER \
- "{ 'class': 'InvalidParameter', 'data': { 'name': %s } }"
-
-#define QERR_INVALID_PARAMETER_COMBINATION \
- "{ 'class': 'InvalidParameterCombination', 'data': {} }"
-
-#define QERR_INVALID_PARAMETER_TYPE \
- "{ 'class': 'InvalidParameterType', 'data': { 'name': %s,'expected': %s } }"
-
-#define QERR_INVALID_PARAMETER_VALUE \
- "{ 'class': 'InvalidParameterValue', 'data': { 'name': %s, 'expected': %s } }"
-
-#define QERR_INVALID_PASSWORD \
- "{ 'class': 'InvalidPassword', 'data': {} }"
-
-#define QERR_IO_ERROR \
- "{ 'class': 'IOError', 'data': {} }"
-
-#define QERR_JSON_PARSE_ERROR \
- "{ 'class': 'JSONParseError', 'data': { 'message': %s } }"
-
-#define QERR_JSON_PARSING \
- "{ 'class': 'JSONParsing', 'data': {} }"
-
-#define QERR_KVM_MISSING_CAP \
- "{ 'class': 'KVMMissingCap', 'data': { 'capability': %s, 'feature': %s } }"
-
-#define QERR_MIGRATION_ACTIVE \
- "{ 'class': 'MigrationActive', 'data': {} }"
-
-#define QERR_MIGRATION_NOT_SUPPORTED \
- "{ 'class': 'MigrationNotSupported', 'data': {'device': %s} }"
-
-#define QERR_MIGRATION_EXPECTED \
- "{ 'class': 'MigrationExpected', 'data': {} }"
-
-#define QERR_MISSING_PARAMETER \
- "{ 'class': 'MissingParameter', 'data': { 'name': %s } }"
-
-#define QERR_NO_BUS_FOR_DEVICE \
- "{ 'class': 'NoBusForDevice', 'data': { 'device': %s, 'bus': %s } }"
-
-#define QERR_NOT_SUPPORTED \
- "{ 'class': 'NotSupported', 'data': {} }"
-
-#define QERR_OPEN_FILE_FAILED \
- "{ 'class': 'OpenFileFailed', 'data': { 'filename': %s } }"
-
-#define QERR_PERMISSION_DENIED \
- "{ 'class': 'PermissionDenied', 'data': {} }"
-
-#define QERR_PROPERTY_NOT_FOUND \
- "{ 'class': 'PropertyNotFound', 'data': { 'device': %s, 'property': %s } }"
-
-#define QERR_PROPERTY_VALUE_BAD \
- "{ 'class': 'PropertyValueBad', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
-
-#define QERR_PROPERTY_VALUE_IN_USE \
- "{ 'class': 'PropertyValueInUse', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
-
-#define QERR_PROPERTY_VALUE_NOT_FOUND \
- "{ 'class': 'PropertyValueNotFound', 'data': { 'device': %s, 'property': %s, 'value': %s } }"
-
-#define QERR_PROPERTY_VALUE_NOT_POWER_OF2 \
- "{ 'class': 'PropertyValueNotPowerOf2', 'data': { " \
- "'device': %s, 'property': %s, 'value': %"PRId64" } }"
-
-#define QERR_PROPERTY_VALUE_OUT_OF_RANGE \
- "{ 'class': 'PropertyValueOutOfRange', 'data': { 'device': %s, 'property': %s, 'value': %"PRId64", 'min': %"PRId64", 'max': %"PRId64" } }"
-
-#define QERR_QGA_COMMAND_FAILED \
- "{ 'class': 'QgaCommandFailed', 'data': { 'message': %s } }"
-
-#define QERR_QGA_LOGGING_FAILED \
- "{ 'class': 'QgaLoggingFailed', 'data': {} }"
-
-#define QERR_QMP_BAD_INPUT_OBJECT \
- "{ 'class': 'QMPBadInputObject', 'data': { 'expected': %s } }"
-
-#define QERR_QMP_BAD_INPUT_OBJECT_MEMBER \
- "{ 'class': 'QMPBadInputObjectMember', 'data': { 'member': %s, 'expected': %s } }"
-
-#define QERR_QMP_EXTRA_INPUT_OBJECT_MEMBER \
- "{ 'class': 'QMPExtraInputObjectMember', 'data': { 'member': %s } }"
-
-#define QERR_RESET_REQUIRED \
- "{ 'class': 'ResetRequired', 'data': {} }"
-
-#define QERR_SET_PASSWD_FAILED \
- "{ 'class': 'SetPasswdFailed', 'data': {} }"
-
-#define QERR_TOO_MANY_FILES \
- "{ 'class': 'TooManyFiles', 'data': {} }"
-
-#define QERR_UNDEFINED_ERROR \
- "{ 'class': 'UndefinedError', 'data': {} }"
-
-#define QERR_UNKNOWN_BLOCK_FORMAT_FEATURE \
- "{ 'class': 'UnknownBlockFormatFeature', 'data': { 'device': %s, 'format': %s, 'feature': %s } }"
-
-#define QERR_UNSUPPORTED \
- "{ 'class': 'Unsupported', 'data': {} }"
-
-#define QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION \
- "{ 'class': 'VirtFSFeatureBlocksMigration', 'data': { 'path': %s, 'tag': %s } }"
-
-#define QERR_VNC_SERVER_FAILED \
- "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
-
-#define QERR_SOCK_CONNECT_INPROGRESS \
- "{ 'class': 'SockConnectInprogress', 'data': {} }"
-
-#define QERR_SOCK_CONNECT_FAILED \
- "{ 'class': 'SockConnectFailed', 'data': {} }"
-
-#define QERR_SOCK_LISTEN_FAILED \
- "{ 'class': 'SockListenFailed', 'data': {} }"
-
-#define QERR_SOCK_BIND_FAILED \
- "{ 'class': 'SockBindFailed', 'data': {} }"
-
-#define QERR_SOCK_CREATE_FAILED \
- "{ 'class': 'SockCreateFailed', 'data': {} }"
-
#endif /* QERROR_H */
--
1.7.11.2.249.g31c7954.dirty
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH 10/11] qerror: switch to qapi generated error macros and table
2012-07-25 16:54 ` [Qemu-devel] [PATCH 10/11] qerror: switch to qapi generated error macros and table Luiz Capitulino
@ 2012-07-26 11:56 ` Markus Armbruster
2012-07-26 14:50 ` Luiz Capitulino
0 siblings, 1 reply; 39+ messages in thread
From: Markus Armbruster @ 2012-07-26 11:56 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: pbonzini, aliguori, qemu-devel, mdroth
Luiz Capitulino <lcapitulino@redhat.com> writes:
> Previous commits added qapi infrastructure to automatically generate
> qerror macros and the qerror table from qapi-schema-errors.json.
>
> This commit drops the current error macros from qerror.h and the error
> table from qerror.c and use the generated ones instead.
>
> Please, note that qapi-error.c is actually _included_ by qerror.c.
> This is hacky, but the alternative is to make the table private to
> qapi-error.c and generate functions to return table entries. I think that
> doesn't pay much off.
Functions? Why can't you simply put
const QErrorStringTable qerror_table[NUMBER_OF_ERRORS];
into qapi-errors.h?
With a literal number instead of NUMBER_OF_ERRORS. If you suffer from
literal-phobia, you can also #define it instead.
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH 10/11] qerror: switch to qapi generated error macros and table
2012-07-26 11:56 ` Markus Armbruster
@ 2012-07-26 14:50 ` Luiz Capitulino
2012-07-26 16:05 ` Markus Armbruster
0 siblings, 1 reply; 39+ messages in thread
From: Luiz Capitulino @ 2012-07-26 14:50 UTC (permalink / raw)
To: Markus Armbruster; +Cc: pbonzini, aliguori, qemu-devel, mdroth
On Thu, 26 Jul 2012 13:56:00 +0200
Markus Armbruster <armbru@redhat.com> wrote:
> Luiz Capitulino <lcapitulino@redhat.com> writes:
>
> > Previous commits added qapi infrastructure to automatically generate
> > qerror macros and the qerror table from qapi-schema-errors.json.
> >
> > This commit drops the current error macros from qerror.h and the error
> > table from qerror.c and use the generated ones instead.
> >
> > Please, note that qapi-error.c is actually _included_ by qerror.c.
> > This is hacky, but the alternative is to make the table private to
> > qapi-error.c and generate functions to return table entries. I think that
> > doesn't pay much off.
>
> Functions? Why can't you simply put
>
> const QErrorStringTable qerror_table[NUMBER_OF_ERRORS];
>
> into qapi-errors.h?
Because it's included by qerror.h, which is included by several files.
I don't like much the idea of including a .c file, but on the other hand
it's only included by qerror.c and qerror.c will probably die in the
near future.
>
> With a literal number instead of NUMBER_OF_ERRORS. If you suffer from
> literal-phobia, you can also #define it instead.
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Qemu-devel] [PATCH 10/11] qerror: switch to qapi generated error macros and table
2012-07-26 14:50 ` Luiz Capitulino
@ 2012-07-26 16:05 ` Markus Armbruster
2012-07-26 16:52 ` Luiz Capitulino
0 siblings, 1 reply; 39+ messages in thread
From: Markus Armbruster @ 2012-07-26 16:05 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: pbonzini, aliguori, qemu-devel, mdroth
Luiz Capitulino <lcapitulino@redhat.com> writes:
> On Thu, 26 Jul 2012 13:56:00 +0200
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Luiz Capitulino <lcapitulino@redhat.com> writes:
>>
>> > Previous commits added qapi infrastructure to automatically generate
>> > qerror macros and the qerror table from qapi-schema-errors.json.
>> >
>> > This commit drops the current error macros from qerror.h and the error
>> > table from qerror.c and use the generated ones instead.
>> >
>> > Please, note that qapi-error.c is actually _included_ by qerror.c.
>> > This is hacky, but the alternative is to make the table private to
>> > qapi-error.c and generate functions to return table entries. I think that
>> > doesn't pay much off.
>>
>> Functions? Why can't you simply put
>>
>> const QErrorStringTable qerror_table[NUMBER_OF_ERRORS];
>>
>> into qapi-errors.h?
>
> Because it's included by qerror.h, which is included by several files.
And what harm does the declaration of qerror_table[] to those files?
> I don't like much the idea of including a .c file, but on the other hand
> it's only included by qerror.c and qerror.c will probably die in the
> near future.
>
>>
>> With a literal number instead of NUMBER_OF_ERRORS. If you suffer from
>> literal-phobia, you can also #define it instead.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [Qemu-devel] [PATCH 10/11] qerror: switch to qapi generated error macros and table
2012-07-26 16:05 ` Markus Armbruster
@ 2012-07-26 16:52 ` Luiz Capitulino
0 siblings, 0 replies; 39+ messages in thread
From: Luiz Capitulino @ 2012-07-26 16:52 UTC (permalink / raw)
To: Markus Armbruster; +Cc: pbonzini, aliguori, qemu-devel, mdroth
On Thu, 26 Jul 2012 18:05:45 +0200
Markus Armbruster <armbru@redhat.com> wrote:
> Luiz Capitulino <lcapitulino@redhat.com> writes:
>
> > On Thu, 26 Jul 2012 13:56:00 +0200
> > Markus Armbruster <armbru@redhat.com> wrote:
> >
> >> Luiz Capitulino <lcapitulino@redhat.com> writes:
> >>
> >> > Previous commits added qapi infrastructure to automatically generate
> >> > qerror macros and the qerror table from qapi-schema-errors.json.
> >> >
> >> > This commit drops the current error macros from qerror.h and the error
> >> > table from qerror.c and use the generated ones instead.
> >> >
> >> > Please, note that qapi-error.c is actually _included_ by qerror.c.
> >> > This is hacky, but the alternative is to make the table private to
> >> > qapi-error.c and generate functions to return table entries. I think that
> >> > doesn't pay much off.
> >>
> >> Functions? Why can't you simply put
> >>
> >> const QErrorStringTable qerror_table[NUMBER_OF_ERRORS];
> >>
> >> into qapi-errors.h?
> >
> > Because it's included by qerror.h, which is included by several files.
>
> And what harm does the declaration of qerror_table[] to those files?
We want to restrict qerror stuff as much as we can. Having it on a header
that's used by several files goes against that.
However, we just had a discussion on irc about not having the schema, so
this patch is probably going to change.
PS: Anthony will send a summary of the discussion to the list later.
>
> > I don't like much the idea of including a .c file, but on the other hand
> > it's only included by qerror.c and qerror.c will probably die in the
> > near future.
> >
> >>
> >> With a literal number instead of NUMBER_OF_ERRORS. If you suffer from
> >> literal-phobia, you can also #define it instead.
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [Qemu-devel] [PATCH 11/11] scripts: update check-qerror.sh
2012-07-25 16:54 [Qemu-devel] [PATCH v2 00/11]: qapi: generate qerrors from qapi-schema-errors.json Luiz Capitulino
` (9 preceding siblings ...)
2012-07-25 16:54 ` [Qemu-devel] [PATCH 10/11] qerror: switch to qapi generated error macros and table Luiz Capitulino
@ 2012-07-25 16:54 ` Luiz Capitulino
2012-07-26 11:57 ` Markus Armbruster
10 siblings, 1 reply; 39+ messages in thread
From: Luiz Capitulino @ 2012-07-25 16:54 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, aliguori, armbru, mdroth
The qerror.h file doesn't contain the macros anymore, the script should
check qapi-schema-errors.json instead.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
scripts/check-qerror.sh | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/scripts/check-qerror.sh b/scripts/check-qerror.sh
index af7fbd5..e397b4f 100755
--- a/scripts/check-qerror.sh
+++ b/scripts/check-qerror.sh
@@ -16,7 +16,5 @@ check_order() {
return 0
}
-check_order 'Definitions in qerror.h must be in alphabetical order:' \
- grep '^#define QERR_' qerror.h
-check_order 'Entries in qerror.c:qerror_table must be in alphabetical order:' \
- sed -n '/^static.*qerror_table\[\]/,/^};/s/QERR_/&/gp' qerror.c
+check_order 'Definitions must be in alphabetical order:' \
+ grep '^# @' qapi-schema-errors.json
--
1.7.11.2.249.g31c7954.dirty
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [Qemu-devel] [PATCH 11/11] scripts: update check-qerror.sh
2012-07-25 16:54 ` [Qemu-devel] [PATCH 11/11] scripts: update check-qerror.sh Luiz Capitulino
@ 2012-07-26 11:57 ` Markus Armbruster
0 siblings, 0 replies; 39+ messages in thread
From: Markus Armbruster @ 2012-07-26 11:57 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: pbonzini, aliguori, qemu-devel, mdroth
Luiz Capitulino <lcapitulino@redhat.com> writes:
> The qerror.h file doesn't contain the macros anymore, the script should
> check qapi-schema-errors.json instead.
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
> scripts/check-qerror.sh | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/check-qerror.sh b/scripts/check-qerror.sh
> index af7fbd5..e397b4f 100755
> --- a/scripts/check-qerror.sh
> +++ b/scripts/check-qerror.sh
> @@ -16,7 +16,5 @@ check_order() {
> return 0
> }
>
> -check_order 'Definitions in qerror.h must be in alphabetical order:' \
> - grep '^#define QERR_' qerror.h
> -check_order 'Entries in qerror.c:qerror_table must be in alphabetical order:' \
> - sed -n '/^static.*qerror_table\[\]/,/^};/s/QERR_/&/gp' qerror.c
> +check_order 'Definitions must be in alphabetical order:' \
> + grep '^# @' qapi-schema-errors.json
I'd add the new rule in the commit that adds qapi-schema-errors.json,
and drop the old rules in the commit that deletes the old definitions.
^ permalink raw reply [flat|nested] 39+ messages in thread