Re: [Qemu-devel] [RFC 00/27]: add new error format
On Mon, 6 Aug 2012 02:35:03 -0400 (EDT) Amos Kong wrote: > - Original Message - > > On Thu, 02 Aug 2012 10:31:28 +0800 > > Amos Kong wrote: > > > > > On 01/08/12 21:29, Luiz Capitulino wrote: > > > > On Wed, 01 Aug 2012 19:33:27 +0800 > > > > Amos Kong wrote: > > > > > > > >> On 31/07/12 22:44, Luiz Capitulino wrote: > > > >>> On Fri, 27 Jul 2012 18:31:41 -0300 > > > >>> Luiz Capitulino wrote: > > > >>> > > > >>>> [Please, read below why this is an RFC] > > > >>>> > > > >>>> This series implements the 'Plan for error handling in QMP' as > > > >>>> described > > > >>>> by Anthony in this email: > > > >> > > > >> > > > >> > > > >> Tested with > > > >> http://repo.or.cz/w/qemu/qmp-unstable.git/shortlog/refs/heads/error/new-format/v1 > > > > > > > > Thanks for testing Amos, but that branch is where I'm working > > > > currently so > > > > the code there is constantly changing. It's better to wait until > > > > I post it > > > > to the list. > > > > > > Got it. > > > > > > > Could you share your test-case, btw? > > > > > > 1. start a migration listen vm > > > x86_64-softmmu/qemu-system-x86_64 -monitor stdio -boot n -vnc :2 > > > -incoming tcp:0:1234 > > > > > > 2. start a migration client vm > > > x86_64-softmmu/qemu-system-x86_64 --enable-kvm -monitor stdio -boot > > > n > > > -vnc :1 > > > > > > 3. execute migration > > > vm2 (qemu) migrate -d tcp:0:1234 > > > vm2 (qemu) info migration > > > > > > expected result: migration should complete successfully > > > > This should work fine with v1 I posted yesterday. > > > > Actually, master is buggy: > > > > (qemu) migrate -d tcp:0: > > migrate: Connection can not be completed immediately > > > I thought it's a note of real socket status, not an error message. That's not something relevant for the user to know because it's temporary, and the user can't do anything about it anyway. > > > > (qemu) > > > > I wonder how you did not get this on your test-case? > > > Anyway, this is also fixed in v1. > > Thanks. > >
Re: [Qemu-devel] [PATCH 15/34] net: inet_connect(), inet_connect_opts(): return -errno
On Mon, 6 Aug 2012 02:52:24 -0400 (EDT)
Amos Kong wrote:
>
>
> - Original Message -
> > [cc: Juan & Amos]
> >
> > Luiz Capitulino writes:
> >
> > > On Wed, 1 Aug 2012 22:02:35 -0300
> > > Luiz Capitulino wrote:
> > >
> > >> Next commit wants to use this.
> > >>
> > >> Signed-off-by: Luiz Capitulino
> > >> ---
> > >>
> > >> This patch is an interesting case, because one of the goal of the
> > >> error
> > >> format that's being replaced was that callers could use it to know
> > >> the
> > >> error cause (with error_is_type().
> > >>
> > >> However, the new error format doesn't allow this as most errors
> > >> are
> > >> class GenericError. So, we'll have to use errno to know the error
> > >> cause,
> > >> this is the case of inet_connect() when called by
> > >> tcp_start_outgoing_migration().
> > >
> > > I'm thinking in doing this differently. Instead of returning errno,
> > > we
> > > could have:
> > >
> > > error_sete(Error **err, ErrorClass err_class, int err_no,
> > > const char *fmt, ...);
> > >
> > > Then we store err_no in Error, and also add error_get_errno().
> > >
> > > Comments?
> >
> > Maybe that'll turn out to be useful elsewhere, but not here.
> >
> > The purpose of PATCH 14+15 is to permit purging error_is_type() from
> > tcp_start_outgoing_migration() in PATCH 16. Let's have a look at all
> > three patches together.
> >
> > This is tcp_start_outgoing_migration() now:
> >
> > int tcp_start_outgoing_migration(MigrationState *s, const char
> > *host_port,
> > Error **errp)
> > {
> > s->get_error = socket_errno;
> > s->write = socket_write;
> > s->close = tcp_close;
> >
> > s->fd = inet_connect(host_port, false, errp);
> >
> > if (!error_is_set(errp)) {
> > migrate_fd_connect(s);
> > } else if (error_is_type(*errp,
> > QERR_SOCKET_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)) {
> > DPRINTF("connect failed\n");
> > return -1;
> > } else if (error_is_type(*errp, QERR_SOCKET_CONNECT_FAILED))
> > {
> > DPRINTF("connect failed\n");
> > migrate_fd_error(s);
> > return -1;
> > } else {
> > DPRINTF("unknown error\n");
> > return -1;
> > }
> >
> > return 0;
> > }
> >
> > Cases:
> >
> > 1. Success
> >
> >Proceeed to migrate_fd_connect().
> >
> > 2. QERR_SOCKET_CONNECT_IN_PROGRESS
> >
> >connect() failed with -EINPROGRESS. Not actually an error. Set
> >up
> >appropriate callback.
> >
> > 3. QERR_SOCKET_CONNECT_FAILED
> >
> >getaddrinfo() succeeded, but could not connect() to any of the
> >addresses. Fail migration with migrate_fd_error().
> >
> > 4. QERR_SOCKET_CREATE_FAILED
> >
> >The error grabbag:
> >
> >* inet_parse() failed, or
> >
> >* inet_connect_opts() misses host and/or port (should never
> >happen)
> >
> >* getaddrinfo() failed
> >
> >Bug: neglects to migrate_fd_error()! Broken in commit d5c5dacc.
>
>
> Before commit d5c5dacc, migrate_fd_error() would not be called
> when fail to create socket.
>
> -s->fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
> -if (s->fd == -1) {
> -DPRINTF("Unable to open socket");
> -return -socket_error();
> -}
>
>
> If you call migrate_fd_error() in this situation, qemu would hang.
How did you test this?
I've added the call to migrate_fd_error() as Markus suggests above and
tested it by changing inet_connect_opts() to trigger all possible errors
and everything seems to work fine.
>
>
> > 5. Anything else
> >
> >Should never happen. Handled exactly like 4.
> >
> > If I undo d5c5dacc's damage (untested), it looks like th
[Qemu-devel] [PATCH 07/35] qerror: QError: drop file, linenr, func
They have never been fully used and conflict with future error
improvements.
Also makes qerror_report() a proper function, as there's no point
in having it as a macro anymore.
Signed-off-by: Luiz Capitulino
---
qerror.c | 20 +++-
qerror.h | 8 +---
2 files changed, 4 insertions(+), 24 deletions(-)
diff --git a/qerror.c b/qerror.c
index e717496..6f9f49c 100644
--- a/qerror.c
+++ b/qerror.c
@@ -404,27 +404,14 @@ static const QErrorStringTable *get_desc_no_fail(const
char *fmt)
/**
* qerror_from_info(): Create a new QError from error information
*
- * The information consists of:
- *
- * - file the file name of where the error occurred
- * - linenr the line number of where the error occurred
- * - func the function name of where the error occurred
- * - fmtJSON printf-like dictionary, there must exist keys 'class' and
- * 'data'
- * - va va_list of all arguments specified by fmt
- *
* Return strong reference.
*/
-static QError *qerror_from_info(const char *file, int linenr, const char *func,
-const char *fmt, va_list *va)
+static QError *qerror_from_info(const char *fmt, va_list *va)
{
QError *qerr;
qerr = qerror_new();
loc_save(&qerr->loc);
-qerr->linenr = linenr;
-qerr->file = file;
-qerr->func = func;
qerr->error = error_obj_from_fmt_no_fail(fmt, va);
qerr->entry = get_desc_no_fail(fmt);
@@ -545,14 +532,13 @@ static void qerror_print(QError *qerror)
QDECREF(qstring);
}
-void qerror_report_internal(const char *file, int linenr, const char *func,
-const char *fmt, ...)
+void qerror_report(const char *fmt, ...)
{
va_list va;
QError *qerror;
va_start(va, fmt);
-qerror = qerror_from_info(file, linenr, func, fmt, &va);
+qerror = qerror_from_info(fmt, &va);
va_end(va);
if (monitor_cur_is_qmp()) {
diff --git a/qerror.h b/qerror.h
index fe8870c..3c0b14c 100644
--- a/qerror.h
+++ b/qerror.h
@@ -27,20 +27,14 @@ typedef struct QError {
QObject_HEAD;
QDict *error;
Location loc;
-int linenr;
-const char *file;
-const char *func;
const QErrorStringTable *entry;
} QError;
QString *qerror_human(const QError *qerror);
-void qerror_report_internal(const char *file, int linenr, const char *func,
-const char *fmt, ...) GCC_FMT_ATTR(4, 5);
+void qerror_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
void qerror_report_err(Error *err);
void assert_no_error(Error *err);
QString *qerror_format(const char *fmt, QDict *error);
-#define qerror_report(fmt, ...) \
-qerror_report_internal(__FILE__, __LINE__, __func__, fmt, ## __VA_ARGS__)
/*
* QError class list
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 35/35] docs: writing-qmp-commands.txt: update error section
Add information about the new error format and improve the text a bit.
Signed-off-by: Luiz Capitulino
---
docs/writing-qmp-commands.txt | 47 +--
1 file changed, 27 insertions(+), 20 deletions(-)
diff --git a/docs/writing-qmp-commands.txt b/docs/writing-qmp-commands.txt
index 0ad51aa..10ecd97 100644
--- a/docs/writing-qmp-commands.txt
+++ b/docs/writing-qmp-commands.txt
@@ -210,19 +210,17 @@ if you don't see these strings, then something went wrong.
=== Errors ===
QMP commands should use the error interface exported by the error.h header
-file. The basic function used to set an error is the error_set() one.
+file. Basically, errors are set by calling the error_set() function.
Let's say we don't accept the string "message" to contain the word "love". If
-it does contain it, we want the "hello-world" command to the return the
-InvalidParameter error.
-
-Only one change is required, and it's in the C implementation:
+it does contain it, we want the "hello-world" command to return an error:
void qmp_hello_world(bool has_message, const char *message, Error **errp)
{
if (has_message) {
if (strstr(message, "love")) {
-error_set(errp, QERR_INVALID_PARAMETER, "message");
+error_set(errp, ERROR_CLASS_GENERIC_ERROR,
+ "the word 'love' is not allowed");
return;
}
printf("%s\n", message);
@@ -231,30 +229,40 @@ void qmp_hello_world(bool has_message, const char
*message, Error **errp)
}
}
-Let's test it. Build qemu, run it as defined in the "Testing" section, and
-then issue the following command:
+The first argument to the error_set() function is the Error pointer to pointer,
+which is passed to all QMP functions. The second argument is a ErrorClass
+value, which should be ERROR_CLASS_GENERIC_ERROR most of the time (more
+details about error classes are given below). The third argument is a human
+description of the error, this is a free-form printf-like string.
+
+Let's test the example above. Build qemu, run it as defined in the "Testing"
+section, and then issue the following command:
-{ "execute": "hello-world", "arguments": { "message": "we love qemu" } }
+{ "execute": "hello-world", "arguments": { "message": "all you need is love" }
}
The QMP server's response should be:
{
"error": {
-"class": "InvalidParameter",
-"desc": "Invalid parameter 'message'",
-"data": {
-"name": "message"
-}
+"class": "GenericError",
+"desc": "the word 'love' is not allowed"
}
}
-Which is the InvalidParameter error.
+As a general rule, all QMP errors should use ERROR_CLASS_GENERIC_ERROR. There
+are two exceptions to this rule:
+
+ 1. A non-generic ErrorClass value exists* for the failure you want to report
+(eg. DeviceNotFound)
+
+ 2. Management applications have to take special action on the failure you
+want to report, hence you have to add a new ErrorClass value so that they
+can check for it
-When you have to return an error but you're unsure what error to return or
-which arguments an error takes, you should look at the qerror.h file. Note
-that you might be required to add new errors if needed.
+If the failure you want to report doesn't fall in one of the two cases above,
+just report ERROR_CLASS_GENERIC_ERROR.
-FIXME: describe better the error API and how to add new errors.
+ * All existing ErrorClass values are defined in the qapi-schema.json file
=== Command Documentation ===
@@ -275,7 +283,6 @@ here goes "hello-world"'s new entry for the
qapi-schema.json file:
# @message: #optional string to be printed
#
# Returns: Nothing on success.
-# If @message contains "love", InvalidParameter
#
# Notes: if @message is not provided, the "Hello, world" string will
#be printed instead
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 18/35] hmp: hmp.h: include qdict.h
hmp.h is relying on qdict.h being provided by qapi-types.h. Fix this, as a future commit will change qapi-types.h not to provide qdict.h. Signed-off-by: Luiz Capitulino --- hmp.h | 1 + 1 file changed, 1 insertion(+) diff --git a/hmp.h b/hmp.h index 8d2b0d7..3275522 100644 --- a/hmp.h +++ b/hmp.h @@ -16,6 +16,7 @@ #include "qemu-common.h" #include "qapi-types.h" +#include "qdict.h" void hmp_info_name(Monitor *mon); void hmp_info_version(Monitor *mon); -- 1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 34/35] error, qerror: drop QDict member
Used to store error information, but it's unused now.
Signed-off-by: Luiz Capitulino
---
error.c | 4
qerror.c | 4
qerror.h | 1 -
3 files changed, 9 deletions(-)
diff --git a/error.c b/error.c
index 0e10373..1f05fc4 100644
--- a/error.c
+++ b/error.c
@@ -19,7 +19,6 @@
struct Error
{
-QDict *obj;
char *msg;
ErrorClass err_class;
};
@@ -51,8 +50,6 @@ Error *error_copy(const Error *err)
err_new = g_malloc0(sizeof(*err));
err_new->msg = g_strdup(err->msg);
err_new->err_class = err->err_class;
-err_new->obj = err->obj;
-QINCREF(err_new->obj);
return err_new;
}
@@ -75,7 +72,6 @@ const char *error_get_pretty(Error *err)
void error_free(Error *err)
{
if (err) {
-QDECREF(err->obj);
g_free(err->msg);
g_free(err);
}
diff --git a/qerror.c b/qerror.c
index ccc52be..0818504 100644
--- a/qerror.c
+++ b/qerror.c
@@ -100,7 +100,6 @@ void qerror_report(ErrorClass eclass, const char *fmt, ...)
/* Evil... */
struct Error
{
-QDict *obj;
char *msg;
ErrorClass err_class;
};
@@ -111,8 +110,6 @@ void qerror_report_err(Error *err)
qerr = qerror_new();
loc_save(&qerr->loc);
-QINCREF(err->obj);
-qerr->error = err->obj;
qerr->err_msg = g_strdup(err->msg);
qerr->err_class = err->err_class;
@@ -154,7 +151,6 @@ static void qerror_destroy_obj(QObject *obj)
assert(obj != NULL);
qerr = qobject_to_qerror(obj);
-QDECREF(qerr->error);
g_free(qerr->err_msg);
g_free(qerr);
}
diff --git a/qerror.h b/qerror.h
index c5ad29f..d0a76a4 100644
--- a/qerror.h
+++ b/qerror.h
@@ -21,7 +21,6 @@
typedef struct QError {
QObject_HEAD;
-QDict *error;
Location loc;
char *err_msg;
ErrorClass err_class;
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 23/35] qerror: qerror_table: don't use C99 struct initializers
This allows for changing QERR_ macros to initialize two struct members
at the same time. See next commit for more details.
Signed-off-by: Luiz Capitulino
---
qerror.c | 276 +++
qerror.h | 2 +-
2 files changed, 139 insertions(+), 139 deletions(-)
diff --git a/qerror.c b/qerror.c
index 452ec69..ff460b0 100644
--- a/qerror.c
+++ b/qerror.c
@@ -44,285 +44,285 @@ static const QType qerror_type = {
*/
static const QErrorStringTable qerror_table[] = {
{
-.error_fmt = QERR_ADD_CLIENT_FAILED,
-.desc = "Could not add client",
+ QERR_ADD_CLIENT_FAILED,
+ "Could not add client",
},
{
-.error_fmt = QERR_AMBIGUOUS_PATH,
-.desc = "Path '%(path)' does not uniquely identify an object"
+ QERR_AMBIGUOUS_PATH,
+ "Path '%(path)' does not uniquely identify an object"
},
{
-.error_fmt = QERR_BAD_BUS_FOR_DEVICE,
-.desc = "Device '%(device)' can't go on a %(bad_bus_type) bus",
+ QERR_BAD_BUS_FOR_DEVICE,
+ "Device '%(device)' can't go on a %(bad_bus_type) bus",
},
{
-.error_fmt = QERR_BASE_NOT_FOUND,
-.desc = "Base '%(base)' not found",
+ QERR_BASE_NOT_FOUND,
+ "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)'",
+ QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
+ "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",
+ QERR_BUS_NO_HOTPLUG,
+ "Bus '%(bus)' does not support hotplugging",
},
{
-.error_fmt = QERR_BUS_NOT_FOUND,
-.desc = "Bus '%(bus)' not found",
+ QERR_BUS_NOT_FOUND,
+ "Bus '%(bus)' not found",
},
{
-.error_fmt = QERR_COMMAND_DISABLED,
-.desc = "The command %(name) has been disabled for this instance",
+ QERR_COMMAND_DISABLED,
+ "The command %(name) has been disabled for this instance",
},
{
-.error_fmt = QERR_COMMAND_NOT_FOUND,
-.desc = "The command %(name) has not been found",
+ QERR_COMMAND_NOT_FOUND,
+ "The command %(name) has not been found",
},
{
-.error_fmt = QERR_DEVICE_ENCRYPTED,
-.desc = "'%(device)' (%(filename)) is encrypted",
+ QERR_DEVICE_ENCRYPTED,
+ "'%(device)' (%(filename)) is encrypted",
},
{
-.error_fmt = QERR_DEVICE_FEATURE_BLOCKS_MIGRATION,
-.desc = "Migration is disabled when using feature '%(feature)' in
device '%(device)'",
+ QERR_DEVICE_FEATURE_BLOCKS_MIGRATION,
+ "Migration is disabled when using feature '%(feature)' in device
'%(device)'",
},
{
-.error_fmt = QERR_DEVICE_HAS_NO_MEDIUM,
-.desc = "Device '%(device)' has no medium",
+ QERR_DEVICE_HAS_NO_MEDIUM,
+ "Device '%(device)' has no medium",
},
{
-.error_fmt = QERR_DEVICE_INIT_FAILED,
-.desc = "Device '%(device)' could not be initialized",
+ QERR_DEVICE_INIT_FAILED,
+ "Device '%(device)' could not be initialized",
},
{
-.error_fmt = QERR_DEVICE_IN_USE,
-.desc = "Device '%(device)' is in use",
+ QERR_DEVICE_IN_USE,
+ "Device '%(device)' is in use",
},
{
-.error_fmt = QERR_DEVICE_IS_READ_ONLY,
-.desc = "Device '%(device)' is read only",
+ QERR_DEVICE_IS_READ_ONLY,
+ "Device '%(device)' is read only",
},
{
-.error_fmt = QERR_DEVICE_LOCKED,
-.desc = "Device '%(device)' is locked",
+ QERR_DEVICE_LOCKED,
+ "Device '%(device)' is locked",
},
{
-.error_fmt = QERR_DEVICE_MULTIPLE_BUSSES,
-.desc = "Device '%(device)' has multiple child busses",
+ QERR_DEVICE_MULTIPLE_BUSSES,
+ "Device '%(device)' has multiple child busses",
},
{
-.error_
[Qemu-devel] [PATCH 21/35] qapi: don't convert enum strings to lowercase
Next commit will introduce enum strings in camel case.
Signed-off-by: Luiz Capitulino
---
scripts/qapi-types.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 9b7da96..cf601ae 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -70,7 +70,7 @@ const char *%(name)s_lookup[] = {
ret += mcgen('''
"%(value)s",
''',
- value=value.lower())
+ value=value)
ret += mcgen('''
NULL,
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 02/35] qerror: QERR_AMBIGUOUS_PATH: drop %(object) from human msg
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
---
qerror.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qerror.c b/qerror.c
index 92c4eff..082de98 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 an object"
},
{
.error_fmt = QERR_BAD_BUS_FOR_DEVICE,
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 25/35] qerror: add proper ErrorClass value for QERR_ macros
This commit replaces the place holder value for the ErrorClass
argument with a proper ErrorClass value for all QERR_ macros.
All current errors are mapped to GenericError, except for errors
CommandNotFound, DeviceEncrypted, DeviceNotActive, DeviceNotFound,
KVMMissingCap and MigrationExpected, which are maintained as they
are today.
Signed-off-by: Luiz Capitulino
---
qerror.h | 140 +++
1 file changed, 70 insertions(+), 70 deletions(-)
diff --git a/qerror.h b/qerror.h
index bcc93f8..4f92218 100644
--- a/qerror.h
+++ b/qerror.h
@@ -45,214 +45,214 @@ char *qerror_format(const char *fmt, QDict *error);
* Use scripts/check-qerror.sh to check.
*/
#define QERR_ADD_CLIENT_FAILED \
--1, "{ 'class': 'AddClientFailed', 'data': {} }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'AddClientFailed', 'data': {} }"
#define QERR_AMBIGUOUS_PATH \
--1, "{ 'class': 'AmbiguousPath', 'data': { 'path': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'AmbiguousPath', 'data': { 'path':
%s } }"
#define QERR_BAD_BUS_FOR_DEVICE \
--1, "{ 'class': 'BadBusForDevice', 'data': { 'device': %s, 'bad_bus_type':
%s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BadBusForDevice', 'data': {
'device': %s, 'bad_bus_type': %s } }"
#define QERR_BASE_NOT_FOUND \
--1, "{ 'class': 'BaseNotFound', 'data': { 'base': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BaseNotFound', 'data': { 'base':
%s } }"
#define QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED \
--1, "{ 'class': 'BlockFormatFeatureNotSupported', 'data': { 'format': %s,
'name': %s, 'feature': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BlockFormatFeatureNotSupported',
'data': { 'format': %s, 'name': %s, 'feature': %s } }"
#define QERR_BUFFER_OVERRUN \
--1, "{ 'class': 'BufferOverrun', 'data': {} }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BufferOverrun', 'data': {} }"
#define QERR_BUS_NO_HOTPLUG \
--1, "{ 'class': 'BusNoHotplug', 'data': { 'bus': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BusNoHotplug', 'data': { 'bus': %s
} }"
#define QERR_BUS_NOT_FOUND \
--1, "{ 'class': 'BusNotFound', 'data': { 'bus': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BusNotFound', 'data': { 'bus': %s
} }"
#define QERR_COMMAND_DISABLED \
--1, "{ 'class': 'CommandDisabled', 'data': { 'name': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'CommandDisabled', 'data': {
'name': %s } }"
#define QERR_COMMAND_NOT_FOUND \
--1, "{ 'class': 'CommandNotFound', 'data': { 'name': %s } }"
+ERROR_CLASS_COMMAND_NOT_FOUND, "{ 'class': 'CommandNotFound', 'data': {
'name': %s } }"
#define QERR_DEVICE_ENCRYPTED \
--1, "{ 'class': 'DeviceEncrypted', 'data': { 'device': %s, 'filename': %s
} }"
+ERROR_CLASS_DEVICE_ENCRYPTED, "{ 'class': 'DeviceEncrypted', 'data': {
'device': %s, 'filename': %s } }"
#define QERR_DEVICE_FEATURE_BLOCKS_MIGRATION \
--1, "{ 'class': 'DeviceFeatureBlocksMigration', 'data': { 'device': %s,
'feature': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceFeatureBlocksMigration',
'data': { 'device': %s, 'feature': %s } }"
#define QERR_DEVICE_HAS_NO_MEDIUM \
--1, "{ 'class': 'DeviceHasNoMedium', 'data': { 'device': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceHasNoMedium', 'data': {
'device': %s } }"
#define QERR_DEVICE_INIT_FAILED \
--1, "{ 'class': 'DeviceInitFailed', 'data': { 'device': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceInitFaile
[Qemu-devel] [PATCH 17/35] block: block_int: include qerror.h
Several block/ files are relying on qerror.h being provided by qapi-types.h. Fix this, as a future commit will change qapi-types.h not to provide qerror.h. Signed-off-by: Luiz Capitulino --- block_int.h | 1 + 1 file changed, 1 insertion(+) diff --git a/block_int.h b/block_int.h index d72317f..00892e8 100644 --- a/block_int.h +++ b/block_int.h @@ -30,6 +30,7 @@ #include "qemu-coroutine.h" #include "qemu-timer.h" #include "qapi-types.h" +#include "qerror.h" #define BLOCK_FLAG_ENCRYPT 1 #define BLOCK_FLAG_COMPAT6 4 -- 1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 20/35] qapi: generate correct enum names for camel case enums
An enum like GenericError in the schema, should generate
GENERIC_ERROR and not GENERICERROR.
Signed-off-by: Luiz Capitulino
---
scripts/qapi-types.py | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 3ed9f04..9b7da96 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -79,6 +79,16 @@ const char *%(name)s_lookup[] = {
''')
return ret
+def generate_enum_name(name):
+if name.isupper():
+return c_fun(name)
+new_name = ''
+for c in c_fun(name):
+if c.isupper():
+new_name += '_'
+new_name += c
+return new_name.lstrip('_').upper()
+
def generate_enum(name, values):
lookup_decl = mcgen('''
extern const char *%(name)s_lookup[];
@@ -100,7 +110,7 @@ typedef enum %(name)s
%(abbrev)s_%(value)s = %(i)d,
''',
abbrev=de_camel_case(name).upper(),
- value=c_fun(value).upper(),
+ value=generate_enum_name(value),
i=i)
i += 1
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 24/35] error, qerror: add ErrorClass argument to error functions
The new argument is added to functions qerror_report() and error_set().
It's stored in Error and QError. qerror_report_err() is also updated to
take care of it.
The QERR_ macros are changed to contain a place holder value for the
new argument, so that the value is used on all current calls to
qerror_report() and error_set() (and also to initialize qerror_table[]).
Next commit will update the QERR_ macros with a proper ErrorClass
value.
Signed-off-by: Luiz Capitulino
---
error.c | 8 +++-
error.h | 5 ++-
qerror.c | 10 +++--
qerror.h | 145 ---
4 files changed, 90 insertions(+), 78 deletions(-)
diff --git a/error.c b/error.c
index 2ade99b..648706a 100644
--- a/error.c
+++ b/error.c
@@ -14,6 +14,7 @@
#include "error.h"
#include "qjson.h"
#include "qdict.h"
+#include "qapi-types.h"
#include "error_int.h"
#include "qerror.h"
@@ -21,9 +22,10 @@ struct Error
{
QDict *obj;
char *msg;
+ErrorClass err_class;
};
-void error_set(Error **errp, const char *fmt, ...)
+void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
{
Error *err;
va_list ap;
@@ -39,6 +41,7 @@ void error_set(Error **errp, const char *fmt, ...)
err->obj = qobject_to_qdict(qobject_from_jsonv(fmt, &ap));
va_end(ap);
err->msg = qerror_format(fmt, err->obj);
+err->err_class = err_class;
*errp = err;
}
@@ -49,6 +52,7 @@ Error *error_copy(const Error *err)
err_new = g_malloc0(sizeof(*err));
err_new->msg = g_strdup(err->msg);
+err_new->err_class = err->err_class;
err_new->obj = err->obj;
QINCREF(err_new->obj);
@@ -97,7 +101,7 @@ void error_free(Error *err)
}
}
-bool error_is_type(Error *err, const char *fmt)
+bool error_is_type(Error *err, ErrorClass err_class, const char *fmt)
{
const char *error_class;
char *ptr;
diff --git a/error.h b/error.h
index 3d9d96d..9678752 100644
--- a/error.h
+++ b/error.h
@@ -13,6 +13,7 @@
#define ERROR_H
#include "compiler.h"
+#include "qapi-types.h"
#include
/**
@@ -26,7 +27,7 @@ typedef struct Error Error;
* Currently, qerror.h defines these error formats. This function is not
* meant to be used outside of QEMU.
*/
-void error_set(Error **err, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
+void error_set(Error **err, ErrorClass err_class, const char *fmt, ...)
GCC_FMT_ATTR(3, 4);
/**
* Returns true if an indirect pointer to an error is pointing to a valid
@@ -70,6 +71,6 @@ void error_free(Error *err);
* Determine if an error is of a speific type (based on the qerror format).
* Non-QEMU users should get the `class' field to identify the error type.
*/
-bool error_is_type(Error *err, const char *fmt);
+bool error_is_type(Error *err, ErrorClass err_class, const char *fmt);
#endif
diff --git a/qerror.c b/qerror.c
index ff460b0..0bf8aec 100644
--- a/qerror.c
+++ b/qerror.c
@@ -386,13 +386,15 @@ static QDict *error_obj_from_fmt_no_fail(const char *fmt,
va_list *va)
*
* Return strong reference.
*/
-static QError *qerror_from_info(const char *fmt, va_list *va)
+static QError *qerror_from_info(ErrorClass err_class, const char *fmt,
+va_list *va)
{
QError *qerr;
qerr = qerror_new();
loc_save(&qerr->loc);
+qerr->err_class = err_class;
qerr->error = error_obj_from_fmt_no_fail(fmt, va);
qerr->err_msg = qerror_format(fmt, qerr->error);
@@ -518,13 +520,13 @@ static void qerror_print(QError *qerror)
QDECREF(qstring);
}
-void qerror_report(const char *fmt, ...)
+void qerror_report(ErrorClass eclass, const char *fmt, ...)
{
va_list va;
QError *qerror;
va_start(va, fmt);
-qerror = qerror_from_info(fmt, &va);
+qerror = qerror_from_info(eclass, fmt, &va);
va_end(va);
if (monitor_cur_is_qmp()) {
@@ -540,6 +542,7 @@ struct Error
{
QDict *obj;
char *msg;
+ErrorClass err_class;
};
void qerror_report_err(Error *err)
@@ -551,6 +554,7 @@ void qerror_report_err(Error *err)
QINCREF(err->obj);
qerr->error = err->obj;
qerr->err_msg = g_strdup(err->msg);
+qerr->err_class = err->err_class;
if (monitor_cur_is_qmp()) {
monitor_set_error(cur_mon, qerr);
diff --git a/qerror.h b/qerror.h
index 2e6a49d..bcc93f8 100644
--- a/qerror.h
+++ b/qerror.h
@@ -16,9 +16,11 @@
#include "qstring.h"
#include "qemu-error.h"
#include "error.h"
+#include "qapi-types.h"
#include
typedef struct QErrorStringTable {
+ErrorClass err_class;
const char *error_fmt;
const char *desc;
} QErrorStringTable;
@@ -28,10 +30,11 @@ typedef struct QError {
QDict *error;
Location loc;
char *err_msg;
+ErrorClass err_class;
} QError;
QString *qe
[Qemu-devel] [PATCH 03/35] qerror: QERR_DEVICE_ENCRYPTED: change error message
Match what HMP commands print on DeviceEncrypted errors.
Signed-off-by: Luiz Capitulino
---
qerror.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qerror.c b/qerror.c
index 082de98..de0a79e 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)' (%(filename)) is encrypted",
},
{
.error_fmt = QERR_DEVICE_FEATURE_BLOCKS_MIGRATION,
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 16/35] qerror: drop QERR_SOCKET_CONNECT_IN_PROGRESS
Unused since last commit.
Signed-off-by: Luiz Capitulino
---
qerror.c | 4
qerror.h | 3 ---
2 files changed, 7 deletions(-)
diff --git a/qerror.c b/qerror.c
index 5d38428..452ec69 100644
--- a/qerror.c
+++ b/qerror.c
@@ -309,10 +309,6 @@ static const QErrorStringTable qerror_table[] = {
.desc = "Could not start VNC server on %(target)",
},
{
-.error_fmt = QERR_SOCKET_CONNECT_IN_PROGRESS,
-.desc = "Connection can not be completed immediately",
-},
-{
.error_fmt = QERR_SOCKET_CONNECT_FAILED,
.desc = "Failed to connect to socket",
},
diff --git a/qerror.h b/qerror.h
index de8497d..52ce58d 100644
--- a/qerror.h
+++ b/qerror.h
@@ -240,9 +240,6 @@ char *qerror_format(const char *fmt, QDict *error);
#define QERR_VNC_SERVER_FAILED \
"{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
-#define QERR_SOCKET_CONNECT_IN_PROGRESS \
-"{ 'class': 'SockConnectInprogress', 'data': {} }"
-
#define QERR_SOCKET_CONNECT_FAILED \
"{ 'class': 'SockConnectFailed', 'data': {} }"
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 06/35] qerror: avoid passing qerr pointer
Helps dropping/modifying qerror functions.
Signed-off-by: Luiz Capitulino
---
qerror.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/qerror.c b/qerror.c
index 7cb7c12..e717496 100644
--- a/qerror.c
+++ b/qerror.c
@@ -346,10 +346,10 @@ static QError *qerror_new(void)
return qerr;
}
-static void GCC_FMT_ATTR(2, 0) qerror_set_data(QError *qerr,
- const char *fmt, va_list *va)
+static QDict *error_obj_from_fmt_no_fail(const char *fmt, va_list *va)
{
QObject *obj;
+QDict *ret;
obj = qobject_from_jsonv(fmt, va);
if (!obj) {
@@ -361,9 +361,8 @@ static void GCC_FMT_ATTR(2, 0) qerror_set_data(QError *qerr,
abort();
}
-qerr->error = qobject_to_qdict(obj);
-
-obj = qdict_get(qerr->error, "class");
+ret = qobject_to_qdict(obj);
+obj = qdict_get(ret, "class");
if (!obj) {
fprintf(stderr, "missing 'class' key in '%s'\n", fmt);
abort();
@@ -372,8 +371,8 @@ static void GCC_FMT_ATTR(2, 0) qerror_set_data(QError *qerr,
fprintf(stderr, "'class' key value should be a string in '%s'\n", fmt);
abort();
}
-
-obj = qdict_get(qerr->error, "data");
+
+obj = qdict_get(ret, "data");
if (!obj) {
fprintf(stderr, "missing 'data' key in '%s'\n", fmt);
abort();
@@ -382,9 +381,11 @@ static void GCC_FMT_ATTR(2, 0) qerror_set_data(QError
*qerr,
fprintf(stderr, "'data' key value should be a dict in '%s'\n", fmt);
abort();
}
+
+return ret;
}
-static void qerror_set_desc(QError *qerr, const char *fmt)
+static const QErrorStringTable *get_desc_no_fail(const char *fmt)
{
int i;
@@ -392,8 +393,7 @@ static void qerror_set_desc(QError *qerr, const char *fmt)
for (i = 0; qerror_table[i].error_fmt; i++) {
if (strcmp(qerror_table[i].error_fmt, fmt) == 0) {
-qerr->entry = &qerror_table[i];
-return;
+return &qerror_table[i];
}
}
@@ -426,8 +426,8 @@ static QError *qerror_from_info(const char *file, int
linenr, const char *func,
qerr->file = file;
qerr->func = func;
-qerror_set_data(qerr, fmt, va);
-qerror_set_desc(qerr, fmt);
+qerr->error = error_obj_from_fmt_no_fail(fmt, va);
+qerr->entry = get_desc_no_fail(fmt);
return qerr;
}
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 13/35] hmp_change(): don't access DeviceEncrypted's data
It's not needed. The device name is already known and
monitor_read_block_device_key() knows how to do the rest. This overly
simplifies hmp_change().
Signed-off-by: Luiz Capitulino
---
hmp.c | 29 ++---
1 file changed, 2 insertions(+), 27 deletions(-)
diff --git a/hmp.c b/hmp.c
index bfcc032..3a9688d 100644
--- a/hmp.c
+++ b/hmp.c
@@ -782,22 +782,6 @@ static void hmp_change_read_arg(Monitor *mon, const char
*password,
monitor_read_command(mon, 1);
}
-static void cb_hmp_change_bdrv_pwd(Monitor *mon, const char *password,
- void *opaque)
-{
-Error *encryption_err = opaque;
-Error *err = NULL;
-const char *device;
-
-device = error_get_field(encryption_err, "device");
-
-qmp_block_passwd(device, password, &err);
-hmp_handle_error(mon, &err);
-error_free(encryption_err);
-
-monitor_read_command(mon, 1);
-}
-
void hmp_change(Monitor *mon, const QDict *qdict)
{
const char *device = qdict_get_str(qdict, "device");
@@ -816,17 +800,8 @@ void hmp_change(Monitor *mon, const QDict *qdict)
qmp_change(device, target, !!arg, arg, &err);
if (error_is_type(err, QERR_DEVICE_ENCRYPTED)) {
-monitor_printf(mon, "%s (%s) is encrypted.\n",
- error_get_field(err, "device"),
- error_get_field(err, "filename"));
-if (!monitor_get_rs(mon)) {
-monitor_printf(mon,
-"terminal does not support password prompting\n");
-error_free(err);
-return;
-}
-readline_start(monitor_get_rs(mon), "Password: ", 1,
- cb_hmp_change_bdrv_pwd, err);
+error_free(err);
+monitor_read_block_device_key(mon, device, NULL, NULL);
return;
}
hmp_handle_error(mon, &err);
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 04/35] qerror: reduce public exposure
qerror will be dropped in a near future, let's reduce its public
exposure by making functions only used in qerror.c static.
Signed-off-by: Luiz Capitulino
---
qerror.c | 10 +-
qerror.h | 5 -
2 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/qerror.c b/qerror.c
index de0a79e..bfb875a 100644
--- a/qerror.c
+++ b/qerror.c
@@ -336,7 +336,7 @@ static const QErrorStringTable qerror_table[] = {
*
* Return strong reference.
*/
-QError *qerror_new(void)
+static QError *qerror_new(void)
{
QError *qerr;
@@ -424,8 +424,8 @@ static void qerror_set_desc(QError *qerr, const char *fmt)
*
* Return strong reference.
*/
-QError *qerror_from_info(const char *file, int linenr, const char *func,
- const char *fmt, va_list *va)
+static QError *qerror_from_info(const char *file, int linenr, const char *func,
+const char *fmt, va_list *va)
{
QError *qerr;
@@ -549,7 +549,7 @@ QString *qerror_human(const QError *qerror)
* it uses error_report() for this, so that the output is routed to the right
* place (ie. stderr or Monitor's device).
*/
-void qerror_print(QError *qerror)
+static void qerror_print(QError *qerror)
{
QString *qstring = qerror_human(qerror);
loc_push_restore(&qerror->loc);
@@ -620,7 +620,7 @@ void assert_no_error(Error *err)
/**
* qobject_to_qerror(): Convert a QObject into a QError
*/
-QError *qobject_to_qerror(const QObject *obj)
+static QError *qobject_to_qerror(const QObject *obj)
{
if (qobject_type(obj) != QTYPE_QERROR) {
return NULL;
diff --git a/qerror.h b/qerror.h
index b4c8758..fe8870c 100644
--- a/qerror.h
+++ b/qerror.h
@@ -33,11 +33,7 @@ typedef struct QError {
const QErrorStringTable *entry;
} QError;
-QError *qerror_new(void);
-QError *qerror_from_info(const char *file, int linenr, const char *func,
- const char *fmt, va_list *va) GCC_FMT_ATTR(4, 0);
QString *qerror_human(const QError *qerror);
-void qerror_print(QError *qerror);
void qerror_report_internal(const char *file, int linenr, const char *func,
const char *fmt, ...) GCC_FMT_ATTR(4, 5);
void qerror_report_err(Error *err);
@@ -45,7 +41,6 @@ void assert_no_error(Error *err);
QString *qerror_format(const char *fmt, QDict *error);
#define qerror_report(fmt, ...) \
qerror_report_internal(__FILE__, __LINE__, __func__, fmt, ## __VA_ARGS__)
-QError *qobject_to_qerror(const QObject *obj);
/*
* QError class list
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH v2 00/35]: add new error format
v2
o rebase on top of master
o fix linux-user build breakage
o maintain DeviceEncrypted error and let hmp_change() use it
o drop patch that changed inet_connect() and inet_connect_opts()
to return -errno
o Simplified tcp_start_outgoing_migration() error handling
o use g_strdup_vprintf() (instead of vsnprintf() plus g_strdup())
o drop errors from command's documentation in qapi-schema.json
o update docs/writing-qmp-commands.txt
This series implements the 'Plan for error handling in QMP' as described
by Anthony in this email:
http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg03764.html
Basically, this replaces almost all error classes by GenericError (the
exception are a few error classes used by libvirt) and drops the error
data memeber. This also adds a free form string to error_set().
On the wire, we go from:
{ "error": { "class": "DeviceNotRemovable",
"data": { "device": "virtio0" },
"desc": "Device 'virtio0' is not removable" } }
to:
{ "error": { "class": "GenericError",
"desc": "Device 'virtio0' is not removable" } }
Internally, we go from:
void error_set(Error **err, const char *fmt, ...);
to:
void error_set(Error **err, ErrorClass err_class, const char *fmt, ...);
Makefile.objs | 1 +
QMP/qmp-spec.txt | 10 +-
block.c | 1 +
block_int.h | 1 +
configure | 10 -
docs/writing-qmp-commands.txt | 47 ++--
error.c | 93 +---
error.h | 34 +--
error_int.h | 29 ---
hmp.c | 75 +++---
hmp.h | 1 +
migration-tcp.c | 22 +-
monitor.c | 83 ++-
nbd.c | 2 +-
qapi-schema.json | 93 +++-
qapi/qmp-core.h | 1 +
qapi/qmp-dispatch.c | 11 +-
qemu-char.c | 2 +-
qemu-ga.c | 5 +-
qemu-sockets.c| 14 +-
qemu_socket.h | 4 +-
qerror.c | 516 ++
qerror.h | 168 ++
qmp-commands.hx | 4 +-
scripts/qapi-types.py | 17 +-
ui/vnc.c | 2 +-
26 files changed, 272 insertions(+), 974 deletions(-)
[Qemu-devel] [PATCH 14/35] net: inet_connect(), inet_connect_opts(): add in_progress argument
It's used to indicate the special case where a valid file-descriptor
is returned (ie. success) but the connection can't be completed
w/o blocking.
Signed-off-by: Luiz Capitulino
---
migration-tcp.c | 2 +-
nbd.c | 2 +-
qemu-char.c | 2 +-
qemu-sockets.c | 14 +++---
qemu_socket.h | 4 ++--
ui/vnc.c| 2 +-
6 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/migration-tcp.c b/migration-tcp.c
index 440804d..18944a4 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -86,7 +86,7 @@ int tcp_start_outgoing_migration(MigrationState *s, const
char *host_port,
s->write = socket_write;
s->close = tcp_close;
-s->fd = inet_connect(host_port, false, errp);
+s->fd = inet_connect(host_port, false, NULL, errp);
if (!error_is_set(errp)) {
migrate_fd_connect(s);
diff --git a/nbd.c b/nbd.c
index dc0adf9..0dd60c5 100644
--- a/nbd.c
+++ b/nbd.c
@@ -162,7 +162,7 @@ int tcp_socket_outgoing(const char *address, uint16_t port)
int tcp_socket_outgoing_spec(const char *address_and_port)
{
-return inet_connect(address_and_port, true, NULL);
+return inet_connect(address_and_port, true, NULL, NULL);
}
int tcp_socket_incoming(const char *address, uint16_t port)
diff --git a/qemu-char.c b/qemu-char.c
index c2aaaee..382c71e 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2446,7 +2446,7 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts
*opts)
if (is_listen) {
fd = inet_listen_opts(opts, 0, NULL);
} else {
-fd = inet_connect_opts(opts, NULL);
+fd = inet_connect_opts(opts, NULL, NULL);
}
}
if (fd < 0) {
diff --git a/qemu-sockets.c b/qemu-sockets.c
index beb2bb6..9cb47d4 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -209,7 +209,7 @@ listen:
return slisten;
}
-int inet_connect_opts(QemuOpts *opts, Error **errp)
+int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp)
{
struct addrinfo ai,*res,*e;
const char *addr;
@@ -224,6 +224,10 @@ int inet_connect_opts(QemuOpts *opts, Error **errp)
ai.ai_family = PF_UNSPEC;
ai.ai_socktype = SOCK_STREAM;
+if (in_progress) {
+*in_progress = false;
+}
+
addr = qemu_opt_get(opts, "host");
port = qemu_opt_get(opts, "port");
block = qemu_opt_get_bool(opts, "block", 0);
@@ -277,6 +281,10 @@ int inet_connect_opts(QemuOpts *opts, Error **errp)
#else
if (!block && (rc == -EINPROGRESS)) {
#endif
+if (in_progress) {
+*in_progress = true;
+}
+
error_set(errp, QERR_SOCKET_CONNECT_IN_PROGRESS);
} else if (rc < 0) {
if (NULL == e->ai_next)
@@ -487,7 +495,7 @@ int inet_listen(const char *str, char *ostr, int olen,
return sock;
}
-int inet_connect(const char *str, bool block, Error **errp)
+int inet_connect(const char *str, bool block, bool *in_progress, Error **errp)
{
QemuOpts *opts;
int sock = -1;
@@ -497,7 +505,7 @@ int inet_connect(const char *str, bool block, Error **errp)
if (block) {
qemu_opt_set(opts, "block", "on");
}
-sock = inet_connect_opts(opts, errp);
+sock = inet_connect_opts(opts, in_progress, errp);
} else {
error_set(errp, QERR_SOCKET_CREATE_FAILED);
}
diff --git a/qemu_socket.h b/qemu_socket.h
index 4689ff3..30ae6af 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -42,8 +42,8 @@ int send_all(int fd, const void *buf, int len1);
int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp);
int inet_listen(const char *str, char *ostr, int olen,
int socktype, int port_offset, Error **errp);
-int inet_connect_opts(QemuOpts *opts, Error **errp);
-int inet_connect(const char *str, bool block, Error **errp);
+int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp);
+int inet_connect(const char *str, bool block, bool *in_progress, Error **errp);
int inet_dgram_opts(QemuOpts *opts);
const char *inet_strfamily(int family);
diff --git a/ui/vnc.c b/ui/vnc.c
index 312ad7f..385e345 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3061,7 +3061,7 @@ int vnc_display_open(DisplayState *ds, const char
*display)
if (strncmp(display, "unix:", 5) == 0)
vs->lsock = unix_connect(display+5);
else
-vs->lsock = inet_connect(display, true, NULL);
+vs->lsock = inet_connect(display, true, NULL, NULL);
if (-1 == vs->lsock) {
g_free(vs->display);
vs->display = NULL;
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 32/35] error, qerror: pass desc string to error calls
This commit changes all QERR_ macros to contain a human message (ie.
the desc string found in qerr_table[]) instead of a json dictionary
in string format.
Before this commit, error_set() and qerror_report() would receive
a json dictionary in string format and build a qobject from it. Now,
both function receive a human message instead and the qobject is
not built anymore.
Signed-off-by: Luiz Capitulino
---
error.c | 3 +-
error.h | 10 ++---
qerror.c | 42 +--
qerror.h | 141 +++
4 files changed, 77 insertions(+), 119 deletions(-)
diff --git a/error.c b/error.c
index 9d7b35f..0e10373 100644
--- a/error.c
+++ b/error.c
@@ -37,9 +37,8 @@ void error_set(Error **errp, ErrorClass err_class, const char
*fmt, ...)
err = g_malloc0(sizeof(*err));
va_start(ap, fmt);
-err->obj = qobject_to_qdict(qobject_from_jsonv(fmt, &ap));
+err->msg = g_strdup_vprintf(fmt, ap);
va_end(ap);
-err->msg = qerror_format(fmt, err->obj);
err->err_class = err_class;
*errp = err;
diff --git a/error.h b/error.h
index 5336fc5..96fc203 100644
--- a/error.h
+++ b/error.h
@@ -17,15 +17,15 @@
#include
/**
- * A class representing internal errors within QEMU. An error has a string
- * typename and optionally a set of named string parameters.
+ * A class representing internal errors within QEMU. An error has a ErrorClass
+ * code and a human message.
*/
typedef struct Error Error;
/**
- * Set an indirect pointer to an error given a printf-style format parameter.
- * Currently, qerror.h defines these error formats. This function is not
- * meant to be used outside of QEMU.
+ * Set an indirect pointer to an error given a ErrorClass value and a
+ * printf-style human message. This function is not meant to be used outside
+ * of QEMU.
*/
void error_set(Error **err, ErrorClass err_class, const char *fmt, ...)
GCC_FMT_ATTR(3, 4);
diff --git a/qerror.c b/qerror.c
index 0bf8aec..dda1427 100644
--- a/qerror.c
+++ b/qerror.c
@@ -342,45 +342,6 @@ static QError *qerror_new(void)
return qerr;
}
-static QDict *error_obj_from_fmt_no_fail(const char *fmt, va_list *va)
-{
-QObject *obj;
-QDict *ret;
-
-obj = qobject_from_jsonv(fmt, va);
-if (!obj) {
-fprintf(stderr, "invalid json in error dict '%s'\n", fmt);
-abort();
-}
-if (qobject_type(obj) != QTYPE_QDICT) {
-fprintf(stderr, "error is not a dict '%s'\n", fmt);
-abort();
-}
-
-ret = qobject_to_qdict(obj);
-obj = qdict_get(ret, "class");
-if (!obj) {
-fprintf(stderr, "missing 'class' key in '%s'\n", fmt);
-abort();
-}
-if (qobject_type(obj) != QTYPE_QSTRING) {
-fprintf(stderr, "'class' key value should be a string in '%s'\n", fmt);
-abort();
-}
-
-obj = qdict_get(ret, "data");
-if (!obj) {
-fprintf(stderr, "missing 'data' key in '%s'\n", fmt);
-abort();
-}
-if (qobject_type(obj) != QTYPE_QDICT) {
-fprintf(stderr, "'data' key value should be a dict in '%s'\n", fmt);
-abort();
-}
-
-return ret;
-}
-
/**
* qerror_from_info(): Create a new QError from error information
*
@@ -394,9 +355,8 @@ static QError *qerror_from_info(ErrorClass err_class, const
char *fmt,
qerr = qerror_new();
loc_save(&qerr->loc);
+qerr->err_msg = g_strdup_vprintf(fmt, *va);
qerr->err_class = err_class;
-qerr->error = error_obj_from_fmt_no_fail(fmt, va);
-qerr->err_msg = qerror_format(fmt, qerr->error);
return qerr;
}
diff --git a/qerror.h b/qerror.h
index 4f92218..057a8f2 100644
--- a/qerror.h
+++ b/qerror.h
@@ -45,214 +45,213 @@ char *qerror_format(const char *fmt, QDict *error);
* Use scripts/check-qerror.sh to check.
*/
#define QERR_ADD_CLIENT_FAILED \
-ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'AddClientFailed', 'data': {} }"
+ERROR_CLASS_GENERIC_ERROR, "Could not add client"
#define QERR_AMBIGUOUS_PATH \
-ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'AmbiguousPath', 'data': { 'path':
%s } }"
+ERROR_CLASS_GENERIC_ERROR, "Path '%s' does not uniquely identify an object"
#define QERR_BAD_BUS_FOR_DEVICE \
-ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BadBusForDevice', 'data': {
'device': %s, 'bad_bus_type': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "Device '%s' can't go on a %s bus"
#define QERR_BASE_NOT_FOUND \
-ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BaseNotFound', 'data': {
[Qemu-devel] [PATCH 31/35] error: drop error_get_qobject()/error_set_qobject()
error_get_qobject() is unused since last commit, error_set_qobject()
has never been used. Also drops error_int.h.
Signed-off-by: Luiz Capitulino
---
error.c | 20
error_int.h | 28
qapi/qmp-dispatch.c | 1 -
qemu-ga.c | 1 -
4 files changed, 50 deletions(-)
delete mode 100644 error_int.h
diff --git a/error.c b/error.c
index b1d5131..9d7b35f 100644
--- a/error.c
+++ b/error.c
@@ -15,7 +15,6 @@
#include "qjson.h"
#include "qdict.h"
#include "qapi-types.h"
-#include "error_int.h"
#include "qerror.h"
struct Error
@@ -91,22 +90,3 @@ void error_propagate(Error **dst_err, Error *local_err)
error_free(local_err);
}
}
-
-QObject *error_get_qobject(Error *err)
-{
-QINCREF(err->obj);
-return QOBJECT(err->obj);
-}
-
-void error_set_qobject(Error **errp, QObject *obj)
-{
-Error *err;
-if (errp == NULL) {
-return;
-}
-err = g_malloc0(sizeof(*err));
-err->obj = qobject_to_qdict(obj);
-qobject_incref(obj);
-
-*errp = err;
-}
diff --git a/error_int.h b/error_int.h
deleted file mode 100644
index 4b00d08..000
--- a/error_int.h
+++ /dev/null
@@ -1,28 +0,0 @@
-/*
- * QEMU Error Objects
- *
- * Copyright IBM, Corp. 2011
- *
- * Authors:
- * Anthony Liguori
- *
- * This work is licensed under the terms of the GNU LGPL, version 2. See
- * the COPYING.LIB file in the top-level directory.
- */
-#ifndef QEMU_ERROR_INT_H
-#define QEMU_ERROR_INT_H
-
-#include "qemu-common.h"
-#include "qobject.h"
-#include "qdict.h"
-#include "error.h"
-
-/**
- * Internal QEMU functions for working with Error.
- *
- * These are used to convert QErrors to Errors
- */
-QObject *error_get_qobject(Error *err);
-void error_set_qobject(Error **errp, QObject *obj);
-
-#endif
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index ec613f8..4085994 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -16,7 +16,6 @@
#include "json-parser.h"
#include "qapi-types.h"
#include "error.h"
-#include "error_int.h"
#include "qerror.h"
static QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp)
diff --git a/qemu-ga.c b/qemu-ga.c
index 39abc50..8f87621 100644
--- a/qemu-ga.c
+++ b/qemu-ga.c
@@ -28,7 +28,6 @@
#include "module.h"
#include "signal.h"
#include "qerror.h"
-#include "error_int.h"
#include "qapi/qmp-core.h"
#include "qga/channel.h"
#ifdef _WIN32
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 26/35] error: add error_get_class()
Signed-off-by: Luiz Capitulino
---
error.c | 5 +
error.h | 5 +
2 files changed, 10 insertions(+)
diff --git a/error.c b/error.c
index 648706a..2d34cde 100644
--- a/error.c
+++ b/error.c
@@ -64,6 +64,11 @@ bool error_is_set(Error **errp)
return (errp && *errp);
}
+ErrorClass error_get_class(const Error *err)
+{
+return err->err_class;
+}
+
const char *error_get_pretty(Error *err)
{
return err->msg;
diff --git a/error.h b/error.h
index 9678752..114e24b 100644
--- a/error.h
+++ b/error.h
@@ -35,6 +35,11 @@ void error_set(Error **err, ErrorClass err_class, const char
*fmt, ...) GCC_FMT_
*/
bool error_is_set(Error **err);
+/*
+ * Get the error class of an error object.
+ */
+ErrorClass error_get_class(const Error *err);
+
/**
* Returns an exact copy of the error passed as an argument.
*/
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 12/35] hmp: hmp_cont(): don't rely on QERR_DEVICE_ENCRYPTED
This commit changes hmp_cont() to loop through all block devices
and proactively set an encryption key for any encrypted device
without a valid one.
This change is needed because QERR_DEVICE_ENCRYPTED is going to be
dropped by a future commit.
Signed-off-by: Luiz Capitulino
---
hmp.c | 43 +--
1 file changed, 25 insertions(+), 18 deletions(-)
diff --git a/hmp.c b/hmp.c
index 25688ab..bfcc032 100644
--- a/hmp.c
+++ b/hmp.c
@@ -612,34 +612,41 @@ void hmp_pmemsave(Monitor *mon, const QDict *qdict)
static void hmp_cont_cb(void *opaque, int err)
{
-Monitor *mon = opaque;
-
if (!err) {
-hmp_cont(mon, NULL);
+qmp_cont(NULL);
}
}
-void hmp_cont(Monitor *mon, const QDict *qdict)
+static bool blockinfo_is_encrypted(const BlockInfo *bdev)
{
-Error *errp = NULL;
-
-qmp_cont(&errp);
-if (error_is_set(&errp)) {
-if (error_is_type(errp, QERR_DEVICE_ENCRYPTED)) {
-const char *device;
+return (bdev->inserted && bdev->inserted->encrypted);
+}
-/* The device is encrypted. Ask the user for the password
- and retry */
+static bool blockinfo_key_is_set(const BlockInfo *bdev)
+{
+return (bdev->inserted && bdev->inserted->valid_encryption_key);
+}
-device = error_get_field(errp, "device");
-assert(device != NULL);
+void hmp_cont(Monitor *mon, const QDict *qdict)
+{
+BlockInfoList *bdev_list, *bdev;
+Error *errp = NULL;
-monitor_read_block_device_key(mon, device, hmp_cont_cb, mon);
-error_free(errp);
-return;
+bdev_list = qmp_query_block(NULL);
+for (bdev = bdev_list; bdev; bdev = bdev->next) {
+if (blockinfo_is_encrypted(bdev->value) &&
+!blockinfo_key_is_set(bdev->value)) {
+monitor_read_block_device_key(mon, bdev->value->device,
+ hmp_cont_cb, NULL);
+goto out;
}
-hmp_handle_error(mon, &errp);
}
+
+qmp_cont(&errp);
+hmp_handle_error(mon, &errp);
+
+out:
+qapi_free_BlockInfoList(bdev_list);
}
void hmp_system_wakeup(Monitor *mon, const QDict *qdict)
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 22/35] qapi-schema: add ErrorClass enum
Signed-off-by: Luiz Capitulino
---
qapi-schema.json | 30 ++
1 file changed, 30 insertions(+)
diff --git a/qapi-schema.json b/qapi-schema.json
index 5805f74..8f670f3 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3,6 +3,36 @@
# QAPI Schema
##
+# @ErrorClass
+#
+# QEMU error classes
+#
+# @GenericError: this is used for errors that don't require a specific error
+#class. This should be the default case for most errors
+#
+# @CommandNotFound: the requested command has not been found
+#
+# @DeviceEncrypted: the requested operation can't be fulfilled because the
+# selected device is encrypted
+#
+# @DeviceNotActive: a device has failed to be become active
+#
+# @DeviceNotFound: the requested device has not been found
+#
+# @KVMMissingCap: the requested operation can't be fulfilled because a
+# required KVM capability is missing
+#
+# @MigrationExpected: the requested operation can't be fulfilled because a
+# migration process is expected
+#
+# Since: 1.2
+##
+{ 'enum': 'ErrorClass',
+ 'data': [ 'GenericError', 'CommandNotFound', 'DeviceEncrypted',
+'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap',
+'MigrationExpected' ] }
+
+##
# @NameInfo:
#
# Guest name information.
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 05/35] qerror: drop qerror_abort()
qerror_abort() depends on the 'file', 'func' and 'linenr' members of
QError. However, these members are going to be dropped by the next
commit, so let's drop qerror_abort() in favor of printing an error
message to stderr plus a call to abort().
Signed-off-by: Luiz Capitulino
---
qerror.c | 41 ++---
1 file changed, 14 insertions(+), 27 deletions(-)
diff --git a/qerror.c b/qerror.c
index bfb875a..7cb7c12 100644
--- a/qerror.c
+++ b/qerror.c
@@ -346,22 +346,6 @@ static QError *qerror_new(void)
return qerr;
}
-static void GCC_FMT_ATTR(2, 3) qerror_abort(const QError *qerr,
-const char *fmt, ...)
-{
-va_list ap;
-
-fprintf(stderr, "qerror: bad call in function '%s':\n", qerr->func);
-fprintf(stderr, "qerror: -> ");
-
-va_start(ap, fmt);
-vfprintf(stderr, fmt, ap);
-va_end(ap);
-
-fprintf(stderr, "\nqerror: call at %s:%d\n", qerr->file, qerr->linenr);
-abort();
-}
-
static void GCC_FMT_ATTR(2, 0) qerror_set_data(QError *qerr,
const char *fmt, va_list *va)
{
@@ -369,28 +353,34 @@ static void GCC_FMT_ATTR(2, 0) qerror_set_data(QError
*qerr,
obj = qobject_from_jsonv(fmt, va);
if (!obj) {
-qerror_abort(qerr, "invalid format '%s'", fmt);
+fprintf(stderr, "invalid json in error dict '%s'\n", fmt);
+abort();
}
if (qobject_type(obj) != QTYPE_QDICT) {
-qerror_abort(qerr, "error format is not a QDict '%s'", fmt);
+fprintf(stderr, "error is not a dict '%s'\n", fmt);
+abort();
}
qerr->error = qobject_to_qdict(obj);
obj = qdict_get(qerr->error, "class");
if (!obj) {
-qerror_abort(qerr, "missing 'class' key in '%s'", fmt);
+fprintf(stderr, "missing 'class' key in '%s'\n", fmt);
+abort();
}
if (qobject_type(obj) != QTYPE_QSTRING) {
-qerror_abort(qerr, "'class' key value should be a QString");
+fprintf(stderr, "'class' key value should be a string in '%s'\n", fmt);
+abort();
}
obj = qdict_get(qerr->error, "data");
if (!obj) {
-qerror_abort(qerr, "missing 'data' key in '%s'", fmt);
+fprintf(stderr, "missing 'data' key in '%s'\n", fmt);
+abort();
}
if (qobject_type(obj) != QTYPE_QDICT) {
-qerror_abort(qerr, "'data' key value should be a QDICT");
+fprintf(stderr, "'data' key value should be a dict in '%s'\n", fmt);
+abort();
}
}
@@ -407,7 +397,8 @@ static void qerror_set_desc(QError *qerr, const char *fmt)
}
}
-qerror_abort(qerr, "error format '%s' not found", fmt);
+fprintf(stderr, "error format '%s' not found\n", fmt);
+abort();
}
/**
@@ -435,10 +426,6 @@ static QError *qerror_from_info(const char *file, int
linenr, const char *func,
qerr->file = file;
qerr->func = func;
-if (!fmt) {
-qerror_abort(qerr, "QDict not specified");
-}
-
qerror_set_data(qerr, fmt, va);
qerror_set_desc(qerr, fmt);
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 19/35] qapi: qapi-types.h: don't include qapi/qapi-types-core.h
qapi-types.h needs only qemu-common.h. Including qapi-types-core.h
causes problems when qerror.h or error.h includes qapi-types.h.
Signed-off-by: Luiz Capitulino
---
scripts/qapi-types.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 4a734f5..3ed9f04 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -253,7 +253,8 @@ fdecl.write(mcgen('''
#ifndef %(guard)s
#define %(guard)s
-#include "qapi/qapi-types-core.h"
+#include "qemu-common.h"
+
''',
guard=guardname(h_file)))
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 01/35] monitor: drop unused monitor debug code
In the old QMP days, this code was used to find out QMP commands that
might be calling monitor_printf() down its call chain.
This is almost impossible to happen today, because the qapi converted
commands don't even have a monitor object. Besides, it's been more than
a year since I used this last time.
Let's just drop it.
Signed-off-by: Luiz Capitulino
---
configure | 10 --
monitor.c | 65 ---
2 files changed, 75 deletions(-)
diff --git a/configure b/configure
index 280726c..030d137 100755
--- a/configure
+++ b/configure
@@ -147,7 +147,6 @@ vhost_net="no"
kvm="no"
gprof="no"
debug_tcg="no"
-debug_mon="no"
debug="no"
strip_opt="yes"
tcg_interpreter="no"
@@ -633,14 +632,9 @@ for opt do
;;
--disable-debug-tcg) debug_tcg="no"
;;
- --enable-debug-mon) debug_mon="yes"
- ;;
- --disable-debug-mon) debug_mon="no"
- ;;
--enable-debug)
# Enable debugging options that aren't excessively noisy
debug_tcg="yes"
- debug_mon="yes"
debug="yes"
strip_opt="no"
;;
@@ -3039,7 +3033,6 @@ echo "host CPU $cpu"
echo "host big endian $bigendian"
echo "target list $target_list"
echo "tcg debug enabled $debug_tcg"
-echo "Mon debug enabled $debug_mon"
echo "gprof enabled $gprof"
echo "sparse enabled$sparse"
echo "strip binaries$strip_opt"
@@ -3132,9 +3125,6 @@ echo "ARCH=$ARCH" >> $config_host_mak
if test "$debug_tcg" = "yes" ; then
echo "CONFIG_DEBUG_TCG=y" >> $config_host_mak
fi
-if test "$debug_mon" = "yes" ; then
- echo "CONFIG_DEBUG_MONITOR=y" >> $config_host_mak
-fi
if test "$debug" = "yes" ; then
echo "CONFIG_DEBUG_EXEC=y" >> $config_host_mak
fi
diff --git a/monitor.c b/monitor.c
index 49dccfe..aa57167 100644
--- a/monitor.c
+++ b/monitor.c
@@ -172,41 +172,11 @@ struct Monitor {
CPUArchState *mon_cpu;
BlockDriverCompletionFunc *password_completion_cb;
void *password_opaque;
-#ifdef CONFIG_DEBUG_MONITOR
-int print_calls_nr;
-#endif
QError *error;
QLIST_HEAD(,mon_fd_t) fds;
QLIST_ENTRY(Monitor) entry;
};
-#ifdef CONFIG_DEBUG_MONITOR
-#define MON_DEBUG(fmt, ...) do {\
-fprintf(stderr, "Monitor: "); \
-fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
-
-static inline void mon_print_count_inc(Monitor *mon)
-{
-mon->print_calls_nr++;
-}
-
-static inline void mon_print_count_init(Monitor *mon)
-{
-mon->print_calls_nr = 0;
-}
-
-static inline int mon_print_count_get(const Monitor *mon)
-{
-return mon->print_calls_nr;
-}
-
-#else /* !CONFIG_DEBUG_MONITOR */
-#define MON_DEBUG(fmt, ...) do { } while (0)
-static inline void mon_print_count_inc(Monitor *mon) { }
-static inline void mon_print_count_init(Monitor *mon) { }
-static inline int mon_print_count_get(const Monitor *mon) { return 0; }
-#endif /* CONFIG_DEBUG_MONITOR */
-
/* QMP checker flags */
#define QMP_ACCEPT_UNKNOWNS 1
@@ -299,8 +269,6 @@ void monitor_vprintf(Monitor *mon, const char *fmt, va_list
ap)
if (!mon)
return;
-mon_print_count_inc(mon);
-
if (monitor_ctrl_mode(mon)) {
return;
}
@@ -3860,8 +3828,6 @@ void monitor_set_error(Monitor *mon, QError *qerror)
if (!mon->error) {
mon->error = qerror;
} else {
-MON_DEBUG("Additional error report at %s:%d\n",
- qerror->file, qerror->linenr);
QDECREF(qerror);
}
}
@@ -3875,36 +3841,7 @@ static void handler_audit(Monitor *mon, const mon_cmd_t
*cmd, int ret)
* Action: Report an internal error to the client if in QMP.
*/
qerror_report(QERR_UNDEFINED_ERROR);
-MON_DEBUG("command '%s' returned failure but did not pass an error\n",
- cmd->name);
}
-
-#ifdef CONFIG_DEBUG_MONITOR
-if (!ret && monitor_has_error(mon)) {
-/*
- * If it returns success, it must not have passed an error.
- *
- * Action: Report the passed error to the client.
- */
-MON_DEBUG("command '%s' returned success but passed an error\n",
- cmd->name);
-}
-
-if (mon_print_count_get(mon) > 0 && strcmp(cmd->name, "info") != 0) {
-/*
- * Handlers should not call Monitor print functions.
- *
- * Action: Ignore them in QMP.
- *
- * (XXX: we don't check any 'info' or 'query' command here
- * because the user print function
[Qemu-devel] [PATCH 28/35] error: drop unused functions
Besides of being unused, they operate on the current error format,
which is going to be replaced soon.
Signed-off-by: Luiz Capitulino
---
error.c | 48
error.h | 16
error_int.h | 1 -
3 files changed, 65 deletions(-)
diff --git a/error.c b/error.c
index 2d34cde..b1d5131 100644
--- a/error.c
+++ b/error.c
@@ -74,29 +74,6 @@ const char *error_get_pretty(Error *err)
return err->msg;
}
-const char *error_get_field(Error *err, const char *field)
-{
-if (strcmp(field, "class") == 0) {
-return qdict_get_str(err->obj, field);
-} else {
-QDict *dict = qdict_get_qdict(err->obj, "data");
-return qdict_get_str(dict, field);
-}
-}
-
-QDict *error_get_data(Error *err)
-{
-QDict *data = qdict_get_qdict(err->obj, "data");
-QINCREF(data);
-return data;
-}
-
-void error_set_field(Error *err, const char *field, const char *value)
-{
-QDict *dict = qdict_get_qdict(err->obj, "data");
-qdict_put(dict, field, qstring_from_str(value));
-}
-
void error_free(Error *err)
{
if (err) {
@@ -106,31 +83,6 @@ void error_free(Error *err)
}
}
-bool error_is_type(Error *err, ErrorClass err_class, const char *fmt)
-{
-const char *error_class;
-char *ptr;
-char *end;
-
-if (!err) {
-return false;
-}
-
-ptr = strstr(fmt, "'class': '");
-assert(ptr != NULL);
-ptr += strlen("'class': '");
-
-end = strchr(ptr, '\'');
-assert(end != NULL);
-
-error_class = error_get_field(err, "class");
-if (strlen(error_class) != end - ptr) {
-return false;
-}
-
-return strncmp(ptr, error_class, end - ptr) == 0;
-}
-
void error_propagate(Error **dst_err, Error *local_err)
{
if (dst_err && !*dst_err) {
diff --git a/error.h b/error.h
index 114e24b..5336fc5 100644
--- a/error.h
+++ b/error.h
@@ -51,16 +51,6 @@ Error *error_copy(const Error *err);
const char *error_get_pretty(Error *err);
/**
- * Get an individual named error field.
- */
-const char *error_get_field(Error *err, const char *field);
-
-/**
- * Get an individual named error field.
- */
-void error_set_field(Error *err, const char *field, const char *value);
-
-/**
* Propagate an error to an indirect pointer to an error. This function will
* always transfer ownership of the error reference and handles the case where
* dst_err is NULL correctly. Errors after the first are discarded.
@@ -72,10 +62,4 @@ void error_propagate(Error **dst_err, Error *local_err);
*/
void error_free(Error *err);
-/**
- * Determine if an error is of a speific type (based on the qerror format).
- * Non-QEMU users should get the `class' field to identify the error type.
- */
-bool error_is_type(Error *err, ErrorClass err_class, const char *fmt);
-
#endif
diff --git a/error_int.h b/error_int.h
index 5e39424..4b00d08 100644
--- a/error_int.h
+++ b/error_int.h
@@ -22,7 +22,6 @@
*
* These are used to convert QErrors to Errors
*/
-QDict *error_get_data(Error *err);
QObject *error_get_qobject(Error *err);
void error_set_qobject(Error **errp, QObject *obj);
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 29/35] qmp: switch to the new error format on the wire
IMPORTANT: this BREAKS QMP's compatibility for the error response.
This commit changes QMP's wire protocol to make use of the simpler
error format introduced by previous commits.
There are two important (and mostly incompatible) changes:
1. Almost all error classes have been replaced by GenericError. The
only classes that are still supported for compatibility with
libvirt are: CommandNotFound, DeviceNotActive, KVMMissingCap,
DeviceNotFound and MigrationExpected
2. The 'data' field of the error dictionary is gone
As an example, an error response like:
{ "error": { "class": "DeviceNotRemovable",
"data": { "device": "virtio0" },
"desc": "Device 'virtio0' is not removable" } }
Will now be emitted as:
{ "error": { "class": "GenericError",
"desc": "Device 'virtio0' is not removable" } }
Signed-off-by: Luiz Capitulino
---
QMP/qmp-spec.txt | 10 +++---
monitor.c| 18 +-
qapi-schema.json | 55 ---
qmp-commands.hx | 4 ++--
4 files changed, 18 insertions(+), 69 deletions(-)
diff --git a/QMP/qmp-spec.txt b/QMP/qmp-spec.txt
index 1ba916c..a277896 100644
--- a/QMP/qmp-spec.txt
+++ b/QMP/qmp-spec.txt
@@ -106,14 +106,11 @@ completed because of an error condition.
The format is:
-{ "error": { "class": json-string, "data": json-object, "desc": json-string },
- "id": json-value }
+{ "error": { "class": json-string, "desc": json-string }, "id": json-value }
Where,
-- The "class" member contains the error class name (eg. "ServiceUnavailable")
-- The "data" member contains specific error data and is defined in a
- per-command basis, it will be an empty json-object if the error has no data
+- The "class" member contains the error class name (eg. "GenericError")
- The "desc" member is a human-readable error message. Clients should
not attempt to parse this message.
- The "id" member contains the transaction identification associated with
@@ -173,8 +170,7 @@ S: {"return": {"enabled": true, "present": true}, "id":
"example"}
--
C: { "execute": }
-S: {"error": {"class": "JSONParsing", "desc": "Invalid JSON syntax", "data":
-{}}}
+S: {"error": {"class": "GenericError", "desc": "Invalid JSON syntax" } }
3.5 Powerdown event
---
diff --git a/monitor.c b/monitor.c
index aa57167..3694590 100644
--- a/monitor.c
+++ b/monitor.c
@@ -353,16 +353,26 @@ static void monitor_json_emitter(Monitor *mon, const
QObject *data)
QDECREF(json);
}
+static QDict *build_qmp_error_dict(const QError *err)
+{
+QObject *obj;
+
+obj = qobject_from_jsonf("{ 'error': { 'class': %s, 'desc': %p } }",
+ ErrorClass_lookup[err->err_class],
+ qerror_human(err));
+
+return qobject_to_qdict(obj);
+}
+
static void monitor_protocol_emitter(Monitor *mon, QObject *data)
{
QDict *qmp;
trace_monitor_protocol_emitter(mon);
-qmp = qdict_new();
-
if (!monitor_has_error(mon)) {
/* success response */
+qmp = qdict_new();
if (data) {
qobject_incref(data);
qdict_put_obj(qmp, "return", data);
@@ -372,9 +382,7 @@ static void monitor_protocol_emitter(Monitor *mon, QObject
*data)
}
} else {
/* error response */
-qdict_put(mon->error->error, "desc", qerror_human(mon->error));
-qdict_put(qmp, "error", mon->error->error);
-QINCREF(mon->error->error);
+qmp = build_qmp_error_dict(mon->error);
QDECREF(mon->error);
mon->error = NULL;
}
diff --git a/qapi-schema.json b/qapi-schema.json
index 8f670f3..b9c05ca 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -657,7 +657,6 @@
# Returns information about the current VNC server
#
# Returns: @VncInfo
-# If VNC support is not compiled in, FeatureDisabled
#
# Since: 0.14.0
##
@@ -1041,9 +1040,6 @@
# virtual address (defaults to CPU 0)
#
# Returns: Nothing on success
-# If @cpu is not a valid VCPU, InvalidParameterValue
-# If @filename cannot be opened, OpenFileFailed
-# If an I/O error occurs while writing the file, IOError
#
# Since: 0.14.0
#
@@ -1064,8 +1060,6 @@
# @filename: the file to save the memory to as binary data
#
# Returns: Nothin
[Qemu-devel] [PATCH 15/35] migration: don't rely on any QERR_SOCKET_*
Use the in_progress argument for QERR_SOCKET_CONNECT_IN_PROGRESS. The
other errors are handled the same by checking if the error is set and
then calling migrate_fd_error() if it's.
It's also necessary to change inet_connect_opts() not to set
QERR_SOCKET_CONNECT_IN_PROGRESS. This error is only used by
tcp_start_outgoing_migration() and not changing it along with the
usage of in_progress would break migration.
Furthermore this commit fixes a bug. Today, there's a spurious error
report when migration succeeds:
(qemu) migrate tcp:0:
migrate: Connection can not be completed immediately
(qemu)
After this commit no spurious error is reported anymore.
Signed-off-by: Luiz Capitulino
---
migration-tcp.c | 22 --
qemu-sockets.c | 2 --
2 files changed, 8 insertions(+), 16 deletions(-)
diff --git a/migration-tcp.c b/migration-tcp.c
index 18944a4..40c277f 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -82,27 +82,21 @@ static void tcp_wait_for_connect(void *opaque)
int tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
Error **errp)
{
+bool in_progress;
+
s->get_error = socket_errno;
s->write = socket_write;
s->close = tcp_close;
-s->fd = inet_connect(host_port, false, NULL, errp);
-
-if (!error_is_set(errp)) {
-migrate_fd_connect(s);
-} else if (error_is_type(*errp, QERR_SOCKET_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)) {
-DPRINTF("connect failed\n");
-return -1;
-} else if (error_is_type(*errp, QERR_SOCKET_CONNECT_FAILED)) {
-DPRINTF("connect failed\n");
+s->fd = inet_connect(host_port, false, &in_progress, errp);
+if (error_is_set(errp)) {
migrate_fd_error(s);
return -1;
+} else if (in_progress) {
+DPRINTF("connect in progress\n");
+qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
} else {
-DPRINTF("unknown error\n");
-return -1;
+migrate_fd_connect(s);
}
return 0;
diff --git a/qemu-sockets.c b/qemu-sockets.c
index 9cb47d4..361d890 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -284,8 +284,6 @@ int inet_connect_opts(QemuOpts *opts, bool *in_progress,
Error **errp)
if (in_progress) {
*in_progress = true;
}
-
-error_set(errp, QERR_SOCKET_CONNECT_IN_PROGRESS);
} else if (rc < 0) {
if (NULL == e->ai_next)
fprintf(stderr, "%s: connect(%s,%s,%s,%s): %s\n", __FUNCTION__,
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 30/35] qemu-ga: switch to the new error format on the wire
IMPORTANT: this BREAKS qemu-ga compatibility for the error response.
Instead of returning something like:
{ "error": { "class": "InvalidParameterValue",
"data": {"name": "mode", "expected": "halt|powerdown|reboot" } } }
qemu-ga now returns:
{ "error": { "class": "GenericError",
"desc": "Parameter 'mode' expects halt|powerdown|reboot" } }
Notice that this is also a bug fix, as qemu-ga wasn't returning the
human message.
Signed-off-by: Luiz Capitulino
---
Makefile.objs | 1 +
qapi/qmp-core.h | 1 +
qapi/qmp-dispatch.c | 10 +-
qemu-ga.c | 4 ++--
4 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/Makefile.objs b/Makefile.objs
index 5ebbcfa..8454b53 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -211,6 +211,7 @@ common-obj-$(CONFIG_SMARTCARD_NSS) += $(libcacard-y)
# qapi
qapi-obj-y = qapi/
+qapi-obj-y += qapi-types.o qapi-visit.o
common-obj-y += qmp-marshal.o qapi-visit.o qapi-types.o
common-obj-y += qmp.o hmp.o
diff --git a/qapi/qmp-core.h b/qapi/qmp-core.h
index b0f64ba..00446cf 100644
--- a/qapi/qmp-core.h
+++ b/qapi/qmp-core.h
@@ -49,6 +49,7 @@ void qmp_disable_command(const char *name);
void qmp_enable_command(const char *name);
bool qmp_command_is_enabled(const char *name);
char **qmp_get_command_list(void);
+QObject *qmp_build_error_object(Error *errp);
#endif
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 122c1a2..ec613f8 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -14,6 +14,7 @@
#include "qemu-objects.h"
#include "qapi/qmp-core.h"
#include "json-parser.h"
+#include "qapi-types.h"
#include "error.h"
#include "error_int.h"
#include "qerror.h"
@@ -109,6 +110,13 @@ static QObject *do_qmp_dispatch(QObject *request, Error
**errp)
return ret;
}
+QObject *qmp_build_error_object(Error *errp)
+{
+return qobject_from_jsonf("{ 'class': %s, 'desc': %s }",
+ ErrorClass_lookup[error_get_class(errp)],
+ error_get_pretty(errp));
+}
+
QObject *qmp_dispatch(QObject *request)
{
Error *err = NULL;
@@ -119,7 +127,7 @@ QObject *qmp_dispatch(QObject *request)
rsp = qdict_new();
if (err) {
-qdict_put_obj(rsp, "error", error_get_qobject(err));
+qdict_put_obj(rsp, "error", qmp_build_error_object(err));
error_free(err);
} else if (ret) {
qdict_put_obj(rsp, "return", ret);
diff --git a/qemu-ga.c b/qemu-ga.c
index f1a39ec..39abc50 100644
--- a/qemu-ga.c
+++ b/qemu-ga.c
@@ -515,7 +515,7 @@ static void process_event(JSONMessageParser *parser, QList
*tokens)
} else {
g_warning("failed to parse event: %s", error_get_pretty(err));
}
-qdict_put_obj(qdict, "error", error_get_qobject(err));
+qdict_put_obj(qdict, "error", qmp_build_error_object(err));
error_free(err);
} else {
qdict = qobject_to_qdict(obj);
@@ -532,7 +532,7 @@ static void process_event(JSONMessageParser *parser, QList
*tokens)
qdict = qdict_new();
g_warning("unrecognized payload format");
error_set(&err, QERR_UNSUPPORTED);
-qdict_put_obj(qdict, "error", error_get_qobject(err));
+qdict_put_obj(qdict, "error", qmp_build_error_object(err));
error_free(err);
}
ret = send_response(s, QOBJECT(qdict));
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 33/35] qerror: drop qerror_table and qerror_format()
They are unused since last commit.
Signed-off-by: Luiz Capitulino
---
qerror.c | 400 ---
qerror.h | 7 --
2 files changed, 407 deletions(-)
diff --git a/qerror.c b/qerror.c
index dda1427..ccc52be 100644
--- a/qerror.c
+++ b/qerror.c
@@ -23,311 +23,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[] = {
-{
- QERR_ADD_CLIENT_FAILED,
- "Could not add client",
-},
-{
- QERR_AMBIGUOUS_PATH,
- "Path '%(path)' does not uniquely identify an object"
-},
-{
- QERR_BAD_BUS_FOR_DEVICE,
- "Device '%(device)' can't go on a %(bad_bus_type) bus",
-},
-{
- QERR_BASE_NOT_FOUND,
- "Base '%(base)' not found",
-},
-{
- QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "Block format '%(format)' used by device '%(name)' does not support
feature '%(feature)'",
-},
-{
- QERR_BUS_NO_HOTPLUG,
- "Bus '%(bus)' does not support hotplugging",
-},
-{
- QERR_BUS_NOT_FOUND,
- "Bus '%(bus)' not found",
-},
-{
- QERR_COMMAND_DISABLED,
- "The command %(name) has been disabled for this instance",
-},
-{
- QERR_COMMAND_NOT_FOUND,
- "The command %(name) has not been found",
-},
-{
- QERR_DEVICE_ENCRYPTED,
- "'%(device)' (%(filename)) is encrypted",
-},
-{
- QERR_DEVICE_FEATURE_BLOCKS_MIGRATION,
- "Migration is disabled when using feature '%(feature)' in device
'%(device)'",
-},
-{
- QERR_DEVICE_HAS_NO_MEDIUM,
- "Device '%(device)' has no medium",
-},
-{
- QERR_DEVICE_INIT_FAILED,
- "Device '%(device)' could not be initialized",
-},
-{
- QERR_DEVICE_IN_USE,
- "Device '%(device)' is in use",
-},
-{
- QERR_DEVICE_IS_READ_ONLY,
- "Device '%(device)' is read only",
-},
-{
- QERR_DEVICE_LOCKED,
- "Device '%(device)' is locked",
-},
-{
- QERR_DEVICE_MULTIPLE_BUSSES,
- "Device '%(device)' has multiple child busses",
-},
-{
- QERR_DEVICE_NO_BUS,
- "Device '%(device)' has no child bus",
-},
-{
- QERR_DEVICE_NO_HOTPLUG,
- "Device '%(device)' does not support hotplugging",
-},
-{
- QERR_DEVICE_NOT_ACTIVE,
- "Device '%(device)' has not been activated",
-},
-{
- QERR_DEVICE_NOT_ENCRYPTED,
- "Device '%(device)' is not encrypted",
-},
-{
- QERR_DEVICE_NOT_FOUND,
- "Device '%(device)' not found",
-},
-{
- QERR_DEVICE_NOT_REMOVABLE,
- "Device '%(device)' is not removable",
-},
-{
- QERR_DUPLICATE_ID,
- "Duplicate ID '%(id)' for %(object)",
-},
-{
- QERR_FD_NOT_FOUND,
- "File descriptor named '%(name)' not found",
-},
-{
- QERR_FD_NOT_SUPPLIED,
- "No file descriptor supplied via SCM_RIGHTS",
-},
-{
- QERR_FEATURE_DISABLED,
- "The feature '%(name)' is not enabled",
-},
-{
- QERR_INVALID_BLOCK_FORMAT,
- "Invalid block format '%(name)'",
-},
-{
- QERR_INVALID_OPTION_GROUP,
- "There is no option group '%(group)'",
-},
-{
- QERR_INVALID_PARAMETER,
- "Invalid parameter '%(name)'",
-},
-{
- QERR_INVALID_PARAMETER_COMBINATION,
- "Invalid parameter combination",
-},
-{
- QERR_INVALID_PARAMETER_TYPE,
- "Invalid parameter type for '%(name)', expected: %(expected)",
-},
-{
- QERR_INVALID_PARAMETER_VALUE,
- "Parameter '%(name)' expects %(
[Qemu-devel] [PATCH 10/35] error: don't delay error message construction
Today, the error message is only constructed when it's used. This commit
changes that to construct the error message when the error object is
built (ie. when the error is reported).
This simplifies the Error object.
Signed-off-by: Luiz Capitulino
---
error.c | 8 +---
qerror.c | 4 +---
2 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/error.c b/error.c
index 3a62592..2ade99b 100644
--- a/error.c
+++ b/error.c
@@ -20,7 +20,6 @@
struct Error
{
QDict *obj;
-const char *fmt;
char *msg;
};
@@ -39,7 +38,7 @@ void error_set(Error **errp, const char *fmt, ...)
va_start(ap, fmt);
err->obj = qobject_to_qdict(qobject_from_jsonv(fmt, &ap));
va_end(ap);
-err->fmt = fmt;
+err->msg = qerror_format(fmt, err->obj);
*errp = err;
}
@@ -50,7 +49,6 @@ Error *error_copy(const Error *err)
err_new = g_malloc0(sizeof(*err));
err_new->msg = g_strdup(err->msg);
-err_new->fmt = err->fmt;
err_new->obj = err->obj;
QINCREF(err_new->obj);
@@ -64,10 +62,6 @@ bool error_is_set(Error **errp)
const char *error_get_pretty(Error *err)
{
-if (err->msg == NULL) {
-err->msg = qerror_format(err->fmt, err->obj);
-}
-
return err->msg;
}
diff --git a/qerror.c b/qerror.c
index a254f88..5d38428 100644
--- a/qerror.c
+++ b/qerror.c
@@ -543,7 +543,6 @@ void qerror_report(const char *fmt, ...)
struct Error
{
QDict *obj;
-const char *fmt;
char *msg;
};
@@ -555,8 +554,7 @@ void qerror_report_err(Error *err)
loc_save(&qerr->loc);
QINCREF(err->obj);
qerr->error = err->obj;
-
-qerr->err_msg = qerror_format(err->fmt, qerr->error);
+qerr->err_msg = g_strdup(err->msg);
if (monitor_cur_is_qmp()) {
monitor_set_error(cur_mon, qerr);
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 27/35] hmp: hmp_change(): use error_get_class()
The error_is_type() function is going to be dropped.
Signed-off-by: Luiz Capitulino
---
hmp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hmp.c b/hmp.c
index 3a9688d..5900251 100644
--- a/hmp.c
+++ b/hmp.c
@@ -799,7 +799,8 @@ void hmp_change(Monitor *mon, const QDict *qdict)
}
qmp_change(device, target, !!arg, arg, &err);
-if (error_is_type(err, QERR_DEVICE_ENCRYPTED)) {
+if (error_is_set(&err) &&
+error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED) {
error_free(err);
monitor_read_block_device_key(mon, device, NULL, NULL);
return;
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 09/35] qerror: don't delay error message construction
Today, the error message is only constructed when it's used. This commit
changes qerror to construct the error message when the error object is
built (ie. when the error is reported).
This eliminates the need of storing a pointer to qerror_table[], which
will be dropped soon, and also simplifies the code.
Signed-off-by: Luiz Capitulino
---
qerror.c | 29 -
qerror.h | 2 +-
2 files changed, 5 insertions(+), 26 deletions(-)
diff --git a/qerror.c b/qerror.c
index d073ed7..a254f88 100644
--- a/qerror.c
+++ b/qerror.c
@@ -385,22 +385,6 @@ static QDict *error_obj_from_fmt_no_fail(const char *fmt,
va_list *va)
return ret;
}
-static const QErrorStringTable *get_desc_no_fail(const char *fmt)
-{
-int i;
-
-// FIXME: inefficient loop
-
-for (i = 0; qerror_table[i].error_fmt; i++) {
-if (strcmp(qerror_table[i].error_fmt, fmt) == 0) {
-return &qerror_table[i];
-}
-}
-
-fprintf(stderr, "error format '%s' not found\n", fmt);
-abort();
-}
-
/**
* qerror_from_info(): Create a new QError from error information
*
@@ -414,7 +398,7 @@ static QError *qerror_from_info(const char *fmt, va_list
*va)
loc_save(&qerr->loc);
qerr->error = error_obj_from_fmt_no_fail(fmt, va);
-qerr->entry = get_desc_no_fail(fmt);
+qerr->err_msg = qerror_format(fmt, qerr->error);
return qerr;
}
@@ -519,7 +503,7 @@ char *qerror_format(const char *fmt, QDict *error)
*/
QString *qerror_human(const QError *qerror)
{
-return qerror_format_desc(qerror->error, qerror->entry);
+return qstring_from_str(qerror->err_msg);
}
/**
@@ -566,19 +550,13 @@ struct Error
void qerror_report_err(Error *err)
{
QError *qerr;
-int i;
qerr = qerror_new();
loc_save(&qerr->loc);
QINCREF(err->obj);
qerr->error = err->obj;
-for (i = 0; qerror_table[i].error_fmt; i++) {
-if (strcmp(qerror_table[i].error_fmt, err->fmt) == 0) {
-qerr->entry = &qerror_table[i];
-break;
-}
-}
+qerr->err_msg = qerror_format(err->fmt, qerr->error);
if (monitor_cur_is_qmp()) {
monitor_set_error(cur_mon, qerr);
@@ -619,5 +597,6 @@ static void qerror_destroy_obj(QObject *obj)
qerr = qobject_to_qerror(obj);
QDECREF(qerr->error);
+g_free(qerr->err_msg);
g_free(qerr);
}
diff --git a/qerror.h b/qerror.h
index aec76b2..de8497d 100644
--- a/qerror.h
+++ b/qerror.h
@@ -27,7 +27,7 @@ typedef struct QError {
QObject_HEAD;
QDict *error;
Location loc;
-const QErrorStringTable *entry;
+char *err_msg;
} QError;
QString *qerror_human(const QError *qerror);
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 11/35] qmp: query-block: add 'valid_encryption_key' field
Signed-off-by: Luiz Capitulino
---
block.c | 1 +
qapi-schema.json | 8 +---
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/block.c b/block.c
index 24323c1..59f6dd8 100644
--- a/block.c
+++ b/block.c
@@ -2445,6 +2445,7 @@ BlockInfoList *qmp_query_block(Error **errp)
info->value->inserted->ro = bs->read_only;
info->value->inserted->drv = g_strdup(bs->drv->format_name);
info->value->inserted->encrypted = bs->encrypted;
+info->value->inserted->valid_encryption_key = bs->valid_key;
if (bs->backing_file[0]) {
info->value->inserted->has_backing_file = true;
info->value->inserted->backing_file =
g_strdup(bs->backing_file);
diff --git a/qapi-schema.json b/qapi-schema.json
index cddf63a..5805f74 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -402,6 +402,8 @@
#
# @encrypted: true if the backing device is encrypted
#
+# @valid_encryption_key: true if a valid encryption key has been set
+#
# @bps: total throughput limit in bytes per second is specified
#
# @bps_rd: read throughput limit in bytes per second is specified
@@ -421,9 +423,9 @@
{ 'type': 'BlockDeviceInfo',
'data': { 'file': 'str', 'ro': 'bool', 'drv': 'str',
'*backing_file': 'str', 'backing_file_depth': 'int',
-'encrypted': 'bool', 'bps': 'int', 'bps_rd': 'int',
-'bps_wr': 'int', 'iops': 'int', 'iops_rd': 'int',
-'iops_wr': 'int'} }
+'encrypted': 'bool', 'valid_encryption_key': 'bool',
+'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
+'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int'} }
##
# @BlockDeviceIoStatus:
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 08/35] qerror: qerror_format(): return an allocated string
Simplifies current and future users.
Signed-off-by: Luiz Capitulino
---
error.c | 5 +
qerror.c | 10 --
qerror.h | 2 +-
3 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/error.c b/error.c
index 58f55a0..3a62592 100644
--- a/error.c
+++ b/error.c
@@ -65,10 +65,7 @@ bool error_is_set(Error **errp)
const char *error_get_pretty(Error *err)
{
if (err->msg == NULL) {
-QString *str;
-str = qerror_format(err->fmt, err->obj);
-err->msg = g_strdup(qstring_get_str(str));
-QDECREF(str);
+err->msg = qerror_format(err->fmt, err->obj);
}
return err->msg;
diff --git a/qerror.c b/qerror.c
index 6f9f49c..d073ed7 100644
--- a/qerror.c
+++ b/qerror.c
@@ -493,9 +493,11 @@ static QString *qerror_format_desc(QDict *error,
return qstring;
}
-QString *qerror_format(const char *fmt, QDict *error)
+char *qerror_format(const char *fmt, QDict *error)
{
const QErrorStringTable *entry = NULL;
+QString *qstr;
+char *ret;
int i;
for (i = 0; qerror_table[i].error_fmt; i++) {
@@ -505,7 +507,11 @@ QString *qerror_format(const char *fmt, QDict *error)
}
}
-return qerror_format_desc(error, entry);
+qstr = qerror_format_desc(error, entry);
+ret = g_strdup(qstring_get_str(qstr));
+QDECREF(qstr);
+
+return ret;
}
/**
diff --git a/qerror.h b/qerror.h
index 3c0b14c..aec76b2 100644
--- a/qerror.h
+++ b/qerror.h
@@ -34,7 +34,7 @@ QString *qerror_human(const QError *qerror);
void qerror_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
void qerror_report_err(Error *err);
void assert_no_error(Error *err);
-QString *qerror_format(const char *fmt, QDict *error);
+char *qerror_format(const char *fmt, QDict *error);
/*
* QError class list
--
1.7.11.2.249.g31c7954.dirty
Re: [Qemu-devel] [PATCH 00/12] Migration next v12
On Mon, 6 Aug 2012 21:42:46 +0300 Orit Wasserman wrote: > Changes from v11: > Fix example for query-migrate-cache-size commands > Move patch 10 (Change total_time to total-time) to patch 9 and fix > comment. Looks good now: Reviewed-by: Luiz Capitulino
Re: [Qemu-devel] [PATCH 35/35] docs: writing-qmp-commands.txt: update error section
On Wed, 08 Aug 2012 14:35:23 +0200
Pavel Hrdina wrote:
> On 08/07/2012 05:53 PM, Luiz Capitulino wrote:
> > Add information about the new error format and improve the text a bit.
> >
> > Signed-off-by: Luiz Capitulino
> > ---
> > docs/writing-qmp-commands.txt | 47
> > +--
> > 1 file changed, 27 insertions(+), 20 deletions(-)
> >
> > diff --git a/docs/writing-qmp-commands.txt b/docs/writing-qmp-commands.txt
> > index 0ad51aa..10ecd97 100644
> > --- a/docs/writing-qmp-commands.txt
> > +++ b/docs/writing-qmp-commands.txt
> > @@ -210,19 +210,17 @@ if you don't see these strings, then something went
> > wrong.
> > === Errors ===
> >
> > QMP commands should use the error interface exported by the error.h header
> > -file. The basic function used to set an error is the error_set() one.
> > +file. Basically, errors are set by calling the error_set() function.
> >
> > Let's say we don't accept the string "message" to contain the word
> > "love". If
> > -it does contain it, we want the "hello-world" command to the return the
> > -InvalidParameter error.
> > -
> > -Only one change is required, and it's in the C implementation:
> > +it does contain it, we want the "hello-world" command to return an error:
> >
> > void qmp_hello_world(bool has_message, const char *message, Error **errp)
> > {
> > if (has_message) {
> > if (strstr(message, "love")) {
> > -error_set(errp, QERR_INVALID_PARAMETER, "message");
> > +error_set(errp, ERROR_CLASS_GENERIC_ERROR,
> > + "the word 'love' is not allowed");
> > return;
> > }
> > printf("%s\n", message);
> > @@ -231,30 +229,40 @@ void qmp_hello_world(bool has_message, const char
> > *message, Error **errp)
> > }
> > }
> >
> > -Let's test it. Build qemu, run it as defined in the "Testing" section, and
> > -then issue the following command:
> > +The first argument to the error_set() function is the Error pointer to
> > pointer,
> > +which is passed to all QMP functions. The second argument is a ErrorClass
> > +value, which should be ERROR_CLASS_GENERIC_ERROR most of the time (more
> > +details about error classes are given below). The third argument is a human
> > +description of the error, this is a free-form printf-like string.
> > +
> > +Let's test the example above. Build qemu, run it as defined in the
> > "Testing"
> > +section, and then issue the following command:
> >
> > -{ "execute": "hello-world", "arguments": { "message": "we love qemu" } }
> > +{ "execute": "hello-world", "arguments": { "message": "all you need is
> > love" } }
> >
> > The QMP server's response should be:
> >
> > {
> > "error": {
> > -"class": "InvalidParameter",
> > -"desc": "Invalid parameter 'message'",
> > -"data": {
> > -"name": "message"
> > -}
> > +"class": "GenericError",
>
> you have here trailing white-space ^
I've fixed it for v3, thanks.
>
> > +"desc": "the word 'love' is not allowed"
> > }
> > }
> >
> > -Which is the InvalidParameter error.
> > +As a general rule, all QMP errors should use ERROR_CLASS_GENERIC_ERROR.
> > There
> > +are two exceptions to this rule:
> > +
> > + 1. A non-generic ErrorClass value exists* for the failure you want to
> > report
> > +(eg. DeviceNotFound)
> > +
> > + 2. Management applications have to take special action on the failure you
> > +want to report, hence you have to add a new ErrorClass value so that
> > they
> > +can check for it
> >
> > -When you have to return an error but you're unsure what error to return or
> > -which arguments an error takes, you should look at the qerror.h file. Note
> > -that you might be required to add new errors if needed.
> > +If the failure you want to report doesn't fall in one of the two cases
> > above,
> > +just report ERROR_CLASS_GENERIC_ERROR.
> >
> > -FIXME: describe better the error API and how to add new errors.
> > + * All existing ErrorClass values are defined in the qapi-schema.json file
> >
> > === Command Documentation ===
> >
> > @@ -275,7 +283,6 @@ here goes "hello-world"'s new entry for the
> > qapi-schema.json file:
> > # @message: #optional string to be printed
> > #
> > # Returns: Nothing on success.
> > -# If @message contains "love", InvalidParameter
> > #
> > # Notes: if @message is not provided, the "Hello, world" string will
> > #be printed instead
>
Re: [Qemu-devel] [PATCH v7 2/6] qapi: Introduce add-fd, remove-fd, query-fdsets
On Wed, 08 Aug 2012 15:07:02 -0400
Corey Bryant wrote:
>
>
> On 08/07/2012 06:16 PM, Eric Blake wrote:
> > On 08/07/2012 11:07 AM, Corey Bryant wrote:
> >
> +#
> +# Since: 1.2.0
> >>>
> >>> We're not very consistent on '1.2' vs. '1.2.0' in since listings, but
> >>> that's probably worth a global cleanup closer to hard freeze.
> >>>
> >>
> >> I'll make a note of it. Or does Luiz usually do a cleanup?
> >
> > No idea.
> >
>
> Luiz, were you planning to take a pass at cleaning up the "since"
> release? If not let me know and I can submit a patch. Just let me know
> which to use, '1.2' or '1.2.0'.
I'd appreciate it if you submit a patch. I guess we should use 1.2.0.
>
> +##
> +{ 'type': 'FdsetFdInfo', 'data': {'fd': 'int', 'removed': 'bool'} }
> >>>
> >>> Is it worth providing any additional information? For example, knowing
> >>> whether the fd is O_RDONLY, O_WRONLY, or O_RDWR might be beneficial to
> >>> management apps trying to discover what fds are already present after a
> >>> reconnection, in order to decide whether to close them without having to
> >>> resort to /proc/$qemupid/fdinfo/nnn lookups. It might even be worth
> >>> marking such information optional, present only when 'removed':false.
> >>>
> >>
> >> It makes sense but I'd like to limit the new functionality at this point
> >> so that I can get this support into QEMU 1.2. Can this be added as a
> >> follow up patch?
> >
> > I'm personally okay with the idea of adding additional output fields in
> > later releases, but I know that has been questioned in the past, so you
> > may want to get buy-in from Luiz or Anthony. I guess the other thing is
> > to see what libvirt would actually find useful, once I complete some
> > counterpart libvirt patches. If libvirt can get by without any extra
> > information and without needing to hack things from procfs, then it's
> > not worth you spending the effort coding something that will be ignored;
> > conversely, if a piece of info is so important that I end up hacking
> > procfs anyways, that says we have a hole in QMP. I'm okay waiting for now.
> >
>
> Assuming the list of to-do's for the next patch version remains minimal,
> I'll go ahead and add support to return the access mode flags from
> query-fdsets. Also, I'm going to remove in-use from the returned data,
> because it is always going to be true.
>
Re: [Qemu-devel] [PATCH v7 2/6] qapi: Introduce add-fd, remove-fd, query-fdsets
On Wed, 08 Aug 2012 16:52:41 -0400 Corey Bryant wrote: > > > On 08/08/2012 04:48 PM, Luiz Capitulino wrote: > > On Wed, 08 Aug 2012 15:07:02 -0400 > > Corey Bryant wrote: > > > >> > >> > >> On 08/07/2012 06:16 PM, Eric Blake wrote: > >>> On 08/07/2012 11:07 AM, Corey Bryant wrote: > >>> > >>>>>> +# > >>>>>> +# Since: 1.2.0 > >>>>> > >>>>> We're not very consistent on '1.2' vs. '1.2.0' in since listings, but > >>>>> that's probably worth a global cleanup closer to hard freeze. > >>>>> > >>>> > >>>> I'll make a note of it. Or does Luiz usually do a cleanup? > >>> > >>> No idea. > >>> > >> > >> Luiz, were you planning to take a pass at cleaning up the "since" > >> release? If not let me know and I can submit a patch. Just let me know > >> which to use, '1.2' or '1.2.0'. > > > > I'd appreciate it if you submit a patch. I guess we should use 1.2.0. > > > > Sure I'll do that. > > Btw, should I be using error_set(errp, ERROR_CLASS_GENERIC_ERROR, "...") > and have a dependency on your patch series? Or should I stick with the > old error messages? That would preferable, yes. Specially if you're adding new errors. But I'm not exactly sure when my series will be merged, this depends on what review will bring.
Re: [Qemu-devel] [PATCH v7 2/6] qapi: Introduce add-fd, remove-fd, query-fdsets
On Wed, 08 Aug 2012 17:18:33 -0400 Corey Bryant wrote: > > > On 08/08/2012 05:13 PM, Luiz Capitulino wrote: > > On Wed, 08 Aug 2012 16:52:41 -0400 > > Corey Bryant wrote: > > > >> > >> > >> On 08/08/2012 04:48 PM, Luiz Capitulino wrote: > >>> On Wed, 08 Aug 2012 15:07:02 -0400 > >>> Corey Bryant wrote: > >>> > >>>> > >>>> > >>>> On 08/07/2012 06:16 PM, Eric Blake wrote: > >>>>> On 08/07/2012 11:07 AM, Corey Bryant wrote: > >>>>> > >>>>>>>> +# > >>>>>>>> +# Since: 1.2.0 > >>>>>>> > >>>>>>> We're not very consistent on '1.2' vs. '1.2.0' in since listings, but > >>>>>>> that's probably worth a global cleanup closer to hard freeze. > >>>>>>> > >>>>>> > >>>>>> I'll make a note of it. Or does Luiz usually do a cleanup? > >>>>> > >>>>> No idea. > >>>>> > >>>> > >>>> Luiz, were you planning to take a pass at cleaning up the "since" > >>>> release? If not let me know and I can submit a patch. Just let me know > >>>> which to use, '1.2' or '1.2.0'. > >>> > >>> I'd appreciate it if you submit a patch. I guess we should use 1.2.0. > >>> > >> > >> Sure I'll do that. > >> > >> Btw, should I be using error_set(errp, ERROR_CLASS_GENERIC_ERROR, "...") > >> and have a dependency on your patch series? Or should I stick with the > >> old error messages? > > > > That would preferable, yes. Specially if you're adding new errors. > > > > But I'm not exactly sure when my series will be merged, this depends on > > what review will bring. > > > > Are you planning on getting in QEMU 1.2? Absolutely.
[Qemu-devel] [PATCH 0/4]: qmp: WAKEUP event related fixes
Fixes the RESET event being emitted on wakeup from S3. Changes when the WAKEUP event is emitted and a few doc fixes. QMP/qmp-events.txt | 277 - vl.c | 18 +++- 2 files changed, 164 insertions(+), 131 deletions(-)
[Qemu-devel] [PATCH 3/4] qmp: qmp-events.txt: put events in alphabetical order
Signed-off-by: Luiz Capitulino
---
QMP/qmp-events.txt | 266 ++---
1 file changed, 130 insertions(+), 136 deletions(-)
diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
index b8afedb..e37a04e 100644
--- a/QMP/qmp-events.txt
+++ b/QMP/qmp-events.txt
@@ -1,6 +1,23 @@
QEMU Monitor Protocol Events
+BALLOON_CHANGE
+--
+
+Emitted when the guest changes the actual BALLOON level. This
+value is equivalent to the 'actual' field return by the
+'query-balloon' command
+
+Data:
+
+- "actual": actual level of the guest memory balloon in bytes (json-number)
+
+Example:
+
+{ "event": "BALLOON_CHANGE",
+"data": { "actual": 944766976 },
+"timestamp": { "seconds": 1267020223, "microseconds": 435656 } }
+
BLOCK_IO_ERROR
--
@@ -26,6 +43,57 @@ Example:
Note: If action is "stop", a STOP event will eventually follow the
BLOCK_IO_ERROR event.
+BLOCK_JOB_CANCELLED
+---
+
+Emitted when a block job has been cancelled.
+
+Data:
+
+- "type": Job type ("stream" for image streaming, json-string)
+- "device": Device name (json-string)
+- "len": Maximum progress value (json-int)
+- "offset": Current progress value (json-int)
+ On success this is equal to len.
+ On failure this is less than len.
+- "speed":Rate limit, bytes per second (json-int)
+
+Example:
+
+{ "event": "BLOCK_JOB_CANCELLED",
+ "data": { "type": "stream", "device": "virtio-disk0",
+ "len": 10737418240, "offset": 134217728,
+ "speed": 0 },
+ "timestamp": { "seconds": 1267061043, "microseconds": 959568 } }
+
+BLOCK_JOB_COMPLETED
+---
+
+Emitted when a block job has completed.
+
+Data:
+
+- "type": Job type ("stream" for image streaming, json-string)
+- "device": Device name (json-string)
+- "len": Maximum progress value (json-int)
+- "offset": Current progress value (json-int)
+ On success this is equal to len.
+ On failure this is less than len.
+- "speed":Rate limit, bytes per second (json-int)
+- "error":Error message (json-string, optional)
+ Only present on failure. This field contains a human-readable
+ error message. There are no semantics other than that streaming
+ has failed and clients should not try to interpret the error
+ string.
+
+Example:
+
+{ "event": "BLOCK_JOB_COMPLETED",
+ "data": { "type": "stream", "device": "virtio-disk0",
+ "len": 10737418240, "offset": 10737418240,
+ "speed": 0 },
+ "timestamp": { "seconds": 1267061043, "microseconds": 959568 } }
+
DEVICE_TRAY_MOVED
-
@@ -98,6 +166,68 @@ Example:
Note: If the command-line option "-no-shutdown" has been specified, a STOP
event will eventually follow the SHUTDOWN event.
+SPICE_CONNECTED, SPICE_DISCONNECTED
+---
+
+Emitted when a SPICE client connects or disconnects.
+
+Data:
+
+- "server": Server information (json-object)
+ - "host": IP address (json-string)
+ - "port": port number (json-string)
+ - "family": address family (json-string, "ipv4" or "ipv6")
+- "client": Client information (json-object)
+ - "host": IP address (json-string)
+ - "port": port number (json-string)
+ - "family": address family (json-string, "ipv4" or "ipv6")
+
+Example:
+
+{ "timestamp": {"seconds": 1290688046, "microseconds": 388707},
+ "event": "SPICE_CONNECTED",
+ "data": {
+"server": { "port": "5920", "family": "ipv4", "host": "127.0.0.1"},
+"client": {"port": "52873", "family": "ipv4", "host": "127.0.0.1"}
+}}
+
+SPICE_INITIALIZED
+-
+
+Emitted after initial handshake and authentication takes place (if any)
+and the SPICE channel is up'n'running
+
+Data:
+
+- "server": Server information (json-object)
+ - "host": IP address (json-string)
+ - "port": port number (json-string)
+ - "family": address family (json-string, "ipv4" or "ipv6")
[Qemu-devel] [PATCH 2/4] qmp: emit the WAKEUP event when the guest is put to run
Today, the WAKEUP event is emitted when a wakeup _request_ is made.
This could be the system_wakeup command, for example.
A better semantic would be to emit the event when the guest is
already running, as that's what matters in the end. This commit does
that change.
In theory, this could break compatibility. In practice, it shouldn't
happen though, as clients shouldn't rely on timing characteristics of
the events. That is, a client relying that the guest is not running
when the event arrives may break if the event arrives after the guest
is already running.
This commit also adds the missing documentation for the WAKEUP event.
Signed-off-by: Luiz Capitulino
---
QMP/qmp-events.txt | 13 +
vl.c | 2 +-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
index 9ba7079..b8afedb 100644
--- a/QMP/qmp-events.txt
+++ b/QMP/qmp-events.txt
@@ -264,6 +264,19 @@ Example:
}}
+WAKEUP
+--
+
+Emitted when the guest has been waken up from S3 and is running.
+
+Data: None.
+
+Example:
+
+{ "event": "WATCHDOG",
+ "timestamp": { "seconds": 1344522075, "microseconds": 745528 } }
+
+
WATCHDOG
diff --git a/vl.c b/vl.c
index b642637..9d783b7 100644
--- a/vl.c
+++ b/vl.c
@@ -1460,7 +1460,6 @@ void qemu_system_wakeup_request(WakeupReason reason)
return;
}
runstate_set(RUN_STATE_RUNNING);
-monitor_protocol_event(QEVENT_WAKEUP, NULL);
notifier_list_notify(&wakeup_notifiers, &reason);
wakeup_requested = 1;
qemu_notify_event();
@@ -1547,6 +1546,7 @@ static bool main_loop_should_exit(void)
cpu_synchronize_all_states();
qemu_system_reset(VMRESET_SILENT);
resume_all_vcpus();
+monitor_protocol_event(QEVENT_WAKEUP, NULL);
}
if (qemu_powerdown_requested()) {
monitor_protocol_event(QEVENT_POWERDOWN, NULL);
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 1/4] qmp: don't emit the RESET event on wakeup from S3
QEMU is basically using reset logic when waking up from S3. This
causes the QMP RESET event to be emitted, which is wrong. Also,
the runstate checks done in reset are not necessary for S3 wakeup.
Fix this by untangling wakeup from reset logic and passing
VMRESET_SILENT to qemu_system_reset() to avoid emitting the RESET
event.
Signed-off-by: Luiz Capitulino
---
vl.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/vl.c b/vl.c
index e71cb30..b642637 100644
--- a/vl.c
+++ b/vl.c
@@ -1293,6 +1293,7 @@ static pid_t shutdown_pid;
static int powerdown_requested;
static int debug_requested;
static int suspend_requested;
+static int wakeup_requested;
static NotifierList suspend_notifiers =
NOTIFIER_LIST_INITIALIZER(suspend_notifiers);
static NotifierList wakeup_notifiers =
@@ -1347,6 +1348,13 @@ static int qemu_suspend_requested(void)
return r;
}
+static int qemu_wakeup_requested(void)
+{
+int r = wakeup_requested;
+wakeup_requested = 0;
+return r;
+}
+
int qemu_powerdown_requested(void)
{
int r = powerdown_requested;
@@ -1454,7 +1462,7 @@ void qemu_system_wakeup_request(WakeupReason reason)
runstate_set(RUN_STATE_RUNNING);
monitor_protocol_event(QEVENT_WAKEUP, NULL);
notifier_list_notify(&wakeup_notifiers, &reason);
-reset_requested = 1;
+wakeup_requested = 1;
qemu_notify_event();
}
@@ -1534,6 +1542,12 @@ static bool main_loop_should_exit(void)
runstate_set(RUN_STATE_PAUSED);
}
}
+if (qemu_wakeup_requested()) {
+pause_all_vcpus();
+cpu_synchronize_all_states();
+qemu_system_reset(VMRESET_SILENT);
+resume_all_vcpus();
+}
if (qemu_powerdown_requested()) {
monitor_protocol_event(QEVENT_POWERDOWN, NULL);
qemu_irq_raise(qemu_system_powerdown);
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 4/4] qmp: qmp-events.txt: add missing doc for the SUSPEND event
Signed-off-by: Luiz Capitulino
---
QMP/qmp-events.txt | 12
1 file changed, 12 insertions(+)
diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
index e37a04e..2001a71 100644
--- a/QMP/qmp-events.txt
+++ b/QMP/qmp-events.txt
@@ -240,6 +240,18 @@ Example:
{ "event": "STOP",
"timestamp": { "seconds": 1267041730, "microseconds": 281295 } }
+SUSPEND
+---
+
+Emitted when guest enters S3 state.
+
+Data: None.
+
+Example:
+
+{ "event": "SUSPEND",
+ "timestamp": { "seconds": 1344456160, "microseconds": 309119 } }
+
VNC_CONNECTED
-
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 5/5] qmp: add SUSPEND_DISK event
Emitted when the guest makes a request to enter S4 state.
There are three possible ways of having this event, as described here:
http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg02307.html
I've decided to add a new event and make it indepedent of SHUTDOWN.
This means that the SHUTDOWN event will eventually follow the
SUSPEND_DISK event.
I've choosen this way because of two reasons:
1. Having an indepedent event makes it possible to query for its
existence by using query-events
2. In the future, we may allow the user to change what QEMU should
do as a result of the guest entering S4. So it's a good idea to
keep both events separated
Signed-off-by: Luiz Capitulino
---
This is on top of:
[PATCH 0/4]: qmp: WAKEUP event related fixes
QMP/qmp-events.txt | 14 ++
hw/acpi.c | 2 ++
monitor.c | 1 +
monitor.h | 1 +
4 files changed, 18 insertions(+)
diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
index 2001a71..1c51bee 100644
--- a/QMP/qmp-events.txt
+++ b/QMP/qmp-events.txt
@@ -252,6 +252,20 @@ Example:
{ "event": "SUSPEND",
"timestamp": { "seconds": 1344456160, "microseconds": 309119 } }
+SUSPEND_DISK
+
+
+Emitted when the guest makes a request to enter S4 state.
+
+Data: None.
+
+Example:
+
+{ "event": "SUSPEND_DISK",
+ "timestamp": { "seconds": 1344456160, "microseconds": 309119 } }
+
+Note: QEMU shutdowns when entering S4 state.
+
VNC_CONNECTED
-
diff --git a/hw/acpi.c b/hw/acpi.c
index effc7ec..f7950be 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -22,6 +22,7 @@
#include "hw.h"
#include "pc.h"
#include "acpi.h"
+#include "monitor.h"
struct acpi_table_header {
uint16_t _length; /* our length, not actual part of the hdr */
@@ -386,6 +387,7 @@ void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val, char s4)
break;
default:
if (sus_typ == s4) { /* S4 request */
+monitor_protocol_event(QEVENT_SUSPEND_DISK, NULL);
qemu_system_shutdown_request();
}
break;
diff --git a/monitor.c b/monitor.c
index 49dccfe..4f3c595 100644
--- a/monitor.c
+++ b/monitor.c
@@ -456,6 +456,7 @@ static const char *monitor_event_names[] = {
[QEVENT_BLOCK_JOB_CANCELLED] = "BLOCK_JOB_CANCELLED",
[QEVENT_DEVICE_TRAY_MOVED] = "DEVICE_TRAY_MOVED",
[QEVENT_SUSPEND] = "SUSPEND",
+[QEVENT_SUSPEND_DISK] = "SUSPEND_DISK",
[QEVENT_WAKEUP] = "WAKEUP",
[QEVENT_BALLOON_CHANGE] = "BALLOON_CHANGE",
};
diff --git a/monitor.h b/monitor.h
index 5f4de1b..4ef9a04 100644
--- a/monitor.h
+++ b/monitor.h
@@ -40,6 +40,7 @@ typedef enum MonitorEvent {
QEVENT_BLOCK_JOB_CANCELLED,
QEVENT_DEVICE_TRAY_MOVED,
QEVENT_SUSPEND,
+QEVENT_SUSPEND_DISK,
QEVENT_WAKEUP,
QEVENT_BALLOON_CHANGE,
--
1.7.11.2.249.g31c7954.dirty
Re: [Qemu-devel] [PATCH 5/5] qmp: add SUSPEND_DISK event
On Thu, 9 Aug 2012 14:30:32 -0300 Luiz Capitulino wrote: > Emitted when the guest makes a request to enter S4 state. Oops, sorry for the bad subject. It should be just '[PATCH]'.
Re: [Qemu-devel] [PATCH 3/4] qmp: qmp-events.txt: put events in alphabetical order
On Thu, 09 Aug 2012 13:29:32 -0600 Eric Blake wrote: > On 08/09/2012 11:28 AM, Luiz Capitulino wrote: > > Signed-off-by: Luiz Capitulino > > --- > > QMP/qmp-events.txt | 266 > > ++--- > > 1 file changed, 130 insertions(+), 136 deletions(-) > > Code motion that deletes 6 more lines than it adds? I'm assuming those > were blank lines, and that spacing is more consistent now, but it's > nicer to see a code motion patch with matching diffstat. Yes, blank lines. I'll avoid doing this in the future. > > Reviewed-by: Eric Blake > > > > > diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt > > index b8afedb..e37a04e 100644 > > --- a/QMP/qmp-events.txt > > +++ b/QMP/qmp-events.txt > > Nits to possibly address in a followup patch (do NOT fix them at the > same time as code motion, though): > > > > > +SPICE_CONNECTED, SPICE_DISCONNECTED > > +--- > > + > > +Emitted when a SPICE client connects or disconnects. > > Should this be broken into two listings? > > > +SPICE_INITIALIZED > > +- > > + > > +Emitted after initial handshake and authentication takes place (if any) > > +and the SPICE channel is up'n'running > > Do we like this cutesy abbreviation, or should we spell out 'and'? > > > + > > +Data: > > + > > +- "server": Server information (json-object) > > + - "host": IP address (json-string) > > + - "port": port number (json-string) > > + - "family": address family (json-string, "ipv4" or "ipv6") > > + - "auth": authentication method (json-string, optional) > > +- "client": Client information (json-object) > > + - "host": IP address (json-string) > > + - "port": port number (json-string) > > + - "family": address family (json-string, "ipv4" or "ipv6") > > + - "connection-id": spice connection id. All channels with the same id > > + belong to the same spice session (json-int) > > + - "channel-type": channel type. "1" is the main control channel, filter > > for > > +this one if you want track spice sessions only > > (json-int) > > + - "channel-id": channel id. Usually "0", might be different needed when > > + multiple channels of the same type exist, such as > > multiple > > + display channels in a multihead setup (json-int) > > + - "tls": whevener the channel is encrypted (json-bool) > > s/whevener/whether/ >
Re: [Qemu-devel] [PATCH 2/4] qmp: emit the WAKEUP event when the guest is put to run
On Thu, 09 Aug 2012 11:57:38 -0600 Eric Blake wrote: > On 08/09/2012 11:28 AM, Luiz Capitulino wrote: > > Today, the WAKEUP event is emitted when a wakeup _request_ is made. > > This could be the system_wakeup command, for example. > > > > A better semantic would be to emit the event when the guest is > > already running, as that's what matters in the end. This commit does > > that change. > > > > In theory, this could break compatibility. In practice, it shouldn't > > happen though, as clients shouldn't rely on timing characteristics of > > the events. That is, a client relying that the guest is not running > > when the event arrives may break if the event arrives after the guest > > is already running. > > Yeah, no problem with that semantic change from libvirt. > > > > +WAKEUP > > +-- > > + > > +Emitted when the guest has been waken up from S3 and is running. > > grammar: > > s/has been waken/has woken/ Fixed in v2. If I don't respin I'll merge the fixed version in the qmp branch (I won't respin just because of this). > > or maybe > > s/has been waken up/has been awakened/ >
Re: [Qemu-devel] [PATCH 5/5] qmp: add SUSPEND_DISK event
On Thu, 09 Aug 2012 13:51:10 -0600
Eric Blake wrote:
> On 08/09/2012 11:30 AM, Luiz Capitulino wrote:
> > Emitted when the guest makes a request to enter S4 state.
> >
> > There are three possible ways of having this event, as described here:
> >
> > http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg02307.html
> >
> > I've decided to add a new event and make it indepedent of SHUTDOWN.
> > This means that the SHUTDOWN event will eventually follow the
> > SUSPEND_DISK event.
> >
> > I've choosen this way because of two reasons:
> >
> > 1. Having an indepedent event makes it possible to query for its
> > existence by using query-events
> >
> > 2. In the future, we may allow the user to change what QEMU should
> > do as a result of the guest entering S4. So it's a good idea to
> > keep both events separated
>
> Indeed makes sense as a separate event.
>
> >
> > Signed-off-by: Luiz Capitulino
> > ---
> >
> > This is on top of:
> >
> > [PATCH 0/4]: qmp: WAKEUP event related fixes
> >
> > QMP/qmp-events.txt | 14 ++
> > hw/acpi.c | 2 ++
> > monitor.c | 1 +
> > monitor.h | 1 +
> > 4 files changed, 18 insertions(+)
> >
> > diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
> > index 2001a71..1c51bee 100644
> > --- a/QMP/qmp-events.txt
> > +++ b/QMP/qmp-events.txt
> > @@ -252,6 +252,20 @@ Example:
> > { "event": "SUSPEND",
> > "timestamp": { "seconds": 1344456160, "microseconds": 309119 } }
> >
> > +SUSPEND_DISK
> > +
> > +
> > +Emitted when the guest makes a request to enter S4 state.
> > +
> > +Data: None.
> > +
> > +Example:
> > +
> > +{ "event": "SUSPEND_DISK",
> > + "timestamp": { "seconds": 1344456160, "microseconds": 309119 } }
> > +
> > +Note: QEMU shutdowns when entering S4 state.
>
> s/shutdowns/shuts down/
>
> Is that true when you use the -no-shutdown flag? Do you get this event
> if you did not use the -no-shutdown flag?
Yes, the event is orthogonal to the -no-shutdown flag, but qemu won't
shut down if -no-shutdown is passed. I'll fix that note.
Re: [Qemu-devel] [PATCH 5/5] qmp: add SUSPEND_DISK event
On Thu, 9 Aug 2012 17:48:34 -0300
Luiz Capitulino wrote:
> On Thu, 09 Aug 2012 13:51:10 -0600
> Eric Blake wrote:
>
> > On 08/09/2012 11:30 AM, Luiz Capitulino wrote:
> > > Emitted when the guest makes a request to enter S4 state.
> > >
> > > There are three possible ways of having this event, as described here:
> > >
> > > http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg02307.html
> > >
> > > I've decided to add a new event and make it indepedent of SHUTDOWN.
> > > This means that the SHUTDOWN event will eventually follow the
> > > SUSPEND_DISK event.
> > >
> > > I've choosen this way because of two reasons:
> > >
> > > 1. Having an indepedent event makes it possible to query for its
> > > existence by using query-events
> > >
> > > 2. In the future, we may allow the user to change what QEMU should
> > > do as a result of the guest entering S4. So it's a good idea to
> > > keep both events separated
> >
> > Indeed makes sense as a separate event.
> >
> > >
> > > Signed-off-by: Luiz Capitulino
> > > ---
> > >
> > > This is on top of:
> > >
> > > [PATCH 0/4]: qmp: WAKEUP event related fixes
> > >
> > > QMP/qmp-events.txt | 14 ++
> > > hw/acpi.c | 2 ++
> > > monitor.c | 1 +
> > > monitor.h | 1 +
> > > 4 files changed, 18 insertions(+)
> > >
> > > diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
> > > index 2001a71..1c51bee 100644
> > > --- a/QMP/qmp-events.txt
> > > +++ b/QMP/qmp-events.txt
> > > @@ -252,6 +252,20 @@ Example:
> > > { "event": "SUSPEND",
> > > "timestamp": { "seconds": 1344456160, "microseconds": 309119 } }
> > >
> > > +SUSPEND_DISK
> > > +
> > > +
> > > +Emitted when the guest makes a request to enter S4 state.
> > > +
> > > +Data: None.
> > > +
> > > +Example:
> > > +
> > > +{ "event": "SUSPEND_DISK",
> > > + "timestamp": { "seconds": 1344456160, "microseconds": 309119 } }
> > > +
> > > +Note: QEMU shutdowns when entering S4 state.
> >
> > s/shutdowns/shuts down/
> >
> > Is that true when you use the -no-shutdown flag? Do you get this event
> > if you did not use the -no-shutdown flag?
>
> Yes, the event is orthogonal to the -no-shutdown flag, but qemu won't
> shut down if -no-shutdown is passed. I'll fix that note.
Oh, wait.
QEMU still shut downs, but with -no-shutdown it won't exit. So I think the
note doesn't need any changes.
Eric, you're confusing me :)
Re: [Qemu-devel] [PATCH 11/34] qmp: query-block: add 'valid_encryption_key' field
On Fri, 10 Aug 2012 09:56:11 +0200
Markus Armbruster wrote:
> Revisited this one on review of v2, replying here for context.
>
> Luiz Capitulino writes:
>
> > On Thu, 02 Aug 2012 13:35:54 +0200
> > Markus Armbruster wrote:
> >
> >> Luiz Capitulino writes:
> >>
> >> > Signed-off-by: Luiz Capitulino
> >> > ---
> >> > block.c | 1 +
> >> > qapi-schema.json | 7 +--
> >> > 2 files changed, 6 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/block.c b/block.c
> >> > index b38940b..9c113b8 100644
> >> > --- a/block.c
> >> > +++ b/block.c
> >> > @@ -2445,6 +2445,7 @@ BlockInfoList *qmp_query_block(Error **errp)
> >> > info->value->inserted->ro = bs->read_only;
> >> > info->value->inserted->drv = g_strdup(bs->drv->format_name);
> >> > info->value->inserted->encrypted = bs->encrypted;
> >> > +info->value->inserted->valid_encryption_key = bs->valid_key;
> >> > if (bs->backing_file[0]) {
> >> > info->value->inserted->has_backing_file = true;
> >> > info->value->inserted->backing_file =
> >> > g_strdup(bs->backing_file);
> >> > diff --git a/qapi-schema.json b/qapi-schema.json
> >> > index bc55ed2..1b2d7f5 100644
> >> > --- a/qapi-schema.json
> >> > +++ b/qapi-schema.json
> >> > @@ -400,6 +400,8 @@
> >> > #
> >> > # @encrypted: true if the backing device is encrypted
> >> > #
> >> > +# @valid_encryption_key: true if a valid encryption key has been set
> >> > +#
> >> > # @bps: total throughput limit in bytes per second is specified
> >> > #
> >> > # @bps_rd: read throughput limit in bytes per second is specified
> >> > @@ -419,8 +421,9 @@
> >> > { 'type': 'BlockDeviceInfo',
> >> >'data': { 'file': 'str', 'ro': 'bool', 'drv': 'str',
> >> > '*backing_file': 'str', 'encrypted': 'bool',
> >> > -'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
> >> > -'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int'} }
> >> > +'valid_encryption_key': 'bool', 'bps': 'int',
> >> > +'bps_rd': 'int', 'bps_wr': 'int', 'iops': 'int',
> >> > +'iops_rd': 'int', 'iops_wr': 'int'} }
> >> >
> >> > ##
> >> > # @BlockDeviceIoStatus:
> >>
> >> BlockDeviceInfo is API, isn't it?
> >
> > Yes.
> >
> >> Note that bs->valid_key currently implies bs->encrypted. bs->valid_key
> >> && !bs->encrypted is impossible. Should we make valid_encryption_key
> >> only available when encrypted?
> >
> > I don't think so. It's a bool, so it's ok for it to be false when
> > encrypted is false.
>
> What bothers me is encrypted=false, valid_encryption_key=true.
Disappearing keys is worse, IMHO (assuming that that situation is impossible
in practice, of course).
> >> valid_encryption_key is a bit long for my taste. Yours may be
> >> different.
> >
> > We should choose more descriptive and self-documenting names for the
> > protocol. Besides, I can't think of anything shorter that won't get
> > cryptic.
> >
> > Suggestions are always welcome though :)
>
> valid_encryption_key sounds like the value is the valid key.
That's exactly what it is.
> got_crypt_key? Also avoids "valid". Good, because current encrypted
> formats don't actually validate the key; they happily accept any key.
That's a block layer bug, not QMP's.
QMP clients are going to be misguided by valid_encryption_key the same way
they are with the block_passwd command or how we suffer from it internally
when calling bdrv_set_key() (which also manifests itself in HMP).
Fixing the bug where it is will automatically fix all its instances.
> GIGO. In theory, you can trash a disk that way. In practice, we can
> hope the guest will refuse to touch the disk, because it can't recognize
> partition table / filesystems.
Re: [Qemu-devel] [PATCH 12/34] hmp: hmp_cont(): don't rely on QERR_DEVICE_ENCRYPTED
On Fri, 10 Aug 2012 10:42:23 +0200
Markus Armbruster wrote:
> Luiz Capitulino writes:
>
> > On Thu, 02 Aug 2012 13:53:08 +0200
> > Markus Armbruster wrote:
> >
> >> Luiz Capitulino writes:
> >>
> >> > This commit changes hmp_cont() to loop through all block devices
> >> > and proactively set an encryption key for any encrypted device
> >> > without a valid one.
> >> >
> >> > This change is needed because QERR_DEVICE_ENCRYPTED is going to be
> >> > dropped by a future commit.
> >> >
> >> > Signed-off-by: Luiz Capitulino
> >> > ---
> >> > hmp.c | 43 +--
> >> > 1 file changed, 25 insertions(+), 18 deletions(-)
> >> >
> >> > diff --git a/hmp.c b/hmp.c
> >> > index 6b72a64..1ebeb63 100644
> >> > --- a/hmp.c
> >> > +++ b/hmp.c
> >> > @@ -610,34 +610,41 @@ void hmp_pmemsave(Monitor *mon, const QDict *qdict)
> >> >
> >> > static void hmp_cont_cb(void *opaque, int err)
> >> > {
> >> > -Monitor *mon = opaque;
> >> > -
> >> > if (!err) {
> >> > -hmp_cont(mon, NULL);
> >> > +qmp_cont(NULL);
> >> > }
> >> > }
> >> >
> >> > -void hmp_cont(Monitor *mon, const QDict *qdict)
> >> > +static bool blockinfo_is_encrypted(const BlockInfo *bdev)
> >> > {
> >> > -Error *errp = NULL;
> >> > -
> >> > -qmp_cont(&errp);
> >> > -if (error_is_set(&errp)) {
> >> > -if (error_is_type(errp, QERR_DEVICE_ENCRYPTED)) {
> >> > -const char *device;
> >> > +return (bdev->inserted && bdev->inserted->encrypted);
> >> > +}
> >> >
> >> > -/* The device is encrypted. Ask the user for the password
> >> > - and retry */
> >> > +static bool blockinfo_key_is_set(const BlockInfo *bdev)
> >> > +{
> >> > +return (bdev->inserted && bdev->inserted->valid_encryption_key);
> >> > +}
> >> >
> >> > -device = error_get_field(errp, "device");
> >> > -assert(device != NULL);
> >> > +void hmp_cont(Monitor *mon, const QDict *qdict)
> >> > +{
> >> > +BlockInfoList *bdev_list, *bdev;
> >> > +Error *errp = NULL;
> >> >
> >> > -monitor_read_block_device_key(mon, device, hmp_cont_cb,
> >> > mon);
> >> > -error_free(errp);
> >> > -return;
> >> > +bdev_list = qmp_query_block(NULL);
> >> > +for (bdev = bdev_list; bdev; bdev = bdev->next) {
> >> > +if (blockinfo_is_encrypted(bdev->value) &&
> >> > +!blockinfo_key_is_set(bdev->value)) {
> >> > +monitor_read_block_device_key(mon, bdev->value->device,
> >> > + hmp_cont_cb, NULL);
> >> > +goto out;
> >> > }
> >> > -hmp_handle_error(mon, &errp);
> >> > }
> >> > +
> >> > +qmp_cont(&errp);
> >> > +hmp_handle_error(mon, &errp);
> >> > +
> >> > +out:
> >> > +qapi_free_BlockInfoList(bdev_list);
> >> > }
> >> >
> >> > void hmp_system_wakeup(Monitor *mon, const QDict *qdict)
> >>
> >> Quote my previous analysis:
> >>
> >> Diff makes this change look worse than it is. Odd: M-x ediff gets it
> >> right. Anyway, here's how I think it works:
> >>
> >> Unchanged qmp_cont(): search the bdrv_states for the first encrypted one
> >> without a key. If found, set err argument to QERR_DEVICE_ENCRYPTED.
> >> Other errors unrelated to encrypted devices are also possible.
> >>
> >> hmp_cont() before: try qmp_cont(). If we get QERR_DEVICE_ENCRYPTED,
> >> extract the device from the error object, and prompt for its key, with a
> >> callback that retries hmp_cont() if the key was provided.
> >>
> >> After: search the bdrv_states for an encrypted one without a key. If
> >> there is none, qmp_cont(), no special error handling. If there is one,
> >> prompt for its key, with a call
Re: [Qemu-devel] [PATCH 13/35] hmp_change(): don't access DeviceEncrypted's data
On Fri, 10 Aug 2012 11:02:21 +0200 Markus Armbruster wrote: > Luiz Capitulino writes: > > > It's not needed. The device name is already known and > > monitor_read_block_device_key() knows how to do the rest. This overly > > simplifies hmp_change(). > > "overly"? > > My usual complaint about commit messages is that they fail to explain > the change's purpose. Yours explains your reason just fine, but the > description of what's done falls a bit short. I'd like to see something > like "replace duplicated password prompting code by common > monitor_read_block_device_key()". Done, for v3.
Re: [Qemu-devel] [PATCH 14/35] net: inet_connect(), inet_connect_opts(): add in_progress argument
On Fri, 10 Aug 2012 11:07:53 +0200 Markus Armbruster wrote: > Luiz Capitulino writes: > > > It's used to indicate the special case where a valid file-descriptor > > is returned (ie. success) but the connection can't be completed > > w/o blocking. > > Why callers need that isn't obvious from this patch. It's used in PATCH > 16. Either squash the two to make it obvious, or furher explain the > purpose in this commit message. I'd recommend the former. Done for v3.
Re: [Qemu-devel] [PATCH 15/35] migration: don't rely on any QERR_SOCKET_*
On Fri, 10 Aug 2012 11:13:57 +0200
Markus Armbruster wrote:
> Luiz Capitulino writes:
>
> > Use the in_progress argument for QERR_SOCKET_CONNECT_IN_PROGRESS. The
> > other errors are handled the same by checking if the error is set and
> > then calling migrate_fd_error() if it's.
> >
> > It's also necessary to change inet_connect_opts() not to set
> > QERR_SOCKET_CONNECT_IN_PROGRESS. This error is only used by
> > tcp_start_outgoing_migration() and not changing it along with the
> > usage of in_progress would break migration.
> >
> > Furthermore this commit fixes a bug. Today, there's a spurious error
> > report when migration succeeds:
> >
> > (qemu) migrate tcp:0:
> > migrate: Connection can not be completed immediately
> > (qemu)
> >
> > After this commit no spurious error is reported anymore.
> >
> > Signed-off-by: Luiz Capitulino
> > ---
> > migration-tcp.c | 22 --
> > qemu-sockets.c | 2 --
> > 2 files changed, 8 insertions(+), 16 deletions(-)
> >
> > diff --git a/migration-tcp.c b/migration-tcp.c
> > index 18944a4..40c277f 100644
> > --- a/migration-tcp.c
> > +++ b/migration-tcp.c
> > @@ -82,27 +82,21 @@ static void tcp_wait_for_connect(void *opaque)
> > int tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
> > Error **errp)
> > {
> > +bool in_progress;
> > +
> > s->get_error = socket_errno;
> > s->write = socket_write;
> > s->close = tcp_close;
> >
> > -s->fd = inet_connect(host_port, false, NULL, errp);
> > -
> > -if (!error_is_set(errp)) {
> > -migrate_fd_connect(s);
> > -} else if (error_is_type(*errp, QERR_SOCKET_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)) {
> > -DPRINTF("connect failed\n");
> > -return -1;
> > -} else if (error_is_type(*errp, QERR_SOCKET_CONNECT_FAILED)) {
> > -DPRINTF("connect failed\n");
> > +s->fd = inet_connect(host_port, false, &in_progress, errp);
> > +if (error_is_set(errp)) {
> > migrate_fd_error(s);
> > return -1;
> > +} else if (in_progress) {
> > +DPRINTF("connect in progress\n");
> > +qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
> > } else {
> > -DPRINTF("unknown error\n");
> > -return -1;
> > +migrate_fd_connect(s);
> > }
> >
> > return 0;
>
> I'd prefer
>
>s->fd = inet_connect(host_port, false, &in_progress, errp);
>if (error_is_set(errp)) {
>migrate_fd_error(s);
>return -1;
>}
>if (in_progress) {
>DPRINTF("connect in progress\n");
>qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
>} else {
>migrate_fd_connect(s);
>}
>return 0;
>
> because it separates abnormal and normal code paths more clearly.
Very minor, but I've changed it.
>
> Matter of taste.
>
> > diff --git a/qemu-sockets.c b/qemu-sockets.c
> > index 9cb47d4..361d890 100644
> > --- a/qemu-sockets.c
> > +++ b/qemu-sockets.c
> > @@ -284,8 +284,6 @@ int inet_connect_opts(QemuOpts *opts, bool
> > *in_progress, Error **errp)
> > if (in_progress) {
> > *in_progress = true;
> > }
> > -
> > -error_set(errp, QERR_SOCKET_CONNECT_IN_PROGRESS);
> > } else if (rc < 0) {
> > if (NULL == e->ai_next)
> > fprintf(stderr, "%s: connect(%s,%s,%s,%s): %s\n",
> > __FUNCTION__,
>
Re: [Qemu-devel] [PATCH 3/7] qapi: add query-machines command
On Fri, 10 Aug 2012 09:41:20 -0500
Anthony Liguori wrote:
> Luiz Capitulino writes:
>
> > On Fri, 27 Jul 2012 08:37:15 -0500
> > Anthony Liguori wrote:
> >
> >> This provides the same output as -M ? but in a structured way.
> >>
> >> Signed-off-by: Anthony Liguori
> >> ---
> >> qapi-schema.json | 28
> >> qmp-commands.hx |6 ++
> >> vl.c | 31 +++
> >> 3 files changed, 65 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/qapi-schema.json b/qapi-schema.json
> >> index 28e9914..5b47026 100644
> >> --- a/qapi-schema.json
> >> +++ b/qapi-schema.json
> >> @@ -2200,3 +2200,31 @@
> >> # Since: 0.14.0
> >> ##
> >> { 'command': 'closefd', 'data': {'fdname': 'str'} }
> >> +
> >> +##
> >> +# @MachineInfo:
> >> +#
> >> +# Information describing a machine.
> >> +#
> >> +# @name: the name of the machine
> >> +#
> >> +# @alias: #optional an alias for the machine name
> >> +#
> >> +# @default: #optional whether the machine is default
> >
> > Why is default optional?
>
> Brievity.
Can you elaborate, please?
>
> Regards,
>
> Anthony Liguori
>
> >
> >> +#
> >> +# Since: 1.2.0
> >> +##
> >> +{ 'type': 'MachineInfo',
> >> + 'data': { 'name': 'str', '*alias': 'str',
> >> +'*is-default': 'bool' } }
> >> +
> >> +##
> >> +# @query-machines:
> >> +#
> >> +# Return a list of supported machines
> >> +#
> >> +# Returns: a list of MachineInfo
> >> +#
> >> +# Since: 1.2.0
> >> +##
> >> +{ 'command': 'query-machines', 'returns': ['MachineInfo'] }
> >> diff --git a/qmp-commands.hx b/qmp-commands.hx
> >> index 5c55528..a6f82fc 100644
> >> --- a/qmp-commands.hx
> >> +++ b/qmp-commands.hx
> >> @@ -,3 +,9 @@ EQMP
> >> .mhandler.cmd_new = qmp_marshal_input_device_list_properties,
> >> },
> >>
> >> +{
> >> +.name = "query-machines",
> >> +.args_type = "",
> >> +.mhandler.cmd_new = qmp_marshal_input_query_machines,
> >> +},
> >> +
> >> diff --git a/vl.c b/vl.c
> >> index 8904db1..cd900e0 100644
> >> --- a/vl.c
> >> +++ b/vl.c
> >> @@ -1209,6 +1209,37 @@ QEMUMachine *find_default_machine(void)
> >> return NULL;
> >> }
> >>
> >> +MachineInfoList *qmp_query_machines(Error **errp)
> >> +{
> >> +MachineInfoList *mach_list = NULL;
> >> +QEMUMachine *m;
> >> +
> >> +for (m = first_machine; m; m = m->next) {
> >> +MachineInfoList *entry;
> >> +MachineInfo *info;
> >> +
> >> +info = g_malloc0(sizeof(*info));
> >> +if (m->is_default) {
> >> +info->has_is_default = true;
> >> +info->is_default = true;
> >> +}
> >> +
> >> +if (m->alias) {
> >> +info->has_alias = true;
> >> +info->alias = g_strdup(m->alias);
> >> +}
> >> +
> >> +info->name = g_strdup(m->name);
> >> +
> >> +entry = g_malloc0(sizeof(*entry));
> >> +entry->value = info;
> >> +entry->next = mach_list;
> >> +mach_list = entry;
> >> +}
> >> +
> >> +return mach_list;
> >> +}
> >> +
> >> /***/
> >> /* main execution loop */
> >>
>
Re: [Qemu-devel] [PATCH 3/7] qapi: add query-machines command
On Fri, 10 Aug 2012 11:06:14 -0500
Anthony Liguori wrote:
> Luiz Capitulino writes:
>
> > On Fri, 10 Aug 2012 09:41:20 -0500
> > Anthony Liguori wrote:
> >
> >> Luiz Capitulino writes:
> >>
> >> > On Fri, 27 Jul 2012 08:37:15 -0500
> >> > Anthony Liguori wrote:
> >> >
> >> >> This provides the same output as -M ? but in a structured way.
> >> >>
> >> >> Signed-off-by: Anthony Liguori
> >> >> ---
> >> >> qapi-schema.json | 28
> >> >> qmp-commands.hx |6 ++
> >> >> vl.c | 31 +++
> >> >> 3 files changed, 65 insertions(+), 0 deletions(-)
> >> >>
> >> >> diff --git a/qapi-schema.json b/qapi-schema.json
> >> >> index 28e9914..5b47026 100644
> >> >> --- a/qapi-schema.json
> >> >> +++ b/qapi-schema.json
> >> >> @@ -2200,3 +2200,31 @@
> >> >> # Since: 0.14.0
> >> >> ##
> >> >> { 'command': 'closefd', 'data': {'fdname': 'str'} }
> >> >> +
> >> >> +##
> >> >> +# @MachineInfo:
> >> >> +#
> >> >> +# Information describing a machine.
> >> >> +#
> >> >> +# @name: the name of the machine
> >> >> +#
> >> >> +# @alias: #optional an alias for the machine name
> >> >> +#
> >> >> +# @default: #optional whether the machine is default
> >> >
> >> > Why is default optional?
> >>
> >> Brievity.
> >
> > Can you elaborate, please?
>
> There is only one machine that is default. Having default=false for all
> of the rest just adds a lot of unnecessary information in the response.
I think it's more consistent to have the key (also, there are better ways
to save bytes on the wire if this is an issue), but I don't mind much though.
Re: [Qemu-devel] [PATCH 11/34] qmp: query-block: add 'valid_encryption_key' field
On Fri, 10 Aug 2012 18:35:26 +0200
Markus Armbruster wrote:
> Luiz Capitulino writes:
>
> > On Fri, 10 Aug 2012 09:56:11 +0200
> > Markus Armbruster wrote:
> >
> >> Revisited this one on review of v2, replying here for context.
> >>
> >> Luiz Capitulino writes:
> >>
> >> > On Thu, 02 Aug 2012 13:35:54 +0200
> >> > Markus Armbruster wrote:
> >> >
> >> >> Luiz Capitulino writes:
> >> >>
> >> >> > Signed-off-by: Luiz Capitulino
> >> >> > ---
> >> >> > block.c | 1 +
> >> >> > qapi-schema.json | 7 +--
> >> >> > 2 files changed, 6 insertions(+), 2 deletions(-)
> >> >> >
> >> >> > diff --git a/block.c b/block.c
> >> >> > index b38940b..9c113b8 100644
> >> >> > --- a/block.c
> >> >> > +++ b/block.c
> >> >> > @@ -2445,6 +2445,7 @@ BlockInfoList *qmp_query_block(Error **errp)
> >> >> > info->value->inserted->ro = bs->read_only;
> >> >> > info->value->inserted->drv =
> >> >> > g_strdup(bs->drv->format_name);
> >> >> > info->value->inserted->encrypted = bs->encrypted;
> >> >> > +info->value->inserted->valid_encryption_key =
> >> >> > bs->valid_key;
> >> >> > if (bs->backing_file[0]) {
> >> >> > info->value->inserted->has_backing_file = true;
> >> >> > info->value->inserted->backing_file =
> >> >> > g_strdup(bs->backing_file);
> >> >> > diff --git a/qapi-schema.json b/qapi-schema.json
> >> >> > index bc55ed2..1b2d7f5 100644
> >> >> > --- a/qapi-schema.json
> >> >> > +++ b/qapi-schema.json
> >> >> > @@ -400,6 +400,8 @@
> >> >> > #
> >> >> > # @encrypted: true if the backing device is encrypted
> >> >> > #
> >> >> > +# @valid_encryption_key: true if a valid encryption key has been set
> >> >> > +#
> >> >> > # @bps: total throughput limit in bytes per second is specified
> >> >> > #
> >> >> > # @bps_rd: read throughput limit in bytes per second is specified
> >> >> > @@ -419,8 +421,9 @@
> >> >> > { 'type': 'BlockDeviceInfo',
> >> >> >'data': { 'file': 'str', 'ro': 'bool', 'drv': 'str',
> >> >> > '*backing_file': 'str', 'encrypted': 'bool',
> >> >> > -'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
> >> >> > -'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int'} }
> >> >> > +'valid_encryption_key': 'bool', 'bps': 'int',
> >> >> > +'bps_rd': 'int', 'bps_wr': 'int', 'iops': 'int',
> >> >> > +'iops_rd': 'int', 'iops_wr': 'int'} }
> >> >> >
> >> >> > ##
> >> >> > # @BlockDeviceIoStatus:
> >> >>
> >> >> BlockDeviceInfo is API, isn't it?
> >> >
> >> > Yes.
> >> >
> >> >> Note that bs->valid_key currently implies bs->encrypted. bs->valid_key
> >> >> && !bs->encrypted is impossible. Should we make valid_encryption_key
> >> >> only available when encrypted?
> >> >
> >> > I don't think so. It's a bool, so it's ok for it to be false when
> >> > encrypted is false.
> >>
> >> What bothers me is encrypted=false, valid_encryption_key=true.
> >
> > Disappearing keys is worse, IMHO (assuming that that situation is impossible
> > in practice, of course).
>
> It's fundamentally three states: unencrypted, encrypted-no-key,
> encrypted-got-key. I'm fine with mapping these onto two bools, it's how
> the block layer does it. You may want to consider a single enumeration
> instead.
T
[Qemu-devel] [PATCH 01/35] monitor: drop unused monitor debug code
In the old QMP days, this code was used to find out QMP commands that
might be calling monitor_printf() down its call chain.
This is almost impossible to happen today, because the qapi converted
commands don't even have a monitor object. Besides, it's been more than
a year since I used this last time.
Let's just drop it.
Signed-off-by: Luiz Capitulino
---
configure | 10 --
monitor.c | 65 ---
2 files changed, 75 deletions(-)
diff --git a/configure b/configure
index 280726c..030d137 100755
--- a/configure
+++ b/configure
@@ -147,7 +147,6 @@ vhost_net="no"
kvm="no"
gprof="no"
debug_tcg="no"
-debug_mon="no"
debug="no"
strip_opt="yes"
tcg_interpreter="no"
@@ -633,14 +632,9 @@ for opt do
;;
--disable-debug-tcg) debug_tcg="no"
;;
- --enable-debug-mon) debug_mon="yes"
- ;;
- --disable-debug-mon) debug_mon="no"
- ;;
--enable-debug)
# Enable debugging options that aren't excessively noisy
debug_tcg="yes"
- debug_mon="yes"
debug="yes"
strip_opt="no"
;;
@@ -3039,7 +3033,6 @@ echo "host CPU $cpu"
echo "host big endian $bigendian"
echo "target list $target_list"
echo "tcg debug enabled $debug_tcg"
-echo "Mon debug enabled $debug_mon"
echo "gprof enabled $gprof"
echo "sparse enabled$sparse"
echo "strip binaries$strip_opt"
@@ -3132,9 +3125,6 @@ echo "ARCH=$ARCH" >> $config_host_mak
if test "$debug_tcg" = "yes" ; then
echo "CONFIG_DEBUG_TCG=y" >> $config_host_mak
fi
-if test "$debug_mon" = "yes" ; then
- echo "CONFIG_DEBUG_MONITOR=y" >> $config_host_mak
-fi
if test "$debug" = "yes" ; then
echo "CONFIG_DEBUG_EXEC=y" >> $config_host_mak
fi
diff --git a/monitor.c b/monitor.c
index 49dccfe..aa57167 100644
--- a/monitor.c
+++ b/monitor.c
@@ -172,41 +172,11 @@ struct Monitor {
CPUArchState *mon_cpu;
BlockDriverCompletionFunc *password_completion_cb;
void *password_opaque;
-#ifdef CONFIG_DEBUG_MONITOR
-int print_calls_nr;
-#endif
QError *error;
QLIST_HEAD(,mon_fd_t) fds;
QLIST_ENTRY(Monitor) entry;
};
-#ifdef CONFIG_DEBUG_MONITOR
-#define MON_DEBUG(fmt, ...) do {\
-fprintf(stderr, "Monitor: "); \
-fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
-
-static inline void mon_print_count_inc(Monitor *mon)
-{
-mon->print_calls_nr++;
-}
-
-static inline void mon_print_count_init(Monitor *mon)
-{
-mon->print_calls_nr = 0;
-}
-
-static inline int mon_print_count_get(const Monitor *mon)
-{
-return mon->print_calls_nr;
-}
-
-#else /* !CONFIG_DEBUG_MONITOR */
-#define MON_DEBUG(fmt, ...) do { } while (0)
-static inline void mon_print_count_inc(Monitor *mon) { }
-static inline void mon_print_count_init(Monitor *mon) { }
-static inline int mon_print_count_get(const Monitor *mon) { return 0; }
-#endif /* CONFIG_DEBUG_MONITOR */
-
/* QMP checker flags */
#define QMP_ACCEPT_UNKNOWNS 1
@@ -299,8 +269,6 @@ void monitor_vprintf(Monitor *mon, const char *fmt, va_list
ap)
if (!mon)
return;
-mon_print_count_inc(mon);
-
if (monitor_ctrl_mode(mon)) {
return;
}
@@ -3860,8 +3828,6 @@ void monitor_set_error(Monitor *mon, QError *qerror)
if (!mon->error) {
mon->error = qerror;
} else {
-MON_DEBUG("Additional error report at %s:%d\n",
- qerror->file, qerror->linenr);
QDECREF(qerror);
}
}
@@ -3875,36 +3841,7 @@ static void handler_audit(Monitor *mon, const mon_cmd_t
*cmd, int ret)
* Action: Report an internal error to the client if in QMP.
*/
qerror_report(QERR_UNDEFINED_ERROR);
-MON_DEBUG("command '%s' returned failure but did not pass an error\n",
- cmd->name);
}
-
-#ifdef CONFIG_DEBUG_MONITOR
-if (!ret && monitor_has_error(mon)) {
-/*
- * If it returns success, it must not have passed an error.
- *
- * Action: Report the passed error to the client.
- */
-MON_DEBUG("command '%s' returned success but passed an error\n",
- cmd->name);
-}
-
-if (mon_print_count_get(mon) > 0 && strcmp(cmd->name, "info") != 0) {
-/*
- * Handlers should not call Monitor print functions.
- *
- * Action: Ignore them in QMP.
- *
- * (XXX: we don't check any 'info' or 'query' command here
- * because the user print function
[Qemu-devel] [PATCH 02/35] qerror: QERR_AMBIGUOUS_PATH: drop %(object) from human msg
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
---
qerror.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qerror.c b/qerror.c
index 92c4eff..082de98 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 an object"
},
{
.error_fmt = QERR_BAD_BUS_FOR_DEVICE,
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 14/35] net: inet_connect(), inet_connect_opts(): add in_progress argument
It's used to indicate the special case where a valid file-descriptor
is returned (ie. success) but the connection can't be completed
w/o blocking.
This is needed because QERR_SOCKET_CONNECT_IN_PROGRESS is not
treated like an error and a future commit will drop it.
Signed-off-by: Luiz Capitulino
---
migration-tcp.c | 2 +-
nbd.c | 2 +-
qemu-char.c | 2 +-
qemu-sockets.c | 14 +++---
qemu_socket.h | 4 ++--
ui/vnc.c| 2 +-
6 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/migration-tcp.c b/migration-tcp.c
index 440804d..18944a4 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -86,7 +86,7 @@ int tcp_start_outgoing_migration(MigrationState *s, const
char *host_port,
s->write = socket_write;
s->close = tcp_close;
-s->fd = inet_connect(host_port, false, errp);
+s->fd = inet_connect(host_port, false, NULL, errp);
if (!error_is_set(errp)) {
migrate_fd_connect(s);
diff --git a/nbd.c b/nbd.c
index dc0adf9..0dd60c5 100644
--- a/nbd.c
+++ b/nbd.c
@@ -162,7 +162,7 @@ int tcp_socket_outgoing(const char *address, uint16_t port)
int tcp_socket_outgoing_spec(const char *address_and_port)
{
-return inet_connect(address_and_port, true, NULL);
+return inet_connect(address_and_port, true, NULL, NULL);
}
int tcp_socket_incoming(const char *address, uint16_t port)
diff --git a/qemu-char.c b/qemu-char.c
index c2aaaee..382c71e 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2446,7 +2446,7 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts
*opts)
if (is_listen) {
fd = inet_listen_opts(opts, 0, NULL);
} else {
-fd = inet_connect_opts(opts, NULL);
+fd = inet_connect_opts(opts, NULL, NULL);
}
}
if (fd < 0) {
diff --git a/qemu-sockets.c b/qemu-sockets.c
index beb2bb6..9cb47d4 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -209,7 +209,7 @@ listen:
return slisten;
}
-int inet_connect_opts(QemuOpts *opts, Error **errp)
+int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp)
{
struct addrinfo ai,*res,*e;
const char *addr;
@@ -224,6 +224,10 @@ int inet_connect_opts(QemuOpts *opts, Error **errp)
ai.ai_family = PF_UNSPEC;
ai.ai_socktype = SOCK_STREAM;
+if (in_progress) {
+*in_progress = false;
+}
+
addr = qemu_opt_get(opts, "host");
port = qemu_opt_get(opts, "port");
block = qemu_opt_get_bool(opts, "block", 0);
@@ -277,6 +281,10 @@ int inet_connect_opts(QemuOpts *opts, Error **errp)
#else
if (!block && (rc == -EINPROGRESS)) {
#endif
+if (in_progress) {
+*in_progress = true;
+}
+
error_set(errp, QERR_SOCKET_CONNECT_IN_PROGRESS);
} else if (rc < 0) {
if (NULL == e->ai_next)
@@ -487,7 +495,7 @@ int inet_listen(const char *str, char *ostr, int olen,
return sock;
}
-int inet_connect(const char *str, bool block, Error **errp)
+int inet_connect(const char *str, bool block, bool *in_progress, Error **errp)
{
QemuOpts *opts;
int sock = -1;
@@ -497,7 +505,7 @@ int inet_connect(const char *str, bool block, Error **errp)
if (block) {
qemu_opt_set(opts, "block", "on");
}
-sock = inet_connect_opts(opts, errp);
+sock = inet_connect_opts(opts, in_progress, errp);
} else {
error_set(errp, QERR_SOCKET_CREATE_FAILED);
}
diff --git a/qemu_socket.h b/qemu_socket.h
index 4689ff3..30ae6af 100644
--- a/qemu_socket.h
+++ b/qemu_socket.h
@@ -42,8 +42,8 @@ int send_all(int fd, const void *buf, int len1);
int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp);
int inet_listen(const char *str, char *ostr, int olen,
int socktype, int port_offset, Error **errp);
-int inet_connect_opts(QemuOpts *opts, Error **errp);
-int inet_connect(const char *str, bool block, Error **errp);
+int inet_connect_opts(QemuOpts *opts, bool *in_progress, Error **errp);
+int inet_connect(const char *str, bool block, bool *in_progress, Error **errp);
int inet_dgram_opts(QemuOpts *opts);
const char *inet_strfamily(int family);
diff --git a/ui/vnc.c b/ui/vnc.c
index 312ad7f..385e345 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3061,7 +3061,7 @@ int vnc_display_open(DisplayState *ds, const char
*display)
if (strncmp(display, "unix:", 5) == 0)
vs->lsock = unix_connect(display+5);
else
-vs->lsock = inet_connect(display, true, NULL);
+vs->lsock = inet_connect(display, true, NULL, NULL);
if (-1 == vs->lsock) {
g_free(vs->display);
vs->display = NULL;
--
1.7.11.2.249.g31c7954.dirty
Re: [Qemu-devel] [PATCH 11/34] qmp: query-block: add 'valid_encryption_key' field
On Fri, 10 Aug 2012 19:17:22 +0200
Markus Armbruster wrote:
> Luiz Capitulino writes:
>
> > On Fri, 10 Aug 2012 18:35:26 +0200
> > Markus Armbruster wrote:
> >
> >> Luiz Capitulino writes:
> >>
> >> > On Fri, 10 Aug 2012 09:56:11 +0200
> >> > Markus Armbruster wrote:
> >> >
> >> >> Revisited this one on review of v2, replying here for context.
> >> >>
> >> >> Luiz Capitulino writes:
> >> >>
> >> >> > On Thu, 02 Aug 2012 13:35:54 +0200
> >> >> > Markus Armbruster wrote:
> >> >> >
> >> >> >> Luiz Capitulino writes:
> >> >> >>
> >> >> >> > Signed-off-by: Luiz Capitulino
> >> >> >> > ---
> >> >> >> > block.c | 1 +
> >> >> >> > qapi-schema.json | 7 +--
> >> >> >> > 2 files changed, 6 insertions(+), 2 deletions(-)
> >> >> >> >
> >> >> >> > diff --git a/block.c b/block.c
> >> >> >> > index b38940b..9c113b8 100644
> >> >> >> > --- a/block.c
> >> >> >> > +++ b/block.c
> >> >> >> > @@ -2445,6 +2445,7 @@ BlockInfoList *qmp_query_block(Error **errp)
> >> >> >> > info->value->inserted->ro = bs->read_only;
> >> >> >> > info->value->inserted->drv =
> >> >> >> > g_strdup(bs->drv->format_name);
> >> >> >> > info->value->inserted->encrypted = bs->encrypted;
> >> >> >> > +info->value->inserted->valid_encryption_key =
> >> >> >> > bs->valid_key;
> >> >> >> > if (bs->backing_file[0]) {
> >> >> >> > info->value->inserted->has_backing_file = true;
> >> >> >> > info->value->inserted->backing_file =
> >> >> >> > g_strdup(bs->backing_file);
> >> >> >> > diff --git a/qapi-schema.json b/qapi-schema.json
> >> >> >> > index bc55ed2..1b2d7f5 100644
> >> >> >> > --- a/qapi-schema.json
> >> >> >> > +++ b/qapi-schema.json
> >> >> >> > @@ -400,6 +400,8 @@
> >> >> >> > #
> >> >> >> > # @encrypted: true if the backing device is encrypted
> >> >> >> > #
> >> >> >> > +# @valid_encryption_key: true if a valid encryption key has been
> >> >> >> > set
> >> >> >> > +#
> >> >> >> > # @bps: total throughput limit in bytes per second is specified
> >> >> >> > #
> >> >> >> > # @bps_rd: read throughput limit in bytes per second is specified
> >> >> >> > @@ -419,8 +421,9 @@
> >> >> >> > { 'type': 'BlockDeviceInfo',
> >> >> >> >'data': { 'file': 'str', 'ro': 'bool', 'drv': 'str',
> >> >> >> > '*backing_file': 'str', 'encrypted': 'bool',
> >> >> >> > -'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
> >> >> >> > -'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int'} }
> >> >> >> > +'valid_encryption_key': 'bool', 'bps': 'int',
> >> >> >> > +'bps_rd': 'int', 'bps_wr': 'int', 'iops': 'int',
> >> >> >> > +'iops_rd': 'int', 'iops_wr': 'int'} }
> >> >> >> >
> >> >> >> > ##
> >> >> >> > # @BlockDeviceIoStatus:
> >> >> >>
> >> >> >> BlockDeviceInfo is API, isn't it?
> >> >> >
> >> >> > Yes.
> >> >> >
> >> >> >> Note that bs->valid_key currently implies bs->encrypted.
> >> >> >> bs->valid_key
> >>
[Qemu-devel] [PATCH 10/35] error: don't delay error message construction
Today, the error message is only constructed when it's used. This commit
changes that to construct the error message when the error object is
built (ie. when the error is reported).
This simplifies the Error object.
Signed-off-by: Luiz Capitulino
---
error.c | 8 +---
qerror.c | 4 +---
2 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/error.c b/error.c
index 3a62592..2ade99b 100644
--- a/error.c
+++ b/error.c
@@ -20,7 +20,6 @@
struct Error
{
QDict *obj;
-const char *fmt;
char *msg;
};
@@ -39,7 +38,7 @@ void error_set(Error **errp, const char *fmt, ...)
va_start(ap, fmt);
err->obj = qobject_to_qdict(qobject_from_jsonv(fmt, &ap));
va_end(ap);
-err->fmt = fmt;
+err->msg = qerror_format(fmt, err->obj);
*errp = err;
}
@@ -50,7 +49,6 @@ Error *error_copy(const Error *err)
err_new = g_malloc0(sizeof(*err));
err_new->msg = g_strdup(err->msg);
-err_new->fmt = err->fmt;
err_new->obj = err->obj;
QINCREF(err_new->obj);
@@ -64,10 +62,6 @@ bool error_is_set(Error **errp)
const char *error_get_pretty(Error *err)
{
-if (err->msg == NULL) {
-err->msg = qerror_format(err->fmt, err->obj);
-}
-
return err->msg;
}
diff --git a/qerror.c b/qerror.c
index a254f88..5d38428 100644
--- a/qerror.c
+++ b/qerror.c
@@ -543,7 +543,6 @@ void qerror_report(const char *fmt, ...)
struct Error
{
QDict *obj;
-const char *fmt;
char *msg;
};
@@ -555,8 +554,7 @@ void qerror_report_err(Error *err)
loc_save(&qerr->loc);
QINCREF(err->obj);
qerr->error = err->obj;
-
-qerr->err_msg = qerror_format(err->fmt, qerr->error);
+qerr->err_msg = g_strdup(err->msg);
if (monitor_cur_is_qmp()) {
monitor_set_error(cur_mon, qerr);
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 28/35] error: drop unused functions
Besides of being unused, they operate on the current error format,
which is going to be replaced soon.
Signed-off-by: Luiz Capitulino
---
error.c | 48
error.h | 16
error_int.h | 1 -
3 files changed, 65 deletions(-)
diff --git a/error.c b/error.c
index 2d34cde..b1d5131 100644
--- a/error.c
+++ b/error.c
@@ -74,29 +74,6 @@ const char *error_get_pretty(Error *err)
return err->msg;
}
-const char *error_get_field(Error *err, const char *field)
-{
-if (strcmp(field, "class") == 0) {
-return qdict_get_str(err->obj, field);
-} else {
-QDict *dict = qdict_get_qdict(err->obj, "data");
-return qdict_get_str(dict, field);
-}
-}
-
-QDict *error_get_data(Error *err)
-{
-QDict *data = qdict_get_qdict(err->obj, "data");
-QINCREF(data);
-return data;
-}
-
-void error_set_field(Error *err, const char *field, const char *value)
-{
-QDict *dict = qdict_get_qdict(err->obj, "data");
-qdict_put(dict, field, qstring_from_str(value));
-}
-
void error_free(Error *err)
{
if (err) {
@@ -106,31 +83,6 @@ void error_free(Error *err)
}
}
-bool error_is_type(Error *err, ErrorClass err_class, const char *fmt)
-{
-const char *error_class;
-char *ptr;
-char *end;
-
-if (!err) {
-return false;
-}
-
-ptr = strstr(fmt, "'class': '");
-assert(ptr != NULL);
-ptr += strlen("'class': '");
-
-end = strchr(ptr, '\'');
-assert(end != NULL);
-
-error_class = error_get_field(err, "class");
-if (strlen(error_class) != end - ptr) {
-return false;
-}
-
-return strncmp(ptr, error_class, end - ptr) == 0;
-}
-
void error_propagate(Error **dst_err, Error *local_err)
{
if (dst_err && !*dst_err) {
diff --git a/error.h b/error.h
index 114e24b..5336fc5 100644
--- a/error.h
+++ b/error.h
@@ -51,16 +51,6 @@ Error *error_copy(const Error *err);
const char *error_get_pretty(Error *err);
/**
- * Get an individual named error field.
- */
-const char *error_get_field(Error *err, const char *field);
-
-/**
- * Get an individual named error field.
- */
-void error_set_field(Error *err, const char *field, const char *value);
-
-/**
* Propagate an error to an indirect pointer to an error. This function will
* always transfer ownership of the error reference and handles the case where
* dst_err is NULL correctly. Errors after the first are discarded.
@@ -72,10 +62,4 @@ void error_propagate(Error **dst_err, Error *local_err);
*/
void error_free(Error *err);
-/**
- * Determine if an error is of a speific type (based on the qerror format).
- * Non-QEMU users should get the `class' field to identify the error type.
- */
-bool error_is_type(Error *err, ErrorClass err_class, const char *fmt);
-
#endif
diff --git a/error_int.h b/error_int.h
index 5e39424..4b00d08 100644
--- a/error_int.h
+++ b/error_int.h
@@ -22,7 +22,6 @@
*
* These are used to convert QErrors to Errors
*/
-QDict *error_get_data(Error *err);
QObject *error_get_qobject(Error *err);
void error_set_qobject(Error **errp, QObject *obj);
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 35/35] docs: writing-qmp-commands.txt: update error section
Add information about the new error format and improve the text a bit.
Signed-off-by: Luiz Capitulino
---
docs/writing-qmp-commands.txt | 47 +--
1 file changed, 27 insertions(+), 20 deletions(-)
diff --git a/docs/writing-qmp-commands.txt b/docs/writing-qmp-commands.txt
index 0ad51aa..8349dec 100644
--- a/docs/writing-qmp-commands.txt
+++ b/docs/writing-qmp-commands.txt
@@ -210,19 +210,17 @@ if you don't see these strings, then something went wrong.
=== Errors ===
QMP commands should use the error interface exported by the error.h header
-file. The basic function used to set an error is the error_set() one.
+file. Basically, errors are set by calling the error_set() function.
Let's say we don't accept the string "message" to contain the word "love". If
-it does contain it, we want the "hello-world" command to the return the
-InvalidParameter error.
-
-Only one change is required, and it's in the C implementation:
+it does contain it, we want the "hello-world" command to return an error:
void qmp_hello_world(bool has_message, const char *message, Error **errp)
{
if (has_message) {
if (strstr(message, "love")) {
-error_set(errp, QERR_INVALID_PARAMETER, "message");
+error_set(errp, ERROR_CLASS_GENERIC_ERROR,
+ "the word 'love' is not allowed");
return;
}
printf("%s\n", message);
@@ -231,30 +229,40 @@ void qmp_hello_world(bool has_message, const char
*message, Error **errp)
}
}
-Let's test it. Build qemu, run it as defined in the "Testing" section, and
-then issue the following command:
+The first argument to the error_set() function is the Error pointer to pointer,
+which is passed to all QMP functions. The second argument is a ErrorClass
+value, which should be ERROR_CLASS_GENERIC_ERROR most of the time (more
+details about error classes are given below). The third argument is a human
+description of the error, this is a free-form printf-like string.
+
+Let's test the example above. Build qemu, run it as defined in the "Testing"
+section, and then issue the following command:
-{ "execute": "hello-world", "arguments": { "message": "we love qemu" } }
+{ "execute": "hello-world", "arguments": { "message": "all you need is love" }
}
The QMP server's response should be:
{
"error": {
-"class": "InvalidParameter",
-"desc": "Invalid parameter 'message'",
-"data": {
-"name": "message"
-}
+"class": "GenericError",
+"desc": "the word 'love' is not allowed"
}
}
-Which is the InvalidParameter error.
+As a general rule, all QMP errors should use ERROR_CLASS_GENERIC_ERROR. There
+are two exceptions to this rule:
+
+ 1. A non-generic ErrorClass value exists* for the failure you want to report
+(eg. DeviceNotFound)
+
+ 2. Management applications have to take special action on the failure you
+want to report, hence you have to add a new ErrorClass value so that they
+can check for it
-When you have to return an error but you're unsure what error to return or
-which arguments an error takes, you should look at the qerror.h file. Note
-that you might be required to add new errors if needed.
+If the failure you want to report doesn't fall in one of the two cases above,
+just report ERROR_CLASS_GENERIC_ERROR.
-FIXME: describe better the error API and how to add new errors.
+ * All existing ErrorClass values are defined in the qapi-schema.json file
=== Command Documentation ===
@@ -275,7 +283,6 @@ here goes "hello-world"'s new entry for the
qapi-schema.json file:
# @message: #optional string to be printed
#
# Returns: Nothing on success.
-# If @message contains "love", InvalidParameter
#
# Notes: if @message is not provided, the "Hello, world" string will
#be printed instead
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 13/35] hmp_change(): don't access DeviceEncrypted's data
It's not needed. As the device name is already known, we can replace
the duplicated password prompting code by monitor_read_block_device_key().
This overly simplifies hmp_change().
Signed-off-by: Luiz Capitulino
---
hmp.c | 29 ++---
1 file changed, 2 insertions(+), 27 deletions(-)
diff --git a/hmp.c b/hmp.c
index 4efaf51..54c37d7 100644
--- a/hmp.c
+++ b/hmp.c
@@ -776,22 +776,6 @@ static void hmp_change_read_arg(Monitor *mon, const char
*password,
monitor_read_command(mon, 1);
}
-static void cb_hmp_change_bdrv_pwd(Monitor *mon, const char *password,
- void *opaque)
-{
-Error *encryption_err = opaque;
-Error *err = NULL;
-const char *device;
-
-device = error_get_field(encryption_err, "device");
-
-qmp_block_passwd(device, password, &err);
-hmp_handle_error(mon, &err);
-error_free(encryption_err);
-
-monitor_read_command(mon, 1);
-}
-
void hmp_change(Monitor *mon, const QDict *qdict)
{
const char *device = qdict_get_str(qdict, "device");
@@ -810,17 +794,8 @@ void hmp_change(Monitor *mon, const QDict *qdict)
qmp_change(device, target, !!arg, arg, &err);
if (error_is_type(err, QERR_DEVICE_ENCRYPTED)) {
-monitor_printf(mon, "%s (%s) is encrypted.\n",
- error_get_field(err, "device"),
- error_get_field(err, "filename"));
-if (!monitor_get_rs(mon)) {
-monitor_printf(mon,
-"terminal does not support password prompting\n");
-error_free(err);
-return;
-}
-readline_start(monitor_get_rs(mon), "Password: ", 1,
- cb_hmp_change_bdrv_pwd, err);
+error_free(err);
+monitor_read_block_device_key(mon, device, NULL, NULL);
return;
}
hmp_handle_error(mon, &err);
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 31/35] error: drop error_get_qobject()/error_set_qobject()
error_get_qobject() is unused since last commit, error_set_qobject()
has never been used. Also drops error_int.h.
Signed-off-by: Luiz Capitulino
---
error.c | 20
error_int.h | 28
qapi/qmp-dispatch.c | 1 -
qemu-ga.c | 1 -
4 files changed, 50 deletions(-)
delete mode 100644 error_int.h
diff --git a/error.c b/error.c
index b1d5131..9d7b35f 100644
--- a/error.c
+++ b/error.c
@@ -15,7 +15,6 @@
#include "qjson.h"
#include "qdict.h"
#include "qapi-types.h"
-#include "error_int.h"
#include "qerror.h"
struct Error
@@ -91,22 +90,3 @@ void error_propagate(Error **dst_err, Error *local_err)
error_free(local_err);
}
}
-
-QObject *error_get_qobject(Error *err)
-{
-QINCREF(err->obj);
-return QOBJECT(err->obj);
-}
-
-void error_set_qobject(Error **errp, QObject *obj)
-{
-Error *err;
-if (errp == NULL) {
-return;
-}
-err = g_malloc0(sizeof(*err));
-err->obj = qobject_to_qdict(obj);
-qobject_incref(obj);
-
-*errp = err;
-}
diff --git a/error_int.h b/error_int.h
deleted file mode 100644
index 4b00d08..000
--- a/error_int.h
+++ /dev/null
@@ -1,28 +0,0 @@
-/*
- * QEMU Error Objects
- *
- * Copyright IBM, Corp. 2011
- *
- * Authors:
- * Anthony Liguori
- *
- * This work is licensed under the terms of the GNU LGPL, version 2. See
- * the COPYING.LIB file in the top-level directory.
- */
-#ifndef QEMU_ERROR_INT_H
-#define QEMU_ERROR_INT_H
-
-#include "qemu-common.h"
-#include "qobject.h"
-#include "qdict.h"
-#include "error.h"
-
-/**
- * Internal QEMU functions for working with Error.
- *
- * These are used to convert QErrors to Errors
- */
-QObject *error_get_qobject(Error *err);
-void error_set_qobject(Error **errp, QObject *obj);
-
-#endif
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index ec613f8..4085994 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -16,7 +16,6 @@
#include "json-parser.h"
#include "qapi-types.h"
#include "error.h"
-#include "error_int.h"
#include "qerror.h"
static QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp)
diff --git a/qemu-ga.c b/qemu-ga.c
index 39abc50..8f87621 100644
--- a/qemu-ga.c
+++ b/qemu-ga.c
@@ -28,7 +28,6 @@
#include "module.h"
#include "signal.h"
#include "qerror.h"
-#include "error_int.h"
#include "qapi/qmp-core.h"
#include "qga/channel.h"
#ifdef _WIN32
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 24/35] error, qerror: add ErrorClass argument to error functions
The new argument is added to functions qerror_report() and error_set().
It's stored in Error and QError. qerror_report_err() is also updated to
take care of it.
The QERR_ macros are changed to contain a place holder value for the
new argument, so that the value is used on all current calls to
qerror_report() and error_set() (and also to initialize qerror_table[]).
Next commit will update the QERR_ macros with a proper ErrorClass
value.
Signed-off-by: Luiz Capitulino
---
error.c | 8 +++-
error.h | 5 ++-
qerror.c | 10 +++--
qerror.h | 145 ---
4 files changed, 90 insertions(+), 78 deletions(-)
diff --git a/error.c b/error.c
index 2ade99b..648706a 100644
--- a/error.c
+++ b/error.c
@@ -14,6 +14,7 @@
#include "error.h"
#include "qjson.h"
#include "qdict.h"
+#include "qapi-types.h"
#include "error_int.h"
#include "qerror.h"
@@ -21,9 +22,10 @@ struct Error
{
QDict *obj;
char *msg;
+ErrorClass err_class;
};
-void error_set(Error **errp, const char *fmt, ...)
+void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
{
Error *err;
va_list ap;
@@ -39,6 +41,7 @@ void error_set(Error **errp, const char *fmt, ...)
err->obj = qobject_to_qdict(qobject_from_jsonv(fmt, &ap));
va_end(ap);
err->msg = qerror_format(fmt, err->obj);
+err->err_class = err_class;
*errp = err;
}
@@ -49,6 +52,7 @@ Error *error_copy(const Error *err)
err_new = g_malloc0(sizeof(*err));
err_new->msg = g_strdup(err->msg);
+err_new->err_class = err->err_class;
err_new->obj = err->obj;
QINCREF(err_new->obj);
@@ -97,7 +101,7 @@ void error_free(Error *err)
}
}
-bool error_is_type(Error *err, const char *fmt)
+bool error_is_type(Error *err, ErrorClass err_class, const char *fmt)
{
const char *error_class;
char *ptr;
diff --git a/error.h b/error.h
index 3d9d96d..9678752 100644
--- a/error.h
+++ b/error.h
@@ -13,6 +13,7 @@
#define ERROR_H
#include "compiler.h"
+#include "qapi-types.h"
#include
/**
@@ -26,7 +27,7 @@ typedef struct Error Error;
* Currently, qerror.h defines these error formats. This function is not
* meant to be used outside of QEMU.
*/
-void error_set(Error **err, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
+void error_set(Error **err, ErrorClass err_class, const char *fmt, ...)
GCC_FMT_ATTR(3, 4);
/**
* Returns true if an indirect pointer to an error is pointing to a valid
@@ -70,6 +71,6 @@ void error_free(Error *err);
* Determine if an error is of a speific type (based on the qerror format).
* Non-QEMU users should get the `class' field to identify the error type.
*/
-bool error_is_type(Error *err, const char *fmt);
+bool error_is_type(Error *err, ErrorClass err_class, const char *fmt);
#endif
diff --git a/qerror.c b/qerror.c
index ff460b0..0bf8aec 100644
--- a/qerror.c
+++ b/qerror.c
@@ -386,13 +386,15 @@ static QDict *error_obj_from_fmt_no_fail(const char *fmt,
va_list *va)
*
* Return strong reference.
*/
-static QError *qerror_from_info(const char *fmt, va_list *va)
+static QError *qerror_from_info(ErrorClass err_class, const char *fmt,
+va_list *va)
{
QError *qerr;
qerr = qerror_new();
loc_save(&qerr->loc);
+qerr->err_class = err_class;
qerr->error = error_obj_from_fmt_no_fail(fmt, va);
qerr->err_msg = qerror_format(fmt, qerr->error);
@@ -518,13 +520,13 @@ static void qerror_print(QError *qerror)
QDECREF(qstring);
}
-void qerror_report(const char *fmt, ...)
+void qerror_report(ErrorClass eclass, const char *fmt, ...)
{
va_list va;
QError *qerror;
va_start(va, fmt);
-qerror = qerror_from_info(fmt, &va);
+qerror = qerror_from_info(eclass, fmt, &va);
va_end(va);
if (monitor_cur_is_qmp()) {
@@ -540,6 +542,7 @@ struct Error
{
QDict *obj;
char *msg;
+ErrorClass err_class;
};
void qerror_report_err(Error *err)
@@ -551,6 +554,7 @@ void qerror_report_err(Error *err)
QINCREF(err->obj);
qerr->error = err->obj;
qerr->err_msg = g_strdup(err->msg);
+qerr->err_class = err->err_class;
if (monitor_cur_is_qmp()) {
monitor_set_error(cur_mon, qerr);
diff --git a/qerror.h b/qerror.h
index 2e6a49d..bcc93f8 100644
--- a/qerror.h
+++ b/qerror.h
@@ -16,9 +16,11 @@
#include "qstring.h"
#include "qemu-error.h"
#include "error.h"
+#include "qapi-types.h"
#include
typedef struct QErrorStringTable {
+ErrorClass err_class;
const char *error_fmt;
const char *desc;
} QErrorStringTable;
@@ -28,10 +30,11 @@ typedef struct QError {
QDict *error;
Location loc;
char *err_msg;
+ErrorClass err_class;
} QError;
QString *qe
[Qemu-devel] [PATCH 26/35] error: add error_get_class()
Signed-off-by: Luiz Capitulino
---
error.c | 5 +
error.h | 5 +
2 files changed, 10 insertions(+)
diff --git a/error.c b/error.c
index 648706a..2d34cde 100644
--- a/error.c
+++ b/error.c
@@ -64,6 +64,11 @@ bool error_is_set(Error **errp)
return (errp && *errp);
}
+ErrorClass error_get_class(const Error *err)
+{
+return err->err_class;
+}
+
const char *error_get_pretty(Error *err)
{
return err->msg;
diff --git a/error.h b/error.h
index 9678752..114e24b 100644
--- a/error.h
+++ b/error.h
@@ -35,6 +35,11 @@ void error_set(Error **err, ErrorClass err_class, const char
*fmt, ...) GCC_FMT_
*/
bool error_is_set(Error **err);
+/*
+ * Get the error class of an error object.
+ */
+ErrorClass error_get_class(const Error *err);
+
/**
* Returns an exact copy of the error passed as an argument.
*/
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH v3 00/35]: add new error format
v3
o rebase on top of master
o replace 'valid_encryption_key' with 'encryption_key_missing', fixes
a bug found by Markus
o minor changes (changelogs, white-space fix and others)
Only the following patches have changed:
o [PATCH 11/35] qmp: query-block: add 'encryption_key_missing' field
o [PATCH 12/35] hmp: hmp_cont(): don't rely on QERR_DEVICE_ENCRYPTED
o [PATCH 15/35] migration: don't rely on any QERR_SOCKET_*
o [PATCH 29/35] qmp: switch to the new error format on the wire
o [PATCH 35/35] docs: writing-qmp-commands.txt: update error section
This series implements the 'Plan for error handling in QMP' as described
by Anthony in this email:
http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg03764.html
Basically, this replaces almost all error classes by GenericError (the
exception are a few error classes used by libvirt) and drops the error
data memeber. This also adds a free form string to error_set().
On the wire, we go from:
{ "error": { "class": "DeviceNotRemovable",
"data": { "device": "virtio0" },
"desc": "Device 'virtio0' is not removable" } }
to:
{ "error": { "class": "GenericError",
"desc": "Device 'virtio0' is not removable" } }
Internally, we go from:
void error_set(Error **err, const char *fmt, ...);
to:
void error_set(Error **err, ErrorClass err_class, const char *fmt, ...);
Makefile.objs | 1 +
QMP/qmp-spec.txt | 10 +-
block.c | 1 +
block_int.h | 1 +
configure | 10 -
docs/writing-qmp-commands.txt | 47 ++--
error.c | 93 +---
error.h | 34 +--
error_int.h | 29 ---
hmp.c | 69 ++
hmp.h | 1 +
migration-tcp.c | 22 +-
monitor.c | 83 ++-
nbd.c | 2 +-
qapi-schema.json | 94 +++-
qapi/qmp-core.h | 1 +
qapi/qmp-dispatch.c | 11 +-
qemu-char.c | 2 +-
qemu-ga.c | 5 +-
qemu-sockets.c| 14 +-
qemu_socket.h | 4 +-
qerror.c | 516 ++
qerror.h | 168 ++
qmp-commands.hx | 4 +-
scripts/qapi-types.py | 17 +-
ui/vnc.c | 2 +-
26 files changed, 268 insertions(+), 973 deletions(-)
[Qemu-devel] [PATCH 09/35] qerror: don't delay error message construction
Today, the error message is only constructed when it's used. This commit
changes qerror to construct the error message when the error object is
built (ie. when the error is reported).
This eliminates the need of storing a pointer to qerror_table[], which
will be dropped soon, and also simplifies the code.
Signed-off-by: Luiz Capitulino
---
qerror.c | 29 -
qerror.h | 2 +-
2 files changed, 5 insertions(+), 26 deletions(-)
diff --git a/qerror.c b/qerror.c
index d073ed7..a254f88 100644
--- a/qerror.c
+++ b/qerror.c
@@ -385,22 +385,6 @@ static QDict *error_obj_from_fmt_no_fail(const char *fmt,
va_list *va)
return ret;
}
-static const QErrorStringTable *get_desc_no_fail(const char *fmt)
-{
-int i;
-
-// FIXME: inefficient loop
-
-for (i = 0; qerror_table[i].error_fmt; i++) {
-if (strcmp(qerror_table[i].error_fmt, fmt) == 0) {
-return &qerror_table[i];
-}
-}
-
-fprintf(stderr, "error format '%s' not found\n", fmt);
-abort();
-}
-
/**
* qerror_from_info(): Create a new QError from error information
*
@@ -414,7 +398,7 @@ static QError *qerror_from_info(const char *fmt, va_list
*va)
loc_save(&qerr->loc);
qerr->error = error_obj_from_fmt_no_fail(fmt, va);
-qerr->entry = get_desc_no_fail(fmt);
+qerr->err_msg = qerror_format(fmt, qerr->error);
return qerr;
}
@@ -519,7 +503,7 @@ char *qerror_format(const char *fmt, QDict *error)
*/
QString *qerror_human(const QError *qerror)
{
-return qerror_format_desc(qerror->error, qerror->entry);
+return qstring_from_str(qerror->err_msg);
}
/**
@@ -566,19 +550,13 @@ struct Error
void qerror_report_err(Error *err)
{
QError *qerr;
-int i;
qerr = qerror_new();
loc_save(&qerr->loc);
QINCREF(err->obj);
qerr->error = err->obj;
-for (i = 0; qerror_table[i].error_fmt; i++) {
-if (strcmp(qerror_table[i].error_fmt, err->fmt) == 0) {
-qerr->entry = &qerror_table[i];
-break;
-}
-}
+qerr->err_msg = qerror_format(err->fmt, qerr->error);
if (monitor_cur_is_qmp()) {
monitor_set_error(cur_mon, qerr);
@@ -619,5 +597,6 @@ static void qerror_destroy_obj(QObject *obj)
qerr = qobject_to_qerror(obj);
QDECREF(qerr->error);
+g_free(qerr->err_msg);
g_free(qerr);
}
diff --git a/qerror.h b/qerror.h
index aec76b2..de8497d 100644
--- a/qerror.h
+++ b/qerror.h
@@ -27,7 +27,7 @@ typedef struct QError {
QObject_HEAD;
QDict *error;
Location loc;
-const QErrorStringTable *entry;
+char *err_msg;
} QError;
QString *qerror_human(const QError *qerror);
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 23/35] qerror: qerror_table: don't use C99 struct initializers
This allows for changing QERR_ macros to initialize two struct members
at the same time. See next commit for more details.
Signed-off-by: Luiz Capitulino
---
qerror.c | 276 +++
qerror.h | 2 +-
2 files changed, 139 insertions(+), 139 deletions(-)
diff --git a/qerror.c b/qerror.c
index 452ec69..ff460b0 100644
--- a/qerror.c
+++ b/qerror.c
@@ -44,285 +44,285 @@ static const QType qerror_type = {
*/
static const QErrorStringTable qerror_table[] = {
{
-.error_fmt = QERR_ADD_CLIENT_FAILED,
-.desc = "Could not add client",
+ QERR_ADD_CLIENT_FAILED,
+ "Could not add client",
},
{
-.error_fmt = QERR_AMBIGUOUS_PATH,
-.desc = "Path '%(path)' does not uniquely identify an object"
+ QERR_AMBIGUOUS_PATH,
+ "Path '%(path)' does not uniquely identify an object"
},
{
-.error_fmt = QERR_BAD_BUS_FOR_DEVICE,
-.desc = "Device '%(device)' can't go on a %(bad_bus_type) bus",
+ QERR_BAD_BUS_FOR_DEVICE,
+ "Device '%(device)' can't go on a %(bad_bus_type) bus",
},
{
-.error_fmt = QERR_BASE_NOT_FOUND,
-.desc = "Base '%(base)' not found",
+ QERR_BASE_NOT_FOUND,
+ "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)'",
+ QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
+ "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",
+ QERR_BUS_NO_HOTPLUG,
+ "Bus '%(bus)' does not support hotplugging",
},
{
-.error_fmt = QERR_BUS_NOT_FOUND,
-.desc = "Bus '%(bus)' not found",
+ QERR_BUS_NOT_FOUND,
+ "Bus '%(bus)' not found",
},
{
-.error_fmt = QERR_COMMAND_DISABLED,
-.desc = "The command %(name) has been disabled for this instance",
+ QERR_COMMAND_DISABLED,
+ "The command %(name) has been disabled for this instance",
},
{
-.error_fmt = QERR_COMMAND_NOT_FOUND,
-.desc = "The command %(name) has not been found",
+ QERR_COMMAND_NOT_FOUND,
+ "The command %(name) has not been found",
},
{
-.error_fmt = QERR_DEVICE_ENCRYPTED,
-.desc = "'%(device)' (%(filename)) is encrypted",
+ QERR_DEVICE_ENCRYPTED,
+ "'%(device)' (%(filename)) is encrypted",
},
{
-.error_fmt = QERR_DEVICE_FEATURE_BLOCKS_MIGRATION,
-.desc = "Migration is disabled when using feature '%(feature)' in
device '%(device)'",
+ QERR_DEVICE_FEATURE_BLOCKS_MIGRATION,
+ "Migration is disabled when using feature '%(feature)' in device
'%(device)'",
},
{
-.error_fmt = QERR_DEVICE_HAS_NO_MEDIUM,
-.desc = "Device '%(device)' has no medium",
+ QERR_DEVICE_HAS_NO_MEDIUM,
+ "Device '%(device)' has no medium",
},
{
-.error_fmt = QERR_DEVICE_INIT_FAILED,
-.desc = "Device '%(device)' could not be initialized",
+ QERR_DEVICE_INIT_FAILED,
+ "Device '%(device)' could not be initialized",
},
{
-.error_fmt = QERR_DEVICE_IN_USE,
-.desc = "Device '%(device)' is in use",
+ QERR_DEVICE_IN_USE,
+ "Device '%(device)' is in use",
},
{
-.error_fmt = QERR_DEVICE_IS_READ_ONLY,
-.desc = "Device '%(device)' is read only",
+ QERR_DEVICE_IS_READ_ONLY,
+ "Device '%(device)' is read only",
},
{
-.error_fmt = QERR_DEVICE_LOCKED,
-.desc = "Device '%(device)' is locked",
+ QERR_DEVICE_LOCKED,
+ "Device '%(device)' is locked",
},
{
-.error_fmt = QERR_DEVICE_MULTIPLE_BUSSES,
-.desc = "Device '%(device)' has multiple child busses",
+ QERR_DEVICE_MULTIPLE_BUSSES,
+ "Device '%(device)' has multiple child busses",
},
{
-.error_
[Qemu-devel] [PATCH 12/35] hmp: hmp_cont(): don't rely on QERR_DEVICE_ENCRYPTED
This commit changes hmp_cont() to loop through all block devices
and proactively set an encryption key for any encrypted device
missing a key.
This change is needed because QERR_DEVICE_ENCRYPTED is going to be
dropped by a future commit.
Signed-off-by: Luiz Capitulino
---
hmp.c | 37 +++--
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/hmp.c b/hmp.c
index 25688ab..4efaf51 100644
--- a/hmp.c
+++ b/hmp.c
@@ -612,34 +612,35 @@ void hmp_pmemsave(Monitor *mon, const QDict *qdict)
static void hmp_cont_cb(void *opaque, int err)
{
-Monitor *mon = opaque;
-
if (!err) {
-hmp_cont(mon, NULL);
+qmp_cont(NULL);
}
}
+static bool key_is_missing(const BlockInfo *bdev)
+{
+return (bdev->inserted && bdev->inserted->encryption_key_missing);
+}
+
void hmp_cont(Monitor *mon, const QDict *qdict)
{
+BlockInfoList *bdev_list, *bdev;
Error *errp = NULL;
-qmp_cont(&errp);
-if (error_is_set(&errp)) {
-if (error_is_type(errp, QERR_DEVICE_ENCRYPTED)) {
-const char *device;
-
-/* The device is encrypted. Ask the user for the password
- and retry */
-
-device = error_get_field(errp, "device");
-assert(device != NULL);
-
-monitor_read_block_device_key(mon, device, hmp_cont_cb, mon);
-error_free(errp);
-return;
+bdev_list = qmp_query_block(NULL);
+for (bdev = bdev_list; bdev; bdev = bdev->next) {
+if (key_is_missing(bdev->value)) {
+monitor_read_block_device_key(mon, bdev->value->device,
+ hmp_cont_cb, NULL);
+goto out;
}
-hmp_handle_error(mon, &errp);
}
+
+qmp_cont(&errp);
+hmp_handle_error(mon, &errp);
+
+out:
+qapi_free_BlockInfoList(bdev_list);
}
void hmp_system_wakeup(Monitor *mon, const QDict *qdict)
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 20/35] qapi: generate correct enum names for camel case enums
An enum like GenericError in the schema, should generate
GENERIC_ERROR and not GENERICERROR.
Signed-off-by: Luiz Capitulino
---
scripts/qapi-types.py | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 3ed9f04..9b7da96 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -79,6 +79,16 @@ const char *%(name)s_lookup[] = {
''')
return ret
+def generate_enum_name(name):
+if name.isupper():
+return c_fun(name)
+new_name = ''
+for c in c_fun(name):
+if c.isupper():
+new_name += '_'
+new_name += c
+return new_name.lstrip('_').upper()
+
def generate_enum(name, values):
lookup_decl = mcgen('''
extern const char *%(name)s_lookup[];
@@ -100,7 +110,7 @@ typedef enum %(name)s
%(abbrev)s_%(value)s = %(i)d,
''',
abbrev=de_camel_case(name).upper(),
- value=c_fun(value).upper(),
+ value=generate_enum_name(value),
i=i)
i += 1
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 06/35] qerror: avoid passing qerr pointer
Helps dropping/modifying qerror functions.
Signed-off-by: Luiz Capitulino
---
qerror.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/qerror.c b/qerror.c
index 7cb7c12..e717496 100644
--- a/qerror.c
+++ b/qerror.c
@@ -346,10 +346,10 @@ static QError *qerror_new(void)
return qerr;
}
-static void GCC_FMT_ATTR(2, 0) qerror_set_data(QError *qerr,
- const char *fmt, va_list *va)
+static QDict *error_obj_from_fmt_no_fail(const char *fmt, va_list *va)
{
QObject *obj;
+QDict *ret;
obj = qobject_from_jsonv(fmt, va);
if (!obj) {
@@ -361,9 +361,8 @@ static void GCC_FMT_ATTR(2, 0) qerror_set_data(QError *qerr,
abort();
}
-qerr->error = qobject_to_qdict(obj);
-
-obj = qdict_get(qerr->error, "class");
+ret = qobject_to_qdict(obj);
+obj = qdict_get(ret, "class");
if (!obj) {
fprintf(stderr, "missing 'class' key in '%s'\n", fmt);
abort();
@@ -372,8 +371,8 @@ static void GCC_FMT_ATTR(2, 0) qerror_set_data(QError *qerr,
fprintf(stderr, "'class' key value should be a string in '%s'\n", fmt);
abort();
}
-
-obj = qdict_get(qerr->error, "data");
+
+obj = qdict_get(ret, "data");
if (!obj) {
fprintf(stderr, "missing 'data' key in '%s'\n", fmt);
abort();
@@ -382,9 +381,11 @@ static void GCC_FMT_ATTR(2, 0) qerror_set_data(QError
*qerr,
fprintf(stderr, "'data' key value should be a dict in '%s'\n", fmt);
abort();
}
+
+return ret;
}
-static void qerror_set_desc(QError *qerr, const char *fmt)
+static const QErrorStringTable *get_desc_no_fail(const char *fmt)
{
int i;
@@ -392,8 +393,7 @@ static void qerror_set_desc(QError *qerr, const char *fmt)
for (i = 0; qerror_table[i].error_fmt; i++) {
if (strcmp(qerror_table[i].error_fmt, fmt) == 0) {
-qerr->entry = &qerror_table[i];
-return;
+return &qerror_table[i];
}
}
@@ -426,8 +426,8 @@ static QError *qerror_from_info(const char *file, int
linenr, const char *func,
qerr->file = file;
qerr->func = func;
-qerror_set_data(qerr, fmt, va);
-qerror_set_desc(qerr, fmt);
+qerr->error = error_obj_from_fmt_no_fail(fmt, va);
+qerr->entry = get_desc_no_fail(fmt);
return qerr;
}
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 19/35] qapi: qapi-types.h: don't include qapi/qapi-types-core.h
qapi-types.h needs only qemu-common.h. Including qapi-types-core.h
causes problems when qerror.h or error.h includes qapi-types.h.
Signed-off-by: Luiz Capitulino
---
scripts/qapi-types.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 4a734f5..3ed9f04 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -253,7 +253,8 @@ fdecl.write(mcgen('''
#ifndef %(guard)s
#define %(guard)s
-#include "qapi/qapi-types-core.h"
+#include "qemu-common.h"
+
''',
guard=guardname(h_file)))
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 15/35] migration: don't rely on any QERR_SOCKET_*
Use the in_progress argument for QERR_SOCKET_CONNECT_IN_PROGRESS. The
other errors are handled the same by checking if the error is set and
then calling migrate_fd_error() if it's.
It's also necessary to change inet_connect_opts() not to set
QERR_SOCKET_CONNECT_IN_PROGRESS. This error is only used by
tcp_start_outgoing_migration() and not changing it along with the
usage of in_progress would break migration.
Furthermore this commit fixes a bug. Today, there's a spurious error
report when migration succeeds:
(qemu) migrate tcp:0:
migrate: Connection can not be completed immediately
(qemu)
After this commit no spurious error is reported anymore.
Signed-off-by: Luiz Capitulino
---
migration-tcp.c | 22 +-
qemu-sockets.c | 2 --
2 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/migration-tcp.c b/migration-tcp.c
index 18944a4..ac891c3 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -82,27 +82,23 @@ static void tcp_wait_for_connect(void *opaque)
int tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
Error **errp)
{
+bool in_progress;
+
s->get_error = socket_errno;
s->write = socket_write;
s->close = tcp_close;
-s->fd = inet_connect(host_port, false, NULL, errp);
+s->fd = inet_connect(host_port, false, &in_progress, errp);
+if (error_is_set(errp)) {
+migrate_fd_error(s);
+return -1;
+}
-if (!error_is_set(errp)) {
-migrate_fd_connect(s);
-} else if (error_is_type(*errp, QERR_SOCKET_CONNECT_IN_PROGRESS)) {
+if (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)) {
-DPRINTF("connect failed\n");
-return -1;
-} else if (error_is_type(*errp, QERR_SOCKET_CONNECT_FAILED)) {
-DPRINTF("connect failed\n");
-migrate_fd_error(s);
-return -1;
} else {
-DPRINTF("unknown error\n");
-return -1;
+migrate_fd_connect(s);
}
return 0;
diff --git a/qemu-sockets.c b/qemu-sockets.c
index 9cb47d4..361d890 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -284,8 +284,6 @@ int inet_connect_opts(QemuOpts *opts, bool *in_progress,
Error **errp)
if (in_progress) {
*in_progress = true;
}
-
-error_set(errp, QERR_SOCKET_CONNECT_IN_PROGRESS);
} else if (rc < 0) {
if (NULL == e->ai_next)
fprintf(stderr, "%s: connect(%s,%s,%s,%s): %s\n", __FUNCTION__,
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 04/35] qerror: reduce public exposure
qerror will be dropped in a near future, let's reduce its public
exposure by making functions only used in qerror.c static.
Signed-off-by: Luiz Capitulino
---
qerror.c | 10 +-
qerror.h | 5 -
2 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/qerror.c b/qerror.c
index de0a79e..bfb875a 100644
--- a/qerror.c
+++ b/qerror.c
@@ -336,7 +336,7 @@ static const QErrorStringTable qerror_table[] = {
*
* Return strong reference.
*/
-QError *qerror_new(void)
+static QError *qerror_new(void)
{
QError *qerr;
@@ -424,8 +424,8 @@ static void qerror_set_desc(QError *qerr, const char *fmt)
*
* Return strong reference.
*/
-QError *qerror_from_info(const char *file, int linenr, const char *func,
- const char *fmt, va_list *va)
+static QError *qerror_from_info(const char *file, int linenr, const char *func,
+const char *fmt, va_list *va)
{
QError *qerr;
@@ -549,7 +549,7 @@ QString *qerror_human(const QError *qerror)
* it uses error_report() for this, so that the output is routed to the right
* place (ie. stderr or Monitor's device).
*/
-void qerror_print(QError *qerror)
+static void qerror_print(QError *qerror)
{
QString *qstring = qerror_human(qerror);
loc_push_restore(&qerror->loc);
@@ -620,7 +620,7 @@ void assert_no_error(Error *err)
/**
* qobject_to_qerror(): Convert a QObject into a QError
*/
-QError *qobject_to_qerror(const QObject *obj)
+static QError *qobject_to_qerror(const QObject *obj)
{
if (qobject_type(obj) != QTYPE_QERROR) {
return NULL;
diff --git a/qerror.h b/qerror.h
index b4c8758..fe8870c 100644
--- a/qerror.h
+++ b/qerror.h
@@ -33,11 +33,7 @@ typedef struct QError {
const QErrorStringTable *entry;
} QError;
-QError *qerror_new(void);
-QError *qerror_from_info(const char *file, int linenr, const char *func,
- const char *fmt, va_list *va) GCC_FMT_ATTR(4, 0);
QString *qerror_human(const QError *qerror);
-void qerror_print(QError *qerror);
void qerror_report_internal(const char *file, int linenr, const char *func,
const char *fmt, ...) GCC_FMT_ATTR(4, 5);
void qerror_report_err(Error *err);
@@ -45,7 +41,6 @@ void assert_no_error(Error *err);
QString *qerror_format(const char *fmt, QDict *error);
#define qerror_report(fmt, ...) \
qerror_report_internal(__FILE__, __LINE__, __func__, fmt, ## __VA_ARGS__)
-QError *qobject_to_qerror(const QObject *obj);
/*
* QError class list
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 05/35] qerror: drop qerror_abort()
qerror_abort() depends on the 'file', 'func' and 'linenr' members of
QError. However, these members are going to be dropped by the next
commit, so let's drop qerror_abort() in favor of printing an error
message to stderr plus a call to abort().
Signed-off-by: Luiz Capitulino
---
qerror.c | 41 ++---
1 file changed, 14 insertions(+), 27 deletions(-)
diff --git a/qerror.c b/qerror.c
index bfb875a..7cb7c12 100644
--- a/qerror.c
+++ b/qerror.c
@@ -346,22 +346,6 @@ static QError *qerror_new(void)
return qerr;
}
-static void GCC_FMT_ATTR(2, 3) qerror_abort(const QError *qerr,
-const char *fmt, ...)
-{
-va_list ap;
-
-fprintf(stderr, "qerror: bad call in function '%s':\n", qerr->func);
-fprintf(stderr, "qerror: -> ");
-
-va_start(ap, fmt);
-vfprintf(stderr, fmt, ap);
-va_end(ap);
-
-fprintf(stderr, "\nqerror: call at %s:%d\n", qerr->file, qerr->linenr);
-abort();
-}
-
static void GCC_FMT_ATTR(2, 0) qerror_set_data(QError *qerr,
const char *fmt, va_list *va)
{
@@ -369,28 +353,34 @@ static void GCC_FMT_ATTR(2, 0) qerror_set_data(QError
*qerr,
obj = qobject_from_jsonv(fmt, va);
if (!obj) {
-qerror_abort(qerr, "invalid format '%s'", fmt);
+fprintf(stderr, "invalid json in error dict '%s'\n", fmt);
+abort();
}
if (qobject_type(obj) != QTYPE_QDICT) {
-qerror_abort(qerr, "error format is not a QDict '%s'", fmt);
+fprintf(stderr, "error is not a dict '%s'\n", fmt);
+abort();
}
qerr->error = qobject_to_qdict(obj);
obj = qdict_get(qerr->error, "class");
if (!obj) {
-qerror_abort(qerr, "missing 'class' key in '%s'", fmt);
+fprintf(stderr, "missing 'class' key in '%s'\n", fmt);
+abort();
}
if (qobject_type(obj) != QTYPE_QSTRING) {
-qerror_abort(qerr, "'class' key value should be a QString");
+fprintf(stderr, "'class' key value should be a string in '%s'\n", fmt);
+abort();
}
obj = qdict_get(qerr->error, "data");
if (!obj) {
-qerror_abort(qerr, "missing 'data' key in '%s'", fmt);
+fprintf(stderr, "missing 'data' key in '%s'\n", fmt);
+abort();
}
if (qobject_type(obj) != QTYPE_QDICT) {
-qerror_abort(qerr, "'data' key value should be a QDICT");
+fprintf(stderr, "'data' key value should be a dict in '%s'\n", fmt);
+abort();
}
}
@@ -407,7 +397,8 @@ static void qerror_set_desc(QError *qerr, const char *fmt)
}
}
-qerror_abort(qerr, "error format '%s' not found", fmt);
+fprintf(stderr, "error format '%s' not found\n", fmt);
+abort();
}
/**
@@ -435,10 +426,6 @@ static QError *qerror_from_info(const char *file, int
linenr, const char *func,
qerr->file = file;
qerr->func = func;
-if (!fmt) {
-qerror_abort(qerr, "QDict not specified");
-}
-
qerror_set_data(qerr, fmt, va);
qerror_set_desc(qerr, fmt);
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 34/35] error, qerror: drop QDict member
Used to store error information, but it's unused now.
Signed-off-by: Luiz Capitulino
---
error.c | 4
qerror.c | 4
qerror.h | 1 -
3 files changed, 9 deletions(-)
diff --git a/error.c b/error.c
index 0e10373..1f05fc4 100644
--- a/error.c
+++ b/error.c
@@ -19,7 +19,6 @@
struct Error
{
-QDict *obj;
char *msg;
ErrorClass err_class;
};
@@ -51,8 +50,6 @@ Error *error_copy(const Error *err)
err_new = g_malloc0(sizeof(*err));
err_new->msg = g_strdup(err->msg);
err_new->err_class = err->err_class;
-err_new->obj = err->obj;
-QINCREF(err_new->obj);
return err_new;
}
@@ -75,7 +72,6 @@ const char *error_get_pretty(Error *err)
void error_free(Error *err)
{
if (err) {
-QDECREF(err->obj);
g_free(err->msg);
g_free(err);
}
diff --git a/qerror.c b/qerror.c
index ccc52be..0818504 100644
--- a/qerror.c
+++ b/qerror.c
@@ -100,7 +100,6 @@ void qerror_report(ErrorClass eclass, const char *fmt, ...)
/* Evil... */
struct Error
{
-QDict *obj;
char *msg;
ErrorClass err_class;
};
@@ -111,8 +110,6 @@ void qerror_report_err(Error *err)
qerr = qerror_new();
loc_save(&qerr->loc);
-QINCREF(err->obj);
-qerr->error = err->obj;
qerr->err_msg = g_strdup(err->msg);
qerr->err_class = err->err_class;
@@ -154,7 +151,6 @@ static void qerror_destroy_obj(QObject *obj)
assert(obj != NULL);
qerr = qobject_to_qerror(obj);
-QDECREF(qerr->error);
g_free(qerr->err_msg);
g_free(qerr);
}
diff --git a/qerror.h b/qerror.h
index c5ad29f..d0a76a4 100644
--- a/qerror.h
+++ b/qerror.h
@@ -21,7 +21,6 @@
typedef struct QError {
QObject_HEAD;
-QDict *error;
Location loc;
char *err_msg;
ErrorClass err_class;
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 29/35] qmp: switch to the new error format on the wire
IMPORTANT: this BREAKS QMP's compatibility for the error response.
This commit changes QMP's wire protocol to make use of the simpler
error format introduced by previous commits.
There are two important (and mostly incompatible) changes:
1. Almost all error classes have been replaced by GenericError. The
only classes that are still supported for compatibility with
libvirt are: CommandNotFound, DeviceNotActive, KVMMissingCap,
DeviceNotFound and MigrationExpected
2. The 'data' field of the error dictionary is gone
As an example, an error response like:
{ "error": { "class": "DeviceNotRemovable",
"data": { "device": "virtio0" },
"desc": "Device 'virtio0' is not removable" } }
Will now be emitted as:
{ "error": { "class": "GenericError",
"desc": "Device 'virtio0' is not removable" } }
Signed-off-by: Luiz Capitulino
---
QMP/qmp-spec.txt | 10 +++---
monitor.c| 18 +-
qapi-schema.json | 55 ---
qmp-commands.hx | 4 ++--
4 files changed, 18 insertions(+), 69 deletions(-)
diff --git a/QMP/qmp-spec.txt b/QMP/qmp-spec.txt
index 1ba916c..a277896 100644
--- a/QMP/qmp-spec.txt
+++ b/QMP/qmp-spec.txt
@@ -106,14 +106,11 @@ completed because of an error condition.
The format is:
-{ "error": { "class": json-string, "data": json-object, "desc": json-string },
- "id": json-value }
+{ "error": { "class": json-string, "desc": json-string }, "id": json-value }
Where,
-- The "class" member contains the error class name (eg. "ServiceUnavailable")
-- The "data" member contains specific error data and is defined in a
- per-command basis, it will be an empty json-object if the error has no data
+- The "class" member contains the error class name (eg. "GenericError")
- The "desc" member is a human-readable error message. Clients should
not attempt to parse this message.
- The "id" member contains the transaction identification associated with
@@ -173,8 +170,7 @@ S: {"return": {"enabled": true, "present": true}, "id":
"example"}
--
C: { "execute": }
-S: {"error": {"class": "JSONParsing", "desc": "Invalid JSON syntax", "data":
-{}}}
+S: {"error": {"class": "GenericError", "desc": "Invalid JSON syntax" } }
3.5 Powerdown event
---
diff --git a/monitor.c b/monitor.c
index aa57167..3694590 100644
--- a/monitor.c
+++ b/monitor.c
@@ -353,16 +353,26 @@ static void monitor_json_emitter(Monitor *mon, const
QObject *data)
QDECREF(json);
}
+static QDict *build_qmp_error_dict(const QError *err)
+{
+QObject *obj;
+
+obj = qobject_from_jsonf("{ 'error': { 'class': %s, 'desc': %p } }",
+ ErrorClass_lookup[err->err_class],
+ qerror_human(err));
+
+return qobject_to_qdict(obj);
+}
+
static void monitor_protocol_emitter(Monitor *mon, QObject *data)
{
QDict *qmp;
trace_monitor_protocol_emitter(mon);
-qmp = qdict_new();
-
if (!monitor_has_error(mon)) {
/* success response */
+qmp = qdict_new();
if (data) {
qobject_incref(data);
qdict_put_obj(qmp, "return", data);
@@ -372,9 +382,7 @@ static void monitor_protocol_emitter(Monitor *mon, QObject
*data)
}
} else {
/* error response */
-qdict_put(mon->error->error, "desc", qerror_human(mon->error));
-qdict_put(qmp, "error", mon->error->error);
-QINCREF(mon->error->error);
+qmp = build_qmp_error_dict(mon->error);
QDECREF(mon->error);
mon->error = NULL;
}
diff --git a/qapi-schema.json b/qapi-schema.json
index b513935..ec8d919 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -658,7 +658,6 @@
# Returns information about the current VNC server
#
# Returns: @VncInfo
-# If VNC support is not compiled in, FeatureDisabled
#
# Since: 0.14.0
##
@@ -1042,9 +1041,6 @@
# virtual address (defaults to CPU 0)
#
# Returns: Nothing on success
-# If @cpu is not a valid VCPU, InvalidParameterValue
-# If @filename cannot be opened, OpenFileFailed
-# If an I/O error occurs while writing the file, IOError
#
# Since: 0.14.0
#
@@ -1065,8 +1061,6 @@
# @filename: the file to save the memory to as binary data
#
# Returns: Nothin
[Qemu-devel] [PATCH 03/35] qerror: QERR_DEVICE_ENCRYPTED: change error message
Match what HMP commands print on DeviceEncrypted errors.
Signed-off-by: Luiz Capitulino
---
qerror.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qerror.c b/qerror.c
index 082de98..de0a79e 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)' (%(filename)) is encrypted",
},
{
.error_fmt = QERR_DEVICE_FEATURE_BLOCKS_MIGRATION,
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 25/35] qerror: add proper ErrorClass value for QERR_ macros
This commit replaces the place holder value for the ErrorClass
argument with a proper ErrorClass value for all QERR_ macros.
All current errors are mapped to GenericError, except for errors
CommandNotFound, DeviceEncrypted, DeviceNotActive, DeviceNotFound,
KVMMissingCap and MigrationExpected, which are maintained as they
are today.
Signed-off-by: Luiz Capitulino
---
qerror.h | 140 +++
1 file changed, 70 insertions(+), 70 deletions(-)
diff --git a/qerror.h b/qerror.h
index bcc93f8..4f92218 100644
--- a/qerror.h
+++ b/qerror.h
@@ -45,214 +45,214 @@ char *qerror_format(const char *fmt, QDict *error);
* Use scripts/check-qerror.sh to check.
*/
#define QERR_ADD_CLIENT_FAILED \
--1, "{ 'class': 'AddClientFailed', 'data': {} }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'AddClientFailed', 'data': {} }"
#define QERR_AMBIGUOUS_PATH \
--1, "{ 'class': 'AmbiguousPath', 'data': { 'path': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'AmbiguousPath', 'data': { 'path':
%s } }"
#define QERR_BAD_BUS_FOR_DEVICE \
--1, "{ 'class': 'BadBusForDevice', 'data': { 'device': %s, 'bad_bus_type':
%s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BadBusForDevice', 'data': {
'device': %s, 'bad_bus_type': %s } }"
#define QERR_BASE_NOT_FOUND \
--1, "{ 'class': 'BaseNotFound', 'data': { 'base': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BaseNotFound', 'data': { 'base':
%s } }"
#define QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED \
--1, "{ 'class': 'BlockFormatFeatureNotSupported', 'data': { 'format': %s,
'name': %s, 'feature': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BlockFormatFeatureNotSupported',
'data': { 'format': %s, 'name': %s, 'feature': %s } }"
#define QERR_BUFFER_OVERRUN \
--1, "{ 'class': 'BufferOverrun', 'data': {} }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BufferOverrun', 'data': {} }"
#define QERR_BUS_NO_HOTPLUG \
--1, "{ 'class': 'BusNoHotplug', 'data': { 'bus': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BusNoHotplug', 'data': { 'bus': %s
} }"
#define QERR_BUS_NOT_FOUND \
--1, "{ 'class': 'BusNotFound', 'data': { 'bus': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BusNotFound', 'data': { 'bus': %s
} }"
#define QERR_COMMAND_DISABLED \
--1, "{ 'class': 'CommandDisabled', 'data': { 'name': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'CommandDisabled', 'data': {
'name': %s } }"
#define QERR_COMMAND_NOT_FOUND \
--1, "{ 'class': 'CommandNotFound', 'data': { 'name': %s } }"
+ERROR_CLASS_COMMAND_NOT_FOUND, "{ 'class': 'CommandNotFound', 'data': {
'name': %s } }"
#define QERR_DEVICE_ENCRYPTED \
--1, "{ 'class': 'DeviceEncrypted', 'data': { 'device': %s, 'filename': %s
} }"
+ERROR_CLASS_DEVICE_ENCRYPTED, "{ 'class': 'DeviceEncrypted', 'data': {
'device': %s, 'filename': %s } }"
#define QERR_DEVICE_FEATURE_BLOCKS_MIGRATION \
--1, "{ 'class': 'DeviceFeatureBlocksMigration', 'data': { 'device': %s,
'feature': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceFeatureBlocksMigration',
'data': { 'device': %s, 'feature': %s } }"
#define QERR_DEVICE_HAS_NO_MEDIUM \
--1, "{ 'class': 'DeviceHasNoMedium', 'data': { 'device': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceHasNoMedium', 'data': {
'device': %s } }"
#define QERR_DEVICE_INIT_FAILED \
--1, "{ 'class': 'DeviceInitFailed', 'data': { 'device': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'DeviceInitFaile
[Qemu-devel] [PATCH 27/35] hmp: hmp_change(): use error_get_class()
The error_is_type() function is going to be dropped.
Signed-off-by: Luiz Capitulino
---
hmp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hmp.c b/hmp.c
index 54c37d7..9b44dfc 100644
--- a/hmp.c
+++ b/hmp.c
@@ -793,7 +793,8 @@ void hmp_change(Monitor *mon, const QDict *qdict)
}
qmp_change(device, target, !!arg, arg, &err);
-if (error_is_type(err, QERR_DEVICE_ENCRYPTED)) {
+if (error_is_set(&err) &&
+error_get_class(err) == ERROR_CLASS_DEVICE_ENCRYPTED) {
error_free(err);
monitor_read_block_device_key(mon, device, NULL, NULL);
return;
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 22/35] qapi-schema: add ErrorClass enum
Signed-off-by: Luiz Capitulino
---
qapi-schema.json | 30 ++
1 file changed, 30 insertions(+)
diff --git a/qapi-schema.json b/qapi-schema.json
index a62bf68..b513935 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3,6 +3,36 @@
# QAPI Schema
##
+# @ErrorClass
+#
+# QEMU error classes
+#
+# @GenericError: this is used for errors that don't require a specific error
+#class. This should be the default case for most errors
+#
+# @CommandNotFound: the requested command has not been found
+#
+# @DeviceEncrypted: the requested operation can't be fulfilled because the
+# selected device is encrypted
+#
+# @DeviceNotActive: a device has failed to be become active
+#
+# @DeviceNotFound: the requested device has not been found
+#
+# @KVMMissingCap: the requested operation can't be fulfilled because a
+# required KVM capability is missing
+#
+# @MigrationExpected: the requested operation can't be fulfilled because a
+# migration process is expected
+#
+# Since: 1.2
+##
+{ 'enum': 'ErrorClass',
+ 'data': [ 'GenericError', 'CommandNotFound', 'DeviceEncrypted',
+'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap',
+'MigrationExpected' ] }
+
+##
# @NameInfo:
#
# Guest name information.
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 21/35] qapi: don't convert enum strings to lowercase
Next commit will introduce enum strings in camel case.
Signed-off-by: Luiz Capitulino
---
scripts/qapi-types.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 9b7da96..cf601ae 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -70,7 +70,7 @@ const char *%(name)s_lookup[] = {
ret += mcgen('''
"%(value)s",
''',
- value=value.lower())
+ value=value)
ret += mcgen('''
NULL,
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 18/35] hmp: hmp.h: include qdict.h
hmp.h is relying on qdict.h being provided by qapi-types.h. Fix this, as a future commit will change qapi-types.h not to provide qdict.h. Signed-off-by: Luiz Capitulino --- hmp.h | 1 + 1 file changed, 1 insertion(+) diff --git a/hmp.h b/hmp.h index 8d2b0d7..3275522 100644 --- a/hmp.h +++ b/hmp.h @@ -16,6 +16,7 @@ #include "qemu-common.h" #include "qapi-types.h" +#include "qdict.h" void hmp_info_name(Monitor *mon); void hmp_info_version(Monitor *mon); -- 1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 08/35] qerror: qerror_format(): return an allocated string
Simplifies current and future users.
Signed-off-by: Luiz Capitulino
---
error.c | 5 +
qerror.c | 10 --
qerror.h | 2 +-
3 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/error.c b/error.c
index 58f55a0..3a62592 100644
--- a/error.c
+++ b/error.c
@@ -65,10 +65,7 @@ bool error_is_set(Error **errp)
const char *error_get_pretty(Error *err)
{
if (err->msg == NULL) {
-QString *str;
-str = qerror_format(err->fmt, err->obj);
-err->msg = g_strdup(qstring_get_str(str));
-QDECREF(str);
+err->msg = qerror_format(err->fmt, err->obj);
}
return err->msg;
diff --git a/qerror.c b/qerror.c
index 6f9f49c..d073ed7 100644
--- a/qerror.c
+++ b/qerror.c
@@ -493,9 +493,11 @@ static QString *qerror_format_desc(QDict *error,
return qstring;
}
-QString *qerror_format(const char *fmt, QDict *error)
+char *qerror_format(const char *fmt, QDict *error)
{
const QErrorStringTable *entry = NULL;
+QString *qstr;
+char *ret;
int i;
for (i = 0; qerror_table[i].error_fmt; i++) {
@@ -505,7 +507,11 @@ QString *qerror_format(const char *fmt, QDict *error)
}
}
-return qerror_format_desc(error, entry);
+qstr = qerror_format_desc(error, entry);
+ret = g_strdup(qstring_get_str(qstr));
+QDECREF(qstr);
+
+return ret;
}
/**
diff --git a/qerror.h b/qerror.h
index 3c0b14c..aec76b2 100644
--- a/qerror.h
+++ b/qerror.h
@@ -34,7 +34,7 @@ QString *qerror_human(const QError *qerror);
void qerror_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
void qerror_report_err(Error *err);
void assert_no_error(Error *err);
-QString *qerror_format(const char *fmt, QDict *error);
+char *qerror_format(const char *fmt, QDict *error);
/*
* QError class list
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 33/35] qerror: drop qerror_table and qerror_format()
They are unused since last commit.
Signed-off-by: Luiz Capitulino
---
qerror.c | 400 ---
qerror.h | 7 --
2 files changed, 407 deletions(-)
diff --git a/qerror.c b/qerror.c
index dda1427..ccc52be 100644
--- a/qerror.c
+++ b/qerror.c
@@ -23,311 +23,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[] = {
-{
- QERR_ADD_CLIENT_FAILED,
- "Could not add client",
-},
-{
- QERR_AMBIGUOUS_PATH,
- "Path '%(path)' does not uniquely identify an object"
-},
-{
- QERR_BAD_BUS_FOR_DEVICE,
- "Device '%(device)' can't go on a %(bad_bus_type) bus",
-},
-{
- QERR_BASE_NOT_FOUND,
- "Base '%(base)' not found",
-},
-{
- QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
- "Block format '%(format)' used by device '%(name)' does not support
feature '%(feature)'",
-},
-{
- QERR_BUS_NO_HOTPLUG,
- "Bus '%(bus)' does not support hotplugging",
-},
-{
- QERR_BUS_NOT_FOUND,
- "Bus '%(bus)' not found",
-},
-{
- QERR_COMMAND_DISABLED,
- "The command %(name) has been disabled for this instance",
-},
-{
- QERR_COMMAND_NOT_FOUND,
- "The command %(name) has not been found",
-},
-{
- QERR_DEVICE_ENCRYPTED,
- "'%(device)' (%(filename)) is encrypted",
-},
-{
- QERR_DEVICE_FEATURE_BLOCKS_MIGRATION,
- "Migration is disabled when using feature '%(feature)' in device
'%(device)'",
-},
-{
- QERR_DEVICE_HAS_NO_MEDIUM,
- "Device '%(device)' has no medium",
-},
-{
- QERR_DEVICE_INIT_FAILED,
- "Device '%(device)' could not be initialized",
-},
-{
- QERR_DEVICE_IN_USE,
- "Device '%(device)' is in use",
-},
-{
- QERR_DEVICE_IS_READ_ONLY,
- "Device '%(device)' is read only",
-},
-{
- QERR_DEVICE_LOCKED,
- "Device '%(device)' is locked",
-},
-{
- QERR_DEVICE_MULTIPLE_BUSSES,
- "Device '%(device)' has multiple child busses",
-},
-{
- QERR_DEVICE_NO_BUS,
- "Device '%(device)' has no child bus",
-},
-{
- QERR_DEVICE_NO_HOTPLUG,
- "Device '%(device)' does not support hotplugging",
-},
-{
- QERR_DEVICE_NOT_ACTIVE,
- "Device '%(device)' has not been activated",
-},
-{
- QERR_DEVICE_NOT_ENCRYPTED,
- "Device '%(device)' is not encrypted",
-},
-{
- QERR_DEVICE_NOT_FOUND,
- "Device '%(device)' not found",
-},
-{
- QERR_DEVICE_NOT_REMOVABLE,
- "Device '%(device)' is not removable",
-},
-{
- QERR_DUPLICATE_ID,
- "Duplicate ID '%(id)' for %(object)",
-},
-{
- QERR_FD_NOT_FOUND,
- "File descriptor named '%(name)' not found",
-},
-{
- QERR_FD_NOT_SUPPLIED,
- "No file descriptor supplied via SCM_RIGHTS",
-},
-{
- QERR_FEATURE_DISABLED,
- "The feature '%(name)' is not enabled",
-},
-{
- QERR_INVALID_BLOCK_FORMAT,
- "Invalid block format '%(name)'",
-},
-{
- QERR_INVALID_OPTION_GROUP,
- "There is no option group '%(group)'",
-},
-{
- QERR_INVALID_PARAMETER,
- "Invalid parameter '%(name)'",
-},
-{
- QERR_INVALID_PARAMETER_COMBINATION,
- "Invalid parameter combination",
-},
-{
- QERR_INVALID_PARAMETER_TYPE,
- "Invalid parameter type for '%(name)', expected: %(expected)",
-},
-{
- QERR_INVALID_PARAMETER_VALUE,
- "Parameter '%(name)' expects %(
[Qemu-devel] [PATCH 30/35] qemu-ga: switch to the new error format on the wire
IMPORTANT: this BREAKS qemu-ga compatibility for the error response.
Instead of returning something like:
{ "error": { "class": "InvalidParameterValue",
"data": {"name": "mode", "expected": "halt|powerdown|reboot" } } }
qemu-ga now returns:
{ "error": { "class": "GenericError",
"desc": "Parameter 'mode' expects halt|powerdown|reboot" } }
Notice that this is also a bug fix, as qemu-ga wasn't returning the
human message.
Signed-off-by: Luiz Capitulino
---
Makefile.objs | 1 +
qapi/qmp-core.h | 1 +
qapi/qmp-dispatch.c | 10 +-
qemu-ga.c | 4 ++--
4 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/Makefile.objs b/Makefile.objs
index 5ebbcfa..8454b53 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -211,6 +211,7 @@ common-obj-$(CONFIG_SMARTCARD_NSS) += $(libcacard-y)
# qapi
qapi-obj-y = qapi/
+qapi-obj-y += qapi-types.o qapi-visit.o
common-obj-y += qmp-marshal.o qapi-visit.o qapi-types.o
common-obj-y += qmp.o hmp.o
diff --git a/qapi/qmp-core.h b/qapi/qmp-core.h
index b0f64ba..00446cf 100644
--- a/qapi/qmp-core.h
+++ b/qapi/qmp-core.h
@@ -49,6 +49,7 @@ void qmp_disable_command(const char *name);
void qmp_enable_command(const char *name);
bool qmp_command_is_enabled(const char *name);
char **qmp_get_command_list(void);
+QObject *qmp_build_error_object(Error *errp);
#endif
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 122c1a2..ec613f8 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -14,6 +14,7 @@
#include "qemu-objects.h"
#include "qapi/qmp-core.h"
#include "json-parser.h"
+#include "qapi-types.h"
#include "error.h"
#include "error_int.h"
#include "qerror.h"
@@ -109,6 +110,13 @@ static QObject *do_qmp_dispatch(QObject *request, Error
**errp)
return ret;
}
+QObject *qmp_build_error_object(Error *errp)
+{
+return qobject_from_jsonf("{ 'class': %s, 'desc': %s }",
+ ErrorClass_lookup[error_get_class(errp)],
+ error_get_pretty(errp));
+}
+
QObject *qmp_dispatch(QObject *request)
{
Error *err = NULL;
@@ -119,7 +127,7 @@ QObject *qmp_dispatch(QObject *request)
rsp = qdict_new();
if (err) {
-qdict_put_obj(rsp, "error", error_get_qobject(err));
+qdict_put_obj(rsp, "error", qmp_build_error_object(err));
error_free(err);
} else if (ret) {
qdict_put_obj(rsp, "return", ret);
diff --git a/qemu-ga.c b/qemu-ga.c
index f1a39ec..39abc50 100644
--- a/qemu-ga.c
+++ b/qemu-ga.c
@@ -515,7 +515,7 @@ static void process_event(JSONMessageParser *parser, QList
*tokens)
} else {
g_warning("failed to parse event: %s", error_get_pretty(err));
}
-qdict_put_obj(qdict, "error", error_get_qobject(err));
+qdict_put_obj(qdict, "error", qmp_build_error_object(err));
error_free(err);
} else {
qdict = qobject_to_qdict(obj);
@@ -532,7 +532,7 @@ static void process_event(JSONMessageParser *parser, QList
*tokens)
qdict = qdict_new();
g_warning("unrecognized payload format");
error_set(&err, QERR_UNSUPPORTED);
-qdict_put_obj(qdict, "error", error_get_qobject(err));
+qdict_put_obj(qdict, "error", qmp_build_error_object(err));
error_free(err);
}
ret = send_response(s, QOBJECT(qdict));
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 32/35] error, qerror: pass desc string to error calls
This commit changes all QERR_ macros to contain a human message (ie.
the desc string found in qerr_table[]) instead of a json dictionary
in string format.
Before this commit, error_set() and qerror_report() would receive
a json dictionary in string format and build a qobject from it. Now,
both function receive a human message instead and the qobject is
not built anymore.
Signed-off-by: Luiz Capitulino
---
error.c | 3 +-
error.h | 10 ++---
qerror.c | 42 +--
qerror.h | 141 +++
4 files changed, 77 insertions(+), 119 deletions(-)
diff --git a/error.c b/error.c
index 9d7b35f..0e10373 100644
--- a/error.c
+++ b/error.c
@@ -37,9 +37,8 @@ void error_set(Error **errp, ErrorClass err_class, const char
*fmt, ...)
err = g_malloc0(sizeof(*err));
va_start(ap, fmt);
-err->obj = qobject_to_qdict(qobject_from_jsonv(fmt, &ap));
+err->msg = g_strdup_vprintf(fmt, ap);
va_end(ap);
-err->msg = qerror_format(fmt, err->obj);
err->err_class = err_class;
*errp = err;
diff --git a/error.h b/error.h
index 5336fc5..96fc203 100644
--- a/error.h
+++ b/error.h
@@ -17,15 +17,15 @@
#include
/**
- * A class representing internal errors within QEMU. An error has a string
- * typename and optionally a set of named string parameters.
+ * A class representing internal errors within QEMU. An error has a ErrorClass
+ * code and a human message.
*/
typedef struct Error Error;
/**
- * Set an indirect pointer to an error given a printf-style format parameter.
- * Currently, qerror.h defines these error formats. This function is not
- * meant to be used outside of QEMU.
+ * Set an indirect pointer to an error given a ErrorClass value and a
+ * printf-style human message. This function is not meant to be used outside
+ * of QEMU.
*/
void error_set(Error **err, ErrorClass err_class, const char *fmt, ...)
GCC_FMT_ATTR(3, 4);
diff --git a/qerror.c b/qerror.c
index 0bf8aec..dda1427 100644
--- a/qerror.c
+++ b/qerror.c
@@ -342,45 +342,6 @@ static QError *qerror_new(void)
return qerr;
}
-static QDict *error_obj_from_fmt_no_fail(const char *fmt, va_list *va)
-{
-QObject *obj;
-QDict *ret;
-
-obj = qobject_from_jsonv(fmt, va);
-if (!obj) {
-fprintf(stderr, "invalid json in error dict '%s'\n", fmt);
-abort();
-}
-if (qobject_type(obj) != QTYPE_QDICT) {
-fprintf(stderr, "error is not a dict '%s'\n", fmt);
-abort();
-}
-
-ret = qobject_to_qdict(obj);
-obj = qdict_get(ret, "class");
-if (!obj) {
-fprintf(stderr, "missing 'class' key in '%s'\n", fmt);
-abort();
-}
-if (qobject_type(obj) != QTYPE_QSTRING) {
-fprintf(stderr, "'class' key value should be a string in '%s'\n", fmt);
-abort();
-}
-
-obj = qdict_get(ret, "data");
-if (!obj) {
-fprintf(stderr, "missing 'data' key in '%s'\n", fmt);
-abort();
-}
-if (qobject_type(obj) != QTYPE_QDICT) {
-fprintf(stderr, "'data' key value should be a dict in '%s'\n", fmt);
-abort();
-}
-
-return ret;
-}
-
/**
* qerror_from_info(): Create a new QError from error information
*
@@ -394,9 +355,8 @@ static QError *qerror_from_info(ErrorClass err_class, const
char *fmt,
qerr = qerror_new();
loc_save(&qerr->loc);
+qerr->err_msg = g_strdup_vprintf(fmt, *va);
qerr->err_class = err_class;
-qerr->error = error_obj_from_fmt_no_fail(fmt, va);
-qerr->err_msg = qerror_format(fmt, qerr->error);
return qerr;
}
diff --git a/qerror.h b/qerror.h
index 4f92218..057a8f2 100644
--- a/qerror.h
+++ b/qerror.h
@@ -45,214 +45,213 @@ char *qerror_format(const char *fmt, QDict *error);
* Use scripts/check-qerror.sh to check.
*/
#define QERR_ADD_CLIENT_FAILED \
-ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'AddClientFailed', 'data': {} }"
+ERROR_CLASS_GENERIC_ERROR, "Could not add client"
#define QERR_AMBIGUOUS_PATH \
-ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'AmbiguousPath', 'data': { 'path':
%s } }"
+ERROR_CLASS_GENERIC_ERROR, "Path '%s' does not uniquely identify an object"
#define QERR_BAD_BUS_FOR_DEVICE \
-ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BadBusForDevice', 'data': {
'device': %s, 'bad_bus_type': %s } }"
+ERROR_CLASS_GENERIC_ERROR, "Device '%s' can't go on a %s bus"
#define QERR_BASE_NOT_FOUND \
-ERROR_CLASS_GENERIC_ERROR, "{ 'class': 'BaseNotFound', 'data': {
[Qemu-devel] [PATCH 11/35] qmp: query-block: add 'encryption_key_missing' field
Signed-off-by: Luiz Capitulino
---
block.c | 1 +
qapi-schema.json | 9 ++---
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/block.c b/block.c
index 24323c1..016858b 100644
--- a/block.c
+++ b/block.c
@@ -2445,6 +2445,7 @@ BlockInfoList *qmp_query_block(Error **errp)
info->value->inserted->ro = bs->read_only;
info->value->inserted->drv = g_strdup(bs->drv->format_name);
info->value->inserted->encrypted = bs->encrypted;
+info->value->inserted->encryption_key_missing =
bdrv_key_required(bs);
if (bs->backing_file[0]) {
info->value->inserted->has_backing_file = true;
info->value->inserted->backing_file =
g_strdup(bs->backing_file);
diff --git a/qapi-schema.json b/qapi-schema.json
index bd9c450..a62bf68 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -402,6 +402,9 @@
#
# @encrypted: true if the backing device is encrypted
#
+# @encryption_key_missing: true if the backing device is encrypted but an
+# valid encryption key is missing
+#
# @bps: total throughput limit in bytes per second is specified
#
# @bps_rd: read throughput limit in bytes per second is specified
@@ -421,9 +424,9 @@
{ 'type': 'BlockDeviceInfo',
'data': { 'file': 'str', 'ro': 'bool', 'drv': 'str',
'*backing_file': 'str', 'backing_file_depth': 'int',
-'encrypted': 'bool', 'bps': 'int', 'bps_rd': 'int',
-'bps_wr': 'int', 'iops': 'int', 'iops_rd': 'int',
-'iops_wr': 'int'} }
+'encrypted': 'bool', 'encryption_key_missing': 'bool',
+'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
+'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int'} }
##
# @BlockDeviceIoStatus:
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 17/35] block: block_int: include qerror.h
Several block/ files are relying on qerror.h being provided by qapi-types.h. Fix this, as a future commit will change qapi-types.h not to provide qerror.h. Signed-off-by: Luiz Capitulino --- block_int.h | 1 + 1 file changed, 1 insertion(+) diff --git a/block_int.h b/block_int.h index 6c1d9ca..4452f6f 100644 --- a/block_int.h +++ b/block_int.h @@ -30,6 +30,7 @@ #include "qemu-coroutine.h" #include "qemu-timer.h" #include "qapi-types.h" +#include "qerror.h" #define BLOCK_FLAG_ENCRYPT 1 #define BLOCK_FLAG_COMPAT6 4 -- 1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 16/35] qerror: drop QERR_SOCKET_CONNECT_IN_PROGRESS
Unused since last commit.
Signed-off-by: Luiz Capitulino
---
qerror.c | 4
qerror.h | 3 ---
2 files changed, 7 deletions(-)
diff --git a/qerror.c b/qerror.c
index 5d38428..452ec69 100644
--- a/qerror.c
+++ b/qerror.c
@@ -309,10 +309,6 @@ static const QErrorStringTable qerror_table[] = {
.desc = "Could not start VNC server on %(target)",
},
{
-.error_fmt = QERR_SOCKET_CONNECT_IN_PROGRESS,
-.desc = "Connection can not be completed immediately",
-},
-{
.error_fmt = QERR_SOCKET_CONNECT_FAILED,
.desc = "Failed to connect to socket",
},
diff --git a/qerror.h b/qerror.h
index de8497d..52ce58d 100644
--- a/qerror.h
+++ b/qerror.h
@@ -240,9 +240,6 @@ char *qerror_format(const char *fmt, QDict *error);
#define QERR_VNC_SERVER_FAILED \
"{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
-#define QERR_SOCKET_CONNECT_IN_PROGRESS \
-"{ 'class': 'SockConnectInprogress', 'data': {} }"
-
#define QERR_SOCKET_CONNECT_FAILED \
"{ 'class': 'SockConnectFailed', 'data': {} }"
--
1.7.11.2.249.g31c7954.dirty
[Qemu-devel] [PATCH 07/35] qerror: QError: drop file, linenr, func
They have never been fully used and conflict with future error
improvements.
Also makes qerror_report() a proper function, as there's no point
in having it as a macro anymore.
Signed-off-by: Luiz Capitulino
---
qerror.c | 20 +++-
qerror.h | 8 +---
2 files changed, 4 insertions(+), 24 deletions(-)
diff --git a/qerror.c b/qerror.c
index e717496..6f9f49c 100644
--- a/qerror.c
+++ b/qerror.c
@@ -404,27 +404,14 @@ static const QErrorStringTable *get_desc_no_fail(const
char *fmt)
/**
* qerror_from_info(): Create a new QError from error information
*
- * The information consists of:
- *
- * - file the file name of where the error occurred
- * - linenr the line number of where the error occurred
- * - func the function name of where the error occurred
- * - fmtJSON printf-like dictionary, there must exist keys 'class' and
- * 'data'
- * - va va_list of all arguments specified by fmt
- *
* Return strong reference.
*/
-static QError *qerror_from_info(const char *file, int linenr, const char *func,
-const char *fmt, va_list *va)
+static QError *qerror_from_info(const char *fmt, va_list *va)
{
QError *qerr;
qerr = qerror_new();
loc_save(&qerr->loc);
-qerr->linenr = linenr;
-qerr->file = file;
-qerr->func = func;
qerr->error = error_obj_from_fmt_no_fail(fmt, va);
qerr->entry = get_desc_no_fail(fmt);
@@ -545,14 +532,13 @@ static void qerror_print(QError *qerror)
QDECREF(qstring);
}
-void qerror_report_internal(const char *file, int linenr, const char *func,
-const char *fmt, ...)
+void qerror_report(const char *fmt, ...)
{
va_list va;
QError *qerror;
va_start(va, fmt);
-qerror = qerror_from_info(file, linenr, func, fmt, &va);
+qerror = qerror_from_info(fmt, &va);
va_end(va);
if (monitor_cur_is_qmp()) {
diff --git a/qerror.h b/qerror.h
index fe8870c..3c0b14c 100644
--- a/qerror.h
+++ b/qerror.h
@@ -27,20 +27,14 @@ typedef struct QError {
QObject_HEAD;
QDict *error;
Location loc;
-int linenr;
-const char *file;
-const char *func;
const QErrorStringTable *entry;
} QError;
QString *qerror_human(const QError *qerror);
-void qerror_report_internal(const char *file, int linenr, const char *func,
-const char *fmt, ...) GCC_FMT_ATTR(4, 5);
+void qerror_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
void qerror_report_err(Error *err);
void assert_no_error(Error *err);
QString *qerror_format(const char *fmt, QDict *error);
-#define qerror_report(fmt, ...) \
-qerror_report_internal(__FILE__, __LINE__, __func__, fmt, ## __VA_ARGS__)
/*
* QError class list
--
1.7.11.2.249.g31c7954.dirty
Re: [Qemu-devel] [PATCH v3 00/35]: add new error format
On Sat, 11 Aug 2012 09:05:33 +0200
Markus Armbruster wrote:
> Luiz Capitulino writes:
>
> [...]
> > This series implements the 'Plan for error handling in QMP' as described
> > by Anthony in this email:
> >
> > http://lists.gnu.org/archive/html/qemu-devel/2012-07/msg03764.html
> >
> > Basically, this replaces almost all error classes by GenericError (the
> > exception are a few error classes used by libvirt) and drops the error
> > data memeber. This also adds a free form string to error_set().
> >
> > On the wire, we go from:
> >
> > { "error": { "class": "DeviceNotRemovable",
> > "data": { "device": "virtio0" },
> > "desc": "Device 'virtio0' is not removable" } }
> >
> > to:
> >
> > { "error": { "class": "GenericError",
> > "desc": "Device 'virtio0' is not removable" } }
> >
> > Internally, we go from:
> >
> > void error_set(Error **err, const char *fmt, ...);
> >
> > to:
> >
> > void error_set(Error **err, ErrorClass err_class, const char *fmt, ...);
>
> Glad to see this change in good shape in time for the release. Thanks,
> Luiz!
Thank you for your review!
>
> Reviewed-by: Markus Armbruster
>
