| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
When migration completes, unrefing the new connection leads to original
GSocket pending refs, and thus the sockets stay in CLOSE_WAIT.
This is a regression from 8029bd0 where GSocketConnection is kept around
to satisfy old glib.
https://bugzilla.redhat.com/show_bug.cgi?id=1024501
This will also probably fix:
https://bugzilla.redhat.com/show_bug.cgi?id=952375
|
|
Current code works with DIGEST-MD5, but not with PLAIN.
In particular, when using PLAIN, sasl_client_start() returns SASL_OK, which
should not be an error in spite of vague/confusing upstream documentation
about this:
http://asg.andrew.cmu.edu/archive/message.php?mailbox=archive.cyrus-sasl&msg=10104
Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
|
|
Take an approach similar to gthreads, which are considered as low
level/must work features by glib, which aborts on failures.
|
|
|
|
spice_channel_type_to_string() does not return NULL
|
|
Currently, spice-gtk will look in $HOME/.spicec/spice_truststore.pem
by default for its trust certificate store (to verify the certificates
used during SPICE TLS connections).
However, these days, progress is under-way to have a system-wide
certificate store [1].
In order to use it, we only need to call SSL_CTX_set_default_verify_paths()
and it will automatically use the shared system CA store if the distro
is properly setup.
We only try to use that store if there was no user-provided CA file to use,
or if we failed to load it.
[1] https://fedoraproject.org/wiki/Features/SharedSystemCertificates
|
|
coroutine_init() can fail, but spice-channel.c was not checking its return
value, which could lead to some crashes if coroutine_init() failed and we
then try to use coroutine_yieldto()
|
|
When exiting remote-viewer after authenticating through SASL, I got
this crash:
#0 0x0000000100a51870 in ?? ()
#1 0x000000314d20c53e in _sasl_log (conn=<optimized out>, level=5, fmt=0x7fffe49893e8 "DIGEST-MD5 client mech dispose")
at common.c:1985
#2 0x00007fffe4982d88 in digestmd5_client_mech_dispose (conn_context=0xaf1900, utils=0xaefd10) at digestmd5.c:4580
#3 0x000000314d208654 in client_dispose (pconn=0xaf0710) at client.c:332
#4 0x000000314d20b76b in sasl_dispose (pconn=0xa51898) at common.c:851
#5 0x00007ffff7602dc7 in channel_reset (channel=0xa52250, migrating=0) at spice-channel.c:2493
#6 0x00007ffff760f7b7 in spice_inputs_channel_reset (channel=0xa52250, migrating=0) at channel-inputs.c:615
#7 0x00007ffff76030ac in spice_channel_reset (channel=0xa52250, migrating=0) at spice-channel.c:2551
#8 0x00007ffff76031e0 in channel_disconnect (channel=0xa52250) at spice-channel.c:2570
#9 0x00007ffff760283d in spice_channel_coroutine (data=0xa52250) at spice-channel.c:2368
#10 0x00007ffff763d14b in coroutine_trampoline (cc=0xa51900) at coroutine_ucontext.c:58
#11 0x00007ffff763ce30 in continuation_trampoline (i0=10819840, i1=0) at continuation.c:49
#12 0x00000031342479c0 in ?? () from /lib64/libc.so.6
#13 0x0000000000a51cc8 in ?? ()
#14 0x0000000000000000 in ?? ()
It turns out that the sasl_callback_t data passed when calling
sasl_client_new() must be valid until sasl_dispose() is called. I could not
find mentions of this in the official documentation but
https://mail-archives.apache.org/mod_mbox/subversion-dev/201109.mbox/%3C20110908072256.GN25324@ted.stsp.name%3E
describes what happens.
Making the sasl_callback_t structure static should be enough to guarantee
that the data will stay around long enough.
|
|
After the previous commit, spice_channel_switch_protocol() is now
called when needed, but it's not doing anything. What happens is
that spice_channel_switch_protocol() triggers a channel disconnection
and then it queues an idle to reconnect (after having changed the
protocol version to be used).
When spice_channel_recv_link_hdr() returns, we need to jump out of
the coroutine to let the idle trigger and the new channel coroutine
be started. But jumping out of the coroutine will call channel_disconnect()
which calls channel_reset() which disables the idle switch_protocol()
just queued. This causes the connection attempt to be apparently
stuck with nothing happening.
Falling back to the older SPICE protocol is not the only situation
when we need to drop the current connection attempt and reconnect,
we also need to do that when the remote server returns
SPICE_LINK_ERR_NEED_SECURED to let the client know it needs to use
a secure port for this channel. This is handled by the 'switch_tls'
variable set in spice_channel_recv_link_msg and handled in
spice_channel_coroutine().
'switch_tls' does the same thing as spice_channel_switch_protocol(),
except that it calls spice_channel_connect() after channel_disconnect()
has been called, which means the idle queued by channel_connect()
won't get cleared.
This all that commit does, remove the spice_channel_switch_protocol()
method and use the same codepath as 'switch_tls' to handle the
reconnection.
|
|
This partially reverts b19acbc. This commit broke the fallback
to the old protocol as it added a check for c->peer_msg != NULL
before calling switch_protocol(), but mismatch between local
and remote protocol versions is detected before c->peer_msg is
allocated, so:
if (c->peer_msg != NULL && c->link_hdr.major_version != 1) {
SPICE_DEBUG("%s: error, switching to protocol 1 (spice 0.4)",
c->name);
spice_channel_switch_protocol(channel, 1);
return TRUE;
}
will never get triggered when c->peer_hdr.major_version !=
c->link_hdr.major_version
The crash described in b19acbc occurred when calling
spice_channel_recv_link_msg() in spice_channel_coroutine()
after a call to spice_channel_recv_link_hdr() failed and did
not set c->peer_msg.
This commit removes the c>peer_msg check done before calling
spice_channel_switch_protocol() so that it gets called when needed,
but makes sure that we return FALSE to indicate that an error happened
and that we need to reconnect. This way we won't try to call
spice_channel_recv_link_msg() when c->peer_msg is NULL.
|
|
The usbredir channel uses spice_msg_in_raw to get its data, which uses
in->dpos to determine the msg size and that was no longer being set for
non sub-messages.
https://bugs.freedesktop.org/show_bug.cgi?id=69935
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
|
|
Use of coroutines allow to simplify spice_channel_recv_msg(), it doesn't
need to keep current reading state, it can rely on the coroutine stack
for that.
|
|
|
|
The current coroutine channel_iterate() creates a GSource for the socket
reenters the mainloop waiting for IO condition. This is really heavy
usage of mainloop showing up in system time and user space
profiling (10% of CPU time spent in mainloop overhead). Instead flush
all pending input messages before switching context and reentering main
loop.
This is the kind of results I get with a replay:
before:
2179,440455 task-clock # 0,629 CPUs utilized
3 580 context-switches # 0,002 M/sec
207 cpu-migrations # 0,095 K/sec
48 909 page-faults # 0,022 M/sec
7 362 442 301 cycles # 3,378 GHz
5 256 957 520 stalled-cycles-frontend # 71,40% frontend cycles
idle
<not supported> stalled-cycles-backend
5 460 266 981 instructions # 0,74 insns per cycle
# 0,96 stalled cycles
per insn
982 749 523 branches # 450,918 M/sec
20 745 453 branch-misses # 2,11% of all branches
3,463422087 seconds time elapsed
after:
1741,651383 task-clock # 0,522 CPUs utilized
5 093 context-switches # 0,003 M/sec
137 cpu-migrations # 0,079 K/sec
49 033 page-faults # 0,028 M/sec
5 874 567 974 cycles # 3,373 GHz
4 247 059 288 stalled-cycles-frontend # 72,30% frontend cycles
idle
<not supported> stalled-cycles-backend
4 419 666 346 instructions # 0,75 insns per cycle
# 0,96 stalled cycles
per insn
776 972 366 branches # 446,112 M/sec
17 862 170 branch-misses # 2,30% of all branches
3,337472053 seconds time elapsed
|
|
Allow to disable selectively channels, mainly used for testing,
ex: SPICE_DISABLE_CHANNELS=display spicy-stats -p 12345
|
|
This allows to simplify a little bit derived class (no need to override
handle_msg), and allows the base class more flexibility (for example for
filtering messages, as in the following patch)
|
|
And document both spice_channel_string_to_type and
spice_channel_type_to_string.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
|
|
|
|
|
|
|
|
A broken server may reply to switch to TLS again and again. spice-gtk
should only try once.
|
|
There has been reports of recent spice-gtk versions not working on
RHEL6 or Ubuntu 10.04. This happens because these systems have
an older glib version without:
commit a0e1b226a21ca498b301981b0c89e89ad9a31eb1
Author: Dan Winship <danw@gnome.org>
Date: Fri Apr 23 08:47:18 2010 -0400
GSocketConnection: don't close the socket if it's still reffed
When disposing a GSocketConnection, don't explicitly close the
underlying GSocket. The GSocket will close itself if it gets
destroyed, and if it doesn't get destroyed, that presumably means the
app still wants to use it. Eg, this lets you use GSocketClient to
create a GSocketConnection, and then take the GSocket and destroy the
GSocketConnection.
https://bugzilla.gnome.org/show_bug.cgi?id=616855
and spice-gtk commit 0f9a432c "session: allow to connect via HTTP CONNECT
proxy" changed spice_session_channel_open_host to get its socket by doing:
open_host->socket = g_socket_connection_get_socket(connection);
g_object_ref(open_host->socket);
g_object_unref(connection);
(see socket_client_connect_ready)
If glib does not have the commit mentioned above, then this won't
work as expected as open_host->socket will get closed when 'connection'
gets destroyed.
This commit changes spice_session_channel_open_host to return a
GSocketConnection rather than a GSocket so that we can keep the
socket open even on older glib versions.
Huge thanks go to Brad Campbell <brad@fnarfbargle.com> for doing all the
spice-gtk/glib bisecting work.
|
|
$ remote-viewer spice://192.168.0.233:111 # 111 is not a valid spice port
(remote-viewer:29381): GSpice-WARNING **: incomplete link header (-104/16)
Segmentation fault (core dumped)
$ gdb /usr/bin/remote-viewer core
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
Core was generated by `remote-viewer spice://192.168.0.233:111'.
Program terminated with signal 11, Segmentation fault.
switch_tls=0x7f9eb6855b88) at spice-channel.c:1675
warning: Source file is more recent than executable.
1675 switch (c->peer_msg->error) {
(gdb) bt
switch_tls=0x7f9eb6855b88) at spice-channel.c:1675
at spice-channel.c:2299
at coroutine_ucontext.c:58
at continuation.c:49
c->peer_msg->error was accessed without checking the validity of pointer in
spice_channel_recv_link_msg(). Actually, c->peer_msg may be a NULL pointer if
we got a error in spice_channel_recv_link_hdr().
This patch fixes this error.
Signed-off-by: Dunrong Huang <riegamaths@gmail.com>
|
|
In some cases, source and destinations may have different channel
encryption. We need to swap tls state too during seamless migration.
https://bugzilla.redhat.com/show_bug.cgi?id=855870
|
|
|
|
|
|
The commit fcbbc248a8f885f9a9a6e7c47d7aae0c1ab3cd1b changed the way a
channel coroutine is exiting. In particular, it was going through the
coroutine main cleanup (finishing in main coroutine) while switching
to TLS is recycling the channel. That part of the code is a bit
difficult to grasp, but with this refactoring, I think it makes it
easier to understand the reconnection.
|
|
The Spice server doesn't wait until all the data are received by the
remote before closing the socket (that would need SO_LINGER?). Under
some racy conditions, the client may not have received the link reply
indicating the server protocol version mismatch, which would trigger
reconnection with compatible protocol.
It seems to happen on local networks with Windows sockets (error
WSAECONNRESET). To workaround that issue, spice-gtk can try to
reconnect with protocol 1 when a socket error is encoutered during
link-time.
Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=874698
|
|
|
|
|
|
|
|
We can simplify the channel state callback and simplify a little
the code by relying on coroutine state instead.
|
|
A Spice port channel carry arbitrary data between the Spice client and
the Spice server. It may be used to provide additional services on top
of a Spice connection. For example, a channel can be associated with
the qemu monitor for the client to interact with it, just like any
qemu chardev. Or it may be used with various protocols, such as the
Spice Controller.
A port kind is identified simply by a fqdn, such as org.qemu.monitor,
org.spice.spicy.test or org.ovirt.controller...
|
|
Add spice_channel_flush_async() that asynchronously will write all the
pending channel data.
|
|
Avoid the obfuscating many -> indirection by using the
SpiceChannelPrivate *c variable.
|
|
Sadly, OpenSSL doesn't provide a way to load certificate from memory,
but all the functions necessary to do so are actually public, so we
can implement our own version and avoid files, how awesome!
|
|
The open_host() can return FALSE when the connection is discarded or
skipped. Improve the message to not indicate a failure.
https://bugzilla.redhat.com/show_bug.cgi?id=858232
|
|
This way functions called from the channel_reset function can rely
on state accurately reflecting the state. This is necessary to stop
channel-usbredir's reset callback from trying to send the initial
hello message while the channel is no longer in a connected state.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
|
|
It automatically prepends the channel name to the log message
for easier debugging.
Fixes rhbz#822437
|
|
Otherwise, we will not create smartcard/usb channel on the destination
side, and we will create audio channels, no matter if they existed
of didn't exist for the src side.
|
|
When swapping the src and dest channels's, we need to keep
the xmit_queue and msg serials. Their state is expected to stay the same
after migration.
|
|
Flow:
(1) *src* main channel coroutine (main_handle_migrate_begin_seamless):
handles SPICE_MSG_MAIN_MIGRATE_BEGIN_SEAMLESS; yields to the main loop,
supplying it the destination information needed for connection.
(2) main context (migrate_connect):
Establishes a new session for connecting to the destination.
After all the channels are opened (async), their state, except for
the one of the main channel, is modified to
SPICE_CHANNEL_STATE_MIGRATING (see migrate_channel_event_cb);
no reading is done from the channel during this state.
The dest main channel's state is changed to SPICE_CHANNEL_STATE_MIGRATION_HANDSHAKE
(3) *dest* main channel coroutine: sends to the dest server SPICE_MSGC_MAIN_MIGRATE_DST_DO_SEAMLESS
(see spice_channel_recv_auth)
(4) *dest* main channel coroutine: recevices SPICE_MSG_MAIN_MIGRATE_DST_SEAMLESS_ACK/NACK.
adds main_migrate_handshake_done to the main loop.
(5) main context: when all the dest session channels are connected, and the main channel handshake
is done, we yield to the src main channel coroutine (see
migrate_channel_event_cb and main_migrate_handshake_done)
(6) *src* main channel coroutine: sends to the src server
SPICE_MSGC_MAIN_MIGRATE_(CONNECTED|CONNECTED_SEAMLESS|CONNECT_ERROR)
For more details see spice-protocol. commit
1ad5d259cb4b695ec3106de7ccd082e031e7ae11
|
|
We may have several widget trying to re-connect the channels now.
It is fine to return successfully if we are already connecting or
connected.
|
|
This was initially public to eventually let a derived class
implement more capabilities. Even though it is technically
doable to derive and tweak exisiting channels, there is a
lack of support in spice-gtk for doing that.
|
|
Capability BAR for channel FOO can be disabled at runtime by setting
the SPICE_FOO_CAP_BAR environment variable to '0'
Disabling capabilities is useful for testing purpose.
|
|
spice_channel_coroutine returns a void *, but one of its error path
is doing 'return FALSE'. This commit replaces this return with a
'goto cleanup' since this is what is done in the other error paths.
|
|
There are several very unlikely failures where no signal is emitted
to indicate the failure. Since applications rely on these signals
to detect spice-gtk connection failures, it's important to emit
one in all error cases.
|
|
When trying to start remote-viewer with SPICE over TLS with
--spice-ca-file with a wrong filename, the connection fails
but remote-viewer keeps displaying the "Trying to connect"
message. The only hint that something went wrong is:
(remote-viewer:12924): GSpice-WARNING **: loading ca certs from a/home/teuf/foo.crt
This patch makes sure we emit a SPICE_CHANNEL_ERROR_TLS before
giving up on channel creation to inform the application that
an error happened.
|
|
Related: rhbz#821795
The capabilities have been zeroed after channel reset, which have lead to
publish the wrong caps when both port and tls-port are given, and the
channels are secured (first attempt to connect the channel with "port"
has failed; the channel got reset, and then reconnected with "tls-port"
and bad caps). Specifically, the bug causes semi-seamless migration not
to work when part of the channels are secured.
|
|
For windows (mingw32) ENOTSUP is not defined.
Related to 3bfadb8587f59a74d373e26385d348a105c2e425
|
