Remove a race condition testing code with no libusb enabled.
Setting libusb_context field while the libusb event thread was using
causes a potential deadlock in the test.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
This brings in the following changes:
Eduardo Lima (Etrunko) (1):
build: Unconditionally link with libm if found
Fabrice Fontaine (2):
configure.ac: add --enable-tests
meson: add tests option
Frediano Ziglio (24):
proto: Demarshal Smartcard data field
codegen: Add 'chunk' to the output attributes
codegen: Check validity of array members
codegen: Document "chunk" attribute
codegen: Ignore path generating include guards
quic: Remove unused include header
quic: Use G_UNLIKELY in some hot paths
quic: Do not include quic_config.h in quic.h
snd_codec: Avoid some useless casts declaring struct type
snd_codec: Do not include not needed headers
build: Clean up some configure checks
test-quic: Convert image to get more testing (gray, rgb16)
log: Add spice_extra_assert
Reuse new spice_extra_assert macro
marshallers: Avoid some useless pointers in SpiceMarshallerData
lz_decompress: Constify some variable
lz_decompress: Do not execute nested checks
lz_decompress: Simplify loop
lz_decompress: Reindented comment
lz_decompress: Move variable declaration in nested scope
lz_decompress: Read "ctrl" inside loop
lz_compress: Cleanup unused macros
codegen: Check unsafe values alone
snd_codec: Update field names in function documentation
Kevin Pouget (4):
common/recorder.h: do not complain on unused (dummy) recorders
agent-interface: introduce the core of the Agent Interface
agent-interface: add configuration functions
build: Introduce the agent-interface as an alternative instrumentation library
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
The files are not in src so not included anyway.
Acked-by: Francesco Giudici <fgiudici@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Same result but removing the single directly usage of libdrm include
which is not being checked. The header epoxy/egl_generated.h contains
the macro EGL_LINUX_DRM_FOURCC_EXT that we use in spice-widget-egl.c
and that is included in epoxy/egl.h already.
Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This function is defined and called only from usb-device-manager.c.
Avoids to export it making an ABI to maintain in the future.
This API was added in commit 2a256ad0b4
(cfr "add spice_usb_device_manager shared CD related api functions"),
no release contains it so it's fine to remove it.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
check widget was identified by taking the child of the passed widget
but the check widget itself is being passed
Acked-by: Frediano Ziglio <fziglio@redhat.com>
When a shared cd fails to connect, experiences a device error
or gets disconnected, its device should be removed to avoid the
'existing but not connected' state, which is disallowed for cd devices.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Shared CD devices have special lifecycle requirements: they are always
auto-redirected after being created and always destroyed after being
disconnected. Thus the intermediate state of an existing but not connected
device, which is normal for the physical devices, is not supported.
For the devices added using the corresponding command line options it
means that they should be auto-connected on the viewer start.
By default such devices are redirected subject to the 'redirect-on-connect'
filter, which does not fit the shared CD connecting requirements above.
Instead, CDs are redirected unconditionally by this patch.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
The empty CD entry is a placeholder and appears every time the widget is created.
When it is toggled, a file chooser dialog is popped up. If a file or device is
selected, a new CD device is created. The new CD device is auto-connected.
This device is communicated to the widget through the 'device-added' signal.
The list entry responsible for the new device corresponds to the entity
provided by usb-device-manager. The empty CD entry is automatically moved to
the top of the list.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
The following functions are added:
spice_usb_device_manager_create_shared_cd_device
spice_usb_device_manager_is_device_shared_cd
spice_usb_device_manager_remove_shared_cd_device
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This reverts commit 109e575 "channel: Abort migration in delayed
unref" in 2016-05-02 by Pavel Grunt <pgrunt@redhat.com>
This commit is being reverted because it calls
spice_session_abort_migration() on the SpiceSession for the target
host while the function should receive the SpiceSession for current
host.
I can actually reproduce a bug where the password expires for the
current Spice session and the migration is going to fail because of
that. Before reverting this commit, remote-viewer would hang and the
following logs would occur:
- Migration starts on channel-main
> ../src/channel-main.c:2174 migrate_channel_connect 1:0
- Using TLS
> ../src/spice-channel.c:1934 main-1:0: switching to tls
- Following logs are related to failure to connect due Authentication
> ../src/spice-channel.c:2000 main-1:0: use mini header: 1
> ../src/spice-channel.c:1274 main-1:0: link result: reply 7
> ../src/spice-channel.c:2680 main-1:0: Coroutine exit main-1:0
> ../src/spice-channel.c:2871 main-1:0: reset
Remote-viewer would hang because we are in a state of migration and
SpiceMainChannel does not know that it failed because
spice_session_abort_migration() is being called on SpiceSession of
target host and no SpiceChannel::channel-event is being emitted.
Reverting this patch does abort migration thanks to
SpiceChannel::channel-event being emitted and caught by
SpiceMainChannel at migrate_channel_event_cb() with the log:
> ../src/channel-main.c:2247 main-1:0: error or unhandled channel event during migration: 23
> ../src/channel-main.c:2373 main-1:0: migrate failed: some channels failed to connect
Note that the reverted patch mentions the fix of bug [0] which is also
a virt-viewer with a hanging state. Looking back to the logs, the
interesting part around issues around usbredir, that is:
> GSpice-DEBUG: channel-main.c:2236 migration: channel opened chan:0x29ddce0, left 6
> GSpice-DEBUG: spice-channel.c:916 usbredir-9:0: Read error Error receiving data: Connection reset by peer
> GSpice-DEBUG: spice-channel.c:1210 usbredir-9:0: error, switching to protocol 1 (spice 0.4)
> GSpice-DEBUG: spice-channel.c:2433 usbredir-9:0: Coroutine exit usbredir-9:0
> GSpice-DEBUG: spice-channel.c:2455 usbredir-9:0: Open coroutine starting 0x29e10d0
> GSpice-DEBUG: spice-channel.c:2289 usbredir-9:0: Started background coroutine 0x29e0750
> GSpice-DEBUG: spice-channel.c:916 usbredir-9:1: Read error Error receiving data: Connection reset by peer
> GSpice-DEBUG: spice-channel.c:1076 usbredir-9:1: incomplete auth reply (-104/4)
> GSpice-DEBUG: spice-channel.c:916 playback-5:0: Read error Error receiving data: Connection reset by peer
> GSpice-DEBUG: spice-channel.c:1076 playback-5:0: incomplete auth reply (-104/4)
> GSpice-DEBUG: spice-channel.c:916 display-2:0: Read error Error receiving data: Connection reset by peer
> GSpice-DEBUG: spice-channel.c:1076 display-2:0: incomplete auth reply (-104/4)
> GSpice-DEBUG: spice-session.c:1775 connecting 0x7f12bffffa50...
> GSpice-DEBUG: spice-session.c:1850 open host lab.test.me:5900
> GSpice-DEBUG: channel-main.c:2241 error or unhandled channel event during migration: 22
> GSpice-DEBUG: spice-session.c:1665 session: disconnecting 0
> GSpice-DEBUG: spice-channel.c:2160 usbredir-9:1: channel got error
> GSpice-DEBUG: spice-channel.c:2433 usbredir-9:1: Coroutine exit usbredir-9:1
> GSpice-DEBUG: channel-main.c:2241 error or unhandled channel event during migration: 22
So, the expected behavior was happening on error, which is
channel-main aborting the migration due SpiceChannel::channel-event
being emitted with some failure.
Also note that, with this commit reverted, I don't experience any
hanging after migration is aborted. Likely the original bug was fixed.
[0] https://bugzilla.redhat.com/show_bug.cgi?id=1318574
Related: https://bugzilla.redhat.com/show_bug.cgi?id=1695618
Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
This helps to debug plugins load related issues, also factory name
is usually a bit more accurate (if factory exists the elements name
may also include some suffix).
Signed-off-by: Snir Sheriber <ssheribe@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
"spice_device" and "device" points to the same object.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Francesco Giudici <fgiudici@redhat.com>
Now that SpiceUsbBackendDevice and SpiceUsbDevice are the same
structure is useless to pass the same argument twice.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Francesco Giudici <fgiudici@redhat.com>
Now that SpiceUsbBackendDevice and SpiceUsbDevice are the same
there's no much sense to call it.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Francesco Giudici <fgiudici@redhat.com>
This fixes building spice-gtk on Debian 10.
See https://github.com/mesonbuild/meson/issues/4720.
Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
Acked-by: Uri Lublin <uril@redhat.com>
The msg is no longer valid after spice_msg_out_send_internal(),
so it shouldn't be present in c->flushing since the same
memory can be assigned to a new message.
This currently causes issues when transferring multiple big
files at once (tested 4 × 200 MB).
The following can happen in agent_send_msg_queue(),
after msg is flushed:
(task1, task2 refers to two distinct SpiceFileTransferTasks,
one per file)
g_task_return_boolean(task1) ->
file_xfer_data_flushed_cb(task1) ->
spice_file_transfer_task_read_async(task1) ->
g_coroutine_object_notify() [switches from coroutine context to main]
...
file_xfer_read_async_cb(task2) ->
file_xfer_queue_msg_to_agent(task2)
[allocates new msg for task2 at the same address as the old msg] ->
file_xfer_flush_async(task2)
[inserts to c->flushing, but the old msg is still present,
so the data in the hash table changes from task1 to task2]
...
program returns to coroutine context,
g_task_return_boolean(task1) returns ->
g_hash_table_remove() [removes the entry that now contains task2]
Consequently, flush task for task2 never finishes
and it gets stuck.
This issue is present since 2261e50a64,
that replaced g_object_notify() in spice_file_transfer_task_read_async()
with g_coroutine_object_notify().
To solve it, remove the msg from c->flushing
before finishing the flush task.
Signed-off-by: Jakub Janků <jjanku@redhat.com>
agent_msg_queue_many is a variadic function reading parameters
after the third using va_arg.
Specifically it read sizes of buffers using the "gsize" type.
On x64 for Windows platform only first 4 argument of
agent_msg_queue_many are passed by registers while the rest is
passed on the stack. So the size is written in the stack.
On x64 gsize is 64 bit while data_size in
file_xfer_queue_msg_to_agent is an int which is 32 bit.
So in some cases when data_size is stored in the stack in order
to call agent_msg_queue_many from file_xfer_queue_msg_to_agent
the compiler will write only 32 bit, like for instance with:
mov %ebx,0x28(%rsp)
The problem is that agent_msg_queue_many will use "va_arg(args, gsize)"
reading 64 bit instead of 32. In this case the lower 32 bit
part will be the "data_size" but the higher 32 bit part will be the
previous content of the stack, basically garbage.
This will cause the read size to be a huge value and program will
crash.
This could not be exploited the operation will lead to only read
extra bytes and then crash.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
"device" field is just written, never read.
"manager" field can be retrieved using g_task_get_source_object.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Signed-off-by: Snir Sheriber <ssheribe@redhat.com>
If the pointer is freed with g_object_unref the two do the
same.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Signed-off-by: Snir Sheriber <ssheribe@redhat.com>
Simple automatic update of the file to sync with sources
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Francesco Giudici <fgiudici@redhat.com>
This is a common function for adding an error status message.
If an old message exists, checks if the new message is not already contained
in the old one. The new message is ignored if it is, added if it's not.
New message string should be dynamically allocated, it's always g_free-ed.
Acked-by: Frediano Ziglio <fziglio@redhat.com>
It just holds a reference to the primary surface which is being
removed and freed from the hash table. This ends up generating a
warning below in the same function before setting the new primary
surface reference.
Signed-off-by: Victor Toso <victortoso@redhat.com>
In case the drawing on the screen is scaled the scaling required to
invalidate a slightly bigger region.
This is due to the interpolation done during the resize.
So if scaling is performed invalidate also the adjacent pixels.
This fixes https://gitlab.freedesktop.org/spice/spice-gtk/issues/19.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Migration can start with different messages and the code could take
different paths. By having a debug on which message started can help
pinpoint issues faster.
Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Although currently not supported by the code (libusb_context in
SpiceUsbBackend is never NULL), try to support it in order to be able to
have only emulated devices if the libusb layer is failing.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Currently we call this function when the SPICE channel is up
however this function should continue to work as in theory
the channel could avoid to handle the message and stop the flow
(for instance to implement some kind of flow limitation)
and so will need to call this function again.
This was failing in the first USB emulation implementation
causing a crash.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Mock some usb-backend functions to be able to simulate device
attachment and detachment.
Create session and channel to pass some valid pointer anyway.
Emulate channel state correctly.
Make sure HELLO packets are sent correctly at the beginning and
no more afterwards.
Test auto-connect enabled or disabled.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Instead of linking all object inside spice-client-glib build a library
from these object and link to each test.
This will allow to override some object file for mocking purposes.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Just allocate and free to test for base leaks and reference
counting.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
GStreamer is already initialized using gst_init_get_option_group and
gst_init_get_option_group.
From gst_init documentation:
WARNING: This function does not work in the same way as
corresponding functions in other glib-style libraries,
such as gtk_init(). In particular, unknown command line options
cause this function to abort program execution.
This luckily is not true (program won't abort if you pass --gst-foo
for instance) but better to stick to documentation.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Snir Sheriber <ssheribe@redhat.com>
These files would end up to compile empty code, no reason
to compile and link them.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Add implementation of emulated device to build.
Now it is possible to create emulated CD devices.
Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
