summaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2016-07-27potential leaks with file transfer codefile-transferChristophe Fergeau1-0/+4
2016-07-27spicy: Fix spice_file_transfer_task_get_filename leakChristophe Fergeau2-2/+5
It was wrongly annotated as (transfer none) An alternative would be to store what is returned by g_file_get_basename() in SpiceFileTransferTask and return that, this would allow to make spice_file_transfer_task_get_filename() (transfer none) without leaking memory.
2016-07-27file-transfer: Fix SpiceFileTransferTask::file_stream leakChristophe Fergeau1-0/+1
g_file_read_finish() is (transfer full) so we must release the ref we got in _dispose() as it's not done anywhere else in the code.
2016-07-27file-transfer: Fix GTask leaksChristophe Fergeau1-0/+9
The file transfer code calls g_*_async() methods with a GTask as the user_data argument. It needs to own a reference to the GTask for the duration of the async call to keep the GTask alive, however it was never releasing it when it no longer needs it (that is, after calling g_task_return_*).
2016-07-27test-file-transfer: Don't leak GErrorChristophe Fergeau1-0/+2
2016-07-27test-file-transfer: Don't leak GFileInfoChristophe Fergeau1-0/+5
The GFileInfo returned by spice_file_transfer_task_init_task_finish() must be unref'ed when no longer needed.
2016-07-27vmc: Fix leak in spice_vmc_output_stream_write_finish()Christophe Fergeau1-1/+5
We own a reference on the GAsyncResult returned by g_task_propage_pointer() so we have to g_object_unref() it when we no longer need it.
2016-07-27tests: Use WARN_CFLAGSChristophe Fergeau5-6/+7
No -W flags were used when building tests. This commit adds WARN_CFLAGS to the default flags used when building tests, and fixes the various warnings which were reported.
2016-07-27build: Rename SPICE_CFLAGS to WARN_CFLAGSChristophe Fergeau2-6/+3
This only contains -W* flags, so the WARN_CFLAGS naming is much more explicit.
2016-07-27Handle pause key correctlyFrediano Ziglio1-0/+29
Windows does not like Pause key sent with same scancodes as Break. Although is the same physical key the two functions send two completely different set of codes. On key press a E1 1D 45 sequence is generated while on key release a E1 9D C5 is generated. Also some hardware keyboards send press+release at the same time on key press (nothing on release). Tested with Linux and Windows clients. Tested with Linux, Windows and DOS guests. On Windows guest VK_PAUSE was not arriving correctly. An additional send_pause function was added to avoid to change the normal flow too much. This as the keymap table (generated from src/keymaps.csv) can hold only one scancode while this key generate two scancodes (ie E1-1D and 45) for each event. Signed-off-by: Frediano Ziglio <fziglio@redhat.com> Acked-by: Pavel Grunt <pgrunt@redhat.com>
2016-07-26Support more extension codesFrediano Ziglio1-13/+7
Not only E0 prefix but also E1 and E2. They are used in some special cases, one is the pause key. Signed-off-by: Frediano Ziglio <fziglio@redhat.com> Acked-by: Pavel Grunt <pgrunt@redhat.com>
2016-07-14SpiceWidget: limit scope of 'area' variableJonathon Jongsma1-3/+1
'area' was being set within the 'else' branch, but was not actually used. So just move the declaration of 'area' within the if statement where it is actually used. This potentially avoids "set-but-not-used" warnings when GDK_WINDOWING_X11 is not defined.
2016-07-14configure: Add ${top_builddir}/spice-common to COMMON_CFLAGSEduardo Lima (Etrunko)2-1/+1
This fixes the build when done out of the tree, because of recent commit in spice-common with auto-generated files. Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
2016-07-13Use correct variable to print if LZ4 support is to be built.Eduardo Lima (Etrunko)1-1/+1
Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
2016-07-11Usbredir: enable lz4 compressionSnir Sheriber2-7/+124
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_SPICEVMC_CAP_DATA_COMPRESS_LZ4 capability is available && data_size > COMPRESS_THRESHOLD && !AF_LOCAL data will be sent compressed otherwise data will be sent uncompressed (as well if compression has failed) Update required spice-protocol to 0.12.12 Signed-off-by: Snir Sheriber <ssheribe@redhat.com> Acked-by: Victor Toso <victortoso@redhat.com>
2016-07-11Update spice-common submoduleVictor Toso1-0/+0
2016-07-07util: fix off-by-one array accessMarc-André Lureau1-1/+1
Thanks to ASAN, I found this off-by-one memory access in the unix2dos code: /util/unix2dos: ================================================================= ==23589==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000dd2f at pc 0x00000040428e bp 0x7ffd6fc31b90 sp 0x7ffd6fc31b80 READ of size 1 at 0x60200000dd2f thread T0 #0 0x40428d in spice_convert_newlines /home/elmarco/src/spice/spice-gtk/src/spice-util.c:355 #1 0x40443a in spice_unix2dos /home/elmarco/src/spice/spice-gtk/src/spice-util.c:382 #2 0x401eae in test_unix2dos /home/elmarco/src/spice/spice-gtk/tests/util.c:69 #3 0x7fb8bcd81983 (/lib64/libglib-2.0.so.0+0x6e983) #4 0x7fb8bcd81b4e (/lib64/libglib-2.0.so.0+0x6eb4e) #5 0x7fb8bcd81d5d in g_test_run_suite (/lib64/libglib-2.0.so.0+0x6ed5d) #6 0x7fb8bcd81d80 in g_test_run (/lib64/libglib-2.0.so.0+0x6ed80) #7 0x402cce in main /home/elmarco/src/spice/spice-gtk/tests/util.c:207 #8 0x7fb8bc755730 in __libc_start_main (/lib64/libc.so.6+0x20730) #9 0x401818 in _start (/home/elmarco/src/spice/spice-gtk/tests/util+0x401818) 0x60200000dd2f is located 1 bytes to the left of 4-byte region [0x60200000dd30,0x60200000dd34) allocated by thread T0 here: #0 0x7fb8c10421d0 in realloc (/lib64/libasan.so.3+0xc71d0) #1 0x7fb8bcd61f1f in g_realloc (/lib64/libglib-2.0.so.0+0x4ef1f) SUMMARY: AddressSanitizer: heap-buffer-overflow /home/elmarco/src/spice/spice-gtk/src/spice-util.c:355 in spice_convert_newlines Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
2016-07-07doc: fix unused warningMarc-André Lureau1-1/+2
Fix gtk-doc warning by ignoring the new private header. ./spice-gtk-unused.txt:1: warning: 12 unused declarations.They should be added to spice-gtk-sections.txt in the appropriate place. While at it, fix grabsequence header incorrect file name. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
2016-07-07tests: file-transfer agent cancelVictor Toso1-0/+66
Agent only can only send error or cancel from a transfer operation after it was initialized. From SpiceFileTransferTask point of view, error and cancellation is an GError with different code so testing only cancel seems enough. Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-07-07tests: file-transfer cancel on read asyncVictor Toso1-0/+106
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-07-07tests: file-transfer cancel on task initVictor Toso1-0/+68
Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-07-07tests: file-transfer include simple testsVictor Toso2-0/+192
This only includes a simple test for file-transfer with a small summary of the possible situations of the test. As the test is specifically for SpiceFileTransferTask, we don't create a SpiceMainChannel. Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-07-07file-xfer: move to spice-file-transfer-task.cVictor Toso4-704/+800
This patch moves: * GObject boilerplate * External API related to SpiceFileTransferTask * Internal API needed by channel-main * Helpers that belong to this object Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-07-07main: use xfer_task instead of task or selfVictor Toso1-4/+4
Another patch to rename variable and keep consistency across channel-main. Let's avoid task which is common for GTask and self for non SpiceMainChannel objects Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-07-07file-xfer: move debug to SpiceFileTransferTaskVictor Toso1-13/+13
This debug information is simple way to check that the file-transfer is not stalled. Now that we are using the read-async functions, we can move this debug info there and avoid accessing object internals in channel-main.c Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-07-07file-xfer: Keep namespace of private functionVictor Toso1-5/+5
Rename: * file_xfer_close_cb -> spice_file_transfer_task_close_stream_cb As this will be private to SpiceFileTransferTask Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-07-07file-xfer: call user callback once per operationVictor Toso1-40/+61
SpiceFileTransferTask has a callback to be called when operation ended. Til this patch, we were setting the user callback which means that in multiple file-transfers, we were calling the user callback several times. Following the same logic pointed from 113093dd00a1cf10f6d3c3589b7 this is a SpiceMainChannel operation and it should only call the user callback when this operation is over (FileTransferOperation now). Highlights: * Now using GTask to set user callback and callback_data * Handling error on FileTransferOperation: - It is considered an error if no file was successfully transferred due cancellations or any other kind of error; - If any file failed due any error, we will consider the operation a failure even if other files succeed. Note also that SpiceFileTransferTask now does not have a callback and callback_data. The xfer_tasks has ended after its "finish" signal is emitted. Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-07-07file-xfer: call progress_callback per FileTransferOperationVictor Toso1-45/+62
Before this patch, the progress_callback is being called from SpiceFileTransferTask each time that some data is flushed to the agent of this given xfer-task. The progress value is computed by iterating on all available xfer-tasks. This patch intend to fix/change: * The progress_callback should be called only with information related to the FileTransferOperation of given xfer-task; This was also suggested by 113093dd00a1cf10f6d3c3589b7589a184cec081; * In case a SpiceFileTransferTask is cancelled or has an error, we remove the remaining bytes (not sent) of this file from the transfer_size; * The progress_callback should not be done from SpiceFileTransferTask context by (proposed) design. As the transfer operation starts in spice_main_file_copy_async(), FileTransferOperation should handle these calls. Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-07-07file-xfer: inform agent of errors only when task finishedVictor Toso1-10/+0
No need to inform of a problem under spice_file_transfer_task_completed() as the task will be finalized and we can send the error to the agent there. This change is related to split SpiceFileTransferTask from channel-main. Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-07-07file-xfer: a FileTransferOperation per transfer callVictor Toso1-42/+128
Each call to spice_main_file_copy_async will now create a FileTransferOperation which groups all SpiceFileTransferTasks of the copy operation in a GHashTable. To ease the review process, this first patch introduces the structure and organize the logic around the four following helpers: * spice_main_channel_reset_all_xfer_operations * spice_main_channel_find_xfer_task_by_task_id * file_transfer_operation_free * file_transfer_operation_task_finished The _task_finished function is the new callback for "finished" signal from SpiceFileTransferTask. The file_xfer_tasks GHashTable is now mapped as: (key) SpiceFileTransferTask ID -> (value) FileTransferOperation This patch leaves a FIXME regarding progress_callback which will be addressed in a later patch in this series. This change is related to split SpiceFileTransferTask from channel-main. Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-07-07file-xfer: create helper function to send progressVictor Toso1-20/+30
This is an intermediary step for the following patches to make the changes more clear and easy to follow. In file_xfer_send_progress(), I've renamed the variable self to xfer_task as this is not SpiceFileTransferTask function. Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-07-07file-xfer: improve helper function to queue agent messageVictor Toso1-6/+7
This patch changes: * rename function: file_xfer_queue -> file_xfer_queue_msg_to_agent As it makes more clear what this helper function does; * Use buffer provided by spice_file_transfer_task_read_finish() instead of accessing SpiceFileTransferTask's private structure This change is related to split SpiceFileTransferTask from channel-main. Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-07-07main: do not check for xfer-task errorsVictor Toso1-1/+1
The errors related to SpiceFileTransferTask can be present: * before the flush happens, on spice_file_transfer_task_read_async() * during the async flush: - cancel from user which is handled by file_xfer_flush_async - cancel/error from agent, handled on main_agent_handle_xfer_status() file_xfer_flush_finish() should not check internal errors of SpiceFileTransferTask and only be worried about errors regarding the flush itself. Note that with or without this patch, in case of errors from agent, we don't stop the flushing; it is only cancelled from user cancellation. Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-07-07main: let channel-main handle file-xfer messagesVictor Toso1-17/+12
By separating SpiceFileTransferTask from channel-main, we could choose where to put the handler for messages. With the approach based on spice_file_transfer_task_read_async(), we cannot have it under spice-file-transfer-task.c due the need of callback and userdata on _read_async. It is much easier to keep this in channel-main and do not move any VDAgent. This patch reverts 349a52ca2d6af4b31a4f51c38a3292c04460953c changes but it does rename: - function file_xfer_handle_status -> main_agent_handle_xfer_status - variables task to xfer_task Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-07-07main: to let SpiceFileTransferTask handle errorsVictor Toso1-9/+8
* on VD_AGENT_FILE_XFER_STATUS_CAN_SEND_DATA, if the file-transfer is on pending state, spice_file_transfer_task_read_async() will call the callback with error set. * on VD_AGENT_FILE_XFER_STATUS_SUCCESS, if the file-transfer is on pending state, spice_file_transfer_task_completed() will set the error for this task. Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-07-07file-xfer: introduce functions to read file asyncVictor Toso1-43/+122
Introduced functions (private): * void spice_file_transfer_task_read_async() * gssize spice_file_transfer_task_read_finish() For a better abstraction of how to read from SpiceFileTransferTask and handle its data, following the design of other objects like GFile and GInputStream. Due to the logic changes involved, some functions were created or renamed to better address or match its place and purpose: * spice_file_transfer_task_read_stream_cb Callback for the actual read from GInpustStream; This is handling the SpiceFileTransferTask bits only; * file_xfer_read_cb -> file_xfer_read_async_cb Renamed to match _read_async() function; This is handling the data from reading the file by flushing it to the agent. As the _read_async() uses GTask, the error handling is done on the channel-main's callback, after _read_finish() is called. This change is related to split SpiceFileTransferTask from channel-main. Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-07-07file-xfer: introduce functions to init task asyncVictor Toso1-54/+130
Introduced functions (private): * void spice_file_transfer_task_init_task_async() * GFileInfo *spice_file_transfer_task_init_task_finish() The init process of SpiceFileTransferTask does initialize its GFileInputStream (for reading the file) and also its GFileInfo (necessary to protocol in order to start the file-transfer with agent) Due the logic changed involved, some functions were renamed to better match its place and purpose: * file_xfer_info_async_cb -> file_xfer_init_task_async_cb It is channel-main's callback for each _init_task_async() * file_xfer_read_async_cb -> spice_file_transfer_task_read_file_cb * file_xfer_info_async_cb -> spice_file_transfer_task_query_info_cb Both should be private to SpiceFileTransferTask now. As the _init_task_async() uses GTask, some error handling was moved to channel-main's after _init_task_finish() is called. This change is related to split SpiceFileTransferTask from channel-main. Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-07-07file-xfer: introduce _create_tasks()Victor Toso1-56/+91
We can split from file_xfer_send_start_msg_async() the logic in creating the SpiceFileTransferTasks; The rest of the function can be handled at spice_main_file_copy_async(). The new function, spice_file_transfer_task_create_tasks() returns a GHashTable to optimize the access to a SpiceFileTransferTask from its task-id, which is what we receive from the agent. This change is related to split SpiceFileTransferTask from channel-main. Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-07-07file-xfer: get functions for SpiceFileTransferTaskVictor Toso1-3/+29
In order for channel-main to interact with each SpiceFileTransferTask for the file-transfer operation, the following functions are introduced: * spice_file_transfer_task_get_id * spice_file_transfer_task_get_channel * spice_file_transfer_task_get_cancellable Note that "id" property is public and could be acquired by g_object_get but having the helper function is more practical. This change is related to split SpiceFileTransferTask from channel-main. Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-07-07file-xfer: task-id to start with 1Victor Toso1-1/+1
As this is unsigned, we can let 0 be in case of error Acked-by: Jonathon Jongsma <jjongsma@redhat.com>
2016-06-30usb-device-manager: Avoid USB event thread leakChristophe Fergeau1-1/+11
This is a follow-up of the previous commit ('usb-channel: Really stop listening for USB events on disconnection'). Since the USB event thread has to be stopped when we destroy the associated SpiceUsbDeviceManager, spice_usb_device_manager_dispose() should force event_thread_run to FALSE even if spice_usb_device_manager_stop_event_listening() was not enough. When this happens, this means that there is a bug in the internal users of spice_usb_device_manager_start_event_listening(), but with this change, we'll at least warn about it, and avoid a thread leak/potential future crash.
2016-06-30usb-channel: Really stop listening for USB events on disconnectionChristophe Fergeau1-10/+14
When using USB redirection, it's fairly easy to leak the thread handling USB events, which will eventually cause problems in long lived apps. In particular, in virt-manager, one can: - start a VM - connect to it with SPICE - open the USB redirection window - redirect a device - close the SPICE window -> the SpiceUsbDeviceManager instance will be destroyed (including the USB context it owns), but the associated event thread will keep running. Since it's running a loop blocking on libusb_handle_events(priv->context), the loop will eventually try to use the USB context we just destroyed causing a crash. We can get in this situation when redirecting a USB device because we will call spice_usb_device_manager_start_event_listening() in spice_usbredir_channel_open_device(). The matching spice_usb_device_manager_stop_event_listening() call is supposed to happen in spice_usbredir_channel_disconnect_device(), however by the time it's called in the scenario described above, the session associated with the channel will already have been set to NULL in spice_session_channel_destroy(). The session is only needed in order to get the SpiceUsbDeviceManager instance we need to call spice_usb_device_manager_stop_event_listening() on. This patch stores it in SpiceChannelUsbredir instead, this way we don't need SpiceChannel::session to be non-NULL during device disconnection. This should fix the issues described in https://bugzilla.redhat.com/show_bug.cgi?id=1217202 (virt-manager) and most likely https://bugzilla.redhat.com/show_bug.cgi?id=1337007 (gnome-boxes) as well.
2016-06-30usb: Update outdated GSimpleAsyncResult commentChristophe Fergeau1-1/+1
It's still true after the switch to GTask.
2016-06-30usbredir: Use atomic for UsbDeviceManager::event_thread_runChristophe Fergeau1-5/+5
This variable is accessed from 2 different threads (main thread and USB event thread), so some care must be taken to read/write it.
2016-06-30usbredir: mark some functions as internalMarc-André Lureau1-0/+3
For the sake of making clear those are not exported. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
2016-06-30usbredir: Fix GTask leakChristophe Fergeau1-0/+1
spice_usbredir_channel_disconnect_device_async() creates a GTask and then calls g_task_run_in_thread(). This method will take the reference it needs on the GTask, it does not take ownership of the passed-in GTask. This means we need to unref the GTask we created after calling g_task_run_in_thread(), otherwise we are going to leak the GTask, as well as the channel it's associated with. Since it's an USB redir channel, this also causes some USB device manager/USB event thread leaks.
2016-06-21Update NEWS for 0.32 releaseMarc-André Lureau1-0/+30
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
2016-06-21session: Keep brackets around ipv6 hostnamePavel Grunt1-1/+1
According to rfc2732: "To use a literal IPv6 address in a URL, the literal address should be enclosed in "[" and "]" characters." Resolves: rhbz#1331777 Acked-by: Marc-André Lureau <mlureau@redhat.com>
2016-06-21session: Removed write-only variablePavel Grunt1-4/+0
Acked-by: Marc-André Lureau <mlureau@redhat.com>
2016-06-21mailmap: fix my nameMarc-André Lureau1-0/+1
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>