* [Qemu-devel] [PATCH v2 06/21] qemu-char: convert parallel backend to data-driven creation
2015-10-14 15:54 [Qemu-devel] [PATCH v2 00/21] qemu-char: refactoring of chardev creation Paolo Bonzini
@ 2015-10-14 15:54 ` Paolo Bonzini
2015-10-14 20:52 ` Eric Blake
2015-10-14 15:54 ` [Qemu-devel] [PATCH v2 07/21] qemu-char: convert pipe " Paolo Bonzini
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2015-10-14 15:54 UTC (permalink / raw)
To: qemu-devel; +Cc: samuel.thibault
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..ff7722f 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1753,12 +1753,13 @@ 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) {
+ error_setg_errno(errp, errno, "not a parallel port");
close(fd);
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] 8+ messages in thread* Re: [Qemu-devel] [PATCH v2 06/21] qemu-char: convert parallel backend to data-driven creation
2015-10-14 15:54 ` [Qemu-devel] [PATCH v2 06/21] qemu-char: convert parallel backend to data-driven creation Paolo Bonzini
@ 2015-10-14 20:52 ` Eric Blake
0 siblings, 0 replies; 8+ messages in thread
From: Eric Blake @ 2015-10-14 20:52 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: samuel.thibault
[-- Attachment #1: Type: text/plain, Size: 683 bytes --]
On 10/14/2015 09:54 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(-)
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] 8+ messages in thread
* [Qemu-devel] [PATCH v2 07/21] qemu-char: convert pipe backend to data-driven creation
2015-10-14 15:54 [Qemu-devel] [PATCH v2 00/21] qemu-char: refactoring of chardev creation Paolo Bonzini
2015-10-14 15:54 ` [Qemu-devel] [PATCH v2 06/21] qemu-char: convert parallel backend to data-driven creation Paolo Bonzini
@ 2015-10-14 15:54 ` Paolo Bonzini
2015-10-14 20:53 ` Eric Blake
2015-10-14 15:54 ` [Qemu-devel] [PATCH v2 14/21] qemu-char: convert braille " Paolo Bonzini
2015-10-14 16:18 ` [Qemu-devel] [PATCH v2 00/21] qemu-char: refactoring of chardev creation Eric Blake
3 siblings, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2015-10-14 15:54 UTC (permalink / raw)
To: qemu-devel; +Cc: samuel.thibault
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 ff7722f..47db98f 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");
goto fail;
}
s->hrecv = CreateEvent(NULL, TRUE, FALSE, NULL);
if (!s->hrecv) {
- fprintf(stderr, "Failed CreateEvent\n");
+ error_setg(errp, "Failed CreateEvent");
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)", 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");
goto fail;
}
ret = GetOverlappedResult(s->hcom, &ov, &size, TRUE);
if (!ret) {
- fprintf(stderr, "Failed GetOverlappedResult\n");
+ error_setg(errp, "Failed GetOverlappedResult");
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] 8+ messages in thread* [Qemu-devel] [PATCH v2 14/21] qemu-char: convert braille backend to data-driven creation
2015-10-14 15:54 [Qemu-devel] [PATCH v2 00/21] qemu-char: refactoring of chardev creation Paolo Bonzini
2015-10-14 15:54 ` [Qemu-devel] [PATCH v2 06/21] qemu-char: convert parallel backend to data-driven creation Paolo Bonzini
2015-10-14 15:54 ` [Qemu-devel] [PATCH v2 07/21] qemu-char: convert pipe " Paolo Bonzini
@ 2015-10-14 15:54 ` Paolo Bonzini
2015-10-14 20:53 ` Eric Blake
2015-10-14 16:18 ` [Qemu-devel] [PATCH v2 00/21] qemu-char: refactoring of chardev creation Eric Blake
3 siblings, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2015-10-14 15:54 UTC (permalink / raw)
To: qemu-devel; +Cc: samuel.thibault
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
backends/baum.c | 16 +++++++++++-----
include/sysemu/char.h | 3 ---
qemu-char.c | 4 +---
stubs/Makefile.objs | 1 -
stubs/chr-baum-init.c | 7 -------
5 files changed, 12 insertions(+), 19 deletions(-)
delete mode 100644 stubs/chr-baum-init.c
diff --git a/backends/baum.c b/backends/baum.c
index e86a019..723c658 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,16 @@ 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, "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) {
- brlapi_perror("baum_init: brlapi_getDisplaySize");
+ error_setg(errp, "brlapi__getDisplaySize: %s",
+ brlapi_strerror(brlapi_error_location()));
goto fail;
}
@@ -609,7 +614,8 @@ 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, "brlapi__enterTtyMode: %s",
+ brlapi_strerror(brlapi_error_location()));
goto fail;
}
@@ -630,7 +636,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 21a50a5..06163bd 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] 8+ messages in thread* Re: [Qemu-devel] [PATCH v2 00/21] qemu-char: refactoring of chardev creation
2015-10-14 15:54 [Qemu-devel] [PATCH v2 00/21] qemu-char: refactoring of chardev creation Paolo Bonzini
` (2 preceding siblings ...)
2015-10-14 15:54 ` [Qemu-devel] [PATCH v2 14/21] qemu-char: convert braille " Paolo Bonzini
@ 2015-10-14 16:18 ` Eric Blake
3 siblings, 0 replies; 8+ messages in thread
From: Eric Blake @ 2015-10-14 16:18 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: samuel.thibault, Markus Armbruster
[-- Attachment #1: Type: text/plain, Size: 683 bytes --]
On 10/14/2015 09:54 AM, Paolo Bonzini wrote:
> This series rewrites chardev creation to use a new ->create
> member of the CharDriver struct, and to always signal errors
> via Error*.
>
> The advantage is that backend-specific creation functions need
> not be exported anymore for qemu-char.c's usage, and hence do not
> need stubs anymore.
Hmm - I just noticed that this series conflicts with my work at redoing
how qapi union types are laid out (basically, I'm going from
backend->foo to backend->u.foo). I'll list your series as a
prerequisite of mine.
--
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] 8+ messages in thread