summaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
...
* server/red_worker.c:red_process_drawable: rename item to drawableAlon Levy2013-08-141-16/+16
|
* server/red_worker.c:red_process_drawable: rename drawable to red_drawableAlon Levy2013-08-141-8/+9
|
* red_worker: mark DRAW_ALL as brokenAlon Levy2013-08-141-0/+1
| | | | | | | setting DRAW_ALL define doesn't produce correct rendering. Using update_area instead of red_draw_qxl_drawable will work but it shouldn't be required. This is not work I intend to do right now, so marking it for anyone looking at this in the future.
* red_worker: decrease the timeout when flushing commands and waiting for the ↵Yonit Halperin2013-08-061-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | client. 150 seconds is way too long period for holding the guest driver and waiting for a response for the client. This timeout was 15 seconds, but when off-screen surfaces ware introduced it was arbitrarily multiplied by 10. Other existing related bugs emphasize why it is important to decrease the timeout: (1) 994211 - the qxl driver waits for an async-io reponse for 60 seconds and after that, it switches to sync-io mode. Not only that the driver might use invalid data (since it didn't wait for the query to complete), falling back to sync-io mode introduces other errors. (2) 994175 - spice server sometimes doesn't recognize that the client has disconnected. (3) There might be cache inconsistency between the client and the server, and then the display channel waits indefinitely for a cache item (e.g., bug 977998) This patch changes the timeout to 30 seconds. I tested it under wifi +emulating 2.5Mbps network, together with playing video on the guest and changing resolutions in a loop. The timeout didn't expired during my tests. This bug is related to rhbz#964136 (but from rhbz#964136 info it is still not clear why the client wasn't responsive).
* log: improve debug information related to client disconnectionYonit Halperin2013-07-292-6/+10
|
* snd_worker/snd_disconnect_channel: don't call snd_channel_put if the channel ↵Yonit Halperin2013-07-291-10/+10
| | | | | | | | | | | has already been disconnected The snd channels has one reference as long as their socket is active. The playback channel has an additional reference for each frame that is currently filled by the sound device. Once the channel is disconnected (the socket has been freed and the first reference is released) snd_disconnect_channel shouldn't release a reference again.
* snd_worker: fix memory leak of PlaybackChannelYonit Halperin2013-07-291-5/+4
| | | | | | | | | | When the sequence of calls bellow occurs, the PlaybackChannel is not released (snd_channel_put is not called for the samples that refer to the channel). spice_server_playback_get_buffer snd_channel_disconnect spice_server_playback_put_samples
* reds: s/red_client_disconnect/red_channel_client_shutdown inside callbacksYonit Halperin2013-07-291-3/+4
| | | | | | | | | | | | | | When we want to disconnect the main channel from a callback, it is safer to use red_channel_client_shutdown, instead of directly destroying the client. It is also more consistent with how other channels treat errors. red_channel_client_shutdown will trigger socket error in the main channel. Then, main_channel_client_on_disconnect will be called, and eventually, main_dispatcher_client_disconnect. I didn't replace calls to reds_disconnect/reds_client_disconnect in places where those calls were safe && that might need immediate client disconnection.
* decouple disconnection of the main channel from client destructionYonit Halperin2013-07-295-4/+52
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fixes rhbz#918169 Some channels make direct calls to reds/main_channel routines. If these routines try to read/write to the socket, and they get socket error, main_channel_client_on_disconnect is called, and triggers red_client_destroy. In order to prevent accessing expired references to RedClient, RedChannelClient, or other objects (inside the original call, after red_client_destroy has been called) I made the call to red_client_destroy asynchronous with respect to main_channel_client_on_disconnect. I added MAIN_DISPATCHER_CLIENT_DISCONNECT to main_dispatcher. main_channel_client_on_disconnect pushes this msg to the dispatcher, instead of calling directly to reds_client_disconnect. The patch uses RedClient ref-count in order to handle a case where reds_client_disconnect is called directly (e.g., when a new client connects while another one is connected), while there is already CLIENT_DISCONNECT msg pending in the main_dispatcher. Examples: (1) snd_worker.c snd_disconnect_channel() channel->cleanup() //snd_playback_cleanup reds_enable_mm_timer() . . main_channel_push_multi_media_time()...socket_error . . red_client_destory() . . snd_disconnect_channel() channel->cleanup() celt051_encoder_destroy() celt051_encoder_destory() // double release Note that this bug could have been solved by changing the order of calls: e.g., channel->stream = NULL before calling cleanup, and some other changes + reference counting. However, I found other places in the code with similar problems, and I looked for a general solution, at least till we redesign red_channel to handle reference counting more consistently. (2) inputs_channel.c inputs_connect() main_channel_client_push_notify()...socket_error . . red_client_destory() . . red_channel_client_create() // refers to client which is already destroyed (3) reds.c reds_handle_main_link() main_channel_push_init() ...socket error . . red_client_destory() . . main_channel_client_start_net_test(mcc) // refers to mcc which is already destroyed This can explain the assert in rhbz#964136, comment #1 (but not the hang that occurred before).
* main_dispatcher: add ref count protection to RedClient instancesYonit Halperin2013-07-291-2/+4
|
* red_channel: add ref count to RedClientYonit Halperin2013-07-292-5/+35
|
* red_channel: prevent adding and pushing pipe items after a channel_client ↵Yonit Halperin2013-07-291-6/+24
| | | | | | | | | | | | | | | | | | | | | | | | has diconnected Fixes: leaks of pipe items & "red_client_destroy: assertion `rcc->send_data.size == 0'" red_channel_disconnect clears the pipe. It is called only once. After, it was called, not items should be added to the pipe. An example of when this assert can occur: on_new_cursor_channel (red_worker.c), pushes 2 pipe items. When it pushes the first pipe item, if the client has disconnected, it can hit a socket error, and then, red_channel_client_disconnect is called. The second call to adding a pipe item, will add the item to the pipe. red_channel_client_pipe_add_type also calls red_channel_client_push, which will update the send_data.size. Then, the push will also hit a socket error, but red_channel_client_disconnect won't clear the pending pipe item again, since it was already called. When red_client_destory is called, we hit assertion `rcc->send_data.size == 0'. Note that if a pipe item is added to the pipe after red_channel_client_disconnect was called, but without pushing it, we should hit "spice_assert(rcc->pipe_size == 0)".
* server/red_channel: fix unused variableAlon Levy2013-07-281-10/+13
| | | | unused variable 'so_unsent_size' [-Werror=unused-variable]
* server/red_worker.c: remove unused pipe_item_removeAlon Levy2013-07-241-5/+0
|
* TIOCOUTQ -> SIOCOUTQ and portability ifdefsNahum Shalman2013-07-222-2/+12
| | | | | | | | The ioctl on sockets is actually named SIOCOUTQ though its value is identical to TIOCOUTQ which is for terminals. SIOCOUTQ is linux specific so we add a header check and ifdef based on the presence of the header This prevents bogus ioctls on non-Linux platforms
* configure.ac comment typo nitNahum Shalman2013-07-221-1/+1
|
* Release 0.12.4v0.12.4Uri Lublin2013-07-172-2/+15
|
* Update spice-common submodule (get spice-protocol 0.12.6)Uri Lublin2013-07-171-0/+0
|
* syntax-check: trailing whitespaces -- ignore binary filesUri Lublin2013-07-171-0/+2
|
* syntax-check: remove trailing whitespacesUri Lublin2013-07-163-8/+7
| | | | Only whitespace changes in this commit.
* syntax-check: make sure config.h is the first included .h fileUri Lublin2013-07-163-1/+3
|
* syntax-check: use test A && test B instead of test A -a BUri Lublin2013-07-161-1/+1
|
* syntax-check: fix no-newline or empty line at EOFUri Lublin2013-07-162-2/+1
|
* syntax-check: s/the the/the/ in a commentUri Lublin2013-07-162-2/+2
|
* syntax-check: update AUTHORSUri Lublin2013-07-161-0/+6
|
* syntax-check: fix cast_of_argument_to_freeUri Lublin2013-07-161-1/+2
| | | | | | | | In this case, make syntax-check is wrong, and we actually do need the cast. A cast is needed when types are uint64_t <--> pointer Using a local "ptr" variable makes both gcc and syntax-check happy.
* syntax-check: fix avoid_if_before_freeUri Lublin2013-07-161-6/+2
|
* server/tests: fix timer for test_empty_successUri Lublin2013-07-161-1/+9
|
* server/tests: test_display_width_stride: add destroy commandUri Lublin2013-07-161-0/+9
| | | | | Otherwise, the test exits after the first iteration over all tests, on the second attempt to create an already created surface.
* server/tests: remove option from usage if AUTOMATED_TESTS is not configuredUri Lublin2013-07-161-6/+19
|
* server/tests: invalid-option: print the bad argumentUri Lublin2013-07-161-1/+1
| | | | optind points to the next argument to parse.
* server/tests: fix produce_command for create surfaceUri Lublin2013-07-161-2/+4
| | | | | | | | | | | Earlier in this function, test->target_surface is set to 1, which is the only allowed non-primary surface currently. If surface parameters are given (and specifically data is checked) they are being used, otherwise a default surface is used. Earlier in this function, "command" is set to a non-NULL value. Thus, the else part was unreachable code, which is fixed now.
* server/tests: test_display_base: set rect according to appropriate surfaceUri Lublin2013-07-161-5/+2
| | | | | | | | | When surface_id == 0, primary is used. Otherwise (currently 1), secondary is used. Also, remove unused test_width and test_height. Since commit caea7699434c20dceef8fc79d21b8eeb663fbf53, test->width and test->height are used.
* red_channel: replace RING_FOREACH with RING_FOREACH_SAFE in some placesUri Lublin2013-07-161-4/+4
| | | | | | | | This was originally intended to fix the problem fixed by commit 53488f0275d6c8a121af49f7ac817d09ce68090d. What is left are FOREACH loops that are at less risk and maybe safe (no read/write or disconnect/destroy are called from within them).
* red_worker: use a generic SAFE_FOREACH macroUri Lublin2013-07-161-29/+22
| | | | | | Introduce SAFE_FOREACH macro Make other safe iterators use SAFE_FOREACH
* red_worker: delete unused CCC_FOREACHUri Lublin2013-07-161-6/+0
|
* red_worker: make DRAWABLE_FOREACH_DPI safeUri Lublin2013-07-161-9/+11
|
* red_worker: use only DRAWABLE_FOREACH_GLZ_SAFEUri Lublin2013-07-161-9/+2
|
* red_worker: make WORKER_FOREACH_DCC safeUri Lublin2013-07-161-45/+48
| | | | | | | | | | | | | | | | | Specifically, the loop in red_pipes_add_draw can cause spice to abort. In red_worker.c (WORKER_FOREACH_DCC): red_pipes_add_drawable red_pipe_add_drawable red_handle_drawable_surfaces_client_synced red_push_surface_image red_channel_client_push red_channel_client_send red_peer_handle_outgoing reds_stream_writev (if fails -- EPIPE) handler->cb->on_error = red_channel_client_disconnect() red_channel_remove_client() ring_remove() -- of rcc from channel.clients ring.
* red_worker: reuse DCC_FOREACH in WORKER_DCC_FOREACHUri Lublin2013-07-161-7/+4
| | | | The only thing that is needed is to get the channel out of the worker.
* red_worker: use only RCC_FOREACH_SAFEUri Lublin2013-07-161-9/+2
| | | | | | | | | | | RCC_FOREACH may be dangerous The following patches replace FOREACH loops with a SAFE version. Using unsafe loops may cause spice-server to abort (assert fails). Specifically a read/write fail in those loops, may cause the client to disconnect, removing the node currently iterated, which cause spice to abort in ring_next(): -- assertion `pos->next != NULL && pos->prev != NULL' failed
* Use RING_FOREACH_SAFE in red_channel.c functions which are missing itDavid Gibson2013-07-051-4/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently, both red_channel_pipes_add_type() and red_channel_pipes_add_empty_msg() use plaing RING_FOREACH() which is not safe versus removals from the ring within the loop body. Although it's rare, such a removal can occur in both cases. In the case of red_channel_pipes_add_type() we have: red_channel_pipes_add_type() -> red_channel_client_pipe_add_type() -> red_channel_client_push() And in the case of red_channel_client_pipes_add_empty_msg() we have: red_channel_client_pipes_add_empty_msg() -> red_channel_client_pipe_add_empty_msg() -> red_channel_client_push() But red_channel_client_push() can cause a removal from the clients ring if a network error occurs: red_channel_client_push() -> red_channel_client_send() -> red_peer_handle_outgoing() -> handler->cb->on_error callback = red_channel_client_default_peer_on_error() -> red_channel_client_disconnect() -> red_channel_remove_client() -> ring_remove() When this error path does occur, the assertion in RING_FOREACH()'s ring_next() trips, and the process containing the spice server is aborted. i.e. your whole VM dies, as a result of an unfortunately timed network error on the spice channel. Please apply. Signed-off-by: David Gibson <dgibson@redhat.com>
* red_worker: fix for stuck display_channel over WAN (jpeg_enabled=true)Yonit Halperin2013-06-251-0/+4
| | | | | | | | | | | | | | The image descriptor flags shouldn't be copied as is from the flags that were set by the driver. Specifically, the CACHE_ME flag shouldn't be copied, since it is possible that (a) the image won't be cached (b) the image is already cached, but in its lossy version, and we may want to set the bit for CACHE_REPLACE_ME, in order to cache it in its lossless version. In case (b), the client first looks for the CACHE_ME flag, and only if it is not set it looks for CACHE_REPLACE_ME (see canvas_base.c). Since both flags where set, the client ignored REPLACE_ME, and didn't turned off the lossy flag of the cach item. Then, when a request from this lossles item reached the client (FROM_CACHE_LOSSLESS), the client display channel waited endlessly for the lossless version of the image.
* red_worker: improve stream stats readability and ease of parsingYonit Halperin2013-06-242-8/+15
| | | | also added start/end-bit-rate and avg-quality to the final stream stats.
* mjpeg_encoder: add mjpeg_encoder_get_statsYonit Halperin2013-06-242-0/+18
|
* spice: silencing most of the ping/pong loggingYonit Halperin2013-06-241-11/+2
| | | | Those messages are too frequent and don't contribute much
* server: Add support for filtering out agent file-xfer msgs (rhbz#961848)Hans de Goede2013-06-065-7/+37
| | | | Signed-off-by: Hans de Goede <hdegoede@redhat.com>
* red_channel: replace an assert upon threads mismatch with a warningYonit Halperin2013-05-241-3/+25
| | | | | | | | | | | | | | | | | | | | | | | The assert: spice_assert(pthread_equal(pthread_self(), client->thread_id)) and the assert: spice_assert(pthread_equal(pthread_self(), rcc->channel->thread_id)) were coded in order to protect data that is accessed from the main context (red_client and most of the channels), from access by threads of other channels (namely, the display and cursor channels), and vice versa. However, some of the calls to the sound channel interface, and also the char_device interface, can be done from the vcpu thread. It doesn't endanger these channels internal data, since qemu use global mutex for the vcpu and io threads. Thus, pthread_self() can be != channel->thread_id, if one of them is the vcpu thread and the other is the io-thread, and we shouldn't assert. Future plans: A more complete and complicated solution would be to manage our own thread for spice-channels, and push input from qemu to this thread, instead of counting on the global mutex of qemu rhbz#823472
* main_channel: fix double release of migration target dataYonit Halperin2013-05-231-0/+2
| | | | | | | If client_migrate_info was called once with cert-host-subject and then again without cert-host-subject, on a third call to client_migrate info, the cert-host-subject from the first call would have been freed for the second time.
* Log actual address spice-server binds toChristophe Fergeau2013-05-191-5/+10
| | | | | | | | | | | It's not always obvious what address spice-server will bind to, in particular when the 'addr' parameter is omitted on QEMU commandline. The decision of what address to bind to is made in reds_init_socket with a call to getaddrinfo. Surprisingly, that function had a call to getnameinfo() already, but it does not seem to be using the result of that call in any way. This commit moves this call after the socket is successfully bound and add a log message to indicate which address it's bound to.
generated by cgit (git 2.34.1) at 2025-09-29 16:25:08 +0000