* [Qemu-devel] [PATCH 01/21] qemu-char: cleanup qmp_chardev_add
2015-10-12 8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
@ 2015-10-12 8:03 ` Paolo Bonzini
2015-10-12 14:55 ` Eric Blake
2015-10-12 8:03 ` [Qemu-devel] [PATCH 02/21] qemu-char: cleanup HAVE_CHARDEV_* Paolo Bonzini
` (19 subsequent siblings)
20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12 8:03 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru
Use the usual idioms for error propagation.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu-char.c | 56 +++++++++++++++++++++++++++++++-------------------------
1 file changed, 31 insertions(+), 25 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 653ea10..f51c0aa 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4214,6 +4214,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
{
ChardevReturn *ret = g_new0(ChardevReturn, 1);
CharDriverState *base, *chr = NULL;
+ Error *local_err = NULL;
chr = qemu_chr_find(id);
if (chr) {
@@ -4224,22 +4225,22 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
switch (backend->kind) {
case CHARDEV_BACKEND_KIND_FILE:
- chr = qmp_chardev_open_file(backend->file, errp);
+ chr = qmp_chardev_open_file(backend->file, &local_err);
break;
case CHARDEV_BACKEND_KIND_SERIAL:
- chr = qmp_chardev_open_serial(backend->serial, errp);
+ chr = qmp_chardev_open_serial(backend->serial, &local_err);
break;
case CHARDEV_BACKEND_KIND_PARALLEL:
- chr = qmp_chardev_open_parallel(backend->parallel, errp);
+ chr = qmp_chardev_open_parallel(backend->parallel, &local_err);
break;
case CHARDEV_BACKEND_KIND_PIPE:
chr = qemu_chr_open_pipe(backend->pipe);
break;
case CHARDEV_BACKEND_KIND_SOCKET:
- chr = qmp_chardev_open_socket(backend->socket, errp);
+ chr = qmp_chardev_open_socket(backend->socket, &local_err);
break;
case CHARDEV_BACKEND_KIND_UDP:
- chr = qmp_chardev_open_udp(backend->udp, errp);
+ chr = qmp_chardev_open_udp(backend->udp, &local_err);
break;
#ifdef HAVE_CHARDEV_TTY
case CHARDEV_BACKEND_KIND_PTY:
@@ -4252,7 +4253,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
case CHARDEV_BACKEND_KIND_MUX:
base = qemu_chr_find(backend->mux->chardev);
if (base == NULL) {
- error_setg(errp, "mux: base chardev %s not found",
+ error_setg(&local_err, "mux: base chardev %s not found",
backend->mux->chardev);
break;
}
@@ -4290,11 +4291,11 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
break;
case CHARDEV_BACKEND_KIND_RINGBUF:
case CHARDEV_BACKEND_KIND_MEMORY:
- chr = qemu_chr_open_ringbuf(backend->ringbuf, errp);
+ chr = qemu_chr_open_ringbuf(backend->ringbuf, &local_err);
break;
default:
error_setg(errp, "unknown chardev backend (%d)", backend->kind);
- break;
+ goto out_error;
}
/*
@@ -4303,25 +4304,30 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
* error then.
* TODO full conversion to Error API
*/
- if (chr == NULL && errp && !*errp) {
- error_setg(errp, "Failed to create chardev");
- }
- if (chr) {
- chr->label = g_strdup(id);
- chr->avail_connections =
- (backend->kind == CHARDEV_BACKEND_KIND_MUX) ? MAX_MUX : 1;
- if (!chr->filename) {
- chr->filename = g_strdup(ChardevBackendKind_lookup[backend->kind]);
- }
- if (!chr->explicit_be_open) {
- qemu_chr_be_event(chr, CHR_EVENT_OPENED);
+ if (chr == NULL) {
+ if (local_err) {
+ error_propagate(errp, local_err);
+ } else {
+ error_setg(errp, "Failed to create chardev");
}
- QTAILQ_INSERT_TAIL(&chardevs, chr, next);
- return ret;
- } else {
- g_free(ret);
- return NULL;
+ goto out_error;
+ }
+
+ chr->label = g_strdup(id);
+ chr->avail_connections =
+ (backend->kind == CHARDEV_BACKEND_KIND_MUX) ? MAX_MUX : 1;
+ if (!chr->filename) {
+ chr->filename = g_strdup(ChardevBackendKind_lookup[backend->kind]);
}
+ if (!chr->explicit_be_open) {
+ qemu_chr_be_event(chr, CHR_EVENT_OPENED);
+ }
+ QTAILQ_INSERT_TAIL(&chardevs, chr, next);
+ return ret;
+
+out_error:
+ g_free(ret);
+ return NULL;
}
void qmp_chardev_remove(const char *id, Error **errp)
--
2.5.0
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH 01/21] qemu-char: cleanup qmp_chardev_add
2015-10-12 8:03 ` [Qemu-devel] [PATCH 01/21] qemu-char: cleanup qmp_chardev_add Paolo Bonzini
@ 2015-10-12 14:55 ` Eric Blake
0 siblings, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-12 14:55 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: armbru
[-- Attachment #1: Type: text/plain, Size: 1729 bytes --]
On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Use the usual idioms for error propagation.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> qemu-char.c | 56 +++++++++++++++++++++++++++++++-------------------------
> 1 file changed, 31 insertions(+), 25 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index 653ea10..f51c0aa 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -4214,6 +4214,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
> {
> ChardevReturn *ret = g_new0(ChardevReturn, 1);
> CharDriverState *base, *chr = NULL;
> + Error *local_err = NULL;
It's a toss-up on whether 'local_err' or 'err' is the preferred name for
the local variable. I'm fine with either.
> @@ -4303,25 +4304,30 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
> * error then.
> * TODO full conversion to Error API
> */
> - if (chr == NULL && errp && !*errp) {
> - error_setg(errp, "Failed to create chardev");
> - }
> - if (chr) {
> - chr->label = g_strdup(id);
> - chr->avail_connections =
> - (backend->kind == CHARDEV_BACKEND_KIND_MUX) ? MAX_MUX : 1;
> - if (!chr->filename) {
> - chr->filename = g_strdup(ChardevBackendKind_lookup[backend->kind]);
> - }
> - if (!chr->explicit_be_open) {
> - qemu_chr_be_event(chr, CHR_EVENT_OPENED);
> + if (chr == NULL) {
I might write this as if (!chr), but that's cosmetic.
Conversion looks sane.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* [Qemu-devel] [PATCH 02/21] qemu-char: cleanup HAVE_CHARDEV_*
2015-10-12 8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
2015-10-12 8:03 ` [Qemu-devel] [PATCH 01/21] qemu-char: cleanup qmp_chardev_add Paolo Bonzini
@ 2015-10-12 8:03 ` Paolo Bonzini
2015-10-12 14:57 ` Eric Blake
2015-10-12 8:03 ` [Qemu-devel] [PATCH 03/21] qemu-char: add create to register_char_driver Paolo Bonzini
` (18 subsequent siblings)
20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12 8:03 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru
Move the #ifdef up into qmp_chardev_add, and avoid duplicating
the code that reports unavailable backends. Split HAVE_CHARDEV_TTY
into HAVE_CHARDEV_SERIAL and HAVE_CHARDEV_PTY.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu-char.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index f51c0aa..7219d56 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1196,7 +1196,8 @@ static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
|| defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__) \
|| defined(__GLIBC__)
-#define HAVE_CHARDEV_TTY 1
+#define HAVE_CHARDEV_SERIAL 1
+#define HAVE_CHARDEV_PTY 1
typedef struct {
GIOChannel *fd;
@@ -1832,6 +1833,8 @@ static CharDriverState *qemu_chr_open_pp_fd(int fd)
#else /* _WIN32 */
+#define HAVE_CHARDEV_SERIAL 1
+
typedef struct {
int max_size;
HANDLE hcom, hrecv, hsend;
@@ -4069,10 +4072,10 @@ static CharDriverState *qmp_chardev_open_file(ChardevFile *file, Error **errp)
return qemu_chr_open_fd(in, out);
}
+#ifdef HAVE_CHARDEV_SERIAL
static CharDriverState *qmp_chardev_open_serial(ChardevHostdev *serial,
Error **errp)
{
-#ifdef HAVE_CHARDEV_TTY
int fd;
fd = qmp_chardev_open_file_source(serial->device, O_RDWR, errp);
@@ -4081,16 +4084,12 @@ static CharDriverState *qmp_chardev_open_serial(ChardevHostdev *serial,
}
qemu_set_nonblock(fd);
return qemu_chr_open_tty_fd(fd);
-#else
- error_setg(errp, "character device backend type 'serial' not supported");
- return NULL;
-#endif
}
+#ifdef HAVE_CHARDEV_PARPORT
static CharDriverState *qmp_chardev_open_parallel(ChardevHostdev *parallel,
Error **errp)
{
-#ifdef HAVE_CHARDEV_PARPORT
int fd;
fd = qmp_chardev_open_file_source(parallel->device, O_RDWR, errp);
@@ -4098,11 +4097,8 @@ static CharDriverState *qmp_chardev_open_parallel(ChardevHostdev *parallel,
return NULL;
}
return qemu_chr_open_pp_fd(fd);
-#else
- error_setg(errp, "character device backend type 'parallel' not supported");
- return NULL;
-#endif
}
+#endif
#endif /* WIN32 */
@@ -4227,12 +4223,16 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
case CHARDEV_BACKEND_KIND_FILE:
chr = qmp_chardev_open_file(backend->file, &local_err);
break;
+#ifdef HAVE_CHARDEV_SERIAL
case CHARDEV_BACKEND_KIND_SERIAL:
chr = qmp_chardev_open_serial(backend->serial, &local_err);
break;
+#endif
+#ifdef HAVE_CHARDEV_PARPORT
case CHARDEV_BACKEND_KIND_PARALLEL:
chr = qmp_chardev_open_parallel(backend->parallel, &local_err);
break;
+#endif
case CHARDEV_BACKEND_KIND_PIPE:
chr = qemu_chr_open_pipe(backend->pipe);
break;
@@ -4242,7 +4242,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
case CHARDEV_BACKEND_KIND_UDP:
chr = qmp_chardev_open_udp(backend->udp, &local_err);
break;
-#ifdef HAVE_CHARDEV_TTY
+#ifdef HAVE_CHARDEV_PTY
case CHARDEV_BACKEND_KIND_PTY:
chr = qemu_chr_open_pty(id, ret);
break;
--
2.5.0
^ permalink raw reply related [flat|nested] 51+ messages in thread* [Qemu-devel] [PATCH 03/21] qemu-char: add create to register_char_driver
2015-10-12 8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
2015-10-12 8:03 ` [Qemu-devel] [PATCH 01/21] qemu-char: cleanup qmp_chardev_add Paolo Bonzini
2015-10-12 8:03 ` [Qemu-devel] [PATCH 02/21] qemu-char: cleanup HAVE_CHARDEV_* Paolo Bonzini
@ 2015-10-12 8:03 ` Paolo Bonzini
2015-10-12 15:04 ` Eric Blake
2015-10-12 8:03 ` [Qemu-devel] [PATCH 04/21] qemu-char: convert file backend to data-driven creation Paolo Bonzini
` (17 subsequent siblings)
20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12 8:03 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru
Having creation as a member of the CharDriver struct removes the need
to export functions for qemu-char.c's usage. After the conversion,
chardev backends implemented outside qemu-char.c will not need a stub
creation function anymore.
Ultimately all drivers will be converted. For now, support the case
where cd->create == NULL.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
backends/baum.c | 3 +-
backends/msmouse.c | 3 +-
backends/testdev.c | 3 +-
include/sysemu/char.h | 4 +-
qemu-char.c | 213 ++++++++++++++++++++++++++++----------------------
spice-qemu-char.c | 4 +-
ui/console.c | 3 +-
7 files changed, 133 insertions(+), 100 deletions(-)
diff --git a/backends/baum.c b/backends/baum.c
index a17f625..e86a019 100644
--- a/backends/baum.c
+++ b/backends/baum.c
@@ -629,7 +629,8 @@ fail_handle:
static void register_types(void)
{
- register_char_driver("braille", CHARDEV_BACKEND_KIND_BRAILLE, NULL);
+ register_char_driver("braille", CHARDEV_BACKEND_KIND_BRAILLE, NULL,
+ NULL);
}
type_init(register_types);
diff --git a/backends/msmouse.c b/backends/msmouse.c
index 0119110..d50ed47 100644
--- a/backends/msmouse.c
+++ b/backends/msmouse.c
@@ -79,7 +79,8 @@ CharDriverState *qemu_chr_open_msmouse(void)
static void register_types(void)
{
- register_char_driver("msmouse", CHARDEV_BACKEND_KIND_MSMOUSE, NULL);
+ register_char_driver("msmouse", CHARDEV_BACKEND_KIND_MSMOUSE, NULL,
+ NULL);
}
type_init(register_types);
diff --git a/backends/testdev.c b/backends/testdev.c
index 1429152..43787f6 100644
--- a/backends/testdev.c
+++ b/backends/testdev.c
@@ -125,7 +125,8 @@ CharDriverState *chr_testdev_init(void)
static void register_types(void)
{
- register_char_driver("testdev", CHARDEV_BACKEND_KIND_TESTDEV, NULL);
+ register_char_driver("testdev", CHARDEV_BACKEND_KIND_TESTDEV, NULL,
+ NULL);
}
type_init(register_types);
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 832b7fe..4b01a8c 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -345,7 +345,9 @@ bool chr_is_ringbuf(const CharDriverState *chr);
QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename);
void register_char_driver(const char *name, ChardevBackendKind kind,
- void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp));
+ void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp),
+ CharDriverState *(*create)(const char *id, ChardevBackend *backend,
+ ChardevReturn *ret, Error **errp));
/* add an eventfd to the qemu devices that are polled */
CharDriverState *qemu_chr_open_eventfd(int eventfd);
diff --git a/qemu-char.c b/qemu-char.c
index 7219d56..a6411d6 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3644,12 +3644,16 @@ typedef struct CharDriver {
const char *name;
ChardevBackendKind kind;
void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp);
+ CharDriverState *(*create)(const char *id, ChardevBackend *backend,
+ ChardevReturn *ret, Error **errp);
} CharDriver;
static GSList *backends;
void register_char_driver(const char *name, ChardevBackendKind kind,
- void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp))
+ void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp),
+ CharDriverState *(*create)(const char *id, ChardevBackend *backend,
+ ChardevReturn *ret, Error **errp))
{
CharDriver *s;
@@ -3657,6 +3661,7 @@ void register_char_driver(const char *name, ChardevBackendKind kind,
s->name = g_strdup(name);
s->kind = kind;
s->parse = parse;
+ s->create = create;
backends = g_slist_append(backends, s);
}
@@ -4211,6 +4216,8 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
ChardevReturn *ret = g_new0(ChardevReturn, 1);
CharDriverState *base, *chr = NULL;
Error *local_err = NULL;
+ GSList *i;
+ CharDriver *cd;
chr = qemu_chr_find(id);
if (chr) {
@@ -4219,98 +4226,113 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
return NULL;
}
- switch (backend->kind) {
- case CHARDEV_BACKEND_KIND_FILE:
- chr = qmp_chardev_open_file(backend->file, &local_err);
- break;
+ for (i = backends; i; i = i->next) {
+ cd = i->data;
+
+ if (cd->kind == backend->kind && cd->create) {
+ chr = cd->create(id, backend, ret, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ goto out_error;
+ }
+ break;
+ }
+ }
+
+ if (chr == NULL) {
+ switch (backend->kind) {
+ case CHARDEV_BACKEND_KIND_FILE:
+ chr = qmp_chardev_open_file(backend->file, &local_err);
+ break;
#ifdef HAVE_CHARDEV_SERIAL
- case CHARDEV_BACKEND_KIND_SERIAL:
- chr = qmp_chardev_open_serial(backend->serial, &local_err);
- break;
+ case CHARDEV_BACKEND_KIND_SERIAL:
+ chr = qmp_chardev_open_serial(backend->serial, &local_err);
+ break;
#endif
#ifdef HAVE_CHARDEV_PARPORT
- case CHARDEV_BACKEND_KIND_PARALLEL:
- chr = qmp_chardev_open_parallel(backend->parallel, &local_err);
- break;
+ case CHARDEV_BACKEND_KIND_PARALLEL:
+ chr = qmp_chardev_open_parallel(backend->parallel, &local_err);
+ break;
#endif
- case CHARDEV_BACKEND_KIND_PIPE:
- chr = qemu_chr_open_pipe(backend->pipe);
- break;
- case CHARDEV_BACKEND_KIND_SOCKET:
- chr = qmp_chardev_open_socket(backend->socket, &local_err);
- break;
- case CHARDEV_BACKEND_KIND_UDP:
- chr = qmp_chardev_open_udp(backend->udp, &local_err);
- break;
+ case CHARDEV_BACKEND_KIND_PIPE:
+ chr = qemu_chr_open_pipe(backend->pipe);
+ break;
+ case CHARDEV_BACKEND_KIND_SOCKET:
+ chr = qmp_chardev_open_socket(backend->socket, &local_err);
+ break;
+ case CHARDEV_BACKEND_KIND_UDP:
+ chr = qmp_chardev_open_udp(backend->udp, &local_err);
+ break;
#ifdef HAVE_CHARDEV_PTY
- case CHARDEV_BACKEND_KIND_PTY:
- chr = qemu_chr_open_pty(id, ret);
- break;
+ case CHARDEV_BACKEND_KIND_PTY:
+ chr = qemu_chr_open_pty(id, ret);
+ break;
#endif
- case CHARDEV_BACKEND_KIND_NULL:
- chr = qemu_chr_open_null();
- break;
- case CHARDEV_BACKEND_KIND_MUX:
- base = qemu_chr_find(backend->mux->chardev);
- if (base == NULL) {
- error_setg(&local_err, "mux: base chardev %s not found",
- backend->mux->chardev);
+ case CHARDEV_BACKEND_KIND_NULL:
+ chr = qemu_chr_open_null();
+ break;
+ case CHARDEV_BACKEND_KIND_MUX:
+ base = qemu_chr_find(backend->mux->chardev);
+ if (base == NULL) {
+ error_setg(&local_err, "mux: base chardev %s not found",
+ backend->mux->chardev);
+ break;
+ }
+ chr = qemu_chr_open_mux(base);
+ break;
+ case CHARDEV_BACKEND_KIND_MSMOUSE:
+ chr = qemu_chr_open_msmouse();
break;
- }
- chr = qemu_chr_open_mux(base);
- break;
- case CHARDEV_BACKEND_KIND_MSMOUSE:
- chr = qemu_chr_open_msmouse();
- break;
#ifdef CONFIG_BRLAPI
- case CHARDEV_BACKEND_KIND_BRAILLE:
- chr = chr_baum_init();
- break;
+ case CHARDEV_BACKEND_KIND_BRAILLE:
+ chr = chr_baum_init();
+ break;
#endif
- case CHARDEV_BACKEND_KIND_TESTDEV:
- chr = chr_testdev_init();
- break;
- case CHARDEV_BACKEND_KIND_STDIO:
- chr = qemu_chr_open_stdio(backend->stdio);
- break;
+ case CHARDEV_BACKEND_KIND_TESTDEV:
+ chr = chr_testdev_init();
+ break;
+ case CHARDEV_BACKEND_KIND_STDIO:
+ chr = qemu_chr_open_stdio(backend->stdio);
+ break;
#ifdef _WIN32
- case CHARDEV_BACKEND_KIND_CONSOLE:
- chr = qemu_chr_open_win_con();
- break;
+ case CHARDEV_BACKEND_KIND_CONSOLE:
+ chr = qemu_chr_open_win_con();
+ break;
#endif
#ifdef CONFIG_SPICE
- case CHARDEV_BACKEND_KIND_SPICEVMC:
- chr = qemu_chr_open_spice_vmc(backend->spicevmc->type);
- break;
- case CHARDEV_BACKEND_KIND_SPICEPORT:
- chr = qemu_chr_open_spice_port(backend->spiceport->fqdn);
- break;
+ case CHARDEV_BACKEND_KIND_SPICEVMC:
+ chr = qemu_chr_open_spice_vmc(backend->spicevmc->type);
+ break;
+ case CHARDEV_BACKEND_KIND_SPICEPORT:
+ chr = qemu_chr_open_spice_port(backend->spiceport->fqdn);
+ break;
#endif
- case CHARDEV_BACKEND_KIND_VC:
- chr = vc_init(backend->vc);
- break;
- case CHARDEV_BACKEND_KIND_RINGBUF:
- case CHARDEV_BACKEND_KIND_MEMORY:
- chr = qemu_chr_open_ringbuf(backend->ringbuf, &local_err);
- break;
- default:
- error_setg(errp, "unknown chardev backend (%d)", backend->kind);
- goto out_error;
- }
+ case CHARDEV_BACKEND_KIND_VC:
+ chr = vc_init(backend->vc);
+ break;
+ case CHARDEV_BACKEND_KIND_RINGBUF:
+ case CHARDEV_BACKEND_KIND_MEMORY:
+ chr = qemu_chr_open_ringbuf(backend->ringbuf, &local_err);
+ break;
+ default:
+ error_setg(errp, "unknown chardev backend (%d)", backend->kind);
+ goto out_error;
+ }
- /*
- * Character backend open hasn't been fully converted to the Error
- * API. Some opens fail without setting an error. Set a generic
- * error then.
- * TODO full conversion to Error API
- */
- if (chr == NULL) {
- if (local_err) {
- error_propagate(errp, local_err);
- } else {
- error_setg(errp, "Failed to create chardev");
+ /*
+ * Character backend open hasn't been fully converted to the Error
+ * API. Some opens fail without setting an error. Set a generic
+ * error then.
+ * TODO full conversion to Error API
+ */
+ if (chr == NULL) {
+ if (local_err) {
+ error_propagate(errp, local_err);
+ } else {
+ error_setg(errp, "Failed to create chardev");
+ }
+ goto out_error;
}
- goto out_error;
}
chr->label = g_strdup(id);
@@ -4349,32 +4371,37 @@ void qmp_chardev_remove(const char *id, Error **errp)
static void register_types(void)
{
- register_char_driver("null", CHARDEV_BACKEND_KIND_NULL, NULL);
+ register_char_driver("null", CHARDEV_BACKEND_KIND_NULL, NULL,
+ NULL);
register_char_driver("socket", CHARDEV_BACKEND_KIND_SOCKET,
- qemu_chr_parse_socket);
- register_char_driver("udp", CHARDEV_BACKEND_KIND_UDP, qemu_chr_parse_udp);
+ qemu_chr_parse_socket, NULL);
+ register_char_driver("udp", CHARDEV_BACKEND_KIND_UDP, qemu_chr_parse_udp,
+ NULL);
register_char_driver("ringbuf", CHARDEV_BACKEND_KIND_RINGBUF,
- qemu_chr_parse_ringbuf);
+ qemu_chr_parse_ringbuf, NULL);
register_char_driver("file", CHARDEV_BACKEND_KIND_FILE,
- qemu_chr_parse_file_out);
+ qemu_chr_parse_file_out, NULL);
register_char_driver("stdio", CHARDEV_BACKEND_KIND_STDIO,
- qemu_chr_parse_stdio);
+ qemu_chr_parse_stdio, NULL);
register_char_driver("serial", CHARDEV_BACKEND_KIND_SERIAL,
- qemu_chr_parse_serial);
+ qemu_chr_parse_serial, NULL);
register_char_driver("tty", CHARDEV_BACKEND_KIND_SERIAL,
- qemu_chr_parse_serial);
+ qemu_chr_parse_serial, NULL);
register_char_driver("parallel", CHARDEV_BACKEND_KIND_PARALLEL,
- qemu_chr_parse_parallel);
+ qemu_chr_parse_parallel, NULL);
register_char_driver("parport", CHARDEV_BACKEND_KIND_PARALLEL,
- qemu_chr_parse_parallel);
- register_char_driver("pty", CHARDEV_BACKEND_KIND_PTY, NULL);
- register_char_driver("console", CHARDEV_BACKEND_KIND_CONSOLE, NULL);
+ qemu_chr_parse_parallel, NULL);
+ register_char_driver("pty", CHARDEV_BACKEND_KIND_PTY, NULL,
+ NULL);
+ register_char_driver("console", CHARDEV_BACKEND_KIND_CONSOLE, NULL,
+ NULL);
register_char_driver("pipe", CHARDEV_BACKEND_KIND_PIPE,
- qemu_chr_parse_pipe);
- register_char_driver("mux", CHARDEV_BACKEND_KIND_MUX, qemu_chr_parse_mux);
+ qemu_chr_parse_pipe, NULL);
+ register_char_driver("mux", CHARDEV_BACKEND_KIND_MUX, qemu_chr_parse_mux,
+ NULL);
/* Bug-compatibility: */
register_char_driver("memory", CHARDEV_BACKEND_KIND_MEMORY,
- qemu_chr_parse_ringbuf);
+ qemu_chr_parse_ringbuf, NULL);
/* this must be done after machine init, since we register FEs with muxes
* as part of realize functions like serial_isa_realizefn when -nographic
* is specified
diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index d41bb74..e4353ef 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -379,9 +379,9 @@ static void qemu_chr_parse_spice_port(QemuOpts *opts, ChardevBackend *backend,
static void register_types(void)
{
register_char_driver("spicevmc", CHARDEV_BACKEND_KIND_SPICEVMC,
- qemu_chr_parse_spice_vmc);
+ qemu_chr_parse_spice_vmc, NULL);
register_char_driver("spiceport", CHARDEV_BACKEND_KIND_SPICEPORT,
- qemu_chr_parse_spice_port);
+ qemu_chr_parse_spice_port, NULL);
}
type_init(register_types);
diff --git a/ui/console.c b/ui/console.c
index 75fc492..746b23a 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -2030,7 +2030,8 @@ static const TypeInfo qemu_console_info = {
static void register_types(void)
{
type_register_static(&qemu_console_info);
- register_char_driver("vc", CHARDEV_BACKEND_KIND_VC, qemu_chr_parse_vc);
+ register_char_driver("vc", CHARDEV_BACKEND_KIND_VC, qemu_chr_parse_vc,
+ NULL);
}
type_init(register_types);
--
2.5.0
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH 03/21] qemu-char: add create to register_char_driver
2015-10-12 8:03 ` [Qemu-devel] [PATCH 03/21] qemu-char: add create to register_char_driver Paolo Bonzini
@ 2015-10-12 15:04 ` Eric Blake
0 siblings, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:04 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: armbru
[-- Attachment #1: Type: text/plain, Size: 1529 bytes --]
On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Having creation as a member of the CharDriver struct removes the need
> to export functions for qemu-char.c's usage. After the conversion,
> chardev backends implemented outside qemu-char.c will not need a stub
> creation function anymore.
>
> Ultimately all drivers will be converted. For now, support the case
> where cd->create == NULL.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> backends/baum.c | 3 +-
> backends/msmouse.c | 3 +-
> backends/testdev.c | 3 +-
> include/sysemu/char.h | 4 +-
> qemu-char.c | 213 ++++++++++++++++++++++++++++----------------------
> spice-qemu-char.c | 4 +-
> ui/console.c | 3 +-
> 7 files changed, 133 insertions(+), 100 deletions(-)
The diff is a bit ugly because of reindentation, but seems sane.
>
> void register_char_driver(const char *name, ChardevBackendKind kind,
> - void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp))
> + void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp),
> + CharDriverState *(*create)(const char *id, ChardevBackend *backend,
> + ChardevReturn *ret, Error **errp))
A typedef (or two) for the function pointer(s) would make this a bit
easier to write.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* [Qemu-devel] [PATCH 04/21] qemu-char: convert file backend to data-driven creation
2015-10-12 8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
` (2 preceding siblings ...)
2015-10-12 8:03 ` [Qemu-devel] [PATCH 03/21] qemu-char: add create to register_char_driver Paolo Bonzini
@ 2015-10-12 8:03 ` Paolo Bonzini
2015-10-12 15:06 ` Eric Blake
2015-10-12 8:03 ` [Qemu-devel] [PATCH 05/21] qemu-char: convert serial " Paolo Bonzini
` (16 subsequent siblings)
20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12 8:03 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu-char.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index a6411d6..13fd394 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4010,8 +4010,12 @@ QemuOptsList qemu_chardev_opts = {
#ifdef _WIN32
-static CharDriverState *qmp_chardev_open_file(ChardevFile *file, Error **errp)
+static CharDriverState *qmp_chardev_open_file(const char *id,
+ ChardevBackend *backend,
+ ChardevReturn *ret,
+ Error **errp)
{
+ ChardevFile *file = backend->file;
HANDLE out;
if (file->has_in) {
@@ -4055,8 +4059,12 @@ static int qmp_chardev_open_file_source(char *src, int flags,
return fd;
}
-static CharDriverState *qmp_chardev_open_file(ChardevFile *file, Error **errp)
+static CharDriverState *qmp_chardev_open_file(const char *id,
+ ChardevBackend *backend,
+ ChardevReturn *ret,
+ Error **errp)
{
+ ChardevFile *file = backend->file;
int flags, in = -1, out;
flags = O_WRONLY | O_TRUNC | O_CREAT | O_BINARY;
@@ -4242,7 +4250,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
if (chr == NULL) {
switch (backend->kind) {
case CHARDEV_BACKEND_KIND_FILE:
- chr = qmp_chardev_open_file(backend->file, &local_err);
+ abort();
break;
#ifdef HAVE_CHARDEV_SERIAL
case CHARDEV_BACKEND_KIND_SERIAL:
@@ -4380,7 +4388,7 @@ static void register_types(void)
register_char_driver("ringbuf", CHARDEV_BACKEND_KIND_RINGBUF,
qemu_chr_parse_ringbuf, NULL);
register_char_driver("file", CHARDEV_BACKEND_KIND_FILE,
- qemu_chr_parse_file_out, NULL);
+ qemu_chr_parse_file_out, qmp_chardev_open_file);
register_char_driver("stdio", CHARDEV_BACKEND_KIND_STDIO,
qemu_chr_parse_stdio, NULL);
register_char_driver("serial", CHARDEV_BACKEND_KIND_SERIAL,
--
2.5.0
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH 04/21] qemu-char: convert file backend to data-driven creation
2015-10-12 8:03 ` [Qemu-devel] [PATCH 04/21] qemu-char: convert file backend to data-driven creation Paolo Bonzini
@ 2015-10-12 15:06 ` Eric Blake
2015-10-12 15:17 ` Paolo Bonzini
0 siblings, 1 reply; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:06 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: armbru
[-- Attachment #1: Type: text/plain, Size: 1188 bytes --]
On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> qemu-char.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> @@ -4242,7 +4250,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
> if (chr == NULL) {
> switch (backend->kind) {
> case CHARDEV_BACKEND_KIND_FILE:
> - chr = qmp_chardev_open_file(backend->file, &local_err);
> + abort();
> break;
The break is now dead code.
> #ifdef HAVE_CHARDEV_SERIAL
> case CHARDEV_BACKEND_KIND_SERIAL:
> @@ -4380,7 +4388,7 @@ static void register_types(void)
> register_char_driver("ringbuf", CHARDEV_BACKEND_KIND_RINGBUF,
> qemu_chr_parse_ringbuf, NULL);
> register_char_driver("file", CHARDEV_BACKEND_KIND_FILE,
> - qemu_chr_parse_file_out, NULL);
> + qemu_chr_parse_file_out, qmp_chardev_open_file);
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH 04/21] qemu-char: convert file backend to data-driven creation
2015-10-12 15:06 ` Eric Blake
@ 2015-10-12 15:17 ` Paolo Bonzini
0 siblings, 0 replies; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12 15:17 UTC (permalink / raw)
To: Eric Blake, qemu-devel; +Cc: armbru
On 12/10/2015 17:06, Eric Blake wrote:
>> > case CHARDEV_BACKEND_KIND_FILE:
>> > - chr = qmp_chardev_open_file(backend->file, &local_err);
>> > + abort();
>> > break;
> The break is now dead code.
>
They all go away in patch 21. :)
Paolo
^ permalink raw reply [flat|nested] 51+ messages in thread
* [Qemu-devel] [PATCH 05/21] qemu-char: convert serial backend to data-driven creation
2015-10-12 8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
` (3 preceding siblings ...)
2015-10-12 8:03 ` [Qemu-devel] [PATCH 04/21] qemu-char: convert file backend to data-driven creation Paolo Bonzini
@ 2015-10-12 8:03 ` Paolo Bonzini
2015-10-12 15:09 ` Eric Blake
2015-10-12 8:03 ` [Qemu-devel] [PATCH 06/21] qemu-char: convert parallel " Paolo Bonzini
` (15 subsequent siblings)
20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12 8:03 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu-char.c | 48 +++++++++++++++++++++++++++++-------------------
1 file changed, 29 insertions(+), 19 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 13fd394..8567580 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1886,7 +1886,7 @@ static void win_chr_close(CharDriverState *chr)
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}
-static int win_chr_init(CharDriverState *chr, const char *filename)
+static int win_chr_init(CharDriverState *chr, const char *filename, Error **errp)
{
WinCharState *s = chr->opaque;
COMMCONFIG comcfg;
@@ -1897,25 +1897,25 @@ static int win_chr_init(CharDriverState *chr, const char *filename)
s->hsend = CreateEvent(NULL, TRUE, FALSE, NULL);
if (!s->hsend) {
- fprintf(stderr, "Failed CreateEvent\n");
+ error_setg(errp, "Failed CreateEvent");
goto fail;
}
s->hrecv = CreateEvent(NULL, TRUE, FALSE, NULL);
if (!s->hrecv) {
- fprintf(stderr, "Failed CreateEvent\n");
+ error_setg(errp, "Failed CreateEvent");
goto fail;
}
s->hcom = CreateFile(filename, GENERIC_READ|GENERIC_WRITE, 0, NULL,
OPEN_EXISTING, FILE_FLAG_OVERLAPPED, 0);
if (s->hcom == INVALID_HANDLE_VALUE) {
- fprintf(stderr, "Failed CreateFile (%lu)\n", GetLastError());
+ error_setg(errp, "Failed CreateFile (%lu)", GetLastError());
s->hcom = NULL;
goto fail;
}
if (!SetupComm(s->hcom, NRECVBUF, NSENDBUF)) {
- fprintf(stderr, "Failed SetupComm\n");
+ error_setg(errp, "Failed SetupComm");
goto fail;
}
@@ -1926,23 +1926,23 @@ static int win_chr_init(CharDriverState *chr, const char *filename)
CommConfigDialog(filename, NULL, &comcfg);
if (!SetCommState(s->hcom, &comcfg.dcb)) {
- fprintf(stderr, "Failed SetCommState\n");
+ error_setg(errp, "Failed SetCommState");
goto fail;
}
if (!SetCommMask(s->hcom, EV_ERR)) {
- fprintf(stderr, "Failed SetCommMask\n");
+ error_setg(errp, "Failed SetCommMask");
goto fail;
}
cto.ReadIntervalTimeout = MAXDWORD;
if (!SetCommTimeouts(s->hcom, &cto)) {
- fprintf(stderr, "Failed SetCommTimeouts\n");
+ error_setg(errp, "Failed SetCommTimeouts");
goto fail;
}
if (!ClearCommError(s->hcom, &err, &comstat)) {
- fprintf(stderr, "Failed ClearCommError\n");
+ error_setg(errp, "Failed ClearCommError");
goto fail;
}
qemu_add_polling_cb(win_chr_poll, chr);
@@ -2047,7 +2047,8 @@ static int win_chr_poll(void *opaque)
return 0;
}
-static CharDriverState *qemu_chr_open_win_path(const char *filename)
+static CharDriverState *qemu_chr_open_win_path(const char *filename,
+ Error **errp)
{
CharDriverState *chr;
WinCharState *s;
@@ -2058,7 +2059,7 @@ static CharDriverState *qemu_chr_open_win_path(const char *filename)
chr->chr_write = win_chr_write;
chr->chr_close = win_chr_close;
- if (win_chr_init(chr, filename) < 0) {
+ if (win_chr_init(chr, filename, errp) < 0) {
g_free(s);
g_free(chr);
return NULL;
@@ -3465,6 +3466,7 @@ static void qemu_chr_parse_stdio(QemuOpts *opts, ChardevBackend *backend,
backend->stdio->signal = qemu_opt_get_bool(opts, "signal", true);
}
+#ifdef HAVE_CHARDEV_SERIAL
static void qemu_chr_parse_serial(QemuOpts *opts, ChardevBackend *backend,
Error **errp)
{
@@ -3477,6 +3479,7 @@ static void qemu_chr_parse_serial(QemuOpts *opts, ChardevBackend *backend,
backend->serial = g_new0(ChardevHostdev, 1);
backend->serial->device = g_strdup(device);
}
+#endif
static void qemu_chr_parse_parallel(QemuOpts *opts, ChardevBackend *backend,
Error **errp)
@@ -4032,10 +4035,13 @@ static CharDriverState *qmp_chardev_open_file(const char *id,
return qemu_chr_open_win_file(out);
}
-static CharDriverState *qmp_chardev_open_serial(ChardevHostdev *serial,
+static CharDriverState *qmp_chardev_open_serial(const char *id,
+ ChardevBackend *backend,
+ ChardevReturn *ret,
Error **errp)
{
- return qemu_chr_open_win_path(serial->device);
+ ChardevHostdev *serial = backend->serial;
+ return qemu_chr_open_win_path(serial->device, errp);
}
static CharDriverState *qmp_chardev_open_parallel(ChardevHostdev *parallel,
@@ -4086,9 +4092,12 @@ static CharDriverState *qmp_chardev_open_file(const char *id,
}
#ifdef HAVE_CHARDEV_SERIAL
-static CharDriverState *qmp_chardev_open_serial(ChardevHostdev *serial,
+static CharDriverState *qmp_chardev_open_serial(const char *id,
+ ChardevBackend *backend,
+ ChardevReturn *ret,
Error **errp)
{
+ ChardevHostdev *serial = backend->serial;
int fd;
fd = qmp_chardev_open_file_source(serial->device, O_RDWR, errp);
@@ -4098,6 +4107,7 @@ static CharDriverState *qmp_chardev_open_serial(ChardevHostdev *serial,
qemu_set_nonblock(fd);
return qemu_chr_open_tty_fd(fd);
}
+#endif
#ifdef HAVE_CHARDEV_PARPORT
static CharDriverState *qmp_chardev_open_parallel(ChardevHostdev *parallel,
@@ -4252,11 +4262,9 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
case CHARDEV_BACKEND_KIND_FILE:
abort();
break;
-#ifdef HAVE_CHARDEV_SERIAL
case CHARDEV_BACKEND_KIND_SERIAL:
- chr = qmp_chardev_open_serial(backend->serial, &local_err);
+ abort();
break;
-#endif
#ifdef HAVE_CHARDEV_PARPORT
case CHARDEV_BACKEND_KIND_PARALLEL:
chr = qmp_chardev_open_parallel(backend->parallel, &local_err);
@@ -4391,10 +4399,12 @@ static void register_types(void)
qemu_chr_parse_file_out, qmp_chardev_open_file);
register_char_driver("stdio", CHARDEV_BACKEND_KIND_STDIO,
qemu_chr_parse_stdio, NULL);
+#if defined HAVE_CHARDEV_SERIAL
register_char_driver("serial", CHARDEV_BACKEND_KIND_SERIAL,
- qemu_chr_parse_serial, NULL);
+ qemu_chr_parse_serial, qmp_chardev_open_serial);
register_char_driver("tty", CHARDEV_BACKEND_KIND_SERIAL,
- qemu_chr_parse_serial, NULL);
+ qemu_chr_parse_serial, qmp_chardev_open_serial);
+#endif
register_char_driver("parallel", CHARDEV_BACKEND_KIND_PARALLEL,
qemu_chr_parse_parallel, NULL);
register_char_driver("parport", CHARDEV_BACKEND_KIND_PARALLEL,
--
2.5.0
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH 05/21] qemu-char: convert serial backend to data-driven creation
2015-10-12 8:03 ` [Qemu-devel] [PATCH 05/21] qemu-char: convert serial " Paolo Bonzini
@ 2015-10-12 15:09 ` Eric Blake
0 siblings, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:09 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: armbru
[-- Attachment #1: Type: text/plain, Size: 834 bytes --]
On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> qemu-char.c | 48 +++++++++++++++++++++++++++++-------------------
> 1 file changed, 29 insertions(+), 19 deletions(-)
>
> @@ -4252,11 +4262,9 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
> case CHARDEV_BACKEND_KIND_FILE:
> abort();
> break;
> -#ifdef HAVE_CHARDEV_SERIAL
> case CHARDEV_BACKEND_KIND_SERIAL:
> - chr = qmp_chardev_open_serial(backend->serial, &local_err);
> + abort();
> break;
> -#endif
Another dead break after abort().
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* [Qemu-devel] [PATCH 06/21] qemu-char: convert parallel backend to data-driven creation
2015-10-12 8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
` (4 preceding siblings ...)
2015-10-12 8:03 ` [Qemu-devel] [PATCH 05/21] qemu-char: convert serial " Paolo Bonzini
@ 2015-10-12 8:03 ` Paolo Bonzini
2015-10-12 15:13 ` Eric Blake
2015-10-12 8:03 ` [Qemu-devel] [PATCH 07/21] qemu-char: convert pipe " Paolo Bonzini
` (14 subsequent siblings)
20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12 8:03 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru
Conversion to Error * brings better error messages; before:
qemu-system-x86_64: -chardev id=serial,backend=parallel,path=vl.c: Failed to create chardev
After:
qemu-system-x86_64: -chardev id=serial,backend=parallel,path=vl.c: not a parallel port: Inappropriate ioctl for device
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu-char.c | 28 ++++++++++++++++++----------
1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 8567580..84cb8d0 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1753,13 +1753,14 @@ static void pp_close(CharDriverState *chr)
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}
-static CharDriverState *qemu_chr_open_pp_fd(int fd)
+static CharDriverState *qemu_chr_open_pp_fd(int fd, Error **errp)
{
CharDriverState *chr;
ParallelCharDriver *drv;
if (ioctl(fd, PPCLAIM) < 0) {
close(fd);
+ error_setg_errno(errp, errno, "not a parallel port");
return NULL;
}
@@ -1818,7 +1819,7 @@ static int pp_ioctl(CharDriverState *chr, int cmd, void *arg)
return 0;
}
-static CharDriverState *qemu_chr_open_pp_fd(int fd)
+static CharDriverState *qemu_chr_open_pp_fd(int fd, Error **errp)
{
CharDriverState *chr;
@@ -3481,6 +3482,7 @@ static void qemu_chr_parse_serial(QemuOpts *opts, ChardevBackend *backend,
}
#endif
+#ifdef HAVE_CHARDEV_PARPORT
static void qemu_chr_parse_parallel(QemuOpts *opts, ChardevBackend *backend,
Error **errp)
{
@@ -3493,6 +3495,7 @@ static void qemu_chr_parse_parallel(QemuOpts *opts, ChardevBackend *backend,
backend->parallel = g_new0(ChardevHostdev, 1);
backend->parallel->device = g_strdup(device);
}
+#endif
static void qemu_chr_parse_pipe(QemuOpts *opts, ChardevBackend *backend,
Error **errp)
@@ -4044,7 +4047,9 @@ static CharDriverState *qmp_chardev_open_serial(const char *id,
return qemu_chr_open_win_path(serial->device, errp);
}
-static CharDriverState *qmp_chardev_open_parallel(ChardevHostdev *parallel,
+static CharDriverState *qmp_chardev_open_parallel(const char *id,
+ ChardevBackend *backend,
+ ChardevReturn *ret,
Error **errp)
{
error_setg(errp, "character device backend type 'parallel' not supported");
@@ -4110,16 +4115,19 @@ static CharDriverState *qmp_chardev_open_serial(const char *id,
#endif
#ifdef HAVE_CHARDEV_PARPORT
-static CharDriverState *qmp_chardev_open_parallel(ChardevHostdev *parallel,
+static CharDriverState *qmp_chardev_open_parallel(const char *id,
+ ChardevBackend *backend,
+ ChardevReturn *ret,
Error **errp)
{
+ ChardevHostdev *parallel = backend->parallel;
int fd;
fd = qmp_chardev_open_file_source(parallel->device, O_RDWR, errp);
if (fd < 0) {
return NULL;
}
- return qemu_chr_open_pp_fd(fd);
+ return qemu_chr_open_pp_fd(fd, errp);
}
#endif
@@ -4265,11 +4273,9 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
case CHARDEV_BACKEND_KIND_SERIAL:
abort();
break;
-#ifdef HAVE_CHARDEV_PARPORT
case CHARDEV_BACKEND_KIND_PARALLEL:
- chr = qmp_chardev_open_parallel(backend->parallel, &local_err);
+ abort();
break;
-#endif
case CHARDEV_BACKEND_KIND_PIPE:
chr = qemu_chr_open_pipe(backend->pipe);
break;
@@ -4405,10 +4411,12 @@ static void register_types(void)
register_char_driver("tty", CHARDEV_BACKEND_KIND_SERIAL,
qemu_chr_parse_serial, qmp_chardev_open_serial);
#endif
+#ifdef HAVE_CHARDEV_PARPORT
register_char_driver("parallel", CHARDEV_BACKEND_KIND_PARALLEL,
- qemu_chr_parse_parallel, NULL);
+ qemu_chr_parse_parallel, qmp_chardev_open_parallel);
register_char_driver("parport", CHARDEV_BACKEND_KIND_PARALLEL,
- qemu_chr_parse_parallel, NULL);
+ qemu_chr_parse_parallel, qmp_chardev_open_parallel);
+#endif
register_char_driver("pty", CHARDEV_BACKEND_KIND_PTY, NULL,
NULL);
register_char_driver("console", CHARDEV_BACKEND_KIND_CONSOLE, NULL,
--
2.5.0
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH 06/21] qemu-char: convert parallel backend to data-driven creation
2015-10-12 8:03 ` [Qemu-devel] [PATCH 06/21] qemu-char: convert parallel " Paolo Bonzini
@ 2015-10-12 15:13 ` Eric Blake
0 siblings, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:13 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: armbru
[-- Attachment #1: Type: text/plain, Size: 1489 bytes --]
On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Conversion to Error * brings better error messages; before:
>
> qemu-system-x86_64: -chardev id=serial,backend=parallel,path=vl.c: Failed to create chardev
>
> After:
>
> qemu-system-x86_64: -chardev id=serial,backend=parallel,path=vl.c: not a parallel port: Inappropriate ioctl for device
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> qemu-char.c | 28 ++++++++++++++++++----------
> 1 file changed, 18 insertions(+), 10 deletions(-)
>
> +static CharDriverState *qemu_chr_open_pp_fd(int fd, Error **errp)
> {
> CharDriverState *chr;
> ParallelCharDriver *drv;
>
> if (ioctl(fd, PPCLAIM) < 0) {
> close(fd);
> + error_setg_errno(errp, errno, "not a parallel port");
Swap this line with the close(), since close() can clobber errno.
> @@ -4265,11 +4273,9 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
> case CHARDEV_BACKEND_KIND_SERIAL:
> abort();
> break;
> -#ifdef HAVE_CHARDEV_PARPORT
> case CHARDEV_BACKEND_KIND_PARALLEL:
> - chr = qmp_chardev_open_parallel(backend->parallel, &local_err);
> + abort();
> break;
Another dead break.
With the error reporting fixed,
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* [Qemu-devel] [PATCH 07/21] qemu-char: convert pipe backend to data-driven creation
2015-10-12 8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
` (5 preceding siblings ...)
2015-10-12 8:03 ` [Qemu-devel] [PATCH 06/21] qemu-char: convert parallel " Paolo Bonzini
@ 2015-10-12 8:03 ` Paolo Bonzini
2015-10-12 15:16 ` Eric Blake
2015-10-12 8:03 ` [Qemu-devel] [PATCH 08/21] qemu-char: convert socket " Paolo Bonzini
` (13 subsequent siblings)
20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12 8:03 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu-char.c | 37 +++++++++++++++++++++----------------
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 84cb8d0..3545cd8 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1078,18 +1078,17 @@ static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out)
return chr;
}
-static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
+static CharDriverState *qemu_chr_open_pipe(const char *id,
+ ChardevBackend *backend,
+ ChardevReturn *ret,
+ Error **errp)
{
+ ChardevHostdev *opts = backend->pipe;
int fd_in, fd_out;
char filename_in[CHR_MAX_FILENAME_SIZE];
char filename_out[CHR_MAX_FILENAME_SIZE];
const char *filename = opts->device;
- if (filename == NULL) {
- fprintf(stderr, "chardev: pipe: no filename given\n");
- return NULL;
- }
-
snprintf(filename_in, CHR_MAX_FILENAME_SIZE, "%s.in", filename);
snprintf(filename_out, CHR_MAX_FILENAME_SIZE, "%s.out", filename);
TFR(fd_in = qemu_open(filename_in, O_RDWR | O_BINARY));
@@ -1101,6 +1100,7 @@ static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
close(fd_out);
TFR(fd_in = fd_out = qemu_open(filename, O_RDWR | O_BINARY));
if (fd_in < 0) {
+ error_setg_file_open(errp, errno, filename);
return NULL;
}
}
@@ -2084,7 +2084,8 @@ static int win_chr_pipe_poll(void *opaque)
return 0;
}
-static int win_chr_pipe_init(CharDriverState *chr, const char *filename)
+static int win_chr_pipe_init(CharDriverState *chr, const char *filename,
+ Error **errp)
{
WinCharState *s = chr->opaque;
OVERLAPPED ov;
@@ -2096,12 +2097,12 @@ static int win_chr_pipe_init(CharDriverState *chr, const char *filename)
s->hsend = CreateEvent(NULL, TRUE, FALSE, NULL);
if (!s->hsend) {
- fprintf(stderr, "Failed CreateEvent\n");
+ error_setg(errp, "Failed CreateEvent\n");
goto fail;
}
s->hrecv = CreateEvent(NULL, TRUE, FALSE, NULL);
if (!s->hrecv) {
- fprintf(stderr, "Failed CreateEvent\n");
+ error_setg(errp, "Failed CreateEvent\n");
goto fail;
}
@@ -2111,7 +2112,7 @@ static int win_chr_pipe_init(CharDriverState *chr, const char *filename)
PIPE_WAIT,
MAXCONNECT, NSENDBUF, NRECVBUF, NTIMEOUT, NULL);
if (s->hcom == INVALID_HANDLE_VALUE) {
- fprintf(stderr, "Failed CreateNamedPipe (%lu)\n", GetLastError());
+ error_setg(errp, "Failed CreateNamedPipe (%lu)\n", GetLastError());
s->hcom = NULL;
goto fail;
}
@@ -2120,13 +2121,13 @@ static int win_chr_pipe_init(CharDriverState *chr, const char *filename)
ov.hEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
ret = ConnectNamedPipe(s->hcom, &ov);
if (ret) {
- fprintf(stderr, "Failed ConnectNamedPipe\n");
+ error_setg(errp, "Failed ConnectNamedPipe\n");
goto fail;
}
ret = GetOverlappedResult(s->hcom, &ov, &size, TRUE);
if (!ret) {
- fprintf(stderr, "Failed GetOverlappedResult\n");
+ error_setg(errp, "Failed GetOverlappedResult\n");
if (ov.hEvent) {
CloseHandle(ov.hEvent);
ov.hEvent = NULL;
@@ -2147,8 +2148,12 @@ static int win_chr_pipe_init(CharDriverState *chr, const char *filename)
}
-static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
+static CharDriverState *qemu_chr_open_pipe(const char *id,
+ ChardevBackend *backend,
+ ChardevReturn *ret,
+ Error **errp)
{
+ ChardevHostdev *opts = backend->pipe;
const char *filename = opts->device;
CharDriverState *chr;
WinCharState *s;
@@ -2159,7 +2164,7 @@ static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
chr->chr_write = win_chr_write;
chr->chr_close = win_chr_close;
- if (win_chr_pipe_init(chr, filename) < 0) {
+ if (win_chr_pipe_init(chr, filename, errp) < 0) {
g_free(s);
g_free(chr);
return NULL;
@@ -4277,7 +4282,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
abort();
break;
case CHARDEV_BACKEND_KIND_PIPE:
- chr = qemu_chr_open_pipe(backend->pipe);
+ abort();
break;
case CHARDEV_BACKEND_KIND_SOCKET:
chr = qmp_chardev_open_socket(backend->socket, &local_err);
@@ -4422,7 +4427,7 @@ static void register_types(void)
register_char_driver("console", CHARDEV_BACKEND_KIND_CONSOLE, NULL,
NULL);
register_char_driver("pipe", CHARDEV_BACKEND_KIND_PIPE,
- qemu_chr_parse_pipe, NULL);
+ qemu_chr_parse_pipe, qemu_chr_open_pipe);
register_char_driver("mux", CHARDEV_BACKEND_KIND_MUX, qemu_chr_parse_mux,
NULL);
/* Bug-compatibility: */
--
2.5.0
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH 07/21] qemu-char: convert pipe backend to data-driven creation
2015-10-12 8:03 ` [Qemu-devel] [PATCH 07/21] qemu-char: convert pipe " Paolo Bonzini
@ 2015-10-12 15:16 ` Eric Blake
2015-10-12 15:18 ` Paolo Bonzini
0 siblings, 1 reply; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:16 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: armbru
[-- Attachment #1: Type: text/plain, Size: 2301 bytes --]
On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> qemu-char.c | 37 +++++++++++++++++++++----------------
> 1 file changed, 21 insertions(+), 16 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index 84cb8d0..3545cd8 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -1078,18 +1078,17 @@ static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out)
> return chr;
> }
>
> -static CharDriverState *qemu_chr_open_pipe(ChardevHostdev *opts)
> +static CharDriverState *qemu_chr_open_pipe(const char *id,
> + ChardevBackend *backend,
> + ChardevReturn *ret,
> + Error **errp)
> {
> + ChardevHostdev *opts = backend->pipe;
> int fd_in, fd_out;
> char filename_in[CHR_MAX_FILENAME_SIZE];
> char filename_out[CHR_MAX_FILENAME_SIZE];
> const char *filename = opts->device;
>
> - if (filename == NULL) {
> - fprintf(stderr, "chardev: pipe: no filename given\n");
> - return NULL;
> - }
> -
> snprintf(filename_in, CHR_MAX_FILENAME_SIZE, "%s.in", filename);
Do we need assert(filename) here?
> @@ -2096,12 +2097,12 @@ static int win_chr_pipe_init(CharDriverState *chr, const char *filename)
>
> s->hsend = CreateEvent(NULL, TRUE, FALSE, NULL);
> if (!s->hsend) {
> - fprintf(stderr, "Failed CreateEvent\n");
> + error_setg(errp, "Failed CreateEvent\n");
> goto fail;
> }
> s->hrecv = CreateEvent(NULL, TRUE, FALSE, NULL);
> if (!s->hrecv) {
> - fprintf(stderr, "Failed CreateEvent\n");
> + error_setg(errp, "Failed CreateEvent\n");
> goto fail;
Drop the trailing \n, here and throughout this file
> @@ -4277,7 +4282,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
> abort();
> break;
> case CHARDEV_BACKEND_KIND_PIPE:
> - chr = qemu_chr_open_pipe(backend->pipe);
> + abort();
> break;
another dead break
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH 07/21] qemu-char: convert pipe backend to data-driven creation
2015-10-12 15:16 ` Eric Blake
@ 2015-10-12 15:18 ` Paolo Bonzini
2015-10-12 15:21 ` Eric Blake
0 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12 15:18 UTC (permalink / raw)
To: Eric Blake, qemu-devel; +Cc: armbru
On 12/10/2015 17:16, Eric Blake wrote:
>> rp)
>> > {
>> > + ChardevHostdev *opts = backend->pipe;
>> > int fd_in, fd_out;
>> > char filename_in[CHR_MAX_FILENAME_SIZE];
>> > char filename_out[CHR_MAX_FILENAME_SIZE];
>> > const char *filename = opts->device;
>> >
>> > - if (filename == NULL) {
>> > - fprintf(stderr, "chardev: pipe: no filename given\n");
>> > - return NULL;
>> > - }
>> > -
>> > snprintf(filename_in, CHR_MAX_FILENAME_SIZE, "%s.in", filename);
> Do we need assert(filename) here?
>
>
No, "device" is not optional in the definition of ChardevHostdev.
Paolo
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH 07/21] qemu-char: convert pipe backend to data-driven creation
2015-10-12 15:18 ` Paolo Bonzini
@ 2015-10-12 15:21 ` Eric Blake
0 siblings, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:21 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: armbru
[-- Attachment #1: Type: text/plain, Size: 939 bytes --]
On 10/12/2015 09:18 AM, Paolo Bonzini wrote:
>
>
> On 12/10/2015 17:16, Eric Blake wrote:
>>> rp)
>>>> {
>>>> + ChardevHostdev *opts = backend->pipe;
>>>> int fd_in, fd_out;
>>>> char filename_in[CHR_MAX_FILENAME_SIZE];
>>>> char filename_out[CHR_MAX_FILENAME_SIZE];
>>>> const char *filename = opts->device;
>>>>
>>>> - if (filename == NULL) {
>>>> - fprintf(stderr, "chardev: pipe: no filename given\n");
>>>> - return NULL;
>>>> - }
>>>> -
>>>> snprintf(filename_in, CHR_MAX_FILENAME_SIZE, "%s.in", filename);
>> Do we need assert(filename) here?
>>
>>
>
> No, "device" is not optional in the definition of ChardevHostdev.
Okay, then with the \n gone,
Reviewed-by: Eric Blake <eblake@redhat.com>
(and I'll quit complaining about dead break)
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* [Qemu-devel] [PATCH 08/21] qemu-char: convert socket backend to data-driven creation
2015-10-12 8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
` (6 preceding siblings ...)
2015-10-12 8:03 ` [Qemu-devel] [PATCH 07/21] qemu-char: convert pipe " Paolo Bonzini
@ 2015-10-12 8:03 ` Paolo Bonzini
2015-10-12 15:20 ` Eric Blake
2015-10-12 8:03 ` [Qemu-devel] [PATCH 09/21] qemu-char: convert UDP " Paolo Bonzini
` (12 subsequent siblings)
20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12 8:03 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu-char.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 3545cd8..459ed5c 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4163,11 +4163,14 @@ static gboolean socket_reconnect_timeout(gpointer opaque)
return false;
}
-static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock,
+static CharDriverState *qmp_chardev_open_socket(const char *id,
+ ChardevBackend *backend,
+ ChardevReturn *ret,
Error **errp)
{
CharDriverState *chr;
TCPCharDriver *s;
+ ChardevSocket *sock = backend->socket;
SocketAddress *addr = sock->addr;
bool do_nodelay = sock->has_nodelay ? sock->nodelay : false;
bool is_listen = sock->has_server ? sock->server : true;
@@ -4285,7 +4288,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
abort();
break;
case CHARDEV_BACKEND_KIND_SOCKET:
- chr = qmp_chardev_open_socket(backend->socket, &local_err);
+ abort();
break;
case CHARDEV_BACKEND_KIND_UDP:
chr = qmp_chardev_open_udp(backend->udp, &local_err);
@@ -4401,7 +4404,7 @@ static void register_types(void)
register_char_driver("null", CHARDEV_BACKEND_KIND_NULL, NULL,
NULL);
register_char_driver("socket", CHARDEV_BACKEND_KIND_SOCKET,
- qemu_chr_parse_socket, NULL);
+ qemu_chr_parse_socket, qmp_chardev_open_socket);
register_char_driver("udp", CHARDEV_BACKEND_KIND_UDP, qemu_chr_parse_udp,
NULL);
register_char_driver("ringbuf", CHARDEV_BACKEND_KIND_RINGBUF,
--
2.5.0
^ permalink raw reply related [flat|nested] 51+ messages in thread* [Qemu-devel] [PATCH 09/21] qemu-char: convert UDP backend to data-driven creation
2015-10-12 8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
` (7 preceding siblings ...)
2015-10-12 8:03 ` [Qemu-devel] [PATCH 08/21] qemu-char: convert socket " Paolo Bonzini
@ 2015-10-12 8:03 ` Paolo Bonzini
2015-10-12 15:22 ` Eric Blake
2015-10-12 8:03 ` [Qemu-devel] [PATCH 10/21] qemu-char: convert pty " Paolo Bonzini
` (11 subsequent siblings)
20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12 8:03 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu-char.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 459ed5c..79c0c05 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4232,9 +4232,12 @@ static CharDriverState *qmp_chardev_open_socket(const char *id,
return chr;
}
-static CharDriverState *qmp_chardev_open_udp(ChardevUdp *udp,
+static CharDriverState *qmp_chardev_open_udp(const char *id,
+ ChardevBackend *backend,
+ ChardevReturn *ret,
Error **errp)
{
+ ChardevUdp *udp = backend->udp;
int fd;
fd = socket_dgram(udp->remote, udp->local, errp);
@@ -4291,7 +4294,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
abort();
break;
case CHARDEV_BACKEND_KIND_UDP:
- chr = qmp_chardev_open_udp(backend->udp, &local_err);
+ abort();
break;
#ifdef HAVE_CHARDEV_PTY
case CHARDEV_BACKEND_KIND_PTY:
@@ -4406,7 +4409,7 @@ static void register_types(void)
register_char_driver("socket", CHARDEV_BACKEND_KIND_SOCKET,
qemu_chr_parse_socket, qmp_chardev_open_socket);
register_char_driver("udp", CHARDEV_BACKEND_KIND_UDP, qemu_chr_parse_udp,
- NULL);
+ qmp_chardev_open_udp);
register_char_driver("ringbuf", CHARDEV_BACKEND_KIND_RINGBUF,
qemu_chr_parse_ringbuf, NULL);
register_char_driver("file", CHARDEV_BACKEND_KIND_FILE,
--
2.5.0
^ permalink raw reply related [flat|nested] 51+ messages in thread* [Qemu-devel] [PATCH 10/21] qemu-char: convert pty backend to data-driven creation
2015-10-12 8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
` (8 preceding siblings ...)
2015-10-12 8:03 ` [Qemu-devel] [PATCH 09/21] qemu-char: convert UDP " Paolo Bonzini
@ 2015-10-12 8:03 ` Paolo Bonzini
2015-10-12 15:22 ` Eric Blake
2015-10-12 8:03 ` [Qemu-devel] [PATCH 11/21] qemu-char: convert null " Paolo Bonzini
` (10 subsequent siblings)
20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12 8:03 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu-char.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 79c0c05..c36cbf0 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1390,7 +1390,9 @@ static void pty_chr_close(struct CharDriverState *chr)
}
static CharDriverState *qemu_chr_open_pty(const char *id,
- ChardevReturn *ret)
+ ChardevBackend *backend,
+ ChardevReturn *ret,
+ Error **errp)
{
CharDriverState *chr;
PtyCharDriver *s;
@@ -1399,6 +1401,7 @@ static CharDriverState *qemu_chr_open_pty(const char *id,
master_fd = qemu_openpty_raw(&slave_fd, pty_name);
if (master_fd < 0) {
+ error_setg_errno(errp, errno, "Failed to create PTY");
return NULL;
}
@@ -4296,11 +4299,9 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
case CHARDEV_BACKEND_KIND_UDP:
abort();
break;
-#ifdef HAVE_CHARDEV_PTY
case CHARDEV_BACKEND_KIND_PTY:
- chr = qemu_chr_open_pty(id, ret);
+ abort();
break;
-#endif
case CHARDEV_BACKEND_KIND_NULL:
chr = qemu_chr_open_null();
break;
@@ -4428,8 +4429,10 @@ static void register_types(void)
register_char_driver("parport", CHARDEV_BACKEND_KIND_PARALLEL,
qemu_chr_parse_parallel, qmp_chardev_open_parallel);
#endif
+#ifdef HAVE_CHARDEV_PTY
register_char_driver("pty", CHARDEV_BACKEND_KIND_PTY, NULL,
- NULL);
+ qemu_chr_open_pty);
+#endif
register_char_driver("console", CHARDEV_BACKEND_KIND_CONSOLE, NULL,
NULL);
register_char_driver("pipe", CHARDEV_BACKEND_KIND_PIPE,
--
2.5.0
^ permalink raw reply related [flat|nested] 51+ messages in thread* [Qemu-devel] [PATCH 11/21] qemu-char: convert null backend to data-driven creation
2015-10-12 8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
` (9 preceding siblings ...)
2015-10-12 8:03 ` [Qemu-devel] [PATCH 10/21] qemu-char: convert pty " Paolo Bonzini
@ 2015-10-12 8:03 ` Paolo Bonzini
2015-10-12 15:23 ` Eric Blake
2015-10-12 8:03 ` [Qemu-devel] [PATCH 12/21] qemu-char: convert mux " Paolo Bonzini
` (9 subsequent siblings)
20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12 8:03 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu-char.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index c36cbf0..3e48a47 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -383,7 +383,10 @@ static int null_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
return len;
}
-static CharDriverState *qemu_chr_open_null(void)
+static CharDriverState *qemu_chr_open_null(const char *id,
+ ChardevBackend *backend,
+ ChardevReturn *ret,
+ Error **errp)
{
CharDriverState *chr;
@@ -4303,7 +4306,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
abort();
break;
case CHARDEV_BACKEND_KIND_NULL:
- chr = qemu_chr_open_null();
+ abort();
break;
case CHARDEV_BACKEND_KIND_MUX:
base = qemu_chr_find(backend->mux->chardev);
@@ -4406,7 +4409,7 @@ void qmp_chardev_remove(const char *id, Error **errp)
static void register_types(void)
{
register_char_driver("null", CHARDEV_BACKEND_KIND_NULL, NULL,
- NULL);
+ qemu_chr_open_null);
register_char_driver("socket", CHARDEV_BACKEND_KIND_SOCKET,
qemu_chr_parse_socket, qmp_chardev_open_socket);
register_char_driver("udp", CHARDEV_BACKEND_KIND_UDP, qemu_chr_parse_udp,
--
2.5.0
^ permalink raw reply related [flat|nested] 51+ messages in thread* [Qemu-devel] [PATCH 12/21] qemu-char: convert mux backend to data-driven creation
2015-10-12 8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
` (10 preceding siblings ...)
2015-10-12 8:03 ` [Qemu-devel] [PATCH 11/21] qemu-char: convert null " Paolo Bonzini
@ 2015-10-12 8:03 ` Paolo Bonzini
2015-10-12 15:24 ` Eric Blake
2015-10-12 8:03 ` [Qemu-devel] [PATCH 13/21] qemu-char: convert msmouse " Paolo Bonzini
` (8 subsequent siblings)
20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12 8:03 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu-char.c | 25 ++++++++++++++-----------
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 3e48a47..8e3a79a 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -682,11 +682,20 @@ static GSource *mux_chr_add_watch(CharDriverState *s, GIOCondition cond)
return d->drv->chr_add_watch(d->drv, cond);
}
-static CharDriverState *qemu_chr_open_mux(CharDriverState *drv)
+static CharDriverState *qemu_chr_open_mux(const char *id,
+ ChardevBackend *backend,
+ ChardevReturn *ret, Error **errp)
{
- CharDriverState *chr;
+ ChardevMux *mux = backend->mux;
+ CharDriverState *chr, *drv;
MuxDriver *d;
+ drv = qemu_chr_find(mux->chardev);
+ if (drv == NULL) {
+ error_setg(errp, "mux: base chardev %s not found", mux->chardev);
+ return NULL;
+ }
+
chr = qemu_chr_alloc();
d = g_new0(MuxDriver, 1);
@@ -4257,7 +4266,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
Error **errp)
{
ChardevReturn *ret = g_new0(ChardevReturn, 1);
- CharDriverState *base, *chr = NULL;
+ CharDriverState *chr = NULL;
Error *local_err = NULL;
GSList *i;
CharDriver *cd;
@@ -4309,13 +4318,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
abort();
break;
case CHARDEV_BACKEND_KIND_MUX:
- base = qemu_chr_find(backend->mux->chardev);
- if (base == NULL) {
- error_setg(&local_err, "mux: base chardev %s not found",
- backend->mux->chardev);
- break;
- }
- chr = qemu_chr_open_mux(base);
+ abort();
break;
case CHARDEV_BACKEND_KIND_MSMOUSE:
chr = qemu_chr_open_msmouse();
@@ -4441,7 +4444,7 @@ static void register_types(void)
register_char_driver("pipe", CHARDEV_BACKEND_KIND_PIPE,
qemu_chr_parse_pipe, qemu_chr_open_pipe);
register_char_driver("mux", CHARDEV_BACKEND_KIND_MUX, qemu_chr_parse_mux,
- NULL);
+ qemu_chr_open_mux);
/* Bug-compatibility: */
register_char_driver("memory", CHARDEV_BACKEND_KIND_MEMORY,
qemu_chr_parse_ringbuf, NULL);
--
2.5.0
^ permalink raw reply related [flat|nested] 51+ messages in thread* [Qemu-devel] [PATCH 13/21] qemu-char: convert msmouse backend to data-driven creation
2015-10-12 8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
` (11 preceding siblings ...)
2015-10-12 8:03 ` [Qemu-devel] [PATCH 12/21] qemu-char: convert mux " Paolo Bonzini
@ 2015-10-12 8:03 ` Paolo Bonzini
2015-10-12 15:25 ` Eric Blake
2015-10-12 8:03 ` [Qemu-devel] [PATCH 14/21] qemu-char: convert braille " Paolo Bonzini
` (7 subsequent siblings)
20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12 8:03 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
backends/msmouse.c | 7 +++++--
include/sysemu/char.h | 3 ---
qemu-char.c | 2 +-
stubs/Makefile.objs | 1 -
stubs/chr-msmouse.c | 7 -------
5 files changed, 6 insertions(+), 14 deletions(-)
delete mode 100644 stubs/chr-msmouse.c
diff --git a/backends/msmouse.c b/backends/msmouse.c
index d50ed47..0126fa0 100644
--- a/backends/msmouse.c
+++ b/backends/msmouse.c
@@ -63,7 +63,10 @@ static void msmouse_chr_close (struct CharDriverState *chr)
g_free (chr);
}
-CharDriverState *qemu_chr_open_msmouse(void)
+static CharDriverState *qemu_chr_open_msmouse(const char *id,
+ ChardevBackend *backend,
+ ChardevReturn *ret,
+ Error **errp)
{
CharDriverState *chr;
@@ -80,7 +83,7 @@ CharDriverState *qemu_chr_open_msmouse(void)
static void register_types(void)
{
register_char_driver("msmouse", CHARDEV_BACKEND_KIND_MSMOUSE, NULL,
- NULL);
+ qemu_chr_open_msmouse);
}
type_init(register_types);
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 4b01a8c..2fe8275 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -356,9 +356,6 @@ extern int term_escape_char;
CharDriverState *qemu_char_get_next_serial(void);
-/* msmouse */
-CharDriverState *qemu_chr_open_msmouse(void);
-
/* testdev.c */
CharDriverState *chr_testdev_init(void);
diff --git a/qemu-char.c b/qemu-char.c
index 8e3a79a..8425891 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4321,7 +4321,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
abort();
break;
case CHARDEV_BACKEND_KIND_MSMOUSE:
- chr = qemu_chr_open_msmouse();
+ abort();
break;
#ifdef CONFIG_BRLAPI
case CHARDEV_BACKEND_KIND_BRAILLE:
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 85e4e81..63988ca 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -1,7 +1,6 @@
stub-obj-y += arch-query-cpu-def.o
stub-obj-y += bdrv-commit-all.o
stub-obj-y += chr-baum-init.o
-stub-obj-y += chr-msmouse.o
stub-obj-y += chr-testdev.o
stub-obj-y += clock-warp.o
stub-obj-y += cpu-get-clock.o
diff --git a/stubs/chr-msmouse.c b/stubs/chr-msmouse.c
deleted file mode 100644
index 812f8b0..0000000
--- a/stubs/chr-msmouse.c
+++ /dev/null
@@ -1,7 +0,0 @@
-#include "qemu-common.h"
-#include "sysemu/char.h"
-
-CharDriverState *qemu_chr_open_msmouse(void)
-{
- return 0;
-}
--
2.5.0
^ permalink raw reply related [flat|nested] 51+ messages in thread* [Qemu-devel] [PATCH 14/21] qemu-char: convert braille backend to data-driven creation
2015-10-12 8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
` (12 preceding siblings ...)
2015-10-12 8:03 ` [Qemu-devel] [PATCH 13/21] qemu-char: convert msmouse " Paolo Bonzini
@ 2015-10-12 8:03 ` Paolo Bonzini
2015-10-12 15:30 ` Eric Blake
2015-10-12 8:03 ` [Qemu-devel] [PATCH 15/21] qemu-char: convert testdev " Paolo Bonzini
` (6 subsequent siblings)
20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12 8:03 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
backends/baum.c | 13 ++++++++-----
include/sysemu/char.h | 3 ---
qemu-char.c | 4 +---
stubs/Makefile.objs | 1 -
stubs/chr-baum-init.c | 7 -------
5 files changed, 9 insertions(+), 19 deletions(-)
delete mode 100644 stubs/chr-baum-init.c
diff --git a/backends/baum.c b/backends/baum.c
index e86a019..281be30 100644
--- a/backends/baum.c
+++ b/backends/baum.c
@@ -561,7 +561,10 @@ static void baum_close(struct CharDriverState *chr)
g_free(baum);
}
-CharDriverState *chr_baum_init(void)
+static CharDriverState *chr_baum_init(const char *id,
+ ChardevBackend *backend,
+ ChardevReturn *ret,
+ Error **errp)
{
BaumDriverState *baum;
CharDriverState *chr;
@@ -586,14 +589,14 @@ CharDriverState *chr_baum_init(void)
baum->brlapi_fd = brlapi__openConnection(handle, NULL, NULL);
if (baum->brlapi_fd == -1) {
- brlapi_perror("baum_init: brlapi_openConnection");
+ error_setg(errp, "baum_init: brlapi_openConnection");
goto fail_handle;
}
baum->cellCount_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, baum_cellCount_timer_cb, baum);
if (brlapi__getDisplaySize(handle, &baum->x, &baum->y) == -1) {
- brlapi_perror("baum_init: brlapi_getDisplaySize");
+ error_setg(errp, "baum_init: brlapi_getDisplaySize");
goto fail;
}
@@ -609,7 +612,7 @@ CharDriverState *chr_baum_init(void)
tty = BRLAPI_TTY_DEFAULT;
if (brlapi__enterTtyMode(handle, tty, NULL) == -1) {
- brlapi_perror("baum_init: brlapi_enterTtyMode");
+ error_setg(errp, "baum_init: brlapi_enterTtyMode");
goto fail;
}
@@ -630,7 +633,7 @@ fail_handle:
static void register_types(void)
{
register_char_driver("braille", CHARDEV_BACKEND_KIND_BRAILLE, NULL,
- NULL);
+ chr_baum_init);
}
type_init(register_types);
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 2fe8275..77415ec 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -359,9 +359,6 @@ CharDriverState *qemu_char_get_next_serial(void);
/* testdev.c */
CharDriverState *chr_testdev_init(void);
-/* baum.c */
-CharDriverState *chr_baum_init(void);
-
/* console.c */
typedef CharDriverState *(VcHandler)(ChardevVC *vc);
diff --git a/qemu-char.c b/qemu-char.c
index 8425891..96f40f3 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4323,11 +4323,9 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
case CHARDEV_BACKEND_KIND_MSMOUSE:
abort();
break;
-#ifdef CONFIG_BRLAPI
case CHARDEV_BACKEND_KIND_BRAILLE:
- chr = chr_baum_init();
+ abort();
break;
-#endif
case CHARDEV_BACKEND_KIND_TESTDEV:
chr = chr_testdev_init();
break;
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 63988ca..8cfa5a2 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -1,6 +1,5 @@
stub-obj-y += arch-query-cpu-def.o
stub-obj-y += bdrv-commit-all.o
-stub-obj-y += chr-baum-init.o
stub-obj-y += chr-testdev.o
stub-obj-y += clock-warp.o
stub-obj-y += cpu-get-clock.o
diff --git a/stubs/chr-baum-init.c b/stubs/chr-baum-init.c
deleted file mode 100644
index f5cc6ce..0000000
--- a/stubs/chr-baum-init.c
+++ /dev/null
@@ -1,7 +0,0 @@
-#include "qemu-common.h"
-#include "sysemu/char.h"
-
-CharDriverState *chr_baum_init(void)
-{
- return NULL;
-}
--
2.5.0
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH 14/21] qemu-char: convert braille backend to data-driven creation
2015-10-12 8:03 ` [Qemu-devel] [PATCH 14/21] qemu-char: convert braille " Paolo Bonzini
@ 2015-10-12 15:30 ` Eric Blake
2015-10-12 15:41 ` Samuel Thibault
0 siblings, 1 reply; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:30 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: armbru
[-- Attachment #1: Type: text/plain, Size: 1462 bytes --]
On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> backends/baum.c | 13 ++++++++-----
> include/sysemu/char.h | 3 ---
> qemu-char.c | 4 +---
> stubs/Makefile.objs | 1 -
> stubs/chr-baum-init.c | 7 -------
> 5 files changed, 9 insertions(+), 19 deletions(-)
> delete mode 100644 stubs/chr-baum-init.c
>
> @@ -586,14 +589,14 @@ CharDriverState *chr_baum_init(void)
>
> baum->brlapi_fd = brlapi__openConnection(handle, NULL, NULL);
> if (baum->brlapi_fd == -1) {
> - brlapi_perror("baum_init: brlapi_openConnection");
> + error_setg(errp, "baum_init: brlapi_openConnection");
error_setg() already tracks function name, so you could drop 'baum_init:
'. Also, I assume that brlapi_perror() adds additional information to
the error message it prints, such as conversion of a brlapi-specific
error message in the same manner in which perror() converts errno and in
which error_setg_errno() would be used. So I don't know if this
conversion is the best. But I'm unfamiliar with brlapi_* in general, to
know if there is anything better to use, so I'd rather get a second
opinion on this.
'
Other than the possibility of the incorrect conversion of (several)
error messages, the rest of the patch looks fine to me.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH 14/21] qemu-char: convert braille backend to data-driven creation
2015-10-12 15:30 ` Eric Blake
@ 2015-10-12 15:41 ` Samuel Thibault
2015-10-14 15:51 ` Paolo Bonzini
0 siblings, 1 reply; 51+ messages in thread
From: Samuel Thibault @ 2015-10-12 15:41 UTC (permalink / raw)
To: Eric Blake; +Cc: Paolo Bonzini, qemu-devel, armbru
Eric Blake, le Mon 12 Oct 2015 09:30:12 -0600, a écrit :
> Also, I assume that brlapi_perror() adds additional information to
> the error message it prints, such as conversion of a brlapi-specific
> error message in the same manner in which perror() converts errno and in
> which error_setg_errno() would be used.
Yes. Such additional information is really useful to debug brlapi
issues.
> So I don't know if this
> conversion is the best. But I'm unfamiliar with brlapi_* in general, to
> know if there is anything better to use,
brlapi_error_t * brlapi_error_location(void);
const char * brlapi_strerror(const brlapi_error_t *error);
So brlapi_strerror(brlapi_error_location()) will return what you want,
i.e. the string that brlapi_error() would have printed.
> so I'd rather get a second opinion on this.
Cc-ing me would have been useful, "braille" just happened to catch my
eye by luck while looking over qemu mails :)
Samuel
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [Qemu-devel] [PATCH 14/21] qemu-char: convert braille backend to data-driven creation
2015-10-12 15:41 ` Samuel Thibault
@ 2015-10-14 15:51 ` Paolo Bonzini
2015-10-14 15:55 ` Eric Blake
2015-10-14 16:34 ` Samuel Thibault
0 siblings, 2 replies; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-14 15:51 UTC (permalink / raw)
To: Samuel Thibault, Eric Blake; +Cc: qemu-devel, armbru
On 12/10/2015 17:41, Samuel Thibault wrote:
> Eric Blake, le Mon 12 Oct 2015 09:30:12 -0600, a écrit :
>> Also, I assume that brlapi_perror() adds additional information to
>> the error message it prints, such as conversion of a brlapi-specific
>> error message in the same manner in which perror() converts errno and in
>> which error_setg_errno() would be used.
>
> Yes. Such additional information is really useful to debug brlapi
> issues.
>
>> So I don't know if this
>> conversion is the best. But I'm unfamiliar with brlapi_* in general, to
>> know if there is anything better to use,
>
> brlapi_error_t * brlapi_error_location(void);
> const char * brlapi_strerror(const brlapi_error_t *error);
>
> So brlapi_strerror(brlapi_error_location()) will return what you want,
> i.e. the string that brlapi_error() would have printed.
Is it okay to squash this?
diff --git a/backends/baum.c b/backends/baum.c
index 281be30..723c658 100644
--- a/backends/baum.c
+++ b/backends/baum.c
@@ -589,14 +589,16 @@ static CharDriverState *chr_baum_init(const char *id,
baum->brlapi_fd = brlapi__openConnection(handle, NULL, NULL);
if (baum->brlapi_fd == -1) {
- error_setg(errp, "baum_init: brlapi_openConnection");
+ error_setg(errp, "brlapi__openConnection: %s",
+ brlapi_strerror(brlapi_error_location()));
goto fail_handle;
}
baum->cellCount_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, baum_cellCount_timer_cb, baum);
if (brlapi__getDisplaySize(handle, &baum->x, &baum->y) == -1) {
- error_setg(errp, "baum_init: brlapi_getDisplaySize");
+ error_setg(errp, "brlapi__getDisplaySize: %s",
+ brlapi_strerror(brlapi_error_location()));
goto fail;
}
@@ -612,7 +614,8 @@ static CharDriverState *chr_baum_init(const char *id,
tty = BRLAPI_TTY_DEFAULT;
if (brlapi__enterTtyMode(handle, tty, NULL) == -1) {
- error_setg(errp, "baum_init: brlapi_enterTtyMode");
+ error_setg(errp, "brlapi__enterTtyMode: %s",
+ brlapi_strerror(brlapi_error_location()));
goto fail;
}
Thanks for the help,
Paolo
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH 14/21] qemu-char: convert braille backend to data-driven creation
2015-10-14 15:51 ` Paolo Bonzini
@ 2015-10-14 15:55 ` Eric Blake
2015-10-14 16:34 ` Samuel Thibault
1 sibling, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-14 15:55 UTC (permalink / raw)
To: Paolo Bonzini, Samuel Thibault; +Cc: qemu-devel, armbru
[-- Attachment #1: Type: text/plain, Size: 1222 bytes --]
On 10/14/2015 09:51 AM, Paolo Bonzini wrote:
>
>
> On 12/10/2015 17:41, Samuel Thibault wrote:
>> Eric Blake, le Mon 12 Oct 2015 09:30:12 -0600, a écrit :
>>> Also, I assume that brlapi_perror() adds additional information to
>>> the error message it prints, such as conversion of a brlapi-specific
>>> error message in the same manner in which perror() converts errno and in
>>> which error_setg_errno() would be used.
>>
>> Yes. Such additional information is really useful to debug brlapi
>> issues.
>>
>>> So I don't know if this
>>> conversion is the best. But I'm unfamiliar with brlapi_* in general, to
>>> know if there is anything better to use,
>>
>> brlapi_error_t * brlapi_error_location(void);
>> const char * brlapi_strerror(const brlapi_error_t *error);
>>
>> So brlapi_strerror(brlapi_error_location()) will return what you want,
>> i.e. the string that brlapi_error() would have printed.
>
> Is it okay to squash this?
I'd still like Samuel's confirmation, but from my point of view, that
looks like enough to add:
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [Qemu-devel] [PATCH 14/21] qemu-char: convert braille backend to data-driven creation
2015-10-14 15:51 ` Paolo Bonzini
2015-10-14 15:55 ` Eric Blake
@ 2015-10-14 16:34 ` Samuel Thibault
2015-10-14 16:51 ` Paolo Bonzini
1 sibling, 1 reply; 51+ messages in thread
From: Samuel Thibault @ 2015-10-14 16:34 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, armbru
Paolo Bonzini, le Wed 14 Oct 2015 17:51:51 +0200, a écrit :
>
>
> On 12/10/2015 17:41, Samuel Thibault wrote:
> > Eric Blake, le Mon 12 Oct 2015 09:30:12 -0600, a écrit :
> >> Also, I assume that brlapi_perror() adds additional information to
> >> the error message it prints, such as conversion of a brlapi-specific
> >> error message in the same manner in which perror() converts errno and in
> >> which error_setg_errno() would be used.
> >
> > Yes. Such additional information is really useful to debug brlapi
> > issues.
> >
> >> So I don't know if this
> >> conversion is the best. But I'm unfamiliar with brlapi_* in general, to
> >> know if there is anything better to use,
> >
> > brlapi_error_t * brlapi_error_location(void);
> > const char * brlapi_strerror(const brlapi_error_t *error);
> >
> > So brlapi_strerror(brlapi_error_location()) will return what you want,
> > i.e. the string that brlapi_error() would have printed.
>
> Is it okay to squash this?
Yes, this looks right. Building with brlapi-dev installed would confirm
for sure that it builds, of course :)
Samuel
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [Qemu-devel] [PATCH 14/21] qemu-char: convert braille backend to data-driven creation
2015-10-14 16:34 ` Samuel Thibault
@ 2015-10-14 16:51 ` Paolo Bonzini
0 siblings, 0 replies; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-14 16:51 UTC (permalink / raw)
To: Samuel Thibault; +Cc: qemu-devel, armbru
On 14/10/2015 18:34, Samuel Thibault wrote:
> Paolo Bonzini, le Wed 14 Oct 2015 17:51:51 +0200, a écrit :
>>
>>
>> On 12/10/2015 17:41, Samuel Thibault wrote:
>>> Eric Blake, le Mon 12 Oct 2015 09:30:12 -0600, a écrit :
>>>> Also, I assume that brlapi_perror() adds additional information to
>>>> the error message it prints, such as conversion of a brlapi-specific
>>>> error message in the same manner in which perror() converts errno and in
>>>> which error_setg_errno() would be used.
>>>
>>> Yes. Such additional information is really useful to debug brlapi
>>> issues.
>>>
>>>> So I don't know if this
>>>> conversion is the best. But I'm unfamiliar with brlapi_* in general, to
>>>> know if there is anything better to use,
>>>
>>> brlapi_error_t * brlapi_error_location(void);
>>> const char * brlapi_strerror(const brlapi_error_t *error);
>>>
>>> So brlapi_strerror(brlapi_error_location()) will return what you want,
>>> i.e. the string that brlapi_error() would have printed.
>>
>> Is it okay to squash this?
>
> Yes, this looks right. Building with brlapi-dev installed would confirm
> for sure that it builds, of course :)
That I checked. :)
Paolo
^ permalink raw reply [flat|nested] 51+ messages in thread
* [Qemu-devel] [PATCH 15/21] qemu-char: convert testdev backend to data-driven creation
2015-10-12 8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
` (13 preceding siblings ...)
2015-10-12 8:03 ` [Qemu-devel] [PATCH 14/21] qemu-char: convert braille " Paolo Bonzini
@ 2015-10-12 8:03 ` Paolo Bonzini
2015-10-12 15:49 ` Eric Blake
2015-10-12 8:03 ` [Qemu-devel] [PATCH 16/21] qemu-char: convert stdio " Paolo Bonzini
` (5 subsequent siblings)
20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12 8:03 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
backends/testdev.c | 7 +++++--
include/sysemu/char.h | 3 ---
qemu-char.c | 2 +-
stubs/Makefile.objs | 1 -
stubs/chr-testdev.c | 7 -------
5 files changed, 6 insertions(+), 14 deletions(-)
delete mode 100644 stubs/chr-testdev.c
diff --git a/backends/testdev.c b/backends/testdev.c
index 43787f6..26d5c73 100644
--- a/backends/testdev.c
+++ b/backends/testdev.c
@@ -108,7 +108,10 @@ static void testdev_close(struct CharDriverState *chr)
g_free(testdev);
}
-CharDriverState *chr_testdev_init(void)
+static CharDriverState *chr_testdev_init(const char *id,
+ ChardevBackend *backend,
+ ChardevReturn *ret,
+ Error **errp)
{
TestdevCharState *testdev;
CharDriverState *chr;
@@ -126,7 +129,7 @@ CharDriverState *chr_testdev_init(void)
static void register_types(void)
{
register_char_driver("testdev", CHARDEV_BACKEND_KIND_TESTDEV, NULL,
- NULL);
+ chr_testdev_init);
}
type_init(register_types);
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 77415ec..5c28c16 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -356,9 +356,6 @@ extern int term_escape_char;
CharDriverState *qemu_char_get_next_serial(void);
-/* testdev.c */
-CharDriverState *chr_testdev_init(void);
-
/* console.c */
typedef CharDriverState *(VcHandler)(ChardevVC *vc);
diff --git a/qemu-char.c b/qemu-char.c
index 96f40f3..72658d2 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4327,7 +4327,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
abort();
break;
case CHARDEV_BACKEND_KIND_TESTDEV:
- chr = chr_testdev_init();
+ abort();
break;
case CHARDEV_BACKEND_KIND_STDIO:
chr = qemu_chr_open_stdio(backend->stdio);
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 8cfa5a2..b5322a2 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -1,6 +1,5 @@
stub-obj-y += arch-query-cpu-def.o
stub-obj-y += bdrv-commit-all.o
-stub-obj-y += chr-testdev.o
stub-obj-y += clock-warp.o
stub-obj-y += cpu-get-clock.o
stub-obj-y += cpu-get-icount.o
diff --git a/stubs/chr-testdev.c b/stubs/chr-testdev.c
deleted file mode 100644
index 23112a2..0000000
--- a/stubs/chr-testdev.c
+++ /dev/null
@@ -1,7 +0,0 @@
-#include "qemu-common.h"
-#include "sysemu/char.h"
-
-CharDriverState *chr_testdev_init(void)
-{
- return 0;
-}
--
2.5.0
^ permalink raw reply related [flat|nested] 51+ messages in thread* [Qemu-devel] [PATCH 16/21] qemu-char: convert stdio backend to data-driven creation
2015-10-12 8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
` (14 preceding siblings ...)
2015-10-12 8:03 ` [Qemu-devel] [PATCH 15/21] qemu-char: convert testdev " Paolo Bonzini
@ 2015-10-12 8:03 ` Paolo Bonzini
2015-10-12 15:50 ` Eric Blake
2015-10-12 8:03 ` [Qemu-devel] [PATCH 17/21] qemu-char: convert console " Paolo Bonzini
` (4 subsequent siblings)
20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12 8:03 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru
The backend now always returns errors via the Error* argument.
This avoids a double error message. Before:
qemu-system-x86_64: -chardev stdio,id=base: cannot use stdio with -daemonize
qemu-system-x86_64: -chardev stdio,id=base: Failed to create chardev
After:
qemu-system-x86_64: -chardev stdio,id=base: cannot use stdio with -daemonize
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu-char.c | 57 +++++++++++++++++++++++++++++++++++++++------------------
1 file changed, 39 insertions(+), 18 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 72658d2..74ca66d 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1168,19 +1168,23 @@ static void qemu_chr_close_stdio(struct CharDriverState *chr)
fd_chr_close(chr);
}
-static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
+static CharDriverState *qemu_chr_open_stdio(const char *id,
+ ChardevBackend *backend,
+ ChardevReturn *ret,
+ Error **errp)
{
+ ChardevStdio *opts = backend->stdio;
CharDriverState *chr;
struct sigaction act;
if (is_daemonized()) {
- error_report("cannot use stdio with -daemonize");
+ error_setg(errp, "cannot use stdio with -daemonize");
return NULL;
}
if (stdio_in_use) {
- error_report("cannot use stdio by multiple character devices");
- exit(1);
+ error_setg(errp, "cannot use stdio by multiple character devices");
+ return NULL;
}
stdio_in_use = true;
@@ -2341,7 +2345,10 @@ static void win_stdio_close(CharDriverState *chr)
g_free(chr);
}
-static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
+static CharDriverState *qemu_chr_open_stdio(const char *id,
+ ChardevBackend *backend,
+ ChardevReturn *ret,
+ Error **errp)
{
CharDriverState *chr;
WinStdioCharState *stdio;
@@ -2353,8 +2360,8 @@ static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
stdio->hStdIn = GetStdHandle(STD_INPUT_HANDLE);
if (stdio->hStdIn == INVALID_HANDLE_VALUE) {
- fprintf(stderr, "cannot open stdio: invalid handle\n");
- exit(1);
+ error_setg(errp, "cannot open stdio: invalid handle");
+ return NULL;
}
is_console = GetConsoleMode(stdio->hStdIn, &dwMode) != 0;
@@ -2366,25 +2373,30 @@ static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
if (is_console) {
if (qemu_add_wait_object(stdio->hStdIn,
win_stdio_wait_func, chr)) {
- fprintf(stderr, "qemu_add_wait_object: failed\n");
+ error_setg(errp, "qemu_add_wait_object: failed");
+ goto err1;
}
} else {
DWORD dwId;
stdio->hInputReadyEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
stdio->hInputDoneEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
- stdio->hInputThread = CreateThread(NULL, 0, win_stdio_thread,
- chr, 0, &dwId);
-
- if (stdio->hInputThread == INVALID_HANDLE_VALUE
- || stdio->hInputReadyEvent == INVALID_HANDLE_VALUE
+ if (stdio->hInputReadyEvent == INVALID_HANDLE_VALUE
|| stdio->hInputDoneEvent == INVALID_HANDLE_VALUE) {
- fprintf(stderr, "cannot create stdio thread or event\n");
- exit(1);
+ error_setg(errp, "cannot create event");
+ goto err2;
}
if (qemu_add_wait_object(stdio->hInputReadyEvent,
win_stdio_thread_wait_func, chr)) {
- fprintf(stderr, "qemu_add_wait_object: failed\n");
+ error_setg(errp, "qemu_add_wait_object: failed");
+ goto err2;
+ }
+ stdio->hInputThread = CreateThread(NULL, 0, win_stdio_thread,
+ chr, 0, &dwId);
+
+ if (stdio->hInputThread == INVALID_HANDLE_VALUE) {
+ error_setg(errp, "cannot create stdio thread");
+ goto err3;
}
}
@@ -2402,6 +2414,15 @@ static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
qemu_chr_fe_set_echo(chr, false);
return chr;
+
+err3:
+ qemu_del_wait_object(stdio->hInputReadyEvent, NULL, NULL);
+err2:
+ CloseHandle(stdio->hInputReadyEvent);
+ CloseHandle(stdio->hInputDoneEvent);
+err1:
+ qemu_del_wait_object(stdio->hStdIn, NULL, NULL);
+ return NULL;
}
#endif /* !_WIN32 */
@@ -4330,7 +4351,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
abort();
break;
case CHARDEV_BACKEND_KIND_STDIO:
- chr = qemu_chr_open_stdio(backend->stdio);
+ abort();
break;
#ifdef _WIN32
case CHARDEV_BACKEND_KIND_CONSOLE:
@@ -4420,7 +4441,7 @@ static void register_types(void)
register_char_driver("file", CHARDEV_BACKEND_KIND_FILE,
qemu_chr_parse_file_out, qmp_chardev_open_file);
register_char_driver("stdio", CHARDEV_BACKEND_KIND_STDIO,
- qemu_chr_parse_stdio, NULL);
+ qemu_chr_parse_stdio, qemu_chr_open_stdio);
#if defined HAVE_CHARDEV_SERIAL
register_char_driver("serial", CHARDEV_BACKEND_KIND_SERIAL,
qemu_chr_parse_serial, qmp_chardev_open_serial);
--
2.5.0
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [Qemu-devel] [PATCH 16/21] qemu-char: convert stdio backend to data-driven creation
2015-10-12 8:03 ` [Qemu-devel] [PATCH 16/21] qemu-char: convert stdio " Paolo Bonzini
@ 2015-10-12 15:50 ` Eric Blake
0 siblings, 0 replies; 51+ messages in thread
From: Eric Blake @ 2015-10-12 15:50 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: armbru
[-- Attachment #1: Type: text/plain, Size: 786 bytes --]
On 10/12/2015 02:03 AM, Paolo Bonzini wrote:
> The backend now always returns errors via the Error* argument.
> This avoids a double error message. Before:
>
> qemu-system-x86_64: -chardev stdio,id=base: cannot use stdio with -daemonize
> qemu-system-x86_64: -chardev stdio,id=base: Failed to create chardev
>
> After:
>
> qemu-system-x86_64: -chardev stdio,id=base: cannot use stdio with -daemonize
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> qemu-char.c | 57 +++++++++++++++++++++++++++++++++++++++------------------
> 1 file changed, 39 insertions(+), 18 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* [Qemu-devel] [PATCH 17/21] qemu-char: convert console backend to data-driven creation
2015-10-12 8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
` (15 preceding siblings ...)
2015-10-12 8:03 ` [Qemu-devel] [PATCH 16/21] qemu-char: convert stdio " Paolo Bonzini
@ 2015-10-12 8:03 ` Paolo Bonzini
2015-10-12 15:50 ` Eric Blake
2015-10-12 8:03 ` [Qemu-devel] [PATCH 18/21] qemu-char: convert spice " Paolo Bonzini
` (3 subsequent siblings)
20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12 8:03 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu-char.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 74ca66d..535db73 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2204,7 +2204,10 @@ static CharDriverState *qemu_chr_open_win_file(HANDLE fd_out)
return chr;
}
-static CharDriverState *qemu_chr_open_win_con(void)
+static CharDriverState *qemu_chr_open_win_con(const char *id,
+ ChardevBackend *backend,
+ ChardevReturn *ret,
+ Error **errp)
{
return qemu_chr_open_win_file(GetStdHandle(STD_OUTPUT_HANDLE));
}
@@ -4353,11 +4356,9 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
case CHARDEV_BACKEND_KIND_STDIO:
abort();
break;
-#ifdef _WIN32
case CHARDEV_BACKEND_KIND_CONSOLE:
- chr = qemu_chr_open_win_con();
+ abort();
break;
-#endif
#ifdef CONFIG_SPICE
case CHARDEV_BACKEND_KIND_SPICEVMC:
chr = qemu_chr_open_spice_vmc(backend->spicevmc->type);
@@ -4458,8 +4459,10 @@ static void register_types(void)
register_char_driver("pty", CHARDEV_BACKEND_KIND_PTY, NULL,
qemu_chr_open_pty);
#endif
+#ifdef _WIN32
register_char_driver("console", CHARDEV_BACKEND_KIND_CONSOLE, NULL,
- NULL);
+ qemu_chr_open_win_con);
+#endif
register_char_driver("pipe", CHARDEV_BACKEND_KIND_PIPE,
qemu_chr_parse_pipe, qemu_chr_open_pipe);
register_char_driver("mux", CHARDEV_BACKEND_KIND_MUX, qemu_chr_parse_mux,
--
2.5.0
^ permalink raw reply related [flat|nested] 51+ messages in thread* [Qemu-devel] [PATCH 18/21] qemu-char: convert spice backend to data-driven creation
2015-10-12 8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
` (16 preceding siblings ...)
2015-10-12 8:03 ` [Qemu-devel] [PATCH 17/21] qemu-char: convert console " Paolo Bonzini
@ 2015-10-12 8:03 ` Paolo Bonzini
2015-10-12 15:51 ` Eric Blake
2015-10-12 8:03 ` [Qemu-devel] [PATCH 19/21] qemu-char: convert vc " Paolo Bonzini
` (2 subsequent siblings)
20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12 8:03 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/ui/qemu-spice.h | 2 --
qemu-char.c | 6 ++----
spice-qemu-char.c | 21 ++++++++++++---------
stubs/Makefile.objs | 1 -
stubs/qemu-chr-open-spice.c | 14 --------------
5 files changed, 14 insertions(+), 30 deletions(-)
delete mode 100644 stubs/qemu-chr-open-spice.c
diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
index 0dff422..f9ce357 100644
--- a/include/ui/qemu-spice.h
+++ b/include/ui/qemu-spice.h
@@ -43,9 +43,7 @@ int qemu_spice_set_pw_expire(time_t expires);
int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
const char *subject);
-CharDriverState *qemu_chr_open_spice_vmc(const char *type);
#if SPICE_SERVER_VERSION >= 0x000c02
-CharDriverState *qemu_chr_open_spice_port(const char *name);
void qemu_spice_register_ports(void);
#else
static inline CharDriverState *qemu_chr_open_spice_port(const char *name)
diff --git a/qemu-char.c b/qemu-char.c
index 535db73..43205ae 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4359,14 +4359,12 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
case CHARDEV_BACKEND_KIND_CONSOLE:
abort();
break;
-#ifdef CONFIG_SPICE
case CHARDEV_BACKEND_KIND_SPICEVMC:
- chr = qemu_chr_open_spice_vmc(backend->spicevmc->type);
+ abort();
break;
case CHARDEV_BACKEND_KIND_SPICEPORT:
- chr = qemu_chr_open_spice_port(backend->spiceport->fqdn);
+ abort();
break;
-#endif
case CHARDEV_BACKEND_KIND_VC:
chr = vc_init(backend->vc);
break;
diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index e4353ef..a20fb5c 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -296,15 +296,14 @@ static CharDriverState *chr_open(const char *subtype,
return chr;
}
-CharDriverState *qemu_chr_open_spice_vmc(const char *type)
+static CharDriverState *qemu_chr_open_spice_vmc(const char *id,
+ ChardevBackend *backend,
+ ChardevReturn *ret,
+ Error **errp)
{
+ const char *type = backend->spicevmc->type;
const char **psubtype = spice_server_char_device_recognized_subtypes();
- if (type == NULL) {
- fprintf(stderr, "spice-qemu-char: missing name parameter\n");
- print_allowed_subtypes();
- return NULL;
- }
for (; *psubtype != NULL; ++psubtype) {
if (strcmp(type, *psubtype) == 0) {
break;
@@ -320,8 +319,12 @@ CharDriverState *qemu_chr_open_spice_vmc(const char *type)
}
#if SPICE_SERVER_VERSION >= 0x000c02
-CharDriverState *qemu_chr_open_spice_port(const char *name)
+static CharDriverState *qemu_chr_open_spice_port(const char *id,
+ ChardevBackend *backend,
+ ChardevReturn *ret,
+ Error **errp)
{
+ const char *name = backend->spiceport->fqdn;
CharDriverState *chr;
SpiceCharDriver *s;
@@ -379,9 +382,9 @@ static void qemu_chr_parse_spice_port(QemuOpts *opts, ChardevBackend *backend,
static void register_types(void)
{
register_char_driver("spicevmc", CHARDEV_BACKEND_KIND_SPICEVMC,
- qemu_chr_parse_spice_vmc, NULL);
+ qemu_chr_parse_spice_vmc, qemu_chr_open_spice_vmc);
register_char_driver("spiceport", CHARDEV_BACKEND_KIND_SPICEPORT,
- qemu_chr_parse_spice_port, NULL);
+ qemu_chr_parse_spice_port, qemu_chr_open_spice_port);
}
type_init(register_types);
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index b5322a2..6d4363d 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -20,7 +20,6 @@ stub-obj-y += mon-is-qmp.o
stub-obj-y += mon-printf.o
stub-obj-y += monitor-init.o
stub-obj-y += notify-event.o
-stub-obj-$(CONFIG_SPICE) += qemu-chr-open-spice.o
stub-obj-y += qtest.o
stub-obj-y += reset.o
stub-obj-y += runstate-check.o
diff --git a/stubs/qemu-chr-open-spice.c b/stubs/qemu-chr-open-spice.c
deleted file mode 100644
index f1c4849..0000000
--- a/stubs/qemu-chr-open-spice.c
+++ /dev/null
@@ -1,14 +0,0 @@
-#include "qemu-common.h"
-#include "ui/qemu-spice.h"
-
-CharDriverState *qemu_chr_open_spice_vmc(const char *type)
-{
- return NULL;
-}
-
-#if SPICE_SERVER_VERSION >= 0x000c02
-CharDriverState *qemu_chr_open_spice_port(const char *name)
-{
- return NULL;
-}
-#endif
--
2.5.0
^ permalink raw reply related [flat|nested] 51+ messages in thread* [Qemu-devel] [PATCH 19/21] qemu-char: convert vc backend to data-driven creation
2015-10-12 8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
` (17 preceding siblings ...)
2015-10-12 8:03 ` [Qemu-devel] [PATCH 18/21] qemu-char: convert spice " Paolo Bonzini
@ 2015-10-12 8:03 ` Paolo Bonzini
2015-10-12 15:53 ` Eric Blake
2015-10-12 8:03 ` [Qemu-devel] [PATCH 20/21] qemu-char: convert ringbuf " Paolo Bonzini
2015-10-12 8:03 ` [Qemu-devel] [PATCH 21/21] qemu-char: cleanup after completed conversion to cd->create Paolo Bonzini
20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12 8:03 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/sysemu/char.h | 5 ++---
qemu-char.c | 2 +-
stubs/Makefile.objs | 1 -
stubs/vc-init.c | 7 -------
ui/console.c | 10 ++++++----
ui/gtk.c | 2 +-
6 files changed, 10 insertions(+), 17 deletions(-)
delete mode 100644 stubs/vc-init.c
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 5c28c16..edf7669 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -357,8 +357,7 @@ extern int term_escape_char;
CharDriverState *qemu_char_get_next_serial(void);
/* console.c */
-typedef CharDriverState *(VcHandler)(ChardevVC *vc);
-
+typedef CharDriverState *(VcHandler)(ChardevVC *vc, Error **errp);
void register_vc_handler(VcHandler *handler);
-CharDriverState *vc_init(ChardevVC *vc);
+
#endif
diff --git a/qemu-char.c b/qemu-char.c
index 43205ae..7ef1293 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4366,7 +4366,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
abort();
break;
case CHARDEV_BACKEND_KIND_VC:
- chr = vc_init(backend->vc);
+ abort();
break;
case CHARDEV_BACKEND_KIND_RINGBUF:
case CHARDEV_BACKEND_KIND_MEMORY:
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 6d4363d..1862f84 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -27,7 +27,6 @@ stub-obj-y += set-fd-handler.o
stub-obj-y += slirp.o
stub-obj-y += sysbus.o
stub-obj-y += uuid.o
-stub-obj-y += vc-init.o
stub-obj-y += vm-stop.o
stub-obj-y += vmstate.o
stub-obj-$(CONFIG_WIN32) += fd-register.o
diff --git a/stubs/vc-init.c b/stubs/vc-init.c
deleted file mode 100644
index 308dfa0..0000000
--- a/stubs/vc-init.c
+++ /dev/null
@@ -1,7 +0,0 @@
-#include "qemu-common.h"
-#include "sysemu/char.h"
-
-CharDriverState *vc_init(ChardevVC *vc)
-{
- return 0;
-}
diff --git a/ui/console.c b/ui/console.c
index 746b23a..5e94b0f 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1899,7 +1899,7 @@ static void text_console_do_init(CharDriverState *chr, DisplayState *ds)
chr->init(chr);
}
-static CharDriverState *text_console_init(ChardevVC *vc)
+static CharDriverState *text_console_init(ChardevVC *vc, Error **errp)
{
CharDriverState *chr;
QemuConsole *s;
@@ -1930,6 +1930,7 @@ static CharDriverState *text_console_init(ChardevVC *vc)
if (!s) {
g_free(chr);
+ error_setg(errp, "cannot create text console");
return NULL;
}
@@ -1949,9 +1950,10 @@ static CharDriverState *text_console_init(ChardevVC *vc)
static VcHandler *vc_handler = text_console_init;
-CharDriverState *vc_init(ChardevVC *vc)
+static CharDriverState *vc_init(const char *id, ChardevBackend *backend,
+ ChardevReturn *ret, Error **errp)
{
- return vc_handler(vc);
+ return vc_handler(backend->vc, errp);
}
void register_vc_handler(VcHandler *handler)
@@ -2031,7 +2033,7 @@ static void register_types(void)
{
type_register_static(&qemu_console_info);
register_char_driver("vc", CHARDEV_BACKEND_KIND_VC, qemu_chr_parse_vc,
- NULL);
+ vc_init);
}
type_init(register_types);
diff --git a/ui/gtk.c b/ui/gtk.c
index 3057cdc..9731761 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1522,7 +1522,7 @@ static int gd_vc_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
static int nb_vcs;
static CharDriverState *vcs[MAX_VCS];
-static CharDriverState *gd_vc_handler(ChardevVC *unused)
+static CharDriverState *gd_vc_handler(ChardevVC *unused, Error **errp)
{
CharDriverState *chr;
--
2.5.0
^ permalink raw reply related [flat|nested] 51+ messages in thread* [Qemu-devel] [PATCH 20/21] qemu-char: convert ringbuf backend to data-driven creation
2015-10-12 8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
` (18 preceding siblings ...)
2015-10-12 8:03 ` [Qemu-devel] [PATCH 19/21] qemu-char: convert vc " Paolo Bonzini
@ 2015-10-12 8:03 ` Paolo Bonzini
2015-10-12 15:53 ` Eric Blake
2015-10-12 8:03 ` [Qemu-devel] [PATCH 21/21] qemu-char: cleanup after completed conversion to cd->create Paolo Bonzini
20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12 8:03 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu-char.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index 7ef1293..e0faeb7 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3222,9 +3222,12 @@ static void ringbuf_chr_close(struct CharDriverState *chr)
chr->opaque = NULL;
}
-static CharDriverState *qemu_chr_open_ringbuf(ChardevRingbuf *opts,
+static CharDriverState *qemu_chr_open_ringbuf(const char *id,
+ ChardevBackend *backend,
+ ChardevReturn *ret,
Error **errp)
{
+ ChardevRingbuf *opts = backend->ringbuf;
CharDriverState *chr;
RingBufCharDriver *d;
@@ -4370,7 +4373,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
break;
case CHARDEV_BACKEND_KIND_RINGBUF:
case CHARDEV_BACKEND_KIND_MEMORY:
- chr = qemu_chr_open_ringbuf(backend->ringbuf, &local_err);
+ abort();
break;
default:
error_setg(errp, "unknown chardev backend (%d)", backend->kind);
@@ -4436,7 +4439,7 @@ static void register_types(void)
register_char_driver("udp", CHARDEV_BACKEND_KIND_UDP, qemu_chr_parse_udp,
qmp_chardev_open_udp);
register_char_driver("ringbuf", CHARDEV_BACKEND_KIND_RINGBUF,
- qemu_chr_parse_ringbuf, NULL);
+ qemu_chr_parse_ringbuf, qemu_chr_open_ringbuf);
register_char_driver("file", CHARDEV_BACKEND_KIND_FILE,
qemu_chr_parse_file_out, qmp_chardev_open_file);
register_char_driver("stdio", CHARDEV_BACKEND_KIND_STDIO,
@@ -4467,7 +4470,7 @@ static void register_types(void)
qemu_chr_open_mux);
/* Bug-compatibility: */
register_char_driver("memory", CHARDEV_BACKEND_KIND_MEMORY,
- qemu_chr_parse_ringbuf, NULL);
+ qemu_chr_parse_ringbuf, qemu_chr_open_ringbuf);
/* this must be done after machine init, since we register FEs with muxes
* as part of realize functions like serial_isa_realizefn when -nographic
* is specified
--
2.5.0
^ permalink raw reply related [flat|nested] 51+ messages in thread* [Qemu-devel] [PATCH 21/21] qemu-char: cleanup after completed conversion to cd->create
2015-10-12 8:03 [Qemu-devel] [PATCH 00/22] qemu-char: refactoring of chardev creation Paolo Bonzini
` (19 preceding siblings ...)
2015-10-12 8:03 ` [Qemu-devel] [PATCH 20/21] qemu-char: convert ringbuf " Paolo Bonzini
@ 2015-10-12 8:03 ` Paolo Bonzini
2015-10-12 15:55 ` Eric Blake
20 siblings, 1 reply; 51+ messages in thread
From: Paolo Bonzini @ 2015-10-12 8:03 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru
All backends now return errors through Error*, so the "Failed to
create chardev" placeholder error can only be reached if the backend
is not available (and only from the chardev-add QMP command; instead,
the -chardev command line option fails earlier).
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
qemu-char.c | 80 ++++---------------------------------------------------------
1 file changed, 4 insertions(+), 76 deletions(-)
diff --git a/qemu-char.c b/qemu-char.c
index e0faeb7..8d48e35 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4308,7 +4308,7 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
for (i = backends; i; i = i->next) {
cd = i->data;
- if (cd->kind == backend->kind && cd->create) {
+ if (cd->kind == backend->kind) {
chr = cd->create(id, backend, ret, &local_err);
if (local_err) {
error_propagate(errp, local_err);
@@ -4319,81 +4319,9 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
}
if (chr == NULL) {
- switch (backend->kind) {
- case CHARDEV_BACKEND_KIND_FILE:
- abort();
- break;
- case CHARDEV_BACKEND_KIND_SERIAL:
- abort();
- break;
- case CHARDEV_BACKEND_KIND_PARALLEL:
- abort();
- break;
- case CHARDEV_BACKEND_KIND_PIPE:
- abort();
- break;
- case CHARDEV_BACKEND_KIND_SOCKET:
- abort();
- break;
- case CHARDEV_BACKEND_KIND_UDP:
- abort();
- break;
- case CHARDEV_BACKEND_KIND_PTY:
- abort();
- break;
- case CHARDEV_BACKEND_KIND_NULL:
- abort();
- break;
- case CHARDEV_BACKEND_KIND_MUX:
- abort();
- break;
- case CHARDEV_BACKEND_KIND_MSMOUSE:
- abort();
- break;
- case CHARDEV_BACKEND_KIND_BRAILLE:
- abort();
- break;
- case CHARDEV_BACKEND_KIND_TESTDEV:
- abort();
- break;
- case CHARDEV_BACKEND_KIND_STDIO:
- abort();
- break;
- case CHARDEV_BACKEND_KIND_CONSOLE:
- abort();
- break;
- case CHARDEV_BACKEND_KIND_SPICEVMC:
- abort();
- break;
- case CHARDEV_BACKEND_KIND_SPICEPORT:
- abort();
- break;
- case CHARDEV_BACKEND_KIND_VC:
- abort();
- break;
- case CHARDEV_BACKEND_KIND_RINGBUF:
- case CHARDEV_BACKEND_KIND_MEMORY:
- abort();
- break;
- default:
- error_setg(errp, "unknown chardev backend (%d)", backend->kind);
- goto out_error;
- }
-
- /*
- * Character backend open hasn't been fully converted to the Error
- * API. Some opens fail without setting an error. Set a generic
- * error then.
- * TODO full conversion to Error API
- */
- if (chr == NULL) {
- if (local_err) {
- error_propagate(errp, local_err);
- } else {
- error_setg(errp, "Failed to create chardev");
- }
- goto out_error;
- }
+ assert(!i);
+ error_setg(errp, "chardev backend not available");
+ goto out_error;
}
chr->label = g_strdup(id);
--
2.5.0
^ permalink raw reply related [flat|nested] 51+ messages in thread