This commit reworks a bit the management of RedCompressBuf so that
compress_buf_new/compress_buf_free become unneeded.
Since d25d6ca0 and the introduction of encoder_data_reset,
compress_buf_free is already unused outside of dcc-encoders.c and could
be static. This in turn makes compress_buf_new a bit odd as the matching
destructor is never used in dcc.c.
This commit introduces an encoder_data_init() method which is hiding
the initialization of the EncoderData structure from the dcc.c code,
allowing to get rid of compress_buf_new() calls from dcc.c code.
It also uses this as an opportunity to stop using GSlice for
RedCompressBuf.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
A new function to send fd with unix socket anciliary data.
Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This allow tests programs to link with statically built library to access all symbols
Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
When redirecting a USB webcam over a slow link, it's currently possible
to hit an assertion in spice-server by running cheese (application using
the webcam), killing the client with ctrl+c and then restarting the
client:
qemu-kvm: spicevmc.c:324: spicevmc_red_channel_alloc_msg_rcv_buf:
Assertion `!state->recv_from_client_buf' failed.
This happens when red_peer_handle_incoming tries to allocate memory for
a message using spicevmc:
handler->msg = handler->cb->alloc_msg_buf(handler->opaque, msg_type,
msg_size);
red_peer_handle_incoming() is called when there is client data to be
read, and does
- call alloc_msg_buf() to allocate memory for the message
- read the message
- if the read was partial, return early, the main loop will call again
red_peer_handle_incoming() when there is more data available for that
channel
- parse the message
- call release_msg_buf() to free the message
For channels based on spicevmc (usbredir and port), alloc_msg_buf()
stores message data in SpiceVmcState::recv_from_client_buf and before
allocating new memory, it asserts that it's NULL.
This is what causes this crash in the following scenario:
- SpiceVmc::alloc_msg_buf() is called and allocates memory for a new
message in SpiceVmcState::recv_from_client_buf
- red_peer_handle_incoming() returns early as all the spicevmc message
data hasn't been received yet
- the client gets killed
- the main channel notices the disconnect and calls
main_dispatcher_client_disconnect() which will disconnect all the
channels
- SpiceVmc::on_disconnect is called
- after the new client connects, SpiceVmc::alloc_msg_buf() is called,
notices that SpiceVmcState::recv_from_client_buf is already set, and
asserts()
This commit makes sure the partial SpiceVmcState::recv_from_client_buf
data is cleared on disconnect so that the assert does not trigger.
This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1264113
When COMPRESS_STAT is not set, and RED_WORKER_STAT is set,
stat_time() will be a no-op, but stat_start_time_init() will try to use
stat_info_t::clock. This causes a compile warning on 32 bit arches (not
sure why not on 64 bit builds), as well as an error from valgrind.
Since stat_time() and stat_compress_time() are both doing the same work,
this commit makes them the same function, which ensures
stat_info_t::clock will be set and stat_start_time_init() can be used
regardless of the _init() method which is called.
This avoids compilation errors with -Werror on 32 bit systems as the
pointer size differs from that of a QXLPHYSICAL.
Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
This avoids a compilation error with -Werror on 32 bit systems as the
pointer size differs from that of an uint64_t.
Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
Now we can use the iface parameter to distinguish the context instead
of doing strange assumption on opaque and its state.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
This patch and previous ones want to solve the problem of not having a
context in SpiceCoreInterface. SpiceCoreInterface defines a set of
callbacks to handle events in spice-server. These callbacks allow to
handle timers, watch for file descriptors and send channel events.
All these callbacks do not accept a context (usually in C passed as a
void* parameter) so it is hard for them to differentiate the interface
specified.
Unfortunately this structure is used even internally from different
contexts for instance every RedWorker thread has a different context. To
solve this issue some workarounds are used. Currently for timers a variable
depending on the current thread is used while for watches the opaque
parameter to pass to the event callback is used as it currently points just
to RedChannelClient structure. This however imposes some implicit
maintainance problem in the future. What happens for instance if for some
reason a timer is registered during worker initialization, run in another
thread? What if we decide to register a file descriptor callback for
something not a RedChannelClient? Could be that the program will run
without any issue till some bytes change and weird things could happen.
The implementation of this solution is done implementing an internal "core"
interface that has context specific and use it to differentiate the
context instead of relying on some other, hard to maintain, detail. Then an
adapter structure (name inpired to the adapter pattern) will provide the
internal core interface using the external, public, definition (in the
future this technique can be used to extend the external interface without
breaking the ABI).
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Add wrapper functions for SpiceCoreInterface in order to present
a SpiceCoreInterfaceInternal. These functions just expect
SpiceCoreInterfaceInternal API and forward request to
SpiceCoreInterface.
This allows to change ABI details of internal one.
See comments in "channel: add interface parameters to
SpiceCoreInterfaceInternal" patch.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Define an internal structure that matches 100% the ABI of the public one.
The structure will be changed by following patches.
See comments in "channel: add interface parameters to
SpiceCoreInterfaceInternal" patch.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Use CLOCK_THREAD_CPUTIME_ID instead of getting the clock
with pthread_getcpuclockid.
This avoids to call red_worker_get_clockid. This function returns
uninitialized value at the time DisplayChannel is built resulting in setting
statistics to CLOCK_REALTIME (which is 0) instead to cpu time as expected.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Client can always send report even if a stream is not available.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Instead of returning 0 which could be a valid value returns an invalid
one and check on the caller.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
For zlib+glz compression image if first compressed with glz then the
resulting buffer is compressed again with zlib then the buffer from
zlib is returned to the caller.
However the temporary glz buffer was not freed resulting in a memory
leak.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
As reported in https://bugs.freedesktop.org/show_bug.cgi?id=93520, the
build from git is failing in Archlinux systems with undefined
references to glib symbols.
This patch fixes the problem by simply reordering the order libraries
will be linked. Also, removes duplicate $(GLIB_LIBS) variable.
Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Don't disconnect the display channel, when unsupported compression is
requested from the client. Not changing the compression is enough.
https://bugs.freedesktop.org/show_bug.cgi?id=92821
Acked-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
The size_t definition is different between 32 and 64 bit systems so that
neither '%u' nor '%lu' work for both. '%zu' should be used instead.
Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
The corresponding code has been moved elsewhere during the refactoring.
Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
Some integer type definitions are different between 32 and 64 bit
systems which causes problems in printf. The PRI macros automatically
provide the printf format appropriate for the system.
Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
For coherency use COMMON_BASE macro instead of including single
files.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Do not free memory allocated with C functions (like calloc) using g_free;
although this is possible with default Glib allocator this is not safe.
Also use consistent allocation functions. All other spice-server code
does not use Glib allocations so for coherence do not use them for
watches.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Since commit febaed3, spice.h and spice-experimental.h are listed both
in libspice_server_la_HEADERS and libspice_server_la_SOURCES. Since we
want these headers to be installed, we can remove them from _SOURCES.
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
They clarify the time unit being used and simplify calculations.
Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
They clarify the time unit being used, reduce the need for casts and
simplify calculations.
Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Given that it is used for both cursor and display, COMMON_CLIENT_TIMEOUT
seems more appropriate. Also define it only in red-worker.h.
Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
With multiple cards configured you can have multiple workers running in
different thread.
With such configuration static variables not syncronized could lead
to undefined behavior.
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
The rest of code is using spice_malloc* functions, use them for
consistency.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
