Some mistake in recent patch, thanks to Marc-André's eagle eyes.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
In big endian system GUINT32_TO_LE macro uses the parameter
multiple time causing serial to be incremented multiple times
instead of one.
Avoid side effects using a temporary variable.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
GStreamer is a hard requirement since ac0e50f or v0.36 wich is the
same release that PulseAudio backend was deprecated:
9a4b3bc "build-sys: deprecate the pulseaudio backend" in 2019-01-14
Signed-off-by: Victor Toso <victortoso@redhat.com>
Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com>
The protocol uses a u8 for the selection value. Make sure the given
argument value fits there, or throw a critical.
The other places seem to use u8 variables already.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Delay the release events for 0.5 sec. If no further grab comes in,
then release the grab. Otherwise, let's skip the release. This avoids
some races with clipboard managers.
Related to:
https://gitlab.freedesktop.org/spice/spice-gtk/issues/82
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
On the client side, whenever the grab owner changes (and the clipboard
was previously grabbed), spice-gtk sends a clipboard release followed
immediately by a new grab. But some clipboard managers on the remote
side react to clipboard release events by taking a clipboard grab,
presumably to avoid empty clipboards.
The two grabs, coming from the client and from the remote sides, will
race in both directions, which may confuse the client & remote side,
as both believe the other side is the current grab owner, and thus
further clipboard data requests are likely to fail.
Let's avoid sending a release event when re-grabing.
The race described above may still happen in other rare circunstances,
and will require a protocol change. To avoid the conflict, a discussed
solution could use a clipboard serial number.
Tested with current linux & windows vdagent. Looking at earlier
version of the code, it doesn't seem like subsequent grabs will be
treated as an error.
Signed-off-by: Marc-André Lureau <marcandre.lureau@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>
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>
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>
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>
They are used only inside the module.
Use a macro to simplify declaration which is pretty long.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
Files added without including them in compilation.
They contain implementation of SCSI commands for logical
units of mass-storage device class and USB bulk-only
mass-storage device protocol.
Signed-off-by: Alexander Nezhinsky<anezhins@redhat.com>
Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Redirection of emulated devices requires special approach, as
usbredirhost can't be used for that (it works only with libusb
devices). For emulated devices we create instance of usbredirparser
that implements USB redirection protocol. In order to work with the
same set of protocol capabilities that usbredirhost sets up with
remote side, the parser shall: - not send its own 'hello' to the
server
- initialize the same capabilities that usbredirhost
- receive the same 'hello' response
For that we analyze 'hello' sent by USB redir parser and extract set
of capabilities from it and upon receive of 'hello' response we
provide it also to the parser. When libusb device is redirected via a
channel, instance of usbredirhost serves it. When emulated device is
redirected via a channel, instance of usbredirparser does the job.
Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
