The abstract socket namespace is a non-portable Linux extension. An
attempt to use it elsewhere should fail with ENOENT (the abstract
address looks like a "" pathname, which does not resolve). We report
this failure like
Failed to connect socket abc: No such file or directory
Tolerable, although ENOTSUP would be better.
However, introspection lies: it has @abstract regardless of host
support. Easy enough to fix: since Linux provides them since 2.2,
'if': 'defined(CONFIG_LINUX)' should do.
The above failure becomes
Parameter 'backend.data.addr.data.abstract' is unexpected
I consider this an improvement.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
An optional bool member of a QAPI struct can be false, true, or absent.
The previous commit demonstrated that socket_listen() and
socket_connect() are broken for absent @tight, and indeed QMP chardev-
add also defaults absent member @tight to false instead of true.
In C, QAPI members are represented by two fields, has_MEMBER and MEMBER.
We have:
has_MEMBER MEMBER
false true false
true true true
absent false false/ignore
When has_MEMBER is false, MEMBER should be set to false on write, and
ignored on read.
For QMP, the QAPI visitors handle absent @tight by setting both
@has_tight and @tight to false. unix_listen_saddr() and
unix_connect_saddr() however use @tight only, disregarding @has_tight.
This is wrong and means that absent @tight defaults to false whereas it
should default to true.
The same is true for @has_abstract, though @abstract defaults to
false and therefore has the same behavior for all of QMP, HMP and CLI.
Fix unix_listen_saddr() and unix_connect_saddr() to check
@has_abstract/@has_tight, and to default absent @tight to true.
However, this is only half of the story. HMP chardev-add and CLI
-chardev so far correctly defaulted @tight to true, but defaults to
false again with the above fix for HMP and CLI. In fact, the "tight"
and "abstract" options now break completely.
Digging deeper, we find that qemu_chr_parse_socket() also ignores
@has_tight, leaving it false when it sets @tight. That is also wrong,
but the two wrongs cancelled out. Fix qemu_chr_parse_socket() to set
@has_tight and @has_abstract; writing testcases for HMP and CLI is left
for another day.
Fixes: 776b97d360
Reported-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The test covers only two out of nine combinations. Test all nine.
Four turn out to be broken. Marked /* BUG */.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The abstract sockets test spawns a thread to listen and accept, and a
second one to connect, with a sleep(1) in between to "ensure" the
former is listening when the latter tries to connect. Review fail.
Risks spurious test failure, say when a heavily loaded machine doesn't
schedule the first thread quickly enough. It's also slow.
Listen and accept in the main thread, and start the connect thread in
between. Look ma, no sleep! Run time drops from 2s wall clock to a
few milliseconds.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The thread functions build the SocketAddress from global variable
@abstract_sock_name and the tight flag passed as pointer
argument (either NULL or (gpointer)1). There is no need for such
hackery; simply pass the SocketAddress instead.
While there, dumb down g_rand_int_range() to g_random_int().
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The code tested doesn't care, which is a bug I will fix shortly.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Fixes: 4d3a329af5
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
cur_mon really needs to be coroutine-local as soon as we move monitor
command handlers to coroutines and let them yield. As a first step, just
remove all direct accesses to cur_mon so that we can implement this in
the getter function later.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20201005155855.256490-4-kwolf@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Fixes following errors:
Running test test-util-sockets
ERROR test-util-sockets - missing test plan
# Start of name tests
**
ERROR:../tests/test-util-sockets.c:93:test_socket_fd_pass_name_good: assertion failed (fd != -1): (-1 != -1)
Bail out! ERROR:../tests/test-util-sockets.c:93:test_socket_fd_pass_name_good: assertion failed (fd != -1): (-1 != -1)
First should call to qemu_init_main_loop before socket_init,
then on win32 doesn't support for SOCKET_ADDRESS_TYPE_FD socket type
Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20200915121318.247-21-luoyonggang@gmail.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
After build qemu with '-fsanitize=address' extra-cflags,
'make check' show following leak:
=================================================================
==44580==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 2500 byte(s) in 1 object(s) allocated from:
#0 0x7f1b5a8b8d28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
#1 0x7f1b5a514b10 in g_malloc0 (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x51b10)
#2 0xd79ea4e4c0ad31c3 (<unknown module>)
SUMMARY: AddressSanitizer: 2500 byte(s) leaked in 1 allocation(s).
Call 'g_rand_free' in the end of function to avoid this.
Fixes: 4d3a329af59("tests/util-sockets: add abstract unix socket cases")
Signed-off-by: Li Qiang <liq3ea@163.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: xiaoqiang zhao <zxq_yx_007@163.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
libqemustub.a has been removed in commit ebedb37c8d ("Makefile: Remove
libqemustub.a"). Some remainders have been missed. Remove them now.
Message-Id: <20200804170055.2851-8-thuth@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
add cases to test tight and non-tight for abstract address type
Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Trying to attach a HMP monitor to a chardev that is already in use
results in a crash because monitor_init_hmp() passes &error_abort to
qemu_chr_fe_init():
$ ./x86_64-softmmu/qemu-system-x86_64 --chardev stdio,id=foo --mon foo --mon foo
QEMU 4.2.50 monitor - type 'help' for more information
(qemu) Unexpected error in qemu_chr_fe_init() at chardev/char-fe.c:220:
qemu-system-x86_64: --mon foo: Device 'foo' is in use
Abgebrochen (Speicherabzug geschrieben)
Fix this by allowing monitor_init_hmp() to return an error and passing
any error in qemu_chr_fe_init() to its caller instead of aborting.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200224143008.13362-19-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Trying to attach a QMP monitor to a chardev that is already in use
results in a crash because monitor_init_qmp() passes &error_abort to
qemu_chr_fe_init():
$ ./x86_64-softmmu/qemu-system-x86_64 --chardev stdio,id=foo --mon foo,mode=control --mon foo,mode=control
Unexpected error in qemu_chr_fe_init() at chardev/char-fe.c:220:
qemu-system-x86_64: --mon foo,mode=control: Device 'foo' is in use
Abgebrochen (Speicherabzug geschrieben)
Fix this by allowing monitor_init_qmp() to return an error and passing
any error in qemu_chr_fe_init() to its caller instead of aborting.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200224143008.13362-18-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Current parameter was always one. We continue with that value for now
in all callers.
Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
Moved trace to socket_listen
Skip the tests if socket_check_protocol_support() failed, but do run
g_test_run() to keep TAP harness happy.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Most callers know which monitor type they want to have. Instead of
calling monitor_init() with flags that can describe both types of
monitors, make monitor_init_{hmp,qmp}() public interfaces that take
specific bools instead of flags and call these functions directly.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20190613153405.24769-15-kwolf@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
We commonly want to print to the current monitor if we have one, else
to stdout/stderr. For stderr, have error_printf(). For stdout, all
we have is monitor_vfprintf(), which is rather unwieldy. We often
print to stderr just because error_printf() is easier.
New qemu_printf() and qemu_vprintf() do exactly what's needed. The
next commits will put them to use.
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20190417190641.26814-12-armbru@redhat.com>
@cur_mon is null unless the main thread is running monitor code, either
HMP code within monitor_read(), or QMP code within
monitor_qmp_dispatch().
Use of @cur_mon outside the main thread is therefore unsafe.
Most of its uses are in monitor command handlers. These run in the main
thread.
However, there are also uses hiding elsewhere, such as in
error_vprintf(), and thus error_report(), making these functions unsafe
outside the main thread. No such unsafe uses are known at this time.
Regardless, this is an unnecessary trap. It's an ancient trap, though.
More recently, commit cf869d5317 "qmp: support out-of-band (oob)
execution" spiced things up: the monitor I/O thread assigns to @cur_mon
when executing commands out-of-band. Having two threads save, set and
restore @cur_mon without synchronization is definitely unsafe. We can
end up with @cur_mon null while the main thread runs monitor code, or
non-null while it runs non-monitor code.
We could fix this by making the I/O thread not mess with @cur_mon, but
that would leave the trap armed and ready.
Instead, make @cur_mon thread-local. It's now reliably null unless the
thread is running monitor code.
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[peterx: update subject and commit message written by Markus]
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180720033451.32710-1-peterx@redhat.com>
The SocketAddress 'fd' kind accepts the name of a file descriptor passed
to the monitor with the 'getfd' command. This makes it impossible to use
the 'fd' kind in cases where a monitor is not available. This can apply in
handling command line argv at startup, or simply if internal code wants to
use SocketAddress and pass a numeric FD it has acquired from elsewhere.
Fortunately the 'getfd' command mandated that the FD names must not start
with a leading digit. We can thus safely extend semantics of the
SocketAddress 'fd' kind, to allow a purely numeric name to reference an
file descriptor that QEMU already has open. There will be restrictions on
when each kind can be used.
In codepaths where we are handling a monitor command (ie cur_mon != NULL),
we will only support use of named file descriptors as before. Use of FD
numbers is still not permitted for monitor commands.
In codepaths where we are not handling a monitor command (ie cur_mon ==
NULL), we will not support named file descriptors. Instead we can reference
FD numers explicitly. This allows the app spawning QEMU to intentionally
"leak" a pre-opened socket to QEMU and reference that in a SocketAddress
definition, or for code inside QEMU to pass pre-opened FDs around.
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The SocketAddress struct has an "fd" type, which references the name of a
file descriptor passed over the monitor using the "getfd" command. We
currently blindly assume the FD is a socket, which can lead to hard to
diagnose errors later. This adds an explicit check that the FD is actually
a socket to improve the error diagnosis.
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The fd_is_socket() helper method is useful in a few places, so put it in
the common sockets code. Make the code more compact while moving it.
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
