Commit 767abe7 ("chardev: forbid 'wait' option with client sockets")
is a bit too strict. Current libvirt always set wait=false, and will
thus fail to add client chardev.
Make the code more permissive, allowing wait=false with client socket
chardevs. Deprecate usage of 'wait' with client sockets.
Fixes: 767abe7f49
Cc: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20190415163337.2795-1-marcandre.lureau@redhat.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
We spell out sub/dir/ in sub/dir/trace-events' comments pointing to
source files. That's because when trace-events got split up, the
comments were moved verbatim.
Delete the sub/dir/ part from these comments. Gets rid of several
misspellings.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20190314180929.27722-3-armbru@redhat.com
Message-Id: <20190314180929.27722-3-armbru@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Currently any client which can complete the TLS handshake is able to use
a chardev server. The server admin can turn on the 'verify-peer' option
for the x509 creds to require the client to provide a x509
certificate. This means the client will have to acquire a certificate
from the CA before they are permitted to use the chardev server. This is
still a fairly low bar.
This adds a 'tls-authz=OBJECT-ID' option to the socket chardev backend
which takes the ID of a previously added 'QAuthZ' object instance. This
will be used to validate the client's x509 distinguished name. Clients
failing the check will not be permitted to use the chardev server.
For example to setup authorization that only allows connection from a
client whose x509 certificate distinguished name contains 'CN=fred', you
would use:
$QEMU -object tls-creds-x509,id=tls0,dir=/home/berrange/qemutls,\
endpoint=server,verify-peer=yes \
-object authz-simple,id=authz0,identity=CN=laptop.example.com,,\
O=Example Org,,L=London,,ST=London,,C=GB \
-chardev socket,host=127.0.0.1,port=9000,server,\
tls-creds=tls0,tls-authz=authz0 \
...other qemu args...
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
If the socket is connecting or connected, tcp_chr_update_read_handler will
be called but it should not set the NetListener's callbacks again.
Otherwise, tcp_chr_accept is invoked while the socket is in connected
state and you get an assertion failure.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Spice port registration is delayed until the server is started. But
ports created after are not being registered. If the server is already
started, do vmc_register_interface() to register it from
qemu_chr_open_spice_port().
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Victor Toso <victortoso@redhat.com>
Message-id: 20190221110703.5775-8-marcandre.lureau@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Most chardev backend handle write() as discarded data if underlying
system is disconnected. For unknown historical reasons, the Spice
backend has "reliable" write: it will wait until the client end is
reconnected to do further successful write().
To decide whether it make sense to wait until the client is
reconnected (or queue the writes), let's review Spice chardev usage
and handling of a disconnected client:
* spice vdagent
The agents reopen the virtio port on disconnect. In qemu side,
virtio_serial_close() will also discard pending data.
* usb redirection
A disconnect creates a device disconnection.
* smartcard emulation
Data is discarded in passthru_apdu_from_guest().
(Spice doesn't explicitly open the smartcard char device until
upcoming 0.14.2, commit 69a5cfc74131ec0459f2eb5a231139f5a69a8037)
* spice webdavd
The daemon will restart the service, and reopen the virtio port.
* spice ports (serial console, qemu monitor..)
Depends on the associated device or usage.
- serial, may be throttled or discarded on write, depending on
device
- QMP/HMP monitor have some CLOSED event handling, but want to
flush the write, which will finish when a new client connects.
On disconnect/reconnect, the client starts with fresh sessions. If it
is a seamless migration, the client disconnects after the source
migrated. The handling of source disconnect in qemu is thus irrelevant
for the Spice session migration.
For all these use cases, it is better to discard writes when the
client is disconnected, and require the vm-side device/agent to behave
correctly on CHR_EVENT_CLOSED, to stop reading and writing from
the spice chardev.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Victor Toso <victortoso@redhat.com>
Message-id: 20190221110703.5775-3-marcandre.lureau@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Inform the front-end of disconnected state (spice client
disconnected).
This will wakeup the source handler immediately, so it can detect the
disconnection asap.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Victor Toso <victortoso@redhat.com>
Message-id: 20190221110703.5775-2-marcandre.lureau@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
The lock usage was described with its introduction in commit
9005b2a758. It was necessary because PTY
write() shares more state than GIOChannel with other
operations.
This made char-pty a bit different from other chardev, that only lock
around the write operation. This was apparent in commit
7b3621f47a, which introduced an idle
source to avoid the lock.
By removing the PTY chardev state sharing on write() with previous
patch, we can remove the lock and the idle source.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20190206174328.9736-7-marcandre.lureau@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
This doesn't help much compared to the 1 second poll PTY
timer. I can't think of a use case where this would help.
However, we can simplify the code around chr_write(): the write lock
is no longer needed for other char-pty callbacks (see following
patch).
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20190206174328.9736-6-marcandre.lureau@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Instead of handling mux chardev in a special way in
qemu_chr_fe_set_handlers(), we may use the chr_update_read_handler
class callback instead.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20190206174328.9736-2-marcandre.lureau@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
The correct name is Wacom.
Fix the typo which is present since 378af96155.
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20190213123446.1768-1-philmd@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
This will be needed by vhost-user-test, when each test switches to
its own GMainLoop and GMainContext. Otherwise, for a reconnecting
socket the initial connection will happen on the default GMainContext,
and no one will be listening on it.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20190202110834.24880-1-pbonzini@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
valgrind on the test-char.c code reports that 'struct termios' contains
uninitialized memory.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20190211182442.8542-17-berrange@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
When the 'reconnect' option is given for a client connection, the
qmp_chardev_open_socket_client method will run an asynchronous
connection attempt. The QIOChannel socket executes this is a single use
background thread, so the connection will succeed immediately (assuming
the server is listening). The chardev, however, won't get the result
from this background thread until the main loop starts running and
processes idle callbacks.
Thus when tcp_chr_wait_connected is run s->ioc will be NULL, but the
state will be TCP_CHARDEV_STATE_CONNECTING, and there may already
be an established connection that will be associated with the chardev
by the pending idle callback. tcp_chr_wait_connected doesn't check the
state, only s->ioc, so attempts to establish another connection
synchronously.
If the server allows multiple connections this is unhelpful but not a
fatal problem as the duplicate connection will get ignored by the
tcp_chr_new_client method when it sees the state is already connected.
If the server only supports a single connection, however, the
tcp_chr_wait_connected method will hang forever because the server will
not accept its synchronous connection attempt until the first connection
is closed.
To deal with this tcp_chr_wait_connected needs to synchronize with the
completion of the background connection task. To do this it needs to
create the QIOTask directly and use the qio_task_wait_thread method.
This will cancel the pending idle callback and directly dispatch the
task completion callback, allowing the connection to be associated
with the chardev. If the background connection failed, it can still
attempt a new synchronous connection.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20190211182442.8542-15-berrange@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
In the previous commit
commit 1dc8a6695c
Author: Marc-André Lureau <marcandre.lureau@redhat.com>
Date: Tue Aug 16 12:33:32 2016 +0400
char: fix waiting for TLS and telnet connection
the tcp_chr_wait_connected() method was changed to check for a non-NULL
's->ioc' as a sign that there is already a connection present, as
opposed to checking the "connected" flag to supposedly fix handling of
TLS/telnet connections.
The original code would repeatedly call tcp_chr_wait_connected creating
many connections as 'connected' would never become true. The changed
code would still repeatedly call tcp_chr_wait_connected busy waiting
because s->ioc is set but the chardev will never see CHR_EVENT_OPENED.
IOW, the code is still broken with TLS/telnet, but in a different way.
Checking for a non-NULL 's->ioc' does not mean that a CHR_EVENT_OPENED
will be ready for a TLS/telnet connection. These protocols (and the
websocket protocol) all require the main loop to be running in order
to complete the protocol handshake before emitting CHR_EVENT_OPENED.
The tcp_chr_wait_connected() method is only used during early startup
before a main loop is running, so TLS/telnet/websock connections can
never complete initialization.
Making this work would require changing tcp_chr_wait_connected to run
a main loop. This is quite complex since we must not allow GSource's
that other parts of QEMU have registered to run yet. The current callers
of tcp_chr_wait_connected do not require use of the TLS/telnet/websocket
protocols, so the simplest option is to just forbid this combination
completely for now.
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20190211182442.8542-14-berrange@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
If establishing a client connection fails, the tcp_chr_wait_connected
method should sleep for the reconnect timeout and then retry the
attempt. This ensures the callers don't immediately abort with an
error when the initial connection fails.
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20190211182442.8542-13-berrange@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
The socket connection state is indicated via the 'bool connected' field
in the SocketChardev struct. This variable is somewhat misleading
though, as it is only set to true once the connection has completed all
required handshakes (eg for TLS, telnet or websockets). IOW there is a
period of time in which the socket is connected, but the "connected"
flag is still false.
The socket chardev really has three states that it can be in,
disconnected, connecting and connected and those should be tracked
explicitly.
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20190211182442.8542-12-berrange@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
In qmp_chardev_open_socket the code for connecting client chardevs is
split across two conditionals far apart with some server chardev code in
the middle. Split up the method so that code for client connection setup
is separate from code for server connection setup.
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20190211182442.8542-11-berrange@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
The tcp_chr_wait_connected method can deal with either server or client
chardevs, but some callers only care about one of these possibilities.
The tcp_chr_wait_connected method will also need some refactoring to
reliably deal with its primary goal of allowing a device frontend to
wait for an established connection, which will interfere with other
callers.
Split it into two methods, one responsible for server initiated
connections, the other responsible for client initiated connections.
In doing this split the tcp_char_connect_async() method is renamed
to become consistent with naming of the new methods.
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20190211182442.8542-10-berrange@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
The 'sioc' variable in qmp_chardev_open_socket was unused since
commit 3e7d4d20d3
Author: Peter Xu <peterx@redhat.com>
Date: Tue Mar 6 13:33:17 2018 +0800
chardev: use chardev's gcontext for async connect
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20190211182442.8542-9-berrange@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
If no valid char driver was identified the qemu_chr_parse_compat method
was silent, leaving callers no clue what failed.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20190211182442.8542-8-berrange@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Now that all validation is separated off into a separate method,
we can directly populate the ChardevSocket struct from the
QemuOpts values, avoiding many local variables.
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20190211182442.8542-7-berrange@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
The 'wait'/'nowait' parameter is used to tell server sockets whether to
block until a client is accepted during initialization. Client chardevs
have always silently ignored this option. Various tests were mistakenly
passing this option for their client chardevs.
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20190211182442.8542-6-berrange@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
The 'reconnect' option is used to give the sleep time, in seconds,
before a client socket attempts to re-establish a connection to the
server. It does not make sense to set this for server sockets, as they
will always accept a new client connection immediately after the
previous one went away.
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20190211182442.8542-5-berrange@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
The TLS creds option is not valid with certain address types. The user
config was only checked for errors when parsing legacy QemuOpts, thus
the user could pass unsupported values via QMP.
Pull all code for validating options out into a new method
qmp_chardev_validate_socket, that is called from the main
qmp_chardev_open_socket method. This adds a missing check for rejecting
TLS creds with the vsock address type.
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20190211182442.8542-4-berrange@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
When chardev is multiplexed (mux=on) there are a lot of cases where
CHR_EVENT_OPENED/CHR_EVENT_CLOSED events pairing (expected from
frontend side) is broken. There are either generation of multiple
repeated or extra CHR_EVENT_OPENED events, or CHR_EVENT_CLOSED just
isn't generated at all.
This is mostly because 'qemu_chr_fe_set_handlers()' function makes its
own (and often wrong) implicit decision on updated frontend state and
invokes 'fd_event' callback with 'CHR_EVENT_OPENED'. And even worse,
it doesn't do symmetric action in opposite direction, as someone may
expect (i.e. it doesn't invoke previously set 'fd_event' with
'CHR_EVENT_CLOSED'). Muxed chardev uses trick by calling this function
again to replace callback handlers with its own ones, but it doesn't
account for such side effect.
Fix that using extended version of this function with added argument
for disabling side effect and keep original function for compatibility
with lots of frontends already using this interface and being
"tolerant" to its side effects.
One more source of event duplication is just line of code in
char-mux.c, which does far more than comment above says (obvious fix).
Signed-off-by: Artem Pisarenko <artem.k.pisarenko@gmail.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <7dde6abbd21682857f8294644013173c0b9949b3.1541507990.git.artem.k.pisarenko@gmail.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Looking at chardev/spice.c code, I realize compilation was broken for
a while with spice-server < 0.12.3. Let's bump required version
to 0.12.5, released May 19 2014, instead of adding more #ifdef.
(this patch combines changes from an early version and some of
Frediano "[PATCH 2/2] spice: Bump required spice-server version to
0.12.6")
According to repology, all the distros that are build target platforms
for QEMU include it:
RHEL-7: 0.14.0
Debian (Stretch): 0.12.8
Debian (Jessie): 0.12.5
FreeBSD (ports): 0.14.0
OpenSUSE Leap 15: 0.14.0
Ubuntu (Xenial): 0.12.6
Note that a previous version of this patch was bumping version to
0.12.6. Unfortunately, Debian Jessie (oldstable) is stuck with spice
server 0.12.5, and QEMU should keep building until after 2y of current
stable (Stretch), which will be around June 17th 2019. Qemu 4.1
should thus be free of bumping to spice-server 0.12.6 during 4.1
development cycle.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20181128155932.16171-1-marcandre.lureau@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
QEMU_CHAR_FEATURE_GCONTEXT declares the character device can switch
GMainContext.
Assert we don't switch context when the character device doesn't
provide this feature. Character device users must not violate this
restriction. In particular, user configurations that violate them
must be rejected.
Existing frontend that rely on context switching would now assert() if
the backend doesn't allow it (instead of silently producing undesired
events in the default context). Following patches improve the
situation by reporting an error earlier instead, on the frontend side.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20181205203737.9011-4-marcandre.lureau@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Following the example of qemu_opts_print_help(), indent all entries in
the list of character devices.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
New option "websocket" added to allow using WebSocket protocol for
chardev socket backend.
Example:
-chardev socket,websocket,server,id=...
Signed-off-by: Julia Suvorova <jusual@mail.ru>
Message-Id: <20181018223501.21683-3-jusual@mail.ru>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Upcoming websocket support requires additional parameters in function
headers that are already overloaded. This patch replaces the bunch of
parameters with a single structure pointer.
Signed-off-by: Julia Suvorova <jusual@mail.ru>
Message-Id: <20181018223501.21683-2-jusual@mail.ru>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
A chardev socket created with the 'fd=' argument is not going to
handle reconnection properly by recycling the same fd (or not in a
supported way). Let's forbid this case.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
qemu_chr_parse_socket() fills all ChardevSocket fields, but that
doesn't reflect correctly the arguments given with the options / on
the command line. "reconnect" takes a number as argument, and the
default value is 0, which doesn't help to identify the missing
option. The other arguments have default values that are less
problematic, leave them set by default for now.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Calling error_report() in a function that takes an Error ** argument
is suspicious. Convert a few that are actually help and such to
error_printf().
Improves output of -chardev help from
qemu-system-x86_64: -chardev help: Available chardev backend types:
serial
...
to
Available chardev backend types:
serial
...
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20181017082702.5581-8-armbru@redhat.com>
GLib child source were added with version 2.28. We can use them now
that we bumped our requirement to 2.40.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
This is mostly for readability of the code. Let's make it clear which
callers can create an implicit monitor when the chardev is muxed.
This will also enforce a safer behaviour, as we don't really support
creating monitor anywhere/anytime at the moment. Add an assert() to
make sure the programmer explicitely wanted that behaviour.
There are documented cases, such as: -serial/-parallel/-virtioconsole
and to less extent -debugcon.
Less obvious and questionable ones are -gdb, SLIRP -guestfwd and Xen
console. Add a FIXME note for those, but keep the support for now.
Other qemu_chr_new() callers either have a fixed parameter/filename
string or do not need it, such as -qtest:
* qtest.c: qtest_init()
Afaik, only used by tests/libqtest.c, without mux. I don't think we
support it outside of qemu testing: drop support for implicit mux
monitor (qemu_chr_new() call: no implicit mux now).
* hw/
All with literal @filename argument that doesn't enable mux monitor.
* tests/
All with @filename argument that doesn't enable mux monitor.
On a related note, the list of monitor creation places:
- the chardev creators listed above: all from command line (except
perhaps Xen console?)
- -gdb & hmp gdbserver will create a "GDB monitor command" chardev
that is wired to an HMP monitor.
- -mon command line option
From this short study, I would like to think that a monitor may only
be created in the main thread today, though I remain skeptical :)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
It's possible to write code creating a chardev backend that is not
registered. When it is not user-created, it makes sense to keep it
hidden. Let the associated frontend destroy it also in this case.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
There is no obvious reason to have a loop counter. This limits from
reading several megabytes large buffers in one go, since socket
read/write usually have a limit.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
A socket chardev may not have associated address (when adding client
fd manually for example). But on disconnect, updating socket filename
expects an address and may lead to this crash:
Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
0x0000555555d8c70c in SocketAddress_to_str (prefix=0x555556043062 "disconnected:", addr=0x0, is_listen=false, is_telnet=false) at /home/elmarco/src/qq/chardev/char-socket.c:388
388 switch (addr->type) {
(gdb) bt
#0 0x0000555555d8c70c in SocketAddress_to_str (prefix=0x555556043062 "disconnected:", addr=0x0, is_listen=false, is_telnet=false) at /home/elmarco/src/qq/chardev/char-socket.c:388
#1 0x0000555555d8c8aa in update_disconnected_filename (s=0x555556b1ed00) at /home/elmarco/src/qq/chardev/char-socket.c:419
#2 0x0000555555d8c959 in tcp_chr_disconnect (chr=0x555556b1ed00) at /home/elmarco/src/qq/chardev/char-socket.c:438
#3 0x0000555555d8cba1 in tcp_chr_hup (channel=0x555556b75690, cond=G_IO_HUP, opaque=0x555556b1ed00) at /home/elmarco/src/qq/chardev/char-socket.c:482
#4 0x0000555555da596e in qio_channel_fd_source_dispatch (source=0x555556bb68b0, callback=0x555555d8cb58 <tcp_chr_hup>, user_data=0x555556b1ed00) at /home/elmarco/src/qq/io/channel-watch.c:84
Replace filename with a generic "disconnected:socket" in this case.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
For some reason __APPLE__ was not checked in pty code. However, the #ifdef
is redundant: this file is already compiled only if CONFIG_POSIX, same as
util/qemu-openpty.c which it uses.
Reported-by: Roman Bolshakov <r.bolshakov@yadro.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
So far, tcp_chr_update_read_handler() only updated the read
handler. Let's also update the hup handler.
Factorize the code while at it. (note that s->ioc != NULL when
s->connected)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20180817135224.22971-4-marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This reverts commit 25679e5d58.
This commit broke "reconnect socket" chardev that are created after
"machine_done": they no longer try to connect. It broke also
vhost-user-test that uses chardev while there is no "machine_done"
event.
The goal of this patch was to move the "connect" source to the
frontend context. chr->gcontext is set with
qemu_chr_fe_set_handlers(). But there is no guarantee that it will be
called, so we can't delay connection until then: the chardev should
still attempt to connect during open(). qemu_chr_fe_set_handlers() is
eventually called later and will update the context.
Unless there is a good reason to not use initially the default
context, I think we should revert to the previous state to fix the
regressions.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20180817135224.22971-3-marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This reverts commit 99f2f54174.
See next commit reverting 25679e5d58 as
well for rationale.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20180817135224.22971-2-marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
In the tcp_chr_write function, we checked errno,
but errno was not reset before a read or write operation.
Therefore, this check of errno's actions is often
incorrect after EAGAIN has occurred.
we need check errno together with ret < 0.
Signed-off-by: xinhua.Cao <caoxinhua@huawei.com>
Message-Id: <20180704033642.15996-1-caoxinhua@huawei.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Fixes: 9fc53a10f8
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
On a real serial device, the open can block if the handshake
lines are in a particular state. If a QEMU is passing the serial
device to the guest, the QEMU startup is blocked opening the device
(with a symptom seen as a timeout from libvirt).
Open the serial port with O_NONBLOCK.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The stdio chardev finalize method calls term_exit() to restore the
original terminal settings that were saved in the "oldtty" global. If
the qemu_chr_open_stdio() method exited with an error, we might not have
any original terminal settings saved in "oldtty" yet.
eg
$ qemu-system-x86_64 -monitor stdio -daemonize
qemu-system-x86_64: -monitor stdio: cannot use stdio with -daemonize
will cause QEMU to splatter the terminal settings with an all-zeros
"struct termios", with predictably unpleasant results. Fortunately the
existing "stdio_in_use" flag is suitable witness for whether "oldtty"
contains settings that need restoring.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20180604123043.13985-1-berrange@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
If we see EAGAIN, no data was sent over the socket, so we still have to
retry sending of msgfds next time.
Signed-off-by: linzhecheng <linzhecheng@huawei.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Remove the 'stair-step output' on stdio.
This partially reverts commit 12fb0ac05, which was correct
on the mailing list but got corrupted by the maintainer :p
Introduced-by: 3b876140-c035-dd39-75d0-d54c48128fac@redhat.com
Reported-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Tested-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20180607210818.12727-1-f4bug@amsat.org
Suggested-by: Thomas Huth <thuth@redhat.com>
Tested-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
