The leak is only apparent on Win32. On POSIX platforms destroying a
mutex is not necessary.
Reported-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@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>
The qemu-char.c contains two helper methods send_all
and recv_all. These are in fact declared in sockets.h
so ought to have been in util/qemu-sockets.c. For added
fun the impl of recv_all is completely missing on Win32.
Fortunately there is only a single caller of these
methods, the TPM passthrough code, which is only
ever compiled on Linux. With only a single caller
these helpers are not compelling enough to keep so
inline them in the TPM code, avoiding the need to
fix the missing recv_all on Win32.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <1450879144-17111-1-git-send-email-berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Switch from using g_base64_decode over to qbase64_decode
in order to get error checking of the base64 input data.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Our QA team wants to preserve serial output of the guest in between QEMU
runs to perform post-analysis.
By default this behavior is off (file is truncated each time QEMU is
started or device is plugged).
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Eric Blake <eblake@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <1449211324-17856-1-git-send-email-den@openvz.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This is a case where pty_chr_update_read_handler_locked's lack
of error checking can produce incorrect values. We are not using
SIGUSR1 anymore, so this is quite theoretical, but easy to fix.
Reported-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
We have two issues with our qapi union layout:
1) Even though the QMP wire format spells the tag 'type', the
C code spells it 'kind', requiring some hacks in the generator.
2) The C struct uses an anonymous union, which places all tag
values in the same namespace as all non-variant members. This
leads to spurious collisions if a tag value matches a non-variant
member's name.
Make the conversion to the new layout for character-related
code.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1445898903-12082-19-git-send-email-eblake@redhat.com>
[Commit message tweaked slightly]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
If a chardev is allowed to be created outside of QMP, then it must be
also possible to free it. This is useful for ivshmem that creates
chardev anonymously and must be able to free them.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
The qapi_copy_SocketAddress method is going to be useful
in more places than just qemu-char.c, so move it into
the qemu-sockets.c file to allow its reuse.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
All backends now return errors through Error*, so the "Failed to
create chardev" placeholder error can only be reached if the backend
is not available (and only from the chardev-add QMP command; instead,
the -chardev command line option fails earlier).
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The backend now always returns errors via the Error* argument.
This avoids a double error message. Before:
qemu-system-x86_64: -chardev stdio,id=base: cannot use stdio with -daemonize
qemu-system-x86_64: -chardev stdio,id=base: Failed to create chardev
After:
qemu-system-x86_64: -chardev stdio,id=base: cannot use stdio with -daemonize
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Having creation as a member of the CharDriver struct removes the need
to export functions for qemu-char.c's usage. After the conversion,
chardev backends implemented outside qemu-char.c will not need a stub
creation function anymore.
Ultimately all drivers will be converted. For now, support the case
where cd->create == NULL.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Move the #ifdef up into qmp_chardev_add, and avoid duplicating
the code that reports unavailable backends. Split HAVE_CHARDEV_TTY
into HAVE_CHARDEV_SERIAL and HAVE_CHARDEV_PTY.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
g_new(T, n) is neater than g_malloc(sizeof(T) * n). It's also safer,
for two reasons. One, it catches multiplication overflowing size_t.
Two, it returns T * rather than void *, which lets the compiler catch
more type errors.
This commit only touches allocations with size arguments of the form
sizeof(T). Same Coccinelle semantic patch as in commit b45c03f.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1442231643-23630-1-git-send-email-armbru@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The free() and g_free() functions both happily accept
NULL on any platform QEMU builds on. As such putting a
conditional 'if (foo)' check before calls to 'free(foo)'
merely serves to bloat the lines of code.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
A number of files were including dirent.h but not using any
of the functions it provides
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Commit 812c1057 introduced HUP detection on unix and tcp sockets prior
to a read in tcp_chr_read. This unfortunately broke CloudStack 4.2
which relied on the old behaviour where data on a socket was readable
even if a HUP was present.
A working solution is to properly check the return values from recv,
handling a closed socket once there is no more data to read.
Also enable polling for G_IO_NVAL to ensure the callback is called
for all possible events as these should now be possible to handle
with the improved error detection.
Signed-off-by: Nils Carlson <pyssling@ludd.ltu.se>
Message-Id: <1437338396-22336-1-git-send-email-pyssling@ludd.ltu.se>
[Do not handle EINTR; use socket_error(). - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
img_convert() and img_amend() use qemu_opts_do_parse(), which reports
errors with qerror_report_err(). Its error messages aren't helpful
here, the caller reports one that actually makes sense. Reproducer:
$ qemu-img convert -o backing_format=raw in.img out.img
qemu-img: Invalid parameter 'backing_format'
qemu-img: Invalid options for file format 'raw'
To fix, propagate errors through qemu_opts_do_parse(). This lifts the
error reporting into callers. Drop it from img_convert() and
img_amend(), keep it in qemu_chr_parse_compat(), bdrv_img_create().
Since I'm touching qemu_opts_do_parse() anyway, write a function
comment for it.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
qemu_opt_set() is a wrapper around qemu_opt_set() that reports the
error with qerror_report_err().
Most of its users assume the function can't fail. Make them use
qemu_opt_set_err() with &error_abort, so that should the assumption
ever break, it'll break noisily.
Just two users remain, in util/qemu-config.c. Switch them to
qemu_opt_set_err() as well, then rename qemu_opt_set_err() to
qemu_opt_set().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
qerror_report_err() is a transitional interface to help with
converting existing monitor commands to QMP. It should not be used
elsewhere. Replace by error_report_err() in legacy chardev parser
qemu_chr_parse_compat(). Legacy chardev syntax is not to be used in
QMP.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The monitor's auto-completion feature stopped working when stdio is used
as an input and qemu was resumed after it was suspended (using ctrl-z).
Signed-off-by: Gal Hammer <ghammer@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
tcp_get_fds API discards fds if there's more than 1 of these.
It's tricky to fix this without API changes in the generic case.
However, this API is only used by tests ATM, and tests know how
many fds they expect.
So let's not waste cycles trying to fix this properly:
simply assume at most 16 fds (tests use at most 8 now).
assert if some test tries to get more.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
If reconnect was set, errors wouldn't always be reported.
Fix that and also only report a connect error once until a
connection has been made.
The primary purpose of this is to tell the user that a
connection failed so they can know they need to figure out
what went wrong. So we don't want to spew too much
out here, just enough so they know.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
An error value here would be quite handy and more consistent
with the rest of the code.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
[Make sure SO_ERROR value is passed to error_setg_errno. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
It seems that it might be a good idea to know what is at the remote
end of a socket for tracking down issues. So add that to the
socket filename.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Adds a "reconnect" option to socket backends that gives a reconnect
timeout. This only applies to client sockets. If the other end
of a socket closes the connection, qemu will attempt to reconnect
after the given number of seconds.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
