Commit Graph

2698 Commits

Author SHA1 Message Date
Frediano Ziglio
2f6be0b84d dcc: Make dcc_stop static
Just used by dcc_on_disconnect.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
2017-09-07 12:15:31 +01:00
Frediano Ziglio
e595b18282 tests: Make test-two-servers work
This test runs 2 spice server in one program.
Use two different tcp port to be able to connect to both servers.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-09-07 08:54:18 +01:00
Frediano Ziglio
8f872043b3 tests: Make test-empty-success automated
Add some check that something happened during creation/destruction.
Set as running on "make check".

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-09-07 08:54:18 +01:00
Frediano Ziglio
0905a95d3a tests: Make test-fail-on-null-core-interface an automated test
Update to Glib style to catch warning.
Initialize properly the structure (invalid) provided.
Check results of spice_server_init.
Remove leaks.
Enable as check.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-09-07 08:54:18 +01:00
Frediano Ziglio
544f5d7ed3 tests: Update README
Update tests names.
Remove tetris comments, never available and not planned.
Update some notes.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-09-07 08:54:18 +01:00
Frediano Ziglio
6517ea5cbb test-display-base: Always compile with AUTOMATED_TESTS enabled
There's no need to not compile this feature, it just enable
a parameters which must be passed in order to change test
behaviour.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-09-07 08:54:18 +01:00
Frediano Ziglio
06380a737b tests: Allow to quit the message loop
This allows to end the loop to end some tests.
Currently different tests enter the loop but never exit from
them.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-09-07 08:54:18 +01:00
Frediano Ziglio
eb70454de3 test-display-base: Do not assume LP64 architecture on 64 bit
In C the sizeof(long) can be different than sizeof(void*),
use proper type.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-09-07 08:54:18 +01:00
Frediano Ziglio
6745d048c5 test-display-base: Use spice allocation helpers
Do not use calloc and malloc directly without checking
the result. Use instead spice functions to get a nice
error in case of allocation failures.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-09-07 08:54:18 +01:00
Frediano Ziglio
416c58c348 test-display-base: Avoid cursor move command leaks
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-09-07 08:54:18 +01:00
Frediano Ziglio
230dcf82c3 test-display-base: Avoid global buffer overflow
For some reasons (documented in cursor_init) the function
uses 128 extra bytes of data causing a reading buffer overflow.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-09-07 08:54:18 +01:00
Frediano Ziglio
8bf7c7803d test-display-base: Protect the wake up timer start with a mutex
Timers in spice server are supposed to be used in a single thread
context. To avoid problems, protect the usage from multiple
thread with a mutex.
This sometimes caused a crash.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-09-07 08:54:18 +01:00
Frediano Ziglio
6daf2d115b test-display-base: Avoid usage after free when the wakeup timer is freed
The wakeup timer is used by the worker thread and by the
main thread.
Destroying the object before destroying the worker thread
can lead to use after free.
Destroying the worker thread first makes sure we don't race.
This is detected easily when compiling the test with address sanitizer.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-09-07 08:54:18 +01:00
Frediano Ziglio
574574e425 test-display-base: Protect command ring with a mutex
The ring is accessed by multiple thread.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-09-07 08:54:14 +01:00
Frediano Ziglio
f73fbdcae5 test-display-base: Use unsigned numbers for command ring indexes
As the indexes are used to compute the index inside an array
using modulo operation when a signed value overflows, the
modulo becames negative, causing a buffer underflow.
Unlikely to happens (take lot of time) but is safer that way.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-09-07 06:44:58 +01:00
Frediano Ziglio
6db3ee7f83 common-graphics-channel: Move "qxl" property to DisplayChannel
Only DisplayChannel uses this property.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2017-09-07 06:42:01 +01:00
Frediano Ziglio
8e7d5ac580 cursor-channel: Remove dependency from QXL
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2017-09-07 06:41:49 +01:00
Frediano Ziglio
64f7fa3d0e cursor-channel: Removed unused qxl field from RedCursorPipeItem
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2017-09-07 06:41:38 +01:00
Frediano Ziglio
1c6e7cf73e Release cursor as soon as possible
Cursor resources (basically the shape of it) was retained till
it was used however it was copied so there were no reason to not release
this resource.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2017-09-07 06:41:17 +01:00
Frediano Ziglio
bfb6601348 dcc: Fix NULL pointer dereference attempting to connect duplicate channels
You could easily trigger this issue using multiple monitors and
a modified spice-gtk client with this patch:

