On 01/11/2016 05:44 AM, Daniel P. Berrange wrote: > 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. >
> 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 <berra...@redhat.com> > --- > > -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); Are we leaking anything mutex-related by freeing chr without tearing down the just-initialized chr_write_lock? > > +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; Isn't g_strdup(NULL) safe, such that you could write this as: backend->logfile = g_strdup(logfile); Otherwise looked okay; with the mutex question answered or fixed, you can add: Reviewed-by: Eric Blake <ebl...@redhat.com> -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature