| Message ID | 1452873871-138914-16-git-send-email-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, This patch (commit d0d7708ba29cbcc343364a46bff981e0ff88366f) regresses the following command line: qemu-system-i386 -nodefaults -chardev vc,id=mon0 -mon chardev=mon0 Before: [nothing is print on console] After: QEMU 2.5.50 monitor - type 'help' for more information (qemu) In both cases, a working vc is created in the GTK+ UI. Reverting the commit (and fixing the trivial conflict) makes things work again. Regards, Hervé Le 15/01/2016 17:04, Paolo Bonzini a écrit : > From: "Daniel P. Berrange" <berrange@redhat.com> > > Typically a UNIX guest OS will log boot messages to a serial > port in addition to any graphical console. An admin user > may also wish to use the serial port for an interactive > console. A virtualization management system may wish to > collect system boot messages by logging the serial port, > but also wish to allow admins interactive access. > > Currently providing such a feature forces the mgmt app > to either provide 2 separate serial ports, one for > logging boot messages and one for interactive console > login, or to proxy all output via a separate service > that can multiplex the two needs onto one serial port. > While both are valid approaches, they each have their > own downsides. The former causes confusion and extra > setup work for VM admins creating disk images. The latter > places an extra burden to re-implement much of the QEMU > chardev backends logic in libvirt or even higher level > mgmt apps and adds extra hops in the data transfer path. > > A simpler approach that is satisfactory for many use > cases is to allow the QEMU chardev backends to have a > "logfile" property associated with them. > > $QEMU -chardev socket,host=localhost,port=9000,\ > server=on,nowait,id-charserial0,\ > logfile=/var/log/libvirt/qemu/test-serial0.log > -device isa-serial,chardev=charserial0,id=serial0 > > This patch introduces a 'ChardevCommon' struct which > is setup as a base for all the ChardevBackend types. > Ideally this would be registered directly as a base > against ChardevBackend, rather than each type, but > the QAPI generator doesn't allow that since the > ChardevBackend is a non-discriminated union. The > ChardevCommon struct provides the optional 'logfile' > parameter, as well as 'logappend' which controls > whether QEMU truncates or appends (default truncate). > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > Message-Id: <1452516281-27519-1-git-send-email-berrange@redhat.com> > [Call qemu_chr_parse_common if cd->parse is NULL. - Paolo] > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > backends/baum.c | 7 +- > backends/msmouse.c | 6 +- > gdbstub.c | 3 +- > include/sysemu/char.h | 9 +- > qapi-schema.json | 49 +++++++--- > qemu-char.c | 248 ++++++++++++++++++++++++++++++++++++++++++-------- > qemu-options.hx | 48 ++++++---- > spice-qemu-char.c | 20 +++- > ui/console.c | 6 +- > 9 files changed, 313 insertions(+), 83 deletions(-) > > diff --git a/backends/baum.c b/backends/baum.c > index 723c658..ba32b61 100644 > --- a/backends/baum.c > +++ b/backends/baum.c > @@ -566,6 +566,7 @@ static CharDriverState *chr_baum_init(const char *id, > ChardevReturn *ret, > Error **errp) > { > + ChardevCommon *common = qapi_ChardevDummy_base(backend->u.braille); > BaumDriverState *baum; > CharDriverState *chr; > brlapi_handle_t *handle; > @@ -576,8 +577,12 @@ static CharDriverState *chr_baum_init(const char *id, > #endif > int tty; > > + chr = qemu_chr_alloc(common, errp); > + if (!chr) { > + return NULL; > + } > baum = g_malloc0(sizeof(BaumDriverState)); > - baum->chr = chr = qemu_chr_alloc(); > + baum->chr = chr; > > chr->opaque = baum; > chr->chr_write = baum_write; > diff --git a/backends/msmouse.c b/backends/msmouse.c > index 0126fa0..476dab5 100644 > --- a/backends/msmouse.c > +++ b/backends/msmouse.c > @@ -68,9 +68,13 @@ static CharDriverState *qemu_chr_open_msmouse(const char *id, > ChardevReturn *ret, > Error **errp) > { > + ChardevCommon *common = qapi_ChardevDummy_base(backend->u.msmouse); > CharDriverState *chr; > > - chr = qemu_chr_alloc(); > + chr = qemu_chr_alloc(common, errp); > + if (!chr) { > + return NULL; > + } > chr->chr_write = msmouse_chr_write; > chr->chr_close = msmouse_chr_close; > chr->explicit_be_open = true; > diff --git a/gdbstub.c b/gdbstub.c > index 9c29aa0..1a84c1a 100644 > --- a/gdbstub.c > +++ b/gdbstub.c > @@ -1732,6 +1732,7 @@ int gdbserver_start(const char *device) > char gdbstub_device_name[128]; > CharDriverState *chr = NULL; > CharDriverState *mon_chr; > + ChardevCommon common = { 0 }; > > if (!device) > return -1; > @@ -1768,7 +1769,7 @@ int gdbserver_start(const char *device) > qemu_add_vm_change_state_handler(gdb_vm_state_change, NULL); > > /* Initialize a monitor terminal for gdb */ > - mon_chr = qemu_chr_alloc(); > + mon_chr = qemu_chr_alloc(&common, &error_abort); > mon_chr->chr_write = gdb_monitor_write; > monitor_init(mon_chr, 0); > } else { > diff --git a/include/sysemu/char.h b/include/sysemu/char.h > index aff193f..598dd2b 100644 > --- a/include/sysemu/char.h > +++ b/include/sysemu/char.h > @@ -77,6 +77,7 @@ struct CharDriverState { > void *opaque; > char *label; > char *filename; > + int logfd; > int be_open; > int fe_open; > int explicit_fe_open; > @@ -89,13 +90,15 @@ struct CharDriverState { > }; > > /** > - * @qemu_chr_alloc: > + * qemu_chr_alloc: > + * @backend: the common backend config > + * @errp: pointer to a NULL-initialized error object > * > * Allocate and initialize a new CharDriverState. > * > - * Returns: a newly allocated CharDriverState. > + * Returns: a newly allocated CharDriverState, or NULL on error. > */ > -CharDriverState *qemu_chr_alloc(void); > +CharDriverState *qemu_chr_alloc(ChardevCommon *backend, Error **errp); > > /** > * @qemu_chr_new_from_opts: > diff --git a/qapi-schema.json b/qapi-schema.json > index 2e31733..dabb5ce 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -3093,6 +3093,21 @@ > ## > { 'command': 'screendump', 'data': {'filename': 'str'} } > > + > +## > +# @ChardevCommon: > +# > +# Configuration shared across all chardev backends > +# > +# @logfile: #optional The name of a logfile to save output > +# @logappend: #optional true to append instead of truncate > +# (default to false to truncate) > +# > +# Since: 2.6 > +## > +{ 'struct': 'ChardevCommon', 'data': { '*logfile': 'str', > + '*logappend': 'bool' } } > + > ## > # @ChardevFile: > # > @@ -3107,7 +3122,8 @@ > ## > { 'struct': 'ChardevFile', 'data': { '*in' : 'str', > 'out' : 'str', > - '*append': 'bool' } } > + '*append': 'bool' }, > + 'base': 'ChardevCommon' } > > ## > # @ChardevHostdev: > @@ -3120,7 +3136,8 @@ > # > # Since: 1.4 > ## > -{ 'struct': 'ChardevHostdev', 'data': { 'device' : 'str' } } > +{ 'struct': 'ChardevHostdev', 'data': { 'device' : 'str' }, > + 'base': 'ChardevCommon' } > > ## > # @ChardevSocket: > @@ -3147,7 +3164,8 @@ > '*wait' : 'bool', > '*nodelay' : 'bool', > '*telnet' : 'bool', > - '*reconnect' : 'int' } } > + '*reconnect' : 'int' }, > + 'base': 'ChardevCommon' } > > ## > # @ChardevUdp: > @@ -3160,7 +3178,8 @@ > # Since: 1.5 > ## > { 'struct': 'ChardevUdp', 'data': { 'remote' : 'SocketAddress', > - '*local' : 'SocketAddress' } } > + '*local' : 'SocketAddress' }, > + 'base': 'ChardevCommon' } > > ## > # @ChardevMux: > @@ -3171,7 +3190,8 @@ > # > # Since: 1.5 > ## > -{ 'struct': 'ChardevMux', 'data': { 'chardev' : 'str' } } > +{ 'struct': 'ChardevMux', 'data': { 'chardev' : 'str' }, > + 'base': 'ChardevCommon' } > > ## > # @ChardevStdio: > @@ -3184,7 +3204,9 @@ > # > # Since: 1.5 > ## > -{ 'struct': 'ChardevStdio', 'data': { '*signal' : 'bool' } } > +{ 'struct': 'ChardevStdio', 'data': { '*signal' : 'bool' }, > + 'base': 'ChardevCommon' } > + > > ## > # @ChardevSpiceChannel: > @@ -3195,7 +3217,8 @@ > # > # Since: 1.5 > ## > -{ 'struct': 'ChardevSpiceChannel', 'data': { 'type' : 'str' } } > +{ 'struct': 'ChardevSpiceChannel', 'data': { 'type' : 'str' }, > + 'base': 'ChardevCommon' } > > ## > # @ChardevSpicePort: > @@ -3206,7 +3229,8 @@ > # > # Since: 1.5 > ## > -{ 'struct': 'ChardevSpicePort', 'data': { 'fqdn' : 'str' } } > +{ 'struct': 'ChardevSpicePort', 'data': { 'fqdn' : 'str' }, > + 'base': 'ChardevCommon' } > > ## > # @ChardevVC: > @@ -3223,7 +3247,8 @@ > { 'struct': 'ChardevVC', 'data': { '*width' : 'int', > '*height' : 'int', > '*cols' : 'int', > - '*rows' : 'int' } } > + '*rows' : 'int' }, > + 'base': 'ChardevCommon' } > > ## > # @ChardevRingbuf: > @@ -3234,7 +3259,8 @@ > # > # Since: 1.5 > ## > -{ 'struct': 'ChardevRingbuf', 'data': { '*size' : 'int' } } > +{ 'struct': 'ChardevRingbuf', 'data': { '*size' : 'int' }, > + 'base': 'ChardevCommon' } > > ## > # @ChardevBackend: > @@ -3243,7 +3269,8 @@ > # > # Since: 1.4 (testdev since 2.2) > ## > -{ 'struct': 'ChardevDummy', 'data': { } } > +{ 'struct': 'ChardevDummy', 'data': { }, > + 'base': 'ChardevCommon' } > > { 'union': 'ChardevBackend', 'data': { 'file' : 'ChardevFile', > 'serial' : 'ChardevHostdev', > diff --git a/qemu-char.c b/qemu-char.c > index d7be185..11caa56 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -159,10 +159,33 @@ static int sockaddr_to_str(char *dest, int max_len, > static QTAILQ_HEAD(CharDriverStateHead, CharDriverState) chardevs = > QTAILQ_HEAD_INITIALIZER(chardevs); > > -CharDriverState *qemu_chr_alloc(void) > +static void qemu_chr_free_common(CharDriverState *chr); > + > +CharDriverState *qemu_chr_alloc(ChardevCommon *backend, Error **errp) > { > CharDriverState *chr = g_malloc0(sizeof(CharDriverState)); > qemu_mutex_init(&chr->chr_write_lock); > + > + if (backend->has_logfile) { > + int flags = O_WRONLY | O_CREAT; > + if (backend->has_logappend && > + backend->logappend) { > + flags |= O_APPEND; > + } else { > + flags |= O_TRUNC; > + } > + chr->logfd = qemu_open(backend->logfile, flags, 0666); > + if (chr->logfd < 0) { > + error_setg_errno(errp, errno, > + "Unable to open logfile %s", > + backend->logfile); > + g_free(chr); > + return NULL; > + } > + } else { > + chr->logfd = -1; > + } > + > return chr; > } > > @@ -188,12 +211,45 @@ void qemu_chr_be_generic_open(CharDriverState *s) > qemu_chr_be_event(s, CHR_EVENT_OPENED); > } > > + > +/* Not reporting errors from writing to logfile, as logs are > + * defined to be "best effort" only */ > +static void qemu_chr_fe_write_log(CharDriverState *s, > + const uint8_t *buf, size_t len) > +{ > + size_t done = 0; > + ssize_t ret; > + > + if (s->logfd < 0) { > + return; > + } > + > + while (done < len) { > + do { > + ret = write(s->logfd, buf + done, len - done); > + if (ret == -1 && errno == EAGAIN) { > + g_usleep(100); > + } > + } while (ret == -1 && errno == EAGAIN); > + > + if (ret <= 0) { > + return; > + } > + done += ret; > + } > +} > + > int qemu_chr_fe_write(CharDriverState *s, const uint8_t *buf, int len) > { > int ret; > > qemu_mutex_lock(&s->chr_write_lock); > ret = s->chr_write(s, buf, len); > + > + if (ret > 0) { > + qemu_chr_fe_write_log(s, buf, ret); > + } > + > qemu_mutex_unlock(&s->chr_write_lock); > return ret; > } > @@ -218,6 +274,10 @@ int qemu_chr_fe_write_all(CharDriverState *s, const uint8_t *buf, int len) > > offset += res; > } > + if (offset > 0) { > + qemu_chr_fe_write_log(s, buf, offset); > + } > + > qemu_mutex_unlock(&s->chr_write_lock); > > if (res < 0) { > @@ -365,8 +425,12 @@ static CharDriverState *qemu_chr_open_null(const char *id, > Error **errp) > { > CharDriverState *chr; > + ChardevCommon *common = qapi_ChardevDummy_base(backend->u.null); > > - chr = qemu_chr_alloc(); > + chr = qemu_chr_alloc(common, errp); > + if (!chr) { > + return NULL; > + } > chr->chr_write = null_chr_write; > chr->explicit_be_open = true; > return chr; > @@ -665,6 +729,7 @@ static CharDriverState *qemu_chr_open_mux(const char *id, > ChardevMux *mux = backend->u.mux; > CharDriverState *chr, *drv; > MuxDriver *d; > + ChardevCommon *common = qapi_ChardevMux_base(backend->u.mux); > > drv = qemu_chr_find(mux->chardev); > if (drv == NULL) { > @@ -672,7 +737,10 @@ static CharDriverState *qemu_chr_open_mux(const char *id, > return NULL; > } > > - chr = qemu_chr_alloc(); > + chr = qemu_chr_alloc(common, errp); > + if (!chr) { > + return NULL; > + } > d = g_new0(MuxDriver, 1); > > chr->opaque = d; > @@ -975,12 +1043,16 @@ static void fd_chr_close(struct CharDriverState *chr) > } > > /* open a character device to a unix fd */ > -static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out) > +static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out, > + ChardevCommon *backend, Error **errp) > { > CharDriverState *chr; > FDCharDriver *s; > > - chr = qemu_chr_alloc(); > + chr = qemu_chr_alloc(backend, errp); > + if (!chr) { > + return NULL; > + } > s = g_new0(FDCharDriver, 1); > s->fd_in = io_channel_from_fd(fd_in); > s->fd_out = io_channel_from_fd(fd_out); > @@ -1005,6 +1077,7 @@ static CharDriverState *qemu_chr_open_pipe(const char *id, > char filename_in[CHR_MAX_FILENAME_SIZE]; > char filename_out[CHR_MAX_FILENAME_SIZE]; > const char *filename = opts->device; > + ChardevCommon *common = qapi_ChardevHostdev_base(backend->u.pipe); > > snprintf(filename_in, CHR_MAX_FILENAME_SIZE, "%s.in", filename); > snprintf(filename_out, CHR_MAX_FILENAME_SIZE, "%s.out", filename); > @@ -1021,7 +1094,7 @@ static CharDriverState *qemu_chr_open_pipe(const char *id, > return NULL; > } > } > - return qemu_chr_open_fd(fd_in, fd_out); > + return qemu_chr_open_fd(fd_in, fd_out, common, errp); > } > > /* init terminal so that we can grab keys */ > @@ -1081,6 +1154,7 @@ static CharDriverState *qemu_chr_open_stdio(const char *id, > ChardevStdio *opts = backend->u.stdio; > CharDriverState *chr; > struct sigaction act; > + ChardevCommon *common = qapi_ChardevStdio_base(backend->u.stdio); > > if (is_daemonized()) { > error_setg(errp, "cannot use stdio with -daemonize"); > @@ -1102,7 +1176,7 @@ static CharDriverState *qemu_chr_open_stdio(const char *id, > act.sa_handler = term_stdio_handler; > sigaction(SIGCONT, &act, NULL); > > - chr = qemu_chr_open_fd(0, 1); > + chr = qemu_chr_open_fd(0, 1, common, errp); > chr->chr_close = qemu_chr_close_stdio; > chr->chr_set_echo = qemu_chr_set_echo_stdio; > if (opts->has_signal) { > @@ -1324,6 +1398,7 @@ static CharDriverState *qemu_chr_open_pty(const char *id, > PtyCharDriver *s; > int master_fd, slave_fd; > char pty_name[PATH_MAX]; > + ChardevCommon *common = qapi_ChardevDummy_base(backend->u.pty); > > master_fd = qemu_openpty_raw(&slave_fd, pty_name); > if (master_fd < 0) { > @@ -1334,7 +1409,11 @@ static CharDriverState *qemu_chr_open_pty(const char *id, > close(slave_fd); > qemu_set_nonblock(master_fd); > > - chr = qemu_chr_alloc(); > + chr = qemu_chr_alloc(common, errp); > + if (!chr) { > + close(master_fd); > + return NULL; > + } > > chr->filename = g_strdup_printf("pty:%s", pty_name); > ret->pty = g_strdup(pty_name); > @@ -1557,12 +1636,14 @@ static void qemu_chr_close_tty(CharDriverState *chr) > } > } > > -static CharDriverState *qemu_chr_open_tty_fd(int fd) > +static CharDriverState *qemu_chr_open_tty_fd(int fd, > + ChardevCommon *backend, > + Error **errp) > { > CharDriverState *chr; > > tty_serial_init(fd, 115200, 'N', 8, 1); > - chr = qemu_chr_open_fd(fd, fd); > + chr = qemu_chr_open_fd(fd, fd, backend, errp); > chr->chr_ioctl = tty_serial_ioctl; > chr->chr_close = qemu_chr_close_tty; > return chr; > @@ -1682,7 +1763,9 @@ static void pp_close(CharDriverState *chr) > qemu_chr_be_event(chr, CHR_EVENT_CLOSED); > } > > -static CharDriverState *qemu_chr_open_pp_fd(int fd, Error **errp) > +static CharDriverState *qemu_chr_open_pp_fd(int fd, > + ChardevCommon *backend, > + Error **errp) > { > CharDriverState *chr; > ParallelCharDriver *drv; > @@ -1697,7 +1780,10 @@ static CharDriverState *qemu_chr_open_pp_fd(int fd, Error **errp) > drv->fd = fd; > drv->mode = IEEE1284_MODE_COMPAT; > > - chr = qemu_chr_alloc(); > + chr = qemu_chr_alloc(backend, errp); > + if (!chr) { > + return NULL; > + } > chr->chr_write = null_chr_write; > chr->chr_ioctl = pp_ioctl; > chr->chr_close = pp_close; > @@ -1748,11 +1834,16 @@ static int pp_ioctl(CharDriverState *chr, int cmd, void *arg) > return 0; > } > > -static CharDriverState *qemu_chr_open_pp_fd(int fd, Error **errp) > +static CharDriverState *qemu_chr_open_pp_fd(int fd, > + ChardevBackend *backend, > + Error **errp) > { > CharDriverState *chr; > > - chr = qemu_chr_alloc(); > + chr = qemu_chr_alloc(common, errp); > + if (!chr) { > + return NULL; > + } > chr->opaque = (void *)(intptr_t)fd; > chr->chr_write = null_chr_write; > chr->chr_ioctl = pp_ioctl; > @@ -1978,12 +2069,16 @@ static int win_chr_poll(void *opaque) > } > > static CharDriverState *qemu_chr_open_win_path(const char *filename, > + ChardevCommon *backend, > Error **errp) > { > CharDriverState *chr; > WinCharState *s; > > - chr = qemu_chr_alloc(); > + chr = qemu_chr_alloc(backend, errp); > + if (!chr) { > + return NULL; > + } > s = g_new0(WinCharState, 1); > chr->opaque = s; > chr->chr_write = win_chr_write; > @@ -1991,7 +2086,7 @@ static CharDriverState *qemu_chr_open_win_path(const char *filename, > > if (win_chr_init(chr, filename, errp) < 0) { > g_free(s); > - g_free(chr); > + qemu_chr_free_common(chr); > return NULL; > } > return chr; > @@ -2086,8 +2181,12 @@ static CharDriverState *qemu_chr_open_pipe(const char *id, > const char *filename = opts->device; > CharDriverState *chr; > WinCharState *s; > + ChardevCommon *common = qapi_ChardevHostdev_base(backend->u.pipe); > > - chr = qemu_chr_alloc(); > + chr = qemu_chr_alloc(common, errp); > + if (!chr) { > + return NULL; > + } > s = g_new0(WinCharState, 1); > chr->opaque = s; > chr->chr_write = win_chr_write; > @@ -2095,18 +2194,23 @@ static CharDriverState *qemu_chr_open_pipe(const char *id, > > if (win_chr_pipe_init(chr, filename, errp) < 0) { > g_free(s); > - g_free(chr); > + qemu_chr_free_common(chr); > return NULL; > } > return chr; > } > > -static CharDriverState *qemu_chr_open_win_file(HANDLE fd_out) > +static CharDriverState *qemu_chr_open_win_file(HANDLE fd_out, > + ChardevCommon *backend, > + Error **errp) > { > CharDriverState *chr; > WinCharState *s; > > - chr = qemu_chr_alloc(); > + chr = qemu_chr_alloc(backend, errp); > + if (!chr) { > + return NULL; > + } > s = g_new0(WinCharState, 1); > s->hcom = fd_out; > chr->opaque = s; > @@ -2119,7 +2223,9 @@ static CharDriverState *qemu_chr_open_win_con(const char *id, > ChardevReturn *ret, > Error **errp) > { > - return qemu_chr_open_win_file(GetStdHandle(STD_OUTPUT_HANDLE)); > + ChardevCommon *common = qapi_ChardevDummy_base(backend->u.console); > + return qemu_chr_open_win_file(GetStdHandle(STD_OUTPUT_HANDLE), > + common, errp); > } > > static int win_stdio_write(CharDriverState *chr, const uint8_t *buf, int len) > @@ -2267,8 +2373,12 @@ static CharDriverState *qemu_chr_open_stdio(const char *id, > WinStdioCharState *stdio; > DWORD dwMode; > int is_console = 0; > + ChardevCommon *common = qapi_ChardevStdio_base(backend->u.stdio); > > - chr = qemu_chr_alloc(); > + chr = qemu_chr_alloc(common, errp); > + if (!chr) { > + return NULL; > + } > stdio = g_new0(WinStdioCharState, 1); > > stdio->hStdIn = GetStdHandle(STD_INPUT_HANDLE); > @@ -2440,12 +2550,17 @@ static void udp_chr_close(CharDriverState *chr) > qemu_chr_be_event(chr, CHR_EVENT_CLOSED); > } > > -static CharDriverState *qemu_chr_open_udp_fd(int fd) > +static CharDriverState *qemu_chr_open_udp_fd(int fd, > + ChardevCommon *backend, > + Error **errp) > { > CharDriverState *chr = NULL; > NetCharDriver *s = NULL; > > - chr = qemu_chr_alloc(); > + chr = qemu_chr_alloc(backend, errp); > + if (!chr) { > + return NULL; > + } > s = g_new0(NetCharDriver, 1); > > s->fd = fd; > @@ -2856,7 +2971,7 @@ static int tcp_chr_sync_read(CharDriverState *chr, const uint8_t *buf, int len) > #ifndef _WIN32 > CharDriverState *qemu_chr_open_eventfd(int eventfd) > { > - CharDriverState *chr = qemu_chr_open_fd(eventfd, eventfd); > + CharDriverState *chr = qemu_chr_open_fd(eventfd, eventfd, NULL, NULL); > > if (chr) { > chr->avail_connections = 1; > @@ -3138,10 +3253,14 @@ static CharDriverState *qemu_chr_open_ringbuf(const char *id, > Error **errp) > { > ChardevRingbuf *opts = backend->u.ringbuf; > + ChardevCommon *common = qapi_ChardevRingbuf_base(backend->u.ringbuf); > CharDriverState *chr; > RingBufCharDriver *d; > > - chr = qemu_chr_alloc(); > + chr = qemu_chr_alloc(common, errp); > + if (!chr) { > + return NULL; > + } > d = g_malloc(sizeof(*d)); > > d->size = opts->has_size ? opts->size : 65536; > @@ -3164,7 +3283,7 @@ static CharDriverState *qemu_chr_open_ringbuf(const char *id, > > fail: > g_free(d); > - g_free(chr); > + qemu_chr_free_common(chr); > return NULL; > } > > @@ -3408,6 +3527,18 @@ fail: > return NULL; > } > > +static void qemu_chr_parse_common(QemuOpts *opts, ChardevCommon *backend) > +{ > + const char *logfile = qemu_opt_get(opts, "logfile"); > + > + backend->has_logfile = logfile != NULL; > + backend->logfile = logfile ? g_strdup(logfile) : NULL; > + > + backend->has_logappend = true; > + backend->logappend = qemu_opt_get_bool(opts, "logappend", false); > +} > + > + > static void qemu_chr_parse_file_out(QemuOpts *opts, ChardevBackend *backend, > Error **errp) > { > @@ -3418,6 +3549,7 @@ static void qemu_chr_parse_file_out(QemuOpts *opts, ChardevBackend *backend, > return; > } > backend->u.file = g_new0(ChardevFile, 1); > + qemu_chr_parse_common(opts, qapi_ChardevFile_base(backend->u.file)); > backend->u.file->out = g_strdup(path); > > backend->u.file->has_append = true; > @@ -3428,6 +3560,7 @@ static void qemu_chr_parse_stdio(QemuOpts *opts, ChardevBackend *backend, > Error **errp) > { > backend->u.stdio = g_new0(ChardevStdio, 1); > + qemu_chr_parse_common(opts, qapi_ChardevStdio_base(backend->u.stdio)); > backend->u.stdio->has_signal = true; > backend->u.stdio->signal = qemu_opt_get_bool(opts, "signal", true); > } > @@ -3443,6 +3576,7 @@ static void qemu_chr_parse_serial(QemuOpts *opts, ChardevBackend *backend, > return; > } > backend->u.serial = g_new0(ChardevHostdev, 1); > + qemu_chr_parse_common(opts, qapi_ChardevHostdev_base(backend->u.serial)); > backend->u.serial->device = g_strdup(device); > } > #endif > @@ -3458,6 +3592,7 @@ static void qemu_chr_parse_parallel(QemuOpts *opts, ChardevBackend *backend, > return; > } > backend->u.parallel = g_new0(ChardevHostdev, 1); > + qemu_chr_parse_common(opts, qapi_ChardevHostdev_base(backend->u.parallel)); > backend->u.parallel->device = g_strdup(device); > } > #endif > @@ -3472,6 +3607,7 @@ static void qemu_chr_parse_pipe(QemuOpts *opts, ChardevBackend *backend, > return; > } > backend->u.pipe = g_new0(ChardevHostdev, 1); > + qemu_chr_parse_common(opts, qapi_ChardevHostdev_base(backend->u.pipe)); > backend->u.pipe->device = g_strdup(device); > } > > @@ -3481,6 +3617,7 @@ static void qemu_chr_parse_ringbuf(QemuOpts *opts, ChardevBackend *backend, > int val; > > backend->u.ringbuf = g_new0(ChardevRingbuf, 1); > + qemu_chr_parse_common(opts, qapi_ChardevRingbuf_base(backend->u.ringbuf)); > > val = qemu_opt_get_size(opts, "size", 0); > if (val != 0) { > @@ -3499,6 +3636,7 @@ static void qemu_chr_parse_mux(QemuOpts *opts, ChardevBackend *backend, > return; > } > backend->u.mux = g_new0(ChardevMux, 1); > + qemu_chr_parse_common(opts, qapi_ChardevMux_base(backend->u.mux)); > backend->u.mux->chardev = g_strdup(chardev); > } > > @@ -3527,6 +3665,7 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend, > } > > backend->u.socket = g_new0(ChardevSocket, 1); > + qemu_chr_parse_common(opts, qapi_ChardevSocket_base(backend->u.socket)); > > backend->u.socket->has_nodelay = true; > backend->u.socket->nodelay = do_nodelay; > @@ -3588,6 +3727,7 @@ static void qemu_chr_parse_udp(QemuOpts *opts, ChardevBackend *backend, > } > > backend->u.udp = g_new0(ChardevUdp, 1); > + qemu_chr_parse_common(opts, qapi_ChardevUdp_base(backend->u.udp)); > > addr = g_new0(SocketAddress, 1); > addr->type = SOCKET_ADDRESS_KIND_INET; > @@ -3687,7 +3827,12 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, > error_propagate(errp, local_err); > goto qapi_out; > } > + } else { > + ChardevCommon *cc = g_new0(ChardevCommon, 1); > + qemu_chr_parse_common(opts, cc); > + backend->u.data = cc; > } > + > ret = qmp_chardev_add(bid ? bid : id, backend, errp); > if (!ret) { > goto qapi_out; > @@ -3819,17 +3964,25 @@ void qemu_chr_fe_release(CharDriverState *s) > s->avail_connections++; > } > > -void qemu_chr_free(CharDriverState *chr) > +static void qemu_chr_free_common(CharDriverState *chr) > { > - if (chr->chr_close) { > - chr->chr_close(chr); > - } > g_free(chr->filename); > g_free(chr->label); > qemu_opts_del(chr->opts); > + if (chr->logfd != -1) { > + close(chr->logfd); > + } > g_free(chr); > } > > +void qemu_chr_free(CharDriverState *chr) > +{ > + if (chr->chr_close) { > + chr->chr_close(chr); > + } > + qemu_chr_free_common(chr); > +} > + > void qemu_chr_delete(CharDriverState *chr) > { > QTAILQ_REMOVE(&chardevs, chr, next); > @@ -3982,6 +4135,12 @@ QemuOptsList qemu_chardev_opts = { > },{ > .name = "append", > .type = QEMU_OPT_BOOL, > + },{ > + .name = "logfile", > + .type = QEMU_OPT_STRING, > + },{ > + .name = "logappend", > + .type = QEMU_OPT_BOOL, > }, > { /* end of list */ } > }, > @@ -3995,6 +4154,7 @@ static CharDriverState *qmp_chardev_open_file(const char *id, > Error **errp) > { > ChardevFile *file = backend->u.file; > + ChardevCommon *common = qapi_ChardevFile_base(backend->u.file); > HANDLE out; > > if (file->has_in) { > @@ -4008,7 +4168,7 @@ static CharDriverState *qmp_chardev_open_file(const char *id, > error_setg(errp, "open %s failed", file->out); > return NULL; > } > - return qemu_chr_open_win_file(out); > + return qemu_chr_open_win_file(out, common, errp); > } > > static CharDriverState *qmp_chardev_open_serial(const char *id, > @@ -4017,7 +4177,8 @@ static CharDriverState *qmp_chardev_open_serial(const char *id, > Error **errp) > { > ChardevHostdev *serial = backend->u.serial; > - return qemu_chr_open_win_path(serial->device, errp); > + ChardevCommon *common = qapi_ChardevHostdev_base(backend->u.serial); > + return qemu_chr_open_win_path(serial->device, common, errp); > } > > #else /* WIN32 */ > @@ -4040,6 +4201,7 @@ static CharDriverState *qmp_chardev_open_file(const char *id, > Error **errp) > { > ChardevFile *file = backend->u.file; > + ChardevCommon *common = qapi_ChardevFile_base(backend->u.file); > int flags, in = -1, out; > > flags = O_WRONLY | O_CREAT | O_BINARY; > @@ -4063,7 +4225,7 @@ static CharDriverState *qmp_chardev_open_file(const char *id, > } > } > > - return qemu_chr_open_fd(in, out); > + return qemu_chr_open_fd(in, out, common, errp); > } > > #ifdef HAVE_CHARDEV_SERIAL > @@ -4073,6 +4235,7 @@ static CharDriverState *qmp_chardev_open_serial(const char *id, > Error **errp) > { > ChardevHostdev *serial = backend->u.serial; > + ChardevCommon *common = qapi_ChardevHostdev_base(backend->u.serial); > int fd; > > fd = qmp_chardev_open_file_source(serial->device, O_RDWR, errp); > @@ -4080,7 +4243,7 @@ static CharDriverState *qmp_chardev_open_serial(const char *id, > return NULL; > } > qemu_set_nonblock(fd); > - return qemu_chr_open_tty_fd(fd); > + return qemu_chr_open_tty_fd(fd, common, errp); > } > #endif > > @@ -4091,13 +4254,14 @@ static CharDriverState *qmp_chardev_open_parallel(const char *id, > Error **errp) > { > ChardevHostdev *parallel = backend->u.parallel; > + ChardevCommon *common = qapi_ChardevHostdev_base(backend->u.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, errp); > + return qemu_chr_open_pp_fd(fd, common, errp); > } > #endif > > @@ -4142,8 +4306,12 @@ static CharDriverState *qmp_chardev_open_socket(const char *id, > bool is_telnet = sock->has_telnet ? sock->telnet : false; > bool is_waitconnect = sock->has_wait ? sock->wait : false; > int64_t reconnect = sock->has_reconnect ? sock->reconnect : 0; > + ChardevCommon *common = qapi_ChardevSocket_base(backend->u.socket); > > - chr = qemu_chr_alloc(); > + chr = qemu_chr_alloc(common, errp); > + if (!chr) { > + return NULL; > + } > s = g_new0(TCPCharDriver, 1); > > s->fd = -1; > @@ -4182,8 +4350,7 @@ static CharDriverState *qmp_chardev_open_socket(const char *id, > socket_try_connect(chr); > } else if (!qemu_chr_open_socket_fd(chr, errp)) { > g_free(s); > - g_free(chr->filename); > - g_free(chr); > + qemu_chr_free_common(chr); > return NULL; > } > > @@ -4203,13 +4370,14 @@ static CharDriverState *qmp_chardev_open_udp(const char *id, > Error **errp) > { > ChardevUdp *udp = backend->u.udp; > + ChardevCommon *common = qapi_ChardevUdp_base(backend->u.udp); > int fd; > > fd = socket_dgram(udp->remote, udp->local, errp); > if (fd < 0) { > return NULL; > } > - return qemu_chr_open_udp_fd(fd); > + return qemu_chr_open_udp_fd(fd, common, errp); > } > > ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend, > diff --git a/qemu-options.hx b/qemu-options.hx > index 215d00d..b4763ba 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -2089,40 +2089,43 @@ The general form of a character device option is: > ETEXI > > DEF("chardev", HAS_ARG, QEMU_OPTION_chardev, > - "-chardev null,id=id[,mux=on|off]\n" > + "-chardev null,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" > "-chardev socket,id=id[,host=host],port=port[,to=to][,ipv4][,ipv6][,nodelay][,reconnect=seconds]\n" > - " [,server][,nowait][,telnet][,reconnect=seconds][,mux=on|off] (tcp)\n" > - "-chardev socket,id=id,path=path[,server][,nowait][,telnet][,reconnect=seconds][,mux=on|off] (unix)\n" > + " [,server][,nowait][,telnet][,reconnect=seconds][,mux=on|off]\n" > + " [,logfile=PATH][,logappend=on|off] (tcp)\n" > + "-chardev socket,id=id,path=path[,server][,nowait][,telnet][,reconnect=seconds]\n" > + " [,mux=on|off][,logfile=PATH][,logappend=on|off] (unix)\n" > "-chardev udp,id=id[,host=host],port=port[,localaddr=localaddr]\n" > " [,localport=localport][,ipv4][,ipv6][,mux=on|off]\n" > - "-chardev msmouse,id=id[,mux=on|off]\n" > + " [,logfile=PATH][,logappend=on|off]\n" > + "-chardev msmouse,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" > "-chardev vc,id=id[[,width=width][,height=height]][[,cols=cols][,rows=rows]]\n" > - " [,mux=on|off]\n" > - "-chardev ringbuf,id=id[,size=size]\n" > - "-chardev file,id=id,path=path[,mux=on|off]\n" > - "-chardev pipe,id=id,path=path[,mux=on|off]\n" > + " [,mux=on|off][,logfile=PATH][,logappend=on|off]\n" > + "-chardev ringbuf,id=id[,size=size][,logfile=PATH][,logappend=on|off]\n" > + "-chardev file,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" > + "-chardev pipe,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" > #ifdef _WIN32 > - "-chardev console,id=id[,mux=on|off]\n" > - "-chardev serial,id=id,path=path[,mux=on|off]\n" > + "-chardev console,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" > + "-chardev serial,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" > #else > - "-chardev pty,id=id[,mux=on|off]\n" > - "-chardev stdio,id=id[,mux=on|off][,signal=on|off]\n" > + "-chardev pty,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" > + "-chardev stdio,id=id[,mux=on|off][,signal=on|off][,logfile=PATH][,logappend=on|off]\n" > #endif > #ifdef CONFIG_BRLAPI > - "-chardev braille,id=id[,mux=on|off]\n" > + "-chardev braille,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" > #endif > #if defined(__linux__) || defined(__sun__) || defined(__FreeBSD__) \ > || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__) > - "-chardev serial,id=id,path=path[,mux=on|off]\n" > - "-chardev tty,id=id,path=path[,mux=on|off]\n" > + "-chardev serial,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" > + "-chardev tty,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" > #endif > #if defined(__linux__) || defined(__FreeBSD__) || defined(__DragonFly__) > - "-chardev parallel,id=id,path=path[,mux=on|off]\n" > - "-chardev parport,id=id,path=path[,mux=on|off]\n" > + "-chardev parallel,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" > + "-chardev parport,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" > #endif > #if defined(CONFIG_SPICE) > - "-chardev spicevmc,id=id,name=name[,debug=debug]\n" > - "-chardev spiceport,id=id,name=name[,debug=debug]\n" > + "-chardev spicevmc,id=id,name=name[,debug=debug][,logfile=PATH][,logappend=on|off]\n" > + "-chardev spiceport,id=id,name=name[,debug=debug][,logfile=PATH][,logappend=on|off]\n" > #endif > , QEMU_ARCH_ALL > ) > @@ -2158,7 +2161,12 @@ A character device may be used in multiplexing mode by multiple front-ends. > The key sequence of @key{Control-a} and @key{c} will rotate the input focus > between attached front-ends. Specify @option{mux=on} to enable this mode. > > -Options to each backend are described below. > +Every backend supports the @option{logfile} option, which supplies the path > +to a file to record all data transmitted via the backend. The @option{logappend} > +option controls whether the log file will be truncated or appended to when > +opened. > + > +Further options to each backend are described below. > > @item -chardev null ,id=@var{id} > A void device. This device will not emit any data, and will drop any data it > diff --git a/spice-qemu-char.c b/spice-qemu-char.c > index e70e0f7..8951d7c 100644 > --- a/spice-qemu-char.c > +++ b/spice-qemu-char.c > @@ -271,13 +271,18 @@ static void spice_chr_accept_input(struct CharDriverState *chr) > } > > static CharDriverState *chr_open(const char *subtype, > - void (*set_fe_open)(struct CharDriverState *, int)) > - > + void (*set_fe_open)(struct CharDriverState *, > + int), > + ChardevCommon *backend, > + Error **errp) > { > CharDriverState *chr; > SpiceCharDriver *s; > > - chr = qemu_chr_alloc(); > + chr = qemu_chr_alloc(backend, errp); > + if (!chr) { > + return NULL; > + } > s = g_malloc0(sizeof(SpiceCharDriver)); > s->chr = chr; > s->active = false; > @@ -303,6 +308,7 @@ static CharDriverState *qemu_chr_open_spice_vmc(const char *id, > { > const char *type = backend->u.spicevmc->type; > const char **psubtype = spice_server_char_device_recognized_subtypes(); > + ChardevCommon *common = qapi_ChardevSpiceChannel_base(backend->u.spicevmc); > > for (; *psubtype != NULL; ++psubtype) { > if (strcmp(type, *psubtype) == 0) { > @@ -315,7 +321,7 @@ static CharDriverState *qemu_chr_open_spice_vmc(const char *id, > return NULL; > } > > - return chr_open(type, spice_vmc_set_fe_open); > + return chr_open(type, spice_vmc_set_fe_open, common, errp); > } > > #if SPICE_SERVER_VERSION >= 0x000c02 > @@ -325,6 +331,7 @@ static CharDriverState *qemu_chr_open_spice_port(const char *id, > Error **errp) > { > const char *name = backend->u.spiceport->fqdn; > + ChardevCommon *common = qapi_ChardevSpicePort_base(backend->u.spiceport); > CharDriverState *chr; > SpiceCharDriver *s; > > @@ -333,7 +340,10 @@ static CharDriverState *qemu_chr_open_spice_port(const char *id, > return NULL; > } > > - chr = chr_open("port", spice_port_set_fe_open); > + chr = chr_open("port", spice_port_set_fe_open, common, errp); > + if (!chr) { > + return NULL; > + } > s = chr->opaque; > s->sin.portname = g_strdup(name); > > diff --git a/ui/console.c b/ui/console.c > index 4b65c34..fe950c6 100644 > --- a/ui/console.c > +++ b/ui/console.c > @@ -1953,12 +1953,16 @@ static void text_console_do_init(CharDriverState *chr, DisplayState *ds) > > static CharDriverState *text_console_init(ChardevVC *vc, Error **errp) > { > + ChardevCommon *common = qapi_ChardevVC_base(vc); > CharDriverState *chr; > QemuConsole *s; > unsigned width = 0; > unsigned height = 0; > > - chr = qemu_chr_alloc(); > + chr = qemu_chr_alloc(common, errp); > + if (!chr) { > + return NULL; > + } > > if (vc->has_width) { > width = vc->width; >
On Thu, Jan 21, 2016 at 07:16:25AM +0100, Hervé Poussineau wrote: > Hi, > > This patch (commit d0d7708ba29cbcc343364a46bff981e0ff88366f) regresses the following command line: > qemu-system-i386 -nodefaults -chardev vc,id=mon0 -mon chardev=mon0 > > Before: > [nothing is print on console] > > After: > QEMU 2.5.50 monitor - type 'help' for more information > (qemu) > > In both cases, a working vc is created in the GTK+ UI. > > Reverting the commit (and fixing the trivial conflict) makes things > work again. Thanks for the heads-up, I'll investigate and submit a fix Regards, Daniel
"Daniel P. Berrange" <berrange@redhat.com> writes: > On Thu, Jan 21, 2016 at 07:16:25AM +0100, Hervé Poussineau wrote: >> Hi, >> >> This patch (commit d0d7708ba29cbcc343364a46bff981e0ff88366f) regresses the following command line: >> qemu-system-i386 -nodefaults -chardev vc,id=mon0 -mon chardev=mon0 >> >> Before: >> [nothing is print on console] >> >> After: >> QEMU 2.5.50 monitor - type 'help' for more information >> (qemu) >> >> In both cases, a working vc is created in the GTK+ UI. >> >> Reverting the commit (and fixing the trivial conflict) makes things >> work again. > > Thanks for the heads-up, I'll investigate and submit a fix For the record, it also breaks ivshmem-test when the slow tests are included. I'd be willing to test your fix; got a pointer for me?
On Fri, Feb 12, 2016 at 05:49:38PM +0100, Markus Armbruster wrote: > "Daniel P. Berrange" <berrange@redhat.com> writes: > > > On Thu, Jan 21, 2016 at 07:16:25AM +0100, Hervé Poussineau wrote: > >> Hi, > >> > >> This patch (commit d0d7708ba29cbcc343364a46bff981e0ff88366f) regresses the following command line: > >> qemu-system-i386 -nodefaults -chardev vc,id=mon0 -mon chardev=mon0 > >> > >> Before: > >> [nothing is print on console] > >> > >> After: > >> QEMU 2.5.50 monitor - type 'help' for more information > >> (qemu) > >> > >> In both cases, a working vc is created in the GTK+ UI. > >> > >> Reverting the commit (and fixing the trivial conflict) makes things > >> work again. > > > > Thanks for the heads-up, I'll investigate and submit a fix > > For the record, it also breaks ivshmem-test when the slow tests are > included. I'd be willing to test your fix; got a pointer for me? https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg02738.html Regards, Daniel
"Daniel P. Berrange" <berrange@redhat.com> writes: > On Fri, Feb 12, 2016 at 05:49:38PM +0100, Markus Armbruster wrote: >> "Daniel P. Berrange" <berrange@redhat.com> writes: >> >> > On Thu, Jan 21, 2016 at 07:16:25AM +0100, Hervé Poussineau wrote: >> >> Hi, >> >> >> >> This patch (commit d0d7708ba29cbcc343364a46bff981e0ff88366f) regresses the following command line: >> >> qemu-system-i386 -nodefaults -chardev vc,id=mon0 -mon chardev=mon0 >> >> >> >> Before: >> >> [nothing is print on console] >> >> >> >> After: >> >> QEMU 2.5.50 monitor - type 'help' for more information >> >> (qemu) >> >> >> >> In both cases, a working vc is created in the GTK+ UI. >> >> >> >> Reverting the commit (and fixing the trivial conflict) makes things >> >> work again. >> > >> > Thanks for the heads-up, I'll investigate and submit a fix >> >> For the record, it also breaks ivshmem-test when the slow tests are >> included. I'd be willing to test your fix; got a pointer for me? > > https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg02738.html No luck. Please try the following reproducer: $ make tests/ivshmem-test $ QTEST_QEMU_BINARY='x86_64-softmmu/qemu-system-x86_64' QTEST_QEMU_IMG=qemu-img MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM % 255 + 1))} gtester -k --verbose -m slow tests/ivshmem-test You can instead run make check SPEED=slow, but that runs many more tests.
On Fri, Feb 12, 2016 at 06:12:46PM +0100, Markus Armbruster wrote: > "Daniel P. Berrange" <berrange@redhat.com> writes: > > > On Fri, Feb 12, 2016 at 05:49:38PM +0100, Markus Armbruster wrote: > >> "Daniel P. Berrange" <berrange@redhat.com> writes: > >> > >> > On Thu, Jan 21, 2016 at 07:16:25AM +0100, Hervé Poussineau wrote: > >> >> Hi, > >> >> > >> >> This patch (commit d0d7708ba29cbcc343364a46bff981e0ff88366f) regresses the following command line: > >> >> qemu-system-i386 -nodefaults -chardev vc,id=mon0 -mon chardev=mon0 > >> >> > >> >> Before: > >> >> [nothing is print on console] > >> >> > >> >> After: > >> >> QEMU 2.5.50 monitor - type 'help' for more information > >> >> (qemu) > >> >> > >> >> In both cases, a working vc is created in the GTK+ UI. > >> >> > >> >> Reverting the commit (and fixing the trivial conflict) makes things > >> >> work again. > >> > > >> > Thanks for the heads-up, I'll investigate and submit a fix > >> > >> For the record, it also breaks ivshmem-test when the slow tests are > >> included. I'd be willing to test your fix; got a pointer for me? > > > > https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg02738.html > > No luck. Please try the following reproducer: > > $ make tests/ivshmem-test > $ QTEST_QEMU_BINARY='x86_64-softmmu/qemu-system-x86_64' QTEST_QEMU_IMG=qemu-img MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM % 255 + 1))} gtester -k --verbose -m slow tests/ivshmem-test Yes, I can reproduce that. Will put it on my todo list for next week. Regards, Daniel
On Fri, Feb 12, 2016 at 06:04:59PM +0000, Daniel P. Berrange wrote: > On Fri, Feb 12, 2016 at 06:12:46PM +0100, Markus Armbruster wrote: > > "Daniel P. Berrange" <berrange@redhat.com> writes: > > > > > On Fri, Feb 12, 2016 at 05:49:38PM +0100, Markus Armbruster wrote: > > >> "Daniel P. Berrange" <berrange@redhat.com> writes: > > >> > > >> > On Thu, Jan 21, 2016 at 07:16:25AM +0100, Hervé Poussineau wrote: > > >> >> Hi, > > >> >> > > >> >> This patch (commit d0d7708ba29cbcc343364a46bff981e0ff88366f) regresses the following command line: > > >> >> qemu-system-i386 -nodefaults -chardev vc,id=mon0 -mon chardev=mon0 > > >> >> > > >> >> Before: > > >> >> [nothing is print on console] > > >> >> > > >> >> After: > > >> >> QEMU 2.5.50 monitor - type 'help' for more information > > >> >> (qemu) > > >> >> > > >> >> In both cases, a working vc is created in the GTK+ UI. > > >> >> > > >> >> Reverting the commit (and fixing the trivial conflict) makes things > > >> >> work again. > > >> > > > >> > Thanks for the heads-up, I'll investigate and submit a fix > > >> > > >> For the record, it also breaks ivshmem-test when the slow tests are > > >> included. I'd be willing to test your fix; got a pointer for me? > > > > > > https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg02738.html > > > > No luck. Please try the following reproducer: > > > > $ make tests/ivshmem-test > > $ QTEST_QEMU_BINARY='x86_64-softmmu/qemu-system-x86_64' QTEST_QEMU_IMG=qemu-img MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM % 255 + 1))} gtester -k --verbose -m slow tests/ivshmem-test > > Yes, I can reproduce that. Will put it on my todo list for next week. So I investigated this, and my commit here is not the cause. My commit introduced a crash bug due to NULL pointer reference. This was fixed already in by Marc-Andre in commit 9940c3236f318949c92099163281d5d23a9fcf4f Author: Marc-André Lureau <marcandre.lureau@redhat.com> Date: Mon Dec 21 12:10:13 2015 +0100 ivshmem: use a single eventfd callback, get rid of CharDriver I bisected the test failure again and found this change to blame: commit 428c3ece97179557f2753071fb0ca97a03437267 Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Date: Wed Jan 13 14:59:09 2016 +0000 fix MSI injection on Xen which makes more sense, because the test case failure you identified is specifically with the MSI code in ivshmem Regards, Daniel
"Daniel P. Berrange" <berrange@redhat.com> writes: > On Fri, Feb 12, 2016 at 06:04:59PM +0000, Daniel P. Berrange wrote: >> On Fri, Feb 12, 2016 at 06:12:46PM +0100, Markus Armbruster wrote: [...] >> > >> For the record, it also breaks ivshmem-test when the slow tests are >> > >> included. I'd be willing to test your fix; got a pointer for me? >> > > >> > > https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg02738.html >> > >> > No luck. Please try the following reproducer: >> > >> > $ make tests/ivshmem-test >> > $ QTEST_QEMU_BINARY='x86_64-softmmu/qemu-system-x86_64' QTEST_QEMU_IMG=qemu-img MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM % 255 + 1))} gtester -k --verbose -m slow tests/ivshmem-test >> >> Yes, I can reproduce that. Will put it on my todo list for next week. > > So I investigated this, and my commit here is not the cause. My commit > introduced a crash bug due to NULL pointer reference. This was fixed > already in by Marc-Andre in > > > commit 9940c3236f318949c92099163281d5d23a9fcf4f > Author: Marc-André Lureau <marcandre.lureau@redhat.com> > Date: Mon Dec 21 12:10:13 2015 +0100 > > ivshmem: use a single eventfd callback, get rid of CharDriver > > > I bisected the test failure again and found this change to blame: > > commit 428c3ece97179557f2753071fb0ca97a03437267 > Author: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Date: Wed Jan 13 14:59:09 2016 +0000 > > fix MSI injection on Xen > > which makes more sense, because the test case failure you identified > is specifically with the MSI code in ivshmem Thanks, and sorry for my sloppy bisecting.
diff --git a/backends/baum.c b/backends/baum.c index 723c658..ba32b61 100644 --- a/backends/baum.c +++ b/backends/baum.c @@ -566,6 +566,7 @@ static CharDriverState *chr_baum_init(const char *id, ChardevReturn *ret, Error **errp) { + ChardevCommon *common = qapi_ChardevDummy_base(backend->u.braille); BaumDriverState *baum; CharDriverState *chr; brlapi_handle_t *handle; @@ -576,8 +577,12 @@ static CharDriverState *chr_baum_init(const char *id, #endif int tty; + chr = qemu_chr_alloc(common, errp); + if (!chr) { + return NULL; + } baum = g_malloc0(sizeof(BaumDriverState)); - baum->chr = chr = qemu_chr_alloc(); + baum->chr = chr; chr->opaque = baum; chr->chr_write = baum_write; diff --git a/backends/msmouse.c b/backends/msmouse.c index 0126fa0..476dab5 100644 --- a/backends/msmouse.c +++ b/backends/msmouse.c @@ -68,9 +68,13 @@ static CharDriverState *qemu_chr_open_msmouse(const char *id, ChardevReturn *ret, Error **errp) { + ChardevCommon *common = qapi_ChardevDummy_base(backend->u.msmouse); CharDriverState *chr; - chr = qemu_chr_alloc(); + chr = qemu_chr_alloc(common, errp); + if (!chr) { + return NULL; + } chr->chr_write = msmouse_chr_write; chr->chr_close = msmouse_chr_close; chr->explicit_be_open = true; diff --git a/gdbstub.c b/gdbstub.c index 9c29aa0..1a84c1a 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -1732,6 +1732,7 @@ int gdbserver_start(const char *device) char gdbstub_device_name[128]; CharDriverState *chr = NULL; CharDriverState *mon_chr; + ChardevCommon common = { 0 }; if (!device) return -1; @@ -1768,7 +1769,7 @@ int gdbserver_start(const char *device) qemu_add_vm_change_state_handler(gdb_vm_state_change, NULL); /* Initialize a monitor terminal for gdb */ - mon_chr = qemu_chr_alloc(); + mon_chr = qemu_chr_alloc(&common, &error_abort); mon_chr->chr_write = gdb_monitor_write; monitor_init(mon_chr, 0); } else { diff --git a/include/sysemu/char.h b/include/sysemu/char.h index aff193f..598dd2b 100644 --- a/include/sysemu/char.h +++ b/include/sysemu/char.h @@ -77,6 +77,7 @@ struct CharDriverState { void *opaque; char *label; char *filename; + int logfd; int be_open; int fe_open; int explicit_fe_open; @@ -89,13 +90,15 @@ struct CharDriverState { }; /** - * @qemu_chr_alloc: + * qemu_chr_alloc: + * @backend: the common backend config + * @errp: pointer to a NULL-initialized error object * * Allocate and initialize a new CharDriverState. * - * Returns: a newly allocated CharDriverState. + * Returns: a newly allocated CharDriverState, or NULL on error. */ -CharDriverState *qemu_chr_alloc(void); +CharDriverState *qemu_chr_alloc(ChardevCommon *backend, Error **errp); /** * @qemu_chr_new_from_opts: diff --git a/qapi-schema.json b/qapi-schema.json index 2e31733..dabb5ce 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -3093,6 +3093,21 @@ ## { 'command': 'screendump', 'data': {'filename': 'str'} } + +## +# @ChardevCommon: +# +# Configuration shared across all chardev backends +# +# @logfile: #optional The name of a logfile to save output +# @logappend: #optional true to append instead of truncate +# (default to false to truncate) +# +# Since: 2.6 +## +{ 'struct': 'ChardevCommon', 'data': { '*logfile': 'str', + '*logappend': 'bool' } } + ## # @ChardevFile: # @@ -3107,7 +3122,8 @@ ## { 'struct': 'ChardevFile', 'data': { '*in' : 'str', 'out' : 'str', - '*append': 'bool' } } + '*append': 'bool' }, + 'base': 'ChardevCommon' } ## # @ChardevHostdev: @@ -3120,7 +3136,8 @@ # # Since: 1.4 ## -{ 'struct': 'ChardevHostdev', 'data': { 'device' : 'str' } } +{ 'struct': 'ChardevHostdev', 'data': { 'device' : 'str' }, + 'base': 'ChardevCommon' } ## # @ChardevSocket: @@ -3147,7 +3164,8 @@ '*wait' : 'bool', '*nodelay' : 'bool', '*telnet' : 'bool', - '*reconnect' : 'int' } } + '*reconnect' : 'int' }, + 'base': 'ChardevCommon' } ## # @ChardevUdp: @@ -3160,7 +3178,8 @@ # Since: 1.5 ## { 'struct': 'ChardevUdp', 'data': { 'remote' : 'SocketAddress', - '*local' : 'SocketAddress' } } + '*local' : 'SocketAddress' }, + 'base': 'ChardevCommon' } ## # @ChardevMux: @@ -3171,7 +3190,8 @@ # # Since: 1.5 ## -{ 'struct': 'ChardevMux', 'data': { 'chardev' : 'str' } } +{ 'struct': 'ChardevMux', 'data': { 'chardev' : 'str' }, + 'base': 'ChardevCommon' } ## # @ChardevStdio: @@ -3184,7 +3204,9 @@ # # Since: 1.5 ## -{ 'struct': 'ChardevStdio', 'data': { '*signal' : 'bool' } } +{ 'struct': 'ChardevStdio', 'data': { '*signal' : 'bool' }, + 'base': 'ChardevCommon' } + ## # @ChardevSpiceChannel: @@ -3195,7 +3217,8 @@ # # Since: 1.5 ## -{ 'struct': 'ChardevSpiceChannel', 'data': { 'type' : 'str' } } +{ 'struct': 'ChardevSpiceChannel', 'data': { 'type' : 'str' }, + 'base': 'ChardevCommon' } ## # @ChardevSpicePort: @@ -3206,7 +3229,8 @@ # # Since: 1.5 ## -{ 'struct': 'ChardevSpicePort', 'data': { 'fqdn' : 'str' } } +{ 'struct': 'ChardevSpicePort', 'data': { 'fqdn' : 'str' }, + 'base': 'ChardevCommon' } ## # @ChardevVC: @@ -3223,7 +3247,8 @@ { 'struct': 'ChardevVC', 'data': { '*width' : 'int', '*height' : 'int', '*cols' : 'int', - '*rows' : 'int' } } + '*rows' : 'int' }, + 'base': 'ChardevCommon' } ## # @ChardevRingbuf: @@ -3234,7 +3259,8 @@ # # Since: 1.5 ## -{ 'struct': 'ChardevRingbuf', 'data': { '*size' : 'int' } } +{ 'struct': 'ChardevRingbuf', 'data': { '*size' : 'int' }, + 'base': 'ChardevCommon' } ## # @ChardevBackend: @@ -3243,7 +3269,8 @@ # # Since: 1.4 (testdev since 2.2) ## -{ 'struct': 'ChardevDummy', 'data': { } } +{ 'struct': 'ChardevDummy', 'data': { }, + 'base': 'ChardevCommon' } { 'union': 'ChardevBackend', 'data': { 'file' : 'ChardevFile', 'serial' : 'ChardevHostdev', diff --git a/qemu-char.c b/qemu-char.c index d7be185..11caa56 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -159,10 +159,33 @@ static int sockaddr_to_str(char *dest, int max_len, static QTAILQ_HEAD(CharDriverStateHead, CharDriverState) chardevs = QTAILQ_HEAD_INITIALIZER(chardevs); -CharDriverState *qemu_chr_alloc(void) +static void qemu_chr_free_common(CharDriverState *chr); + +CharDriverState *qemu_chr_alloc(ChardevCommon *backend, Error **errp) { CharDriverState *chr = g_malloc0(sizeof(CharDriverState)); qemu_mutex_init(&chr->chr_write_lock); + + if (backend->has_logfile) { + int flags = O_WRONLY | O_CREAT; + if (backend->has_logappend && + backend->logappend) { + flags |= O_APPEND; + } else { + flags |= O_TRUNC; + } + chr->logfd = qemu_open(backend->logfile, flags, 0666); + if (chr->logfd < 0) { + error_setg_errno(errp, errno, + "Unable to open logfile %s", + backend->logfile); + g_free(chr); + return NULL; + } + } else { + chr->logfd = -1; + } + return chr; } @@ -188,12 +211,45 @@ void qemu_chr_be_generic_open(CharDriverState *s) qemu_chr_be_event(s, CHR_EVENT_OPENED); } + +/* Not reporting errors from writing to logfile, as logs are + * defined to be "best effort" only */ +static void qemu_chr_fe_write_log(CharDriverState *s, + const uint8_t *buf, size_t len) +{ + size_t done = 0; + ssize_t ret; + + if (s->logfd < 0) { + return; + } + + while (done < len) { + do { + ret = write(s->logfd, buf + done, len - done); + if (ret == -1 && errno == EAGAIN) { + g_usleep(100); + } + } while (ret == -1 && errno == EAGAIN); + + if (ret <= 0) { + return; + } + done += ret; + } +} + int qemu_chr_fe_write(CharDriverState *s, const uint8_t *buf, int len) { int ret; qemu_mutex_lock(&s->chr_write_lock); ret = s->chr_write(s, buf, len); + + if (ret > 0) { + qemu_chr_fe_write_log(s, buf, ret); + } + qemu_mutex_unlock(&s->chr_write_lock); return ret; } @@ -218,6 +274,10 @@ int qemu_chr_fe_write_all(CharDriverState *s, const uint8_t *buf, int len) offset += res; } + if (offset > 0) { + qemu_chr_fe_write_log(s, buf, offset); + } + qemu_mutex_unlock(&s->chr_write_lock); if (res < 0) { @@ -365,8 +425,12 @@ static CharDriverState *qemu_chr_open_null(const char *id, Error **errp) { CharDriverState *chr; + ChardevCommon *common = qapi_ChardevDummy_base(backend->u.null); - chr = qemu_chr_alloc(); + chr = qemu_chr_alloc(common, errp); + if (!chr) { + return NULL; + } chr->chr_write = null_chr_write; chr->explicit_be_open = true; return chr; @@ -665,6 +729,7 @@ static CharDriverState *qemu_chr_open_mux(const char *id, ChardevMux *mux = backend->u.mux; CharDriverState *chr, *drv; MuxDriver *d; + ChardevCommon *common = qapi_ChardevMux_base(backend->u.mux); drv = qemu_chr_find(mux->chardev); if (drv == NULL) { @@ -672,7 +737,10 @@ static CharDriverState *qemu_chr_open_mux(const char *id, return NULL; } - chr = qemu_chr_alloc(); + chr = qemu_chr_alloc(common, errp); + if (!chr) { + return NULL; + } d = g_new0(MuxDriver, 1); chr->opaque = d; @@ -975,12 +1043,16 @@ static void fd_chr_close(struct CharDriverState *chr) } /* open a character device to a unix fd */ -static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out) +static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out, + ChardevCommon *backend, Error **errp) { CharDriverState *chr; FDCharDriver *s; - chr = qemu_chr_alloc(); + chr = qemu_chr_alloc(backend, errp); + if (!chr) { + return NULL; + } s = g_new0(FDCharDriver, 1); s->fd_in = io_channel_from_fd(fd_in); s->fd_out = io_channel_from_fd(fd_out); @@ -1005,6 +1077,7 @@ static CharDriverState *qemu_chr_open_pipe(const char *id, char filename_in[CHR_MAX_FILENAME_SIZE]; char filename_out[CHR_MAX_FILENAME_SIZE]; const char *filename = opts->device; + ChardevCommon *common = qapi_ChardevHostdev_base(backend->u.pipe); snprintf(filename_in, CHR_MAX_FILENAME_SIZE, "%s.in", filename); snprintf(filename_out, CHR_MAX_FILENAME_SIZE, "%s.out", filename); @@ -1021,7 +1094,7 @@ static CharDriverState *qemu_chr_open_pipe(const char *id, return NULL; } } - return qemu_chr_open_fd(fd_in, fd_out); + return qemu_chr_open_fd(fd_in, fd_out, common, errp); } /* init terminal so that we can grab keys */ @@ -1081,6 +1154,7 @@ static CharDriverState *qemu_chr_open_stdio(const char *id, ChardevStdio *opts = backend->u.stdio; CharDriverState *chr; struct sigaction act; + ChardevCommon *common = qapi_ChardevStdio_base(backend->u.stdio); if (is_daemonized()) { error_setg(errp, "cannot use stdio with -daemonize"); @@ -1102,7 +1176,7 @@ static CharDriverState *qemu_chr_open_stdio(const char *id, act.sa_handler = term_stdio_handler; sigaction(SIGCONT, &act, NULL); - chr = qemu_chr_open_fd(0, 1); + chr = qemu_chr_open_fd(0, 1, common, errp); chr->chr_close = qemu_chr_close_stdio; chr->chr_set_echo = qemu_chr_set_echo_stdio; if (opts->has_signal) { @@ -1324,6 +1398,7 @@ static CharDriverState *qemu_chr_open_pty(const char *id, PtyCharDriver *s; int master_fd, slave_fd; char pty_name[PATH_MAX]; + ChardevCommon *common = qapi_ChardevDummy_base(backend->u.pty); master_fd = qemu_openpty_raw(&slave_fd, pty_name); if (master_fd < 0) { @@ -1334,7 +1409,11 @@ static CharDriverState *qemu_chr_open_pty(const char *id, close(slave_fd); qemu_set_nonblock(master_fd); - chr = qemu_chr_alloc(); + chr = qemu_chr_alloc(common, errp); + if (!chr) { + close(master_fd); + return NULL; + } chr->filename = g_strdup_printf("pty:%s", pty_name); ret->pty = g_strdup(pty_name); @@ -1557,12 +1636,14 @@ static void qemu_chr_close_tty(CharDriverState *chr) } } -static CharDriverState *qemu_chr_open_tty_fd(int fd) +static CharDriverState *qemu_chr_open_tty_fd(int fd, + ChardevCommon *backend, + Error **errp) { CharDriverState *chr; tty_serial_init(fd, 115200, 'N', 8, 1); - chr = qemu_chr_open_fd(fd, fd); + chr = qemu_chr_open_fd(fd, fd, backend, errp); chr->chr_ioctl = tty_serial_ioctl; chr->chr_close = qemu_chr_close_tty; return chr; @@ -1682,7 +1763,9 @@ static void pp_close(CharDriverState *chr) qemu_chr_be_event(chr, CHR_EVENT_CLOSED); } -static CharDriverState *qemu_chr_open_pp_fd(int fd, Error **errp) +static CharDriverState *qemu_chr_open_pp_fd(int fd, + ChardevCommon *backend, + Error **errp) { CharDriverState *chr; ParallelCharDriver *drv; @@ -1697,7 +1780,10 @@ static CharDriverState *qemu_chr_open_pp_fd(int fd, Error **errp) drv->fd = fd; drv->mode = IEEE1284_MODE_COMPAT; - chr = qemu_chr_alloc(); + chr = qemu_chr_alloc(backend, errp); + if (!chr) { + return NULL; + } chr->chr_write = null_chr_write; chr->chr_ioctl = pp_ioctl; chr->chr_close = pp_close; @@ -1748,11 +1834,16 @@ static int pp_ioctl(CharDriverState *chr, int cmd, void *arg) return 0; } -static CharDriverState *qemu_chr_open_pp_fd(int fd, Error **errp) +static CharDriverState *qemu_chr_open_pp_fd(int fd, + ChardevBackend *backend, + Error **errp) { CharDriverState *chr; - chr = qemu_chr_alloc(); + chr = qemu_chr_alloc(common, errp); + if (!chr) { + return NULL; + } chr->opaque = (void *)(intptr_t)fd; chr->chr_write = null_chr_write; chr->chr_ioctl = pp_ioctl; @@ -1978,12 +2069,16 @@ static int win_chr_poll(void *opaque) } static CharDriverState *qemu_chr_open_win_path(const char *filename, + ChardevCommon *backend, Error **errp) { CharDriverState *chr; WinCharState *s; - chr = qemu_chr_alloc(); + chr = qemu_chr_alloc(backend, errp); + if (!chr) { + return NULL; + } s = g_new0(WinCharState, 1); chr->opaque = s; chr->chr_write = win_chr_write; @@ -1991,7 +2086,7 @@ static CharDriverState *qemu_chr_open_win_path(const char *filename, if (win_chr_init(chr, filename, errp) < 0) { g_free(s); - g_free(chr); + qemu_chr_free_common(chr); return NULL; } return chr; @@ -2086,8 +2181,12 @@ static CharDriverState *qemu_chr_open_pipe(const char *id, const char *filename = opts->device; CharDriverState *chr; WinCharState *s; + ChardevCommon *common = qapi_ChardevHostdev_base(backend->u.pipe); - chr = qemu_chr_alloc(); + chr = qemu_chr_alloc(common, errp); + if (!chr) { + return NULL; + } s = g_new0(WinCharState, 1); chr->opaque = s; chr->chr_write = win_chr_write; @@ -2095,18 +2194,23 @@ static CharDriverState *qemu_chr_open_pipe(const char *id, if (win_chr_pipe_init(chr, filename, errp) < 0) { g_free(s); - g_free(chr); + qemu_chr_free_common(chr); return NULL; } return chr; } -static CharDriverState *qemu_chr_open_win_file(HANDLE fd_out) +static CharDriverState *qemu_chr_open_win_file(HANDLE fd_out, + ChardevCommon *backend, + Error **errp) { CharDriverState *chr; WinCharState *s; - chr = qemu_chr_alloc(); + chr = qemu_chr_alloc(backend, errp); + if (!chr) { + return NULL; + } s = g_new0(WinCharState, 1); s->hcom = fd_out; chr->opaque = s; @@ -2119,7 +2223,9 @@ static CharDriverState *qemu_chr_open_win_con(const char *id, ChardevReturn *ret, Error **errp) { - return qemu_chr_open_win_file(GetStdHandle(STD_OUTPUT_HANDLE)); + ChardevCommon *common = qapi_ChardevDummy_base(backend->u.console); + return qemu_chr_open_win_file(GetStdHandle(STD_OUTPUT_HANDLE), + common, errp); } static int win_stdio_write(CharDriverState *chr, const uint8_t *buf, int len) @@ -2267,8 +2373,12 @@ static CharDriverState *qemu_chr_open_stdio(const char *id, WinStdioCharState *stdio; DWORD dwMode; int is_console = 0; + ChardevCommon *common = qapi_ChardevStdio_base(backend->u.stdio); - chr = qemu_chr_alloc(); + chr = qemu_chr_alloc(common, errp); + if (!chr) { + return NULL; + } stdio = g_new0(WinStdioCharState, 1); stdio->hStdIn = GetStdHandle(STD_INPUT_HANDLE); @@ -2440,12 +2550,17 @@ static void udp_chr_close(CharDriverState *chr) qemu_chr_be_event(chr, CHR_EVENT_CLOSED); } -static CharDriverState *qemu_chr_open_udp_fd(int fd) +static CharDriverState *qemu_chr_open_udp_fd(int fd, + ChardevCommon *backend, + Error **errp) { CharDriverState *chr = NULL; NetCharDriver *s = NULL; - chr = qemu_chr_alloc(); + chr = qemu_chr_alloc(backend, errp); + if (!chr) { + return NULL; + } s = g_new0(NetCharDriver, 1); s->fd = fd; @@ -2856,7 +2971,7 @@ static int tcp_chr_sync_read(CharDriverState *chr, const uint8_t *buf, int len) #ifndef _WIN32 CharDriverState *qemu_chr_open_eventfd(int eventfd) { - CharDriverState *chr = qemu_chr_open_fd(eventfd, eventfd); + CharDriverState *chr = qemu_chr_open_fd(eventfd, eventfd, NULL, NULL); if (chr) { chr->avail_connections = 1; @@ -3138,10 +3253,14 @@ static CharDriverState *qemu_chr_open_ringbuf(const char *id, Error **errp) { ChardevRingbuf *opts = backend->u.ringbuf; + ChardevCommon *common = qapi_ChardevRingbuf_base(backend->u.ringbuf); CharDriverState *chr; RingBufCharDriver *d; - chr = qemu_chr_alloc(); + chr = qemu_chr_alloc(common, errp); + if (!chr) { + return NULL; + } d = g_malloc(sizeof(*d)); d->size = opts->has_size ? opts->size : 65536; @@ -3164,7 +3283,7 @@ static CharDriverState *qemu_chr_open_ringbuf(const char *id, fail: g_free(d); - g_free(chr); + qemu_chr_free_common(chr); return NULL; } @@ -3408,6 +3527,18 @@ fail: return NULL; } +static void qemu_chr_parse_common(QemuOpts *opts, ChardevCommon *backend) +{ + const char *logfile = qemu_opt_get(opts, "logfile"); + + backend->has_logfile = logfile != NULL; + backend->logfile = logfile ? g_strdup(logfile) : NULL; + + backend->has_logappend = true; + backend->logappend = qemu_opt_get_bool(opts, "logappend", false); +} + + static void qemu_chr_parse_file_out(QemuOpts *opts, ChardevBackend *backend, Error **errp) { @@ -3418,6 +3549,7 @@ static void qemu_chr_parse_file_out(QemuOpts *opts, ChardevBackend *backend, return; } backend->u.file = g_new0(ChardevFile, 1); + qemu_chr_parse_common(opts, qapi_ChardevFile_base(backend->u.file)); backend->u.file->out = g_strdup(path); backend->u.file->has_append = true; @@ -3428,6 +3560,7 @@ static void qemu_chr_parse_stdio(QemuOpts *opts, ChardevBackend *backend, Error **errp) { backend->u.stdio = g_new0(ChardevStdio, 1); + qemu_chr_parse_common(opts, qapi_ChardevStdio_base(backend->u.stdio)); backend->u.stdio->has_signal = true; backend->u.stdio->signal = qemu_opt_get_bool(opts, "signal", true); } @@ -3443,6 +3576,7 @@ static void qemu_chr_parse_serial(QemuOpts *opts, ChardevBackend *backend, return; } backend->u.serial = g_new0(ChardevHostdev, 1); + qemu_chr_parse_common(opts, qapi_ChardevHostdev_base(backend->u.serial)); backend->u.serial->device = g_strdup(device); } #endif @@ -3458,6 +3592,7 @@ static void qemu_chr_parse_parallel(QemuOpts *opts, ChardevBackend *backend, return; } backend->u.parallel = g_new0(ChardevHostdev, 1); + qemu_chr_parse_common(opts, qapi_ChardevHostdev_base(backend->u.parallel)); backend->u.parallel->device = g_strdup(device); } #endif @@ -3472,6 +3607,7 @@ static void qemu_chr_parse_pipe(QemuOpts *opts, ChardevBackend *backend, return; } backend->u.pipe = g_new0(ChardevHostdev, 1); + qemu_chr_parse_common(opts, qapi_ChardevHostdev_base(backend->u.pipe)); backend->u.pipe->device = g_strdup(device); } @@ -3481,6 +3617,7 @@ static void qemu_chr_parse_ringbuf(QemuOpts *opts, ChardevBackend *backend, int val; backend->u.ringbuf = g_new0(ChardevRingbuf, 1); + qemu_chr_parse_common(opts, qapi_ChardevRingbuf_base(backend->u.ringbuf)); val = qemu_opt_get_size(opts, "size", 0); if (val != 0) { @@ -3499,6 +3636,7 @@ static void qemu_chr_parse_mux(QemuOpts *opts, ChardevBackend *backend, return; } backend->u.mux = g_new0(ChardevMux, 1); + qemu_chr_parse_common(opts, qapi_ChardevMux_base(backend->u.mux)); backend->u.mux->chardev = g_strdup(chardev); } @@ -3527,6 +3665,7 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend, } backend->u.socket = g_new0(ChardevSocket, 1); + qemu_chr_parse_common(opts, qapi_ChardevSocket_base(backend->u.socket)); backend->u.socket->has_nodelay = true; backend->u.socket->nodelay = do_nodelay; @@ -3588,6 +3727,7 @@ static void qemu_chr_parse_udp(QemuOpts *opts, ChardevBackend *backend, } backend->u.udp = g_new0(ChardevUdp, 1); + qemu_chr_parse_common(opts, qapi_ChardevUdp_base(backend->u.udp)); addr = g_new0(SocketAddress, 1); addr->type = SOCKET_ADDRESS_KIND_INET; @@ -3687,7 +3827,12 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, error_propagate(errp, local_err); goto qapi_out; } + } else { + ChardevCommon *cc = g_new0(ChardevCommon, 1); + qemu_chr_parse_common(opts, cc); + backend->u.data = cc; } + ret = qmp_chardev_add(bid ? bid : id, backend, errp); if (!ret) { goto qapi_out; @@ -3819,17 +3964,25 @@ void qemu_chr_fe_release(CharDriverState *s) s->avail_connections++; } -void qemu_chr_free(CharDriverState *chr) +static void qemu_chr_free_common(CharDriverState *chr) { - if (chr->chr_close) { - chr->chr_close(chr); - } g_free(chr->filename); g_free(chr->label); qemu_opts_del(chr->opts); + if (chr->logfd != -1) { + close(chr->logfd); + } g_free(chr); } +void qemu_chr_free(CharDriverState *chr) +{ + if (chr->chr_close) { + chr->chr_close(chr); + } + qemu_chr_free_common(chr); +} + void qemu_chr_delete(CharDriverState *chr) { QTAILQ_REMOVE(&chardevs, chr, next); @@ -3982,6 +4135,12 @@ QemuOptsList qemu_chardev_opts = { },{ .name = "append", .type = QEMU_OPT_BOOL, + },{ + .name = "logfile", + .type = QEMU_OPT_STRING, + },{ + .name = "logappend", + .type = QEMU_OPT_BOOL, }, { /* end of list */ } }, @@ -3995,6 +4154,7 @@ static CharDriverState *qmp_chardev_open_file(const char *id, Error **errp) { ChardevFile *file = backend->u.file; + ChardevCommon *common = qapi_ChardevFile_base(backend->u.file); HANDLE out; if (file->has_in) { @@ -4008,7 +4168,7 @@ static CharDriverState *qmp_chardev_open_file(const char *id, error_setg(errp, "open %s failed", file->out); return NULL; } - return qemu_chr_open_win_file(out); + return qemu_chr_open_win_file(out, common, errp); } static CharDriverState *qmp_chardev_open_serial(const char *id, @@ -4017,7 +4177,8 @@ static CharDriverState *qmp_chardev_open_serial(const char *id, Error **errp) { ChardevHostdev *serial = backend->u.serial; - return qemu_chr_open_win_path(serial->device, errp); + ChardevCommon *common = qapi_ChardevHostdev_base(backend->u.serial); + return qemu_chr_open_win_path(serial->device, common, errp); } #else /* WIN32 */ @@ -4040,6 +4201,7 @@ static CharDriverState *qmp_chardev_open_file(const char *id, Error **errp) { ChardevFile *file = backend->u.file; + ChardevCommon *common = qapi_ChardevFile_base(backend->u.file); int flags, in = -1, out; flags = O_WRONLY | O_CREAT | O_BINARY; @@ -4063,7 +4225,7 @@ static CharDriverState *qmp_chardev_open_file(const char *id, } } - return qemu_chr_open_fd(in, out); + return qemu_chr_open_fd(in, out, common, errp); } #ifdef HAVE_CHARDEV_SERIAL @@ -4073,6 +4235,7 @@ static CharDriverState *qmp_chardev_open_serial(const char *id, Error **errp) { ChardevHostdev *serial = backend->u.serial; + ChardevCommon *common = qapi_ChardevHostdev_base(backend->u.serial); int fd; fd = qmp_chardev_open_file_source(serial->device, O_RDWR, errp); @@ -4080,7 +4243,7 @@ static CharDriverState *qmp_chardev_open_serial(const char *id, return NULL; } qemu_set_nonblock(fd); - return qemu_chr_open_tty_fd(fd); + return qemu_chr_open_tty_fd(fd, common, errp); } #endif @@ -4091,13 +4254,14 @@ static CharDriverState *qmp_chardev_open_parallel(const char *id, Error **errp) { ChardevHostdev *parallel = backend->u.parallel; + ChardevCommon *common = qapi_ChardevHostdev_base(backend->u.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, errp); + return qemu_chr_open_pp_fd(fd, common, errp); } #endif @@ -4142,8 +4306,12 @@ static CharDriverState *qmp_chardev_open_socket(const char *id, bool is_telnet = sock->has_telnet ? sock->telnet : false; bool is_waitconnect = sock->has_wait ? sock->wait : false; int64_t reconnect = sock->has_reconnect ? sock->reconnect : 0; + ChardevCommon *common = qapi_ChardevSocket_base(backend->u.socket); - chr = qemu_chr_alloc(); + chr = qemu_chr_alloc(common, errp); + if (!chr) { + return NULL; + } s = g_new0(TCPCharDriver, 1); s->fd = -1; @@ -4182,8 +4350,7 @@ static CharDriverState *qmp_chardev_open_socket(const char *id, socket_try_connect(chr); } else if (!qemu_chr_open_socket_fd(chr, errp)) { g_free(s); - g_free(chr->filename); - g_free(chr); + qemu_chr_free_common(chr); return NULL; } @@ -4203,13 +4370,14 @@ static CharDriverState *qmp_chardev_open_udp(const char *id, Error **errp) { ChardevUdp *udp = backend->u.udp; + ChardevCommon *common = qapi_ChardevUdp_base(backend->u.udp); int fd; fd = socket_dgram(udp->remote, udp->local, errp); if (fd < 0) { return NULL; } - return qemu_chr_open_udp_fd(fd); + return qemu_chr_open_udp_fd(fd, common, errp); } ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend, diff --git a/qemu-options.hx b/qemu-options.hx index 215d00d..b4763ba 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2089,40 +2089,43 @@ The general form of a character device option is: ETEXI DEF("chardev", HAS_ARG, QEMU_OPTION_chardev, - "-chardev null,id=id[,mux=on|off]\n" + "-chardev null,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" "-chardev socket,id=id[,host=host],port=port[,to=to][,ipv4][,ipv6][,nodelay][,reconnect=seconds]\n" - " [,server][,nowait][,telnet][,reconnect=seconds][,mux=on|off] (tcp)\n" - "-chardev socket,id=id,path=path[,server][,nowait][,telnet][,reconnect=seconds][,mux=on|off] (unix)\n" + " [,server][,nowait][,telnet][,reconnect=seconds][,mux=on|off]\n" + " [,logfile=PATH][,logappend=on|off] (tcp)\n" + "-chardev socket,id=id,path=path[,server][,nowait][,telnet][,reconnect=seconds]\n" + " [,mux=on|off][,logfile=PATH][,logappend=on|off] (unix)\n" "-chardev udp,id=id[,host=host],port=port[,localaddr=localaddr]\n" " [,localport=localport][,ipv4][,ipv6][,mux=on|off]\n" - "-chardev msmouse,id=id[,mux=on|off]\n" + " [,logfile=PATH][,logappend=on|off]\n" + "-chardev msmouse,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" "-chardev vc,id=id[[,width=width][,height=height]][[,cols=cols][,rows=rows]]\n" - " [,mux=on|off]\n" - "-chardev ringbuf,id=id[,size=size]\n" - "-chardev file,id=id,path=path[,mux=on|off]\n" - "-chardev pipe,id=id,path=path[,mux=on|off]\n" + " [,mux=on|off][,logfile=PATH][,logappend=on|off]\n" + "-chardev ringbuf,id=id[,size=size][,logfile=PATH][,logappend=on|off]\n" + "-chardev file,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" + "-chardev pipe,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" #ifdef _WIN32 - "-chardev console,id=id[,mux=on|off]\n" - "-chardev serial,id=id,path=path[,mux=on|off]\n" + "-chardev console,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" + "-chardev serial,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" #else - "-chardev pty,id=id[,mux=on|off]\n" - "-chardev stdio,id=id[,mux=on|off][,signal=on|off]\n" + "-chardev pty,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" + "-chardev stdio,id=id[,mux=on|off][,signal=on|off][,logfile=PATH][,logappend=on|off]\n" #endif #ifdef CONFIG_BRLAPI - "-chardev braille,id=id[,mux=on|off]\n" + "-chardev braille,id=id[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" #endif #if defined(__linux__) || defined(__sun__) || defined(__FreeBSD__) \ || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__) - "-chardev serial,id=id,path=path[,mux=on|off]\n" - "-chardev tty,id=id,path=path[,mux=on|off]\n" + "-chardev serial,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" + "-chardev tty,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" #endif #if defined(__linux__) || defined(__FreeBSD__) || defined(__DragonFly__) - "-chardev parallel,id=id,path=path[,mux=on|off]\n" - "-chardev parport,id=id,path=path[,mux=on|off]\n" + "-chardev parallel,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" + "-chardev parport,id=id,path=path[,mux=on|off][,logfile=PATH][,logappend=on|off]\n" #endif #if defined(CONFIG_SPICE) - "-chardev spicevmc,id=id,name=name[,debug=debug]\n" - "-chardev spiceport,id=id,name=name[,debug=debug]\n" + "-chardev spicevmc,id=id,name=name[,debug=debug][,logfile=PATH][,logappend=on|off]\n" + "-chardev spiceport,id=id,name=name[,debug=debug][,logfile=PATH][,logappend=on|off]\n" #endif , QEMU_ARCH_ALL ) @@ -2158,7 +2161,12 @@ A character device may be used in multiplexing mode by multiple front-ends. The key sequence of @key{Control-a} and @key{c} will rotate the input focus between attached front-ends. Specify @option{mux=on} to enable this mode. -Options to each backend are described below. +Every backend supports the @option{logfile} option, which supplies the path +to a file to record all data transmitted via the backend. The @option{logappend} +option controls whether the log file will be truncated or appended to when +opened. + +Further options to each backend are described below. @item -chardev null ,id=@var{id} A void device. This device will not emit any data, and will drop any data it diff --git a/spice-qemu-char.c b/spice-qemu-char.c index e70e0f7..8951d7c 100644 --- a/spice-qemu-char.c +++ b/spice-qemu-char.c @@ -271,13 +271,18 @@ static void spice_chr_accept_input(struct CharDriverState *chr) } static CharDriverState *chr_open(const char *subtype, - void (*set_fe_open)(struct CharDriverState *, int)) - + void (*set_fe_open)(struct CharDriverState *, + int), + ChardevCommon *backend, + Error **errp) { CharDriverState *chr; SpiceCharDriver *s; - chr = qemu_chr_alloc(); + chr = qemu_chr_alloc(backend, errp); + if (!chr) { + return NULL; + } s = g_malloc0(sizeof(SpiceCharDriver)); s->chr = chr; s->active = false; @@ -303,6 +308,7 @@ static CharDriverState *qemu_chr_open_spice_vmc(const char *id, { const char *type = backend->u.spicevmc->type; const char **psubtype = spice_server_char_device_recognized_subtypes(); + ChardevCommon *common = qapi_ChardevSpiceChannel_base(backend->u.spicevmc); for (; *psubtype != NULL; ++psubtype) { if (strcmp(type, *psubtype) == 0) { @@ -315,7 +321,7 @@ static CharDriverState *qemu_chr_open_spice_vmc(const char *id, return NULL; } - return chr_open(type, spice_vmc_set_fe_open); + return chr_open(type, spice_vmc_set_fe_open, common, errp); } #if SPICE_SERVER_VERSION >= 0x000c02 @@ -325,6 +331,7 @@ static CharDriverState *qemu_chr_open_spice_port(const char *id, Error **errp) { const char *name = backend->u.spiceport->fqdn; + ChardevCommon *common = qapi_ChardevSpicePort_base(backend->u.spiceport); CharDriverState *chr; SpiceCharDriver *s; @@ -333,7 +340,10 @@ static CharDriverState *qemu_chr_open_spice_port(const char *id, return NULL; } - chr = chr_open("port", spice_port_set_fe_open); + chr = chr_open("port", spice_port_set_fe_open, common, errp); + if (!chr) { + return NULL; + } s = chr->opaque; s->sin.portname = g_strdup(name); diff --git a/ui/console.c b/ui/console.c index 4b65c34..fe950c6 100644 --- a/ui/console.c +++ b/ui/console.c @@ -1953,12 +1953,16 @@ static void text_console_do_init(CharDriverState *chr, DisplayState *ds) static CharDriverState *text_console_init(ChardevVC *vc, Error **errp) { + ChardevCommon *common = qapi_ChardevVC_base(vc); CharDriverState *chr; QemuConsole *s; unsigned width = 0; unsigned height = 0; - chr = qemu_chr_alloc(); + chr = qemu_chr_alloc(common, errp); + if (!chr) { + return NULL; + } if (vc->has_width) { width = vc->width;