--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -1699,6 +1699,7 @@ static gboolean _channel_new(channel_new_t *c)
 {
     g_return_val_if_fail(c != NULL, FALSE);

+    if (c->type == SPICE_CHANNEL_DISPLAY) c->id = 0;
     spice_channel_new(c->session, c->type, c->id);

     g_object_unref(c->session);

This as g_initable_new in this case returns NULL (dcc.c).

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2017-09-06 22:45:50 +01:00
Jonathon Jongsma
abe3e2f422 Dispatcher: validate received message types
Although dispatcher_send_message() does not allow you to send a message
type that is invalid for a dispatcher, it still makes sense to verify
that the type is valid in the receiver. This should only be possible in
the case of some severe problem such as memory corruption, so if it is
invalid, we simply abort.

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2017-09-06 14:20:34 -05:00
Frediano Ziglio
662d197a7d red-worker: Fix leak processing update commands
Is possible to have a leak processing update commands if
the update command is synchronous and the rectangle list
is empty. Note that Qemu always pass an empty list.

If the list is empty display_channel_update fill the list.
This is used to send back the list in case of asynchronous
requests. But in handle_dev_update_async (the callback that
handle the asynchronous case) the list is correctly freed.

This was discovered by accident looking at the code.

Reproduced with a Windows recording file using GCC address
sanitizer and this patch to spice-server-replay:

  --- a/server/red-replay-qxl.c
  +++ b/server/red-replay-qxl.c
  @@ -1280,7 +1280,13 @@ static void replay_handle_dev_input(QXLWorker *worker, SpiceReplay *replay,
           replay->created_primary = FALSE;
           worker->destroy_surfaces(worker);
           break;
  -    case RED_WORKER_MESSAGE_UPDATE:
  +    case RED_WORKER_MESSAGE_UPDATE: {
  +        static uint8_t count = 0;
  +        QXLRect dummy;
  +        QXLRect update = { 0, 0, 100, 100 };
  +        count ^= 1;
  +        worker->update_area(worker, 0, &update, count ? &dummy : NULL, count ? 1 : 0, 0);
  +        } break;
           // XXX do anything? we record the correct bitmaps already.
       case RED_WORKER_MESSAGE_DISPLAY_CONNECT:
           // we want to ignore this one - it is sent on client connection, we

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-09-06 12:54:42 +01:00
Frediano Ziglio
583a291880 cursor-channel: Use a single RedCursorPipeItem to hold the cursor
RedPipeItem already implements reference counting so
this avoid duplicating code to handle a object with reference
counting that points to another object with reference counting
that holds a RedCursorCmd.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-09-06 11:43:36 +01:00
Frediano Ziglio
bfaf660a05 red-channel: Do not push data calling red_channel_pipes_new_add_push
Now the push is done automatically when a PipeItem is added
(cfr commit 5c460de1a3
"worker: push data when clients can receive them"),
forcing a push cause only network fragmentation and is required
only if you are handling data in a loop instead of using the
default loop.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-09-06 11:39:03 +01:00
Frediano Ziglio
ee929dd43a red-channel: Reuse red_channel_pipes_add
Implements red_channel_pipes_add_type and
red_channel_pipes_add_empty_msg using red_channel_pipes_add.
This avoid duplicating items for each client.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-09-06 11:39:03 +01:00
Frediano Ziglio
f8212431d2 Use new red_channel_pipes_add instead of red_channel_pipes_new_add
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-09-06 11:39:03 +01:00
Frediano Ziglio
da3a5cc0ca red-channel: Add red_channel_pipes_add function
Considering that now RedPipeItem have reference counting
and that lot of items are just used to store constant
data to send, using reference counting instead of creating
different items for each client is easier to do.
So this new red_channel_pipes_add allows to add a single item
to all clients.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-09-06 11:39:03 +01:00
Frediano Ziglio
40c658f518 red-channel-client: Remove push call where not necessary
Now the push is done automatically when a PipeItem is added
(cfr commit 5c460de1a3
"worker: push data when clients can receive them"),
forcing a push cause only network fragmentation and is required only if
you are handling data in a polling loop (and thus, you are preventing
the default event loop from running).

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-09-06 11:38:19 +01:00
Jonathon Jongsma
02176e565c Move DispatcherMessage to source file
This is an internal implementation detail

Acked-by: Frediano Ziglio <fziglio@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
2017-09-05 13:50:16 -05:00
Frediano Ziglio
1138c3f54a display-channel: Make some declarations private
display-channel.h contains lots of information used by different
DisplayChannel components.
In the past all RedWorker, CursorChannel and DisplayChannel code was in
a single file. Since lots of code to handle DisplayChannel is still in
RedWorker, display-channel.h contains a lot of declarations so that they
can be accessed from RedWorker.
Moving declarations that are not needed by RedWorker and other external
class components helps to reduce dependencies between RedWorker and
DisplayChannel.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2017-09-02 08:27:26 +01:00
Frediano Ziglio
dd871c01a8 Avoid using global variable for channel IDs
This patch allocates VMC IDs by finding the first ID not used
instead of using a global variable and incrementing the value
for each channel created.
This solves some potential issues:
- remove the global state potentially making possible
  to use multiple SpiceServer on the same process;
- don't potentially overflow the variable. This can happen if
  channels are allocated/deallocated multiple times
  (currently not done by Qemu).

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2017-09-02 08:20:56 +01:00
Jonathon Jongsma
407ad2d63f tests: destroy test object
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2017-09-02 08:07:38 +01:00
Frediano Ziglio
de88e4d6b2 tests: Make variables static where possible
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2017-09-02 08:02:36 +01:00
Frediano Ziglio
55bb4f2395 tests: Remove unused declarations
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2017-09-02 08:02:36 +01:00
Frediano Ziglio
581b289825 tests: Avoid leaking list of commands
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2017-09-02 08:02:36 +01:00
Frediano Ziglio
69356fabcb tests: Pass command list as constant
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2017-09-02 08:02:36 +01:00
Frediano Ziglio
ea58b27aff tests: Reuse G_N_ELEMENTS instead of COUNT
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2017-09-02 08:02:36 +01:00
Frediano Ziglio
03e28721a2 red-client: Prevent RedChannelClient creation when the RedClient is being destroyed
This can happen as the connection is asynchronous so (MT main thread,
CT channel thread):
- MT you get a new connection;
- MT a connection is sent to CT;
- MT you get a disconnection of main channel;
- MT red_client_destroy is called;
- CT you attempt to add the RCC to RedClient.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-08-31 18:05:26 +01:00
Frediano Ziglio
1985b917bd red-qxl: Remove redundant checks
A RedChannelClient is always attached to a valid RedChannel.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-08-31 17:54:46 +01:00
Frediano Ziglio
f712f2637d red-worker: Set thread name if possible
Name will be visible in debugger and /proc filesystem

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-08-31 17:36:18 +01:00
Frediano Ziglio
c8bc09bcb3 red-qxl: Update header guard names
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-08-31 17:30:22 +01:00
Frediano Ziglio
d248714439 red-qxl: Reference RCC pointer in migration request
The message is asynchronous so to avoid the object to potentially
been released before being processed keep a strong reference to
it.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-08-31 17:28:37 +01:00
Christophe Fergeau
b89bd9ef9a worker: Fix 'immediatly' typo in comment
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
2017-08-31 15:51:57 +02:00
Christophe Fergeau
ab1348486d worker: Simplify flush_commands()
red_disconnect_display() is duplicating what red_channel_disconnect()
already does, so red_disconnect_display() and red_disconnect_cursor()
are actually identical code-wise. We can directly call
red_channel_disconnect() from flush_commands() rather than passing a
'red_disconnect_t disconnect' argument to that function.

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2017-08-31 15:51:57 +02:00
Christophe Fergeau
becd0d30f6 cursor: Remove cursor_channel_disconnect()
cursor_channel_disconnect() calls
cursor_channel_client_reset_cursor_cache() on all CursorChannelClient
associated with the current CursorChannel before calling
red_channel_disconnect().

red_channel_disconnect() will iterate over all CursorChannelClient
calling red_channel_client_disconnect(), which will eventually call
CursorChannelClient::on_disconnect. This will in turn
cursor_channel_client_reset_cursor_cache(), so calling it in
cursor_channel_disconnect() before calling red_channel_disconnect() is
redundant.

cursor_channel_disconnect() can thus be replaced by a direct call to
red_channel_disconnect().

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2017-08-31 15:51:57 +02:00
Christophe Fergeau
54eb7716f0 channel: Allow NULL RedChannelClient::on_disconnect()
SoundChannelClient has a stub implementation of
RedChannelClient::on_disconnect(), this commit removes the need for it.

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2017-08-31 15:51:57 +02:00
Christophe Fergeau
5dbfbb4d78 channel: Move RedChannel::on_disconnect to RedChannelClient
This vfunc only has a RedChannelClient * argument, and most of the time,
it operates on RedChannelClient, not on RedChannel. Moreover, the only
time it's used is from RedChannelClient. This commit moves the vfunc to
RedChannelClient, which seems like a better fit for it.

Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
2017-08-31 15:51:57 +02:00
Frediano Ziglio
14aee7cd74 gstreamer: Check if ORC library can work
ORC library is used internally by GStreamer to generate code
dynamically.
If ORC cannot allocate executable memory, the failure causes
an abort(3) to be called.
This happens on some SELinux configurations that disable executable
memory allocation (execmem boolean).
Check that ORC could work before attempting to use GStreamer to
avoid crashes.
While this check is done, the ORC library outputs an error which will
be well visible in Qemu output.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-08-30 15:59:46 +01:00
Frediano Ziglio
20676792a8 Avoid to access data before a NULL check
If you are testing for NULL data this means that variable could be
NULL so avoid to access before the check to make sure the check is hit.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
2017-08-30 15:56:43 +01:00
Frediano Ziglio
1fef2f507d main-channel: Fix multimedia time argument type
The multimedia time is defined as uint32_t.
Use the proper type instead of int.
Currently no arithmetic is done on this value but
just copies so considering that on the architectures
we support sizeof(int) == sizeof(uint32_t) there's
no change in the resulting machine code.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2017-08-29 16:37:21 +01:00