Noting that coding by hand these loop introduced some regression
I'm trying to introduce back from macros.
Before trying something harder to make possible to bind the type of
the content I'm trying some simple macro as were before.
I added the type to avoid some blindly void* casts.
Also the GListIter is introduced to avoid the possibility to exchange
easily some parameters.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Instead of using a Ring (and having a ring item link in every pipe
item), store them in a GQueue. This also necesitated changing
RedCharDeviceVDIPort->priv->read_bufs to a GList as well.
Also Optimise client pipe by passing pipe position instead of data.
This avoids having the search the data scanning all the queue changing
the order of these operations from O(n) to O(1).
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Explicitely discard AGENT_MSG_FILTER_MONITORS_CONFIG messages
from the agent.
Also remove unused AGENT_MSG_FILTER_END
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Make RedsState::mig_target_clients into a GList to improve encapsulation
and maintainability. Also RedsMigTargetClient::pending_links. With
GList, a type implementation can be ignorant of whether they're
contained within a list or not.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Previously we were creating a variable named 'dev_state' and then
apparently not using it. Well, we *were* actually using it, but in a
convoluted sort of way. Creating a new RedCharDevice has a
side-effect of setting itself as the 'st' attribute of
SpiceCharDeviceInstance. So 'dev_state' and 'char_device->st' are in
fact the same variable. But they were being used interchangeably, which
was rather confusing. For example
if (dev_state)
// do something with char_device->st
So this patch doesn't actually change anything, but it makes the code a
bit easier to follow.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Agent properties like file transfer or copy & paste can be disabled by
calling spice_server_set_agent_{copypaste, file_xfer} before the spice
server is initialized. In that case the call crashes the server because
the agent device is created after the initialization.
To avoid the crash this commit introduce a helper function for setting
the agent properties after the server is initialized.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Instead of having all other classes poke into the RedChannelClient
struct to get the RedClient associated with the channel client, call the
accessor function. This commit allows us to encapsulate RedChannelClient
and move it to its own file soon.
Eventually, during a seamless migration, qemu may finish to migrate
before the spice client even finished to connect all channels to
destination and informed the server. In this case,
main_channel_client_migrate_src_complete() will fall back to
switch-host method, and reds_mig_fill_wait_disconnect() is called to
complete the migration (disconnecting all channels).
reds_mig_cleanup() is called when all channels are disconnected, but
reds->mig_wait_connect is still TRUE, and it will call
migrate_connect_complete() instead of the expected
migrate_end_complete(). Setting reds->mig_wait_connect to FALSE when
reds_mig_fill_wait_disconnect() solves the issue.
Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=1352836
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
g_object_new is a variadic function which takes property values.
As the compiler cannot check if these property values are correct,
make sure they are using casts.
This actually fixes a crash in reds.c for 32 bit architectures.
Based on a patch by Frediano Ziglio <fziglio@redhat.com>
Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
Users should not change the list of supported subtypes.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Avoid multiple initializations of this library.
Also initialize using thread safe code to avoid possible race
conditions.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
configure will use GStreamer 1.0 if present and fall back to
GStreamer 0.10 otherwise.
ffenc_mjpeg takes its bitrate as a long so extend set_gstenc_bitrate().
Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
The Spice server administrator can specify the encoder and codec
preferences to optimize for CPU or bandwidth usage. Preferences are
described in a semi-colon separated list of encoder:codec pairs.
The server has a default preference list which can explicitly be
selected by specifying 'auto'.
Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
As the tokens counter were not being reset you could enter in a
situation where client thinks it has more tokens then server which
would eventually lead to client's disconnection from 0c5eca97f1
onwards (before it was crashing).
It is easy to check the above situation if you track the amount of
tokens you have in the client and simply kill and restart the agent
while doing some file transfer: the client could reach more then 13
tokens which should not really be possible.
Based on patch from Frediano Ziglio <fziglio@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This variable was always the same value as
dispatcher_allows_client_mouse.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Possibly used for debugging or an initial recursive lock.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Setting "sin" property is equivalent to call
red_char_device_reset_dev_instance so there is no need for a if/else
as the code is doing mostly (beside setting agent_attached) the
same thing
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
RedCharDevice used for the agent has flow control enabled.
This make possible for red_char_device_write_buffer_get to return NULL.
Handle such situation without crashing avoiding NULL dereference.
This fixes https://bugs.freedesktop.org/show_bug.cgi?id=95416.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
This was proposed by Christophe as improvement over some typesafe
patches.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Make code more type safe. This allow to move or delete structure
fields more safely
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
All other classes using RedPipeItem as base use base as parent name
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Removing an interface cause SpiceBaseInstance->st to be set to NULL.
This pointer was then deferenced in agent code.
As SpiceBaseInstance should not be used after this call make sure
we don't keep pointers to it.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
This code make easier to be sure we don't have dangling pointers
resetting in the function which free the structure.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
The include directory is specified with the -I which is the directory
used directly by #include<>.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
When a client disconnects remove it from the list of clients connected
to the spice char-device.
This was caused by commit 1cec1c5118
("reds: Make VDIPortState a GObject") as the lifespan of RedCharDevice
was changed.
This could be reproduced with:
- start rhel7 machine
- connect remote viewer (RV)
- RV: login
- connect ssh
- SSH: stop agent
- disconnect RV
- SSH: start agent
- connect to RV
and caused (using address sanitizer):
main_channel_handle_parsed: agent start
=================================================================
==29592==ERROR: AddressSanitizer: heap-use-after-free on address 0x60c00001cff0 at pc 0x7fa85b6e8595 bp 0x7ffde3801940 sp 0x7ffde3801930
READ of size 8 at 0x60c00001cff0 thread T0
#0 0x7fa85b6e8594 in red_client_get_main /home/freddy/work/spice-server/server/red-channel.c:2190
#1 0x7fa85b7311e6 in vdi_port_send_msg_to_client /home/freddy/work/spice-server/server/reds.c:880
#2 0x7fa85b69383e in red_char_device_send_msg_to_client /home/freddy/work/spice-server/server/char-device.c:138
#3 0x7fa85b69383e in red_char_device_send_msg_to_clients /home/freddy/work/spice-server/server/char-device.c:356
#4 0x7fa85b69383e in red_char_device_read_from_device /home/freddy/work/spice-server/server/char-device.c:403
#5 0x55a2633b81c1 (/usr/bin/qemu-system-x86_64+0x5561c1)
#6 0x55a2633afe7a (/usr/bin/qemu-system-x86_64+0x54de7a)
#7 0x55a2634cb7b1 (/usr/bin/qemu-system-x86_64+0x6697b1)
#8 0x55a2632078d0 (/usr/bin/qemu-system-x86_64+0x3a58d0)
#9 0x55a26379b2e8 (/usr/bin/qemu-system-x86_64+0x9392e8)
#10 0x55a26379a7a0 (/usr/bin/qemu-system-x86_64+0x9387a0)
#11 0x55a26313fb78 in main (/usr/bin/qemu-system-x86_64+0x2ddb78)
#12 0x7fa85a3cc57f in __libc_start_main (/lib64/libc.so.6+0x2057f)
#13 0x55a26314b0c8 (/usr/bin/qemu-system-x86_64+0x2e90c8)
0x60c00001cff0 is located 48 bytes inside of 128-byte region [0x60c00001cfc0,0x60c00001d040)
freed by thread T0 here:
#0 0x7fa869e3667a in __interceptor_free (/lib64/libasan.so.2+0x9867a)
#1 0x7fa85b6d75f7 in red_client_unref /home/freddy/work/spice-server/server/red-channel.c:2076
#2 0x7fa85b6ead74 in dispatcher_handle_single_read /home/freddy/work/spice-server/server/dispatcher.c:291
#3 0x7fa85b6ead74 in dispatcher_handle_recv_read /home/freddy/work/spice-server/server/dispatcher.c:314
#4 0x55a26379b2e8 (/usr/bin/qemu-system-x86_64+0x9392e8)
#5 0x55a26379a7a0 (/usr/bin/qemu-system-x86_64+0x9387a0)
#6 0x55a26313fb78 in main (/usr/bin/qemu-system-x86_64+0x2ddb78)
#7 0x7fa85a3cc57f in __libc_start_main (/lib64/libc.so.6+0x2057f)
previously allocated by thread T0 here:
#0 0x7fa869e36b19 in __interceptor_calloc (/lib64/libasan.so.2+0x98b19)
#1 0x7fa85b7d6858 in spice_malloc0 /home/freddy/work/spice-server/spice-common/common/mem.c:109
#2 0x7fa85b6e760c in red_client_new /home/freddy/work/spice-server/server/red-channel.c:2053
#3 0x7fa85b7449e4 in reds_handle_main_link /home/freddy/work/spice-server/server/reds.c:1762
#4 0x7fa85b7449e4 in reds_handle_link /home/freddy/work/spice-server/server/reds.c:2002
#5 0x7fa85b745d3a in reds_handle_ticket /home/freddy/work/spice-server/server/reds.c:2056
#6 0x55a26379b2e8 (/usr/bin/qemu-system-x86_64+0x9392e8)
#7 0x55a26379a7a0 (/usr/bin/qemu-system-x86_64+0x9387a0)
#8 0x55a26313fb78 in main (/usr/bin/qemu-system-x86_64+0x2ddb78)
#9 0x7fa85a3cc57f in __libc_start_main (/lib64/libc.so.6+0x2057f)
SUMMARY: AddressSanitizer: heap-use-after-free /home/freddy/work/spice-server/server/red-channel.c:2190 red_client_get_main
Shadow bytes around the buggy address:
0x0c187fffb9a0: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
0x0c187fffb9b0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c187fffb9c0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
0x0c187fffb9d0: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
0x0c187fffb9e0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
=>0x0c187fffb9f0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd[fd]fd
0x0c187fffba00: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
0x0c187fffba10: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c187fffba20: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
0x0c187fffba30: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
0x0c187fffba40: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
This variable belongs to SpiceServerConfig rather than being a static
global variable hidden in sound.c
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
channels_info->num_of_channels is assigned, its value is not used, and
then it's assigned a different value. The first assignment can be
removed.
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Instead of exporting 2 methods to get number of channels, and to fill
channel information, and use that from the main channel code, it's
better to do everything in one go in reds.c, and call that single method
from the main channel code.
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
