This function does not make much sense anymore.
Is called by RedVmcChannel which doesn't use RedChannelClient ACKs
so the variable changed are not used.
Moreover, at red_vmc_channel_constructed() time, there will be no
clients yet, so red_channel_init_outgoing_messages() will be a no-op.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
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>
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>
Mostly of red_channel_destroy calls were preceded by
a call to unregister the channel.
The only exception was the main channel as this channel is
always present and its initialisation is a bit different.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
From spice_server_port_event API you can send port events to
any char device. Although currently this is used only for "port"
devices implemented in spicevmc.c this will allow to support
such events using different objects.
This will be used for instance for a streaming device which
will be a specific SpicePort implementation.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
rcc is already deferenced in red_channel_client_get_client so
checking for NULL after that is uselss.
Also this call is generated from red_channel_client_disconnect
which requires the rcc pointer to be valid.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
This is the default value, and the property is marked as
_CONSTRUCT_ONLY, so we don't need to explicitly force the default when
instantiating a RedChannelClient.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
A few dispose/finalize implementations were not chaining up to their
parent classes, causing memory leaks.
This fixes leaks like the following:
==16240== Memcheck, a memory error detector
==16240== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==16240== Using Valgrind-3.12.0 and LibVEX; rerun with -h for copyright info
==16240== Command: ./test-leaks
==16240==
==16240==
==16240== HEAP SUMMARY:
==16240== in use at exit: 245,490 bytes in 3,638 blocks
==16240== total heap usage: 5,418 allocs, 1,780 frees, 17,676,968 bytes allocated
==16240==
==16240== 16 bytes in 1 blocks are definitely lost in loss record 203 of 771
==16240== at 0x4C2DADE: malloc (vg_replace_malloc.c:298)
==16240== by 0x4C2FC91: realloc (vg_replace_malloc.c:785)
==16240== by 0x52864E: dispatcher_register_handler (dispatcher.c:374)
==16240== by 0x5293E0: main_dispatcher_constructed (main-dispatcher.c:315)
==16240== by 0x7F873DB: g_object_new_internal (gobject.c:1823)
==16240== by 0x7F87EE4: g_object_new_valist (gobject.c:2042)
==16240== by 0x7F86E90: g_object_new (gobject.c:1626)
==16240== by 0x5292A5: main_dispatcher_new (main-dispatcher.c:295)
==16240== by 0x429A0A: do_spice_init (reds.c:3416)
==16240== by 0x42A3F5: spice_server_init (reds.c:3663)
==16240== by 0x4095B1: server_leaks (test-leaks.c:45)
==16240== by 0x844C60A: test_case_run (gtestutils.c:2161)
==16240== by 0x844C9CA: g_test_run_suite_internal (gtestutils.c:2244)
==16240== by 0x844CA73: g_test_run_suite_internal (gtestutils.c:2256)
==16240== by 0x844CC8A: g_test_run_suite (gtestutils.c:2332)
==16240== by 0x844B92C: g_test_run (gtestutils.c:1599)
==16240== by 0x409A0B: main (test-leaks.c:126)
==16240==
==16240== 16 bytes in 1 blocks are definitely lost in loss record 204 of 771
==16240== at 0x4C2DADE: malloc (vg_replace_malloc.c:298)
==16240== by 0x4C2FC91: realloc (vg_replace_malloc.c:785)
==16240== by 0x52864E: dispatcher_register_handler (dispatcher.c:374)
==16240== by 0x5293E0: main_dispatcher_constructed (main-dispatcher.c:315)
==16240== by 0x7F873DB: g_object_new_internal (gobject.c:1823)
==16240== by 0x7F87EE4: g_object_new_valist (gobject.c:2042)
==16240== by 0x7F86E90: g_object_new (gobject.c:1626)
==16240== by 0x5292A5: main_dispatcher_new (main-dispatcher.c:295)
==16240== by 0x429A0A: do_spice_init (reds.c:3416)
==16240== by 0x42A3F5: spice_server_init (reds.c:3663)
==16240== by 0x40BFD4: test_new (test-display-base.c:902)
==16240== by 0x40979D: vmc_leaks (test-leaks.c:92)
==16240== by 0x844C60A: test_case_run (gtestutils.c:2161)
==16240== by 0x844C9CA: g_test_run_suite_internal (gtestutils.c:2244)
==16240== by 0x844CA73: g_test_run_suite_internal (gtestutils.c:2256)
==16240== by 0x844CC8A: g_test_run_suite (gtestutils.c:2332)
==16240== by 0x844B92C: g_test_run (gtestutils.c:1599)
==16240== by 0x409A0B: main (test-leaks.c:126)
==16240==
==16240== 96 bytes in 1 blocks are definitely lost in loss record 638 of 771
==16240== at 0x4C2FA50: calloc (vg_replace_malloc.c:711)
==16240== by 0x8427D3C: g_malloc0 (gmem.c:124)
==16240== by 0x842801F: g_malloc0_n (gmem.c:355)
==16240== by 0x527B44: dispatcher_constructed (dispatcher.c:141)
==16240== by 0x529321: main_dispatcher_constructed (main-dispatcher.c:307)
==16240== by 0x7F873DB: g_object_new_internal (gobject.c:1823)
==16240== by 0x7F87EE4: g_object_new_valist (gobject.c:2042)
==16240== by 0x7F86E90: g_object_new (gobject.c:1626)
==16240== by 0x5292A5: main_dispatcher_new (main-dispatcher.c:295)
==16240== by 0x429A0A: do_spice_init (reds.c:3416)
==16240== by 0x42A3F5: spice_server_init (reds.c:3663)
==16240== by 0x4095B1: server_leaks (test-leaks.c:45)
==16240== by 0x844C60A: test_case_run (gtestutils.c:2161)
==16240== by 0x844C9CA: g_test_run_suite_internal (gtestutils.c:2244)
==16240== by 0x844CA73: g_test_run_suite_internal (gtestutils.c:2256)
==16240== by 0x844CC8A: g_test_run_suite (gtestutils.c:2332)
==16240== by 0x844B92C: g_test_run (gtestutils.c:1599)
==16240== by 0x409A0B: main (test-leaks.c:126)
==16240==
==16240== 96 bytes in 1 blocks are definitely lost in loss record 639 of 771
==16240== at 0x4C2FA50: calloc (vg_replace_malloc.c:711)
==16240== by 0x8427D3C: g_malloc0 (gmem.c:124)
==16240== by 0x842801F: g_malloc0_n (gmem.c:355)
==16240== by 0x527B44: dispatcher_constructed (dispatcher.c:141)
==16240== by 0x529321: main_dispatcher_constructed (main-dispatcher.c:307)
==16240== by 0x7F873DB: g_object_new_internal (gobject.c:1823)
==16240== by 0x7F87EE4: g_object_new_valist (gobject.c:2042)
==16240== by 0x7F86E90: g_object_new (gobject.c:1626)
==16240== by 0x5292A5: main_dispatcher_new (main-dispatcher.c:295)
==16240== by 0x429A0A: do_spice_init (reds.c:3416)
==16240== by 0x42A3F5: spice_server_init (reds.c:3663)
==16240== by 0x40BFD4: test_new (test-display-base.c:902)
==16240== by 0x40979D: vmc_leaks (test-leaks.c:92)
==16240== by 0x844C60A: test_case_run (gtestutils.c:2161)
==16240== by 0x844C9CA: g_test_run_suite_internal (gtestutils.c:2244)
==16240== by 0x844CA73: g_test_run_suite_internal (gtestutils.c:2256)
==16240== by 0x844CC8A: g_test_run_suite (gtestutils.c:2332)
==16240== by 0x844B92C: g_test_run (gtestutils.c:1599)
==16240== by 0x409A0B: main (test-leaks.c:126)
==16240==
==16240== LEAK SUMMARY:
==16240== definitely lost: 224 bytes in 4 blocks
==16240== indirectly lost: 0 bytes in 0 blocks
==16240== possibly lost: 0 bytes in 0 blocks
==16240== still reachable: 207,718 bytes in 3,312 blocks
==16240== of which reachable via heuristic:
==16240== newarray : 1,536 bytes in 16 blocks
==16240== suppressed: 34,548 bytes in 302 blocks
==16240== Reachable blocks (those to which a pointer was found) are not shown.
==16240== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==16240==
==16240== For counts of detected and suppressed errors, rerun with: -v
==16240== ERROR SUMMARY: 4 errors from 4 contexts (suppressed: 20 from 20)
FAIL test-leaks (exit status: 1)
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This commit changes all functions returning TRUE/FALSE from having an
'int' return value to 'bool'.
This way it's obvious that such a function is not going to return
anything else than TRUE or FALSE.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
These vfuncs are more appropriate in RedChannelClient.
The buffer they allocated are related to the client stream
which is managed directly by RedChannelClient.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
For each channel there are two set of capabilities, one
for the common ones and one for the specific ones.
A single set were almost always passed using 2 arguments,
a number of elements and an array but then before using
these were converted to a GArray.
Use a single structure (already available) to pass all
channel capabilites using a single argument.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
spice_new is a bit more safe as return a properly typed pointer.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Most channels don't need to do specific settings for the client socket
so avoid the need to do this setting making easier to setup the client
channnel.
Some improvements and commit subject suggested by Christophe Fergeau.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
TCP_NODELAY flag is set by default for all connection inside
reds.c so there's no need to set again for the single
client channel.
Note that there are still some calls to setsockopt to change this
option.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
red_channel_client_parse() currently does roughly:
if (klass->parser) {
parsed = klass->parser(msg, msg_size, &parsed_size);
klass->handle_parsed(rcc, parsed_size, msg_type, parsed);
} else {
klass->handle_message(rcc, msg_type, msg, msg_size);
}
The handle_parsed implementation expects a void * 'parsed' argument,
which will then be cast to the correct type. There is not really a need
to provide distinct handle_parsed/handle_message vfuncs, instead we can
say that if a RedChannel subclass provides a 'parser' vfunc, then it's
'handle_message' vfunc will be called with the parsed message, otherwise
it will be called with unparsed data (ie what handle_message currently
expects).
This makes the code slightly easier to follow as messages will always be
handled by the same vfunc.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This third argument (and the 'item' member of
RedChannelClient::priv::send_data) was a somewhat roundabout way to keep
the RedPipeItem alive until a message is sent, just in case some data
owned by that pipeitem was added to the marshaller by reference. This
was a rather confusing mechanism, however, since it did not have any
obvious connection to the _add_by_ref() call. It was never very clear
whether you needed to pass an item to this function or not. The previous
series of patches made this parameter unnecessary since the referencing
of the pipe item (or other related structure) is now more explicitly
connected to the calls to spice_marshaller_add_by_ref_full().
Acked-by: Frediano Ziglio <fziglio@redhat.com>
The only time that the pipe item needs to be passed as the third
argument to red_channel_client_init_send_data() is when the pipe item
holds a data buffer that has been added to the marshaller by reference
(spice_marshaller_add_by_ref()) and needs to be kept alive until the
data has been sent. In all other cases, the item does not need to be
kept alive, so we can safely pass NULL for this third parameter.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
The spice_marshaller_add_ref() family of functions is confusing since it
sounds like you're incrementing a reference on the marshaller. What it
is actually doing is adding a data buffer to the marshaller by reference
rather than by value. Changing the function names to _add_by_ref() makes
this clearer.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Pavel Grunt <pgrunt@redhat.com>
These cause compiler errors using RHEL6.
These typedefs are defined in the same file some lines above.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Add a 'self' parameter to all of the char device virtual functions so
that we don't have to play games with the 'opaque' pointer.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Instead of having channel and device object create one the other
create the objects at the beginning and then join them.
This make explicit the code that links the two objects and separate
the objects creation from the linking.
Also remove some boilerplate code.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
After renaming the object to RedVmcChannel, the local variables still
used the old 'state' terminology. Changing these variables to 'channel'
makes things a bit more consistent.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This helper function does nothing but cast the return from
red_channel_client_get_channel(), and it has a confusing name
(_get_state(), but returns a channel)
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This move object destructions from spicevmc_device_disconnect
to RedCharDeviceSpiceVmc destructor functions assuring any possible
RedCharDeviceSpiceVmc object free will clear its references.
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
No reason why this should be done only on spicevmc_device_disconnect.
red_char_device_write_buffer_release can be called with first pointer
NULL.
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Assure field is freed at the end and not used or allocate again.
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Store the channel in the char device object, and unref it in dispose.
Previously, we were just creating a channel and potentially allowing it
to leak. There may be better long-term solutions, but this works for
now.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Internal types use the 'Red' prefix by convention, rather than the
'Spice' prefix. In addition, this type inherits from RedChannel, so
makes the code a lot clearer to call it a 'channel' rather than a
'state'.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Convert the RedChannelClient heirarchy into GObjects. Since the existing
constructors could fail and return NULL, I inherited the base channel
client from GInitable, which introduces a dependency on gio.
When using private structs with GObject, there's a maximum size of (I
think) 64k, which was exceeded by some of the private structs. To avoid
this limitation I changed some members to dynamically allocated.
Encapsulate the structure a bit more and avoid poking into it from other
class implementations. Getting ready to split RedChannelClient out to a
separate file.
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.
Don't poke into the structure to get the channel
This prepares for encapsulating RedChannelClient a bit more and
separating it into its own source file.
Introduced by commit 903c91cd30, this
variable is used only for LZ4 code. Move the declaration to the proper
block of code.
Build log:
spicevmc.c: In function 'handle_compressed_msg':
spicevmc.c:346:14: error: variable 'decompressed' set but not used [-Werror=unused-but-set-variable]
uint8_t *decompressed;
^
Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
This was introduced by commit 903c91cd30.
To fix it, we simply protect the try_compress_lz4 function with proper
ifdef guards.
Build log:
spicevmc.c: In function 'try_compress_lz4':
spicevmc.c:143:5: error: implicit declaration of function 'LZ4_compress_default' [-Werror=implicit-function-declaration]
compressed_data_count = LZ4_compress_default((char*)&msg_item->buf,
^
spicevmc.c:143:5: error: nested extern declaration of 'LZ4_compress_default' [-Werror=nested-externs]
spicevmc.c: At top level:
spicevmc.c:124:24: error: 'try_compress_lz4' defined but not used [-Werror=unused-function]
static RedVmcPipeItem* try_compress_lz4(SpiceVmcState *state, int n, RedVmcPipeItem *msg_item)
^
Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Compressed message type is CompressedData which contains compression
type (1 byte) followed by the uncompressed data size (4 bytes - exists
only if data was compressed) followed by the compressed data
If SPICE_USBREDIR_CAP_DATA_COMPRESS_LZ4 capability is available &&
data_size > COMPRESS_THRESHOLD && !AF_LOCAL data will be sent
compressed otherwise data will be sent uncompressed (also if
compression has failed)
Update the required protocol to 0.12.12
Signed-off-by: Snir Sheriber <ssheribe@redhat.com>
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Frediano Ziglio <fziglio@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>
