Re: [Spice-devel] experimental spice-server branch

2013-10-28 Thread Yonit Halperin
On 10/28/2013 11:34 AM, Marc-André Lureau wrote: On Thu, Oct 24, 2013 at 5:29 PM, Yonit Halperin yhalp...@redhat.com wrote: http://cgit.freedesktop.org/~yhalperi/spice-common/ branch bitmap-crop.mem-control Regarding quic: add x-offset option to bitmap encoding, wouldn't you be able

[Spice-devel] experimental spice-server branch

2013-10-24 Thread Yonit Halperin
Hi, You can find some optimizations I made to spice-server under: http://cgit.freedesktop.org/~yhalperi/spice/ branch bitmap-crop.mem-control together with: http://cgit.freedesktop.org/~yhalperi/spice-common/ branch bitmap-crop.mem-control The first set of patches (till commit a5100670f,

Re: [Spice-devel] RFC: Spice in a 3D word presentation sheets

2013-10-15 Thread Yonit Halperin
Hi, Thanks for the presentation. I agree with Marc-André that I wouldn't be so decisive with Host-side rendering is the only feasible solution. I am not convinced yet that video encoding can be better than using smart compression and caching of 3D objects and client side rendering, at least

Re: [Spice-devel] hardcoded NUM_DRAWABLES too small

2013-10-09 Thread Yonit Halperin
Hi, I think the hardcoded limit was intended for limiting the occupancy of the dev ram with drawables that are referenced by the display channel. If the limit is reasonable it should help with keeping allocations in the driver fluent, and avoiding IO_OOM. An alternative approach is to monitor

[Spice-devel] [PATCH spice-server 1/3] red_worker: cleanup red_clear_surface_drawables_from_pipes

2013-09-26 Thread Yonit Halperin
(1) merge 'force' and 'wait_for_outgoing_item' to one parameter. 'wait_for_outgoing_item' is a derivative of 'force'. (2) move the call to red_wait_outgoing_item to red_clear_surface_drawables_from_pipe --- server/red_worker.c | 30 ++ 1 file changed, 18

[Spice-devel] [PATCH spice-server 3/3] red_worker: disconnect the channel instead of shutdown in case of a blocking method failure

2013-09-26 Thread Yonit Halperin
rhbz#1004443 The methods that trigger waitings on the client pipe require that the waiting will succeed in order to continue, or otherwise, that all the living pipe items will be released (e.g., when we must destroy a surface, we need that all its related pipe items will be released). Shutdown of

[Spice-devel] [PATCH spice-server 2/3] red_channel: cleanup of red_channel_client blocking methods

2013-09-26 Thread Yonit Halperin
(1) receive timeout as a parameter. (2) add a return value and pass the handling of failures to the calling routine. --- server/red_channel.c | 73 ++-- server/red_channel.h | 22 ++-- server/red_worker.c | 55

[Spice-devel] [PATCH spice-server 3/3] red_worker: disconnect the channel instead of shutdown in case of a blocking method failure

2013-09-26 Thread Yonit Halperin
rhbz#1004443 The methods that trigger waitings on the client pipe require that the waiting will succeed in order to continue, or otherwise, that all the living pipe items will be released (e.g., when we must destroy a surface, we need that all its related pipe items will be released). Shutdown of

[Spice-devel] [PATCH spice-server 2/3] red_channel: cleanup of red_channel_client blocking methods

2013-09-26 Thread Yonit Halperin
(1) receive timeout as a parameter. (2) add a return value and pass the handling of failures to the calling routine. --- server/red_channel.c | 73 ++-- server/red_channel.h | 22 ++-- server/red_worker.c | 55

[Spice-devel] [PATCH spice-server 1/3] red_worker: cleanup red_clear_surface_drawables_from_pipes

2013-09-26 Thread Yonit Halperin
(1) merge 'force' and 'wait_for_outgoing_item' to one parameter. 'wait_for_outgoing_item' is a derivative of 'force'. (2) move the call to red_wait_outgoing_item to red_clear_surface_drawables_from_pipe --- server/red_worker.c | 30 ++ 1 file changed, 18

Re: [Spice-devel] [PATCH spice-server 3/3] red_worker: disconnect the channel instead of shutdown in case of a blocking method failure

2013-09-26 Thread Yonit Halperin
PM, Yonit Halperin yhalp...@redhat.com wrote: rhbz#1004443 The methods that trigger waitings on the client pipe require that the waiting will succeed in order to continue, or otherwise, that all the living pipe items will be released (e.g., when we must destroy a surface, we need that all its

Re: [Spice-devel] [PATCH spice-server 1/3] red_worker: cleanup red_clear_surface_drawables_from_pipes

2013-09-20 Thread Yonit Halperin
On 09/18/2013 09:37 AM, Marc-André Lureau wrote: On Wed, Sep 18, 2013 at 3:34 PM, Yonit Halperin yhalp...@redhat.com wrote: On 09/18/2013 09:19 AM, Marc-André Lureau wrote: Hi On Wed, Sep 18, 2013 at 2:58 PM, Yonit Halperin yhalp...@redhat.com wrote: Hi, On 09/17/2013 12:11 PM, Marc

Re: [Spice-devel] [PATCH spice-server 1/3] red_worker: cleanup red_clear_surface_drawables_from_pipes

2013-09-18 Thread Yonit Halperin
Hi, On 09/17/2013 12:11 PM, Marc-André Lureau wrote: On Mon, Sep 16, 2013 at 5:34 PM, Yonit Halperin yhalp...@redhat.com wrote: Hi, 'force' means: wait till there is no pipe item that references the surface. If force = FALSE, try to release any such pipe item, but as long as it doesn't

Re: [Spice-devel] [PATCH spice-server 1/3] red_worker: cleanup red_clear_surface_drawables_from_pipes

2013-09-18 Thread Yonit Halperin
On 09/18/2013 09:19 AM, Marc-André Lureau wrote: Hi On Wed, Sep 18, 2013 at 2:58 PM, Yonit Halperin yhalp...@redhat.com wrote: Hi, On 09/17/2013 12:11 PM, Marc-André Lureau wrote: On Mon, Sep 16, 2013 at 5:34 PM, Yonit Halperin yhalp...@redhat.com wrote: Hi, 'force' means: wait till

Re: [Spice-devel] [PATCH spice-server 1/3] red_worker: cleanup red_clear_surface_drawables_from_pipes

2013-09-16 Thread Yonit Halperin
this comments to the code if it helps. If you could clarified the above, and comment it in the code that would be helpful! :) On Thu, Sep 12, 2013 at 10:04 PM, Yonit Halperin yhalp...@redhat.com wrote: (1) merge 'force' and 'wait_for_outgoing_item' to one parameter. 'wait_for_outgoing_item

[Spice-devel] [PATCH spice-server 1/3] red_worker: cleanup red_clear_surface_drawables_from_pipes

2013-09-12 Thread Yonit Halperin
(1) merge 'force' and 'wait_for_outgoing_item' to one parameter. 'wait_for_outgoing_item' is a derivative of 'force'. (2) move the call to red_wait_outgoing_item to red_clear_surface_drawables_from_pipe --- server/red_worker.c | 20 +++- 1 file changed, 11 insertions(+), 9

[Spice-devel] [PATCH spice-server 3/3] red_worker: disconnect the channel instead of shutdown in case of a blocking method failure

2013-09-12 Thread Yonit Halperin
rhbz#1004443 The methods that trigger waitings on the client pipe require that the waiting will succeed in order to continue, or otherwise, that all the living pipe items will be released (e.g., when we must destroy a surface, we need that all its related pipe items will be released). Shutdown of

[Spice-devel] [PATCH spice-server 2/3] red_channel: cleanup of red_channel_client blocking methods

2013-09-12 Thread Yonit Halperin
(1) receive timeout as a parameter. (2) add a return value and pass the handling of failures to the calling routine. --- server/red_channel.c | 73 ++-- server/red_channel.h | 22 ++-- server/red_worker.c | 53

Re: [Spice-devel] [PATCH spice-gtk 09/14] channel: do not reenter the mainloop at every iteration

2013-09-12 Thread Yonit Halperin
ACK The rest of the patches that haven't been acked in this series, are the same as in the previous series, right? If so, ack series. Cheers, Yonit. On 09/12/2013 08:09 AM, Marc-André Lureau wrote: From: Marc-André Lureau marcandre.lur...@gmail.com The current coroutine channel_iterate()

Re: [Spice-devel] [PATCH spice-gtk] proto: add fake last message in base channel

2013-09-11 Thread Yonit Halperin
Ack On 09/10/2013 05:27 PM, Marc-André Lureau wrote: This is actually for spice-common (although spice-gtk will need later then) and also spice-protocol On Tue, Sep 10, 2013 at 11:13 PM, Marc-André Lureau marcandre.lur...@gmail.com wrote: Make it explicit that 100 is the last value of the

Re: [Spice-devel] [PATCH spice-gtk 03/14] channel: add SPICE_DISABLE_CHANNELS

2013-09-10 Thread Yonit Halperin
On 09/10/2013 10:44 AM, Marc-André Lureau wrote: Allow to disable selectively channels, mainly used for testing, ex: SPICE_DISABLE_CHANNELS=display spicy-stats -p 12345 --- gtk/spice-channel-priv.h | 2 ++ gtk/spice-channel.c | 7 +++ 2 files changed, 9 insertions(+) diff --git

[Spice-devel] [PATCH qxl-win] Revert miniport: halve QXL_IO_UPDATE_IRQ calls

2013-09-09 Thread Yonit Halperin
This reverts commit 49feefa95d3595f04355c4aed53ec5bf26551046. The patch causes the display to get stuck. Till we understand exactly why, I'm reverting it. --- xddm/miniport/qxl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/xddm/miniport/qxl.c b/xddm/miniport/qxl.c index ce37f07..f5d6b48

Re: [Spice-devel] [PATCH spice-gtk 08/10] gtk: use GHashTable in display_cache

2013-09-09 Thread Yonit Halperin
Hi, In general it looks good, but I have several comments: (1) the palette cache shouldn't be shared among the display channels. I.e., there should be one instance per channel, and not one instance in spice-session. (2) I think you changed the ref_count logic of cache item. Now each cache

Re: [Spice-devel] [PATCH spice-gtk 09/10] gtk: use slices for frequently allocated objects

2013-09-09 Thread Yonit Halperin
ack, but what happens when g_slice_new fails? the spice_new and other spice malloc functions handle mallocs that fail and abort in such cases. On 09/08/2013 02:59 PM, Marc-André Lureau wrote: From: Marc-André Lureau marcandre.lur...@gmail.com --- gtk/channel-display.c | 4 ++--

Re: [Spice-devel] [PATCH spice-gtk 01/10] display: use bitfields for surface flags

2013-09-09 Thread Yonit Halperin
On 09/08/2013 02:59 PM, Marc-André Lureau wrote: Checking by value make the flag fields useless.Uunfortunately, when adding more flags, the server will have to ensure it can safely send it, by checking some of new client caps. Good catch. Can you also add a comment about this to spice-protcol

Re: [Spice-devel] [PATCH spice-gtk 02/10] Add SPICE_DISABLE_CHANNELS

2013-09-09 Thread Yonit Halperin
ack. Notice that basically handle_msg of all the channels does the same, expect it uses different msg handlers. It would have been cleaner to use one handle_msg routine, and different msg handlers. Then, instead of checking disable_channel_msg for each msg, we could just set the msg_handlers

Re: [Spice-devel] [PATCH spice-gtk 04/10] display: make the hashtable to destroy the surface

2013-09-09 Thread Yonit Halperin
ack On 09/08/2013 02:59 PM, Marc-André Lureau wrote: Improve a bit the code by using hashtable ownership. --- gtk/channel-display.c | 16 +--- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/gtk/channel-display.c b/gtk/channel-display.c index 7a66558..979ce7b 100644

Re: [Spice-devel] [PATCH spice-gtk 06/10] util: avoid calling getenv for every SPICE_DEBUG

2013-09-09 Thread Yonit Halperin
ack On 09/08/2013 02:59 PM, Marc-André Lureau wrote: From: Marc-André Lureau marcandre.lur...@gmail.com That doesn't seem to really improve performance so much, but that' s what we should do probably. --- gtk/spice-util.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-)

Re: [Spice-devel] [PATCH spice-gtk 07/10] channel: do not reenter the mainloop at every iteration

2013-09-09 Thread Yonit Halperin
Hi, I think that iterate_write iterate_read should be symmetrical: in the same way iterate_write handles all outgoing messages, iterate_read should flush all incoming messages (i.e., the loop in spice_channel_iterate can be moved to iterate_read). But I'll ack it even if you don't change

Re: [Spice-devel] [PATCH spice-gtk 05/10] display: keep a reference to the primary surface

2013-09-09 Thread Yonit Halperin
ack On 09/08/2013 02:59 PM, Marc-André Lureau wrote: Avoid hard-coding surface 0 as being primary, although in practice it always is so far. Also a lot of lookups are primary, so add a shortcut for this special case (~30% apparently), it shows some small lookup speedup. before: real

Re: [Spice-devel] [PATCH spice-gtk 10/10] gtk: simplify spice_channel_recv_msg

2013-09-09 Thread Yonit Halperin
Looks good. see comment bellow. On 09/08/2013 02:59 PM, Marc-André Lureau wrote: From: Marc-André Lureau marcandre.lur...@gmail.com Use of coroutines allow to simplify spice_channel_recv_msg(), it doesn't need to keep current reading state, it can rely on the coroutine stack for that. ---

Re: [Spice-devel] help

2013-08-26 Thread Yonit Halperin
Hi, On 08/26/2013 02:43 AM, 天知道 wrote: Hi, When a video is playing, the client is turned on and off every two seconds. Then assert error appears in certain time given information on the stack。 What do you mean by the client is turned on and off every two seconds?. What are the server and

[Spice-devel] [PATCH 1/2] spice_bitmap_utils: fix dump_bitmap

2013-08-22 Thread Yonit Halperin
--- server/spice_bitmap_utils.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/spice_bitmap_utils.c b/server/spice_bitmap_utils.c index ce0a5ed..7b370a0 100644 --- a/server/spice_bitmap_utils.c +++ b/server/spice_bitmap_utils.c @@ -63,7 +63,7 @@ void

[Spice-devel] [PATCH 2/2] red_worker: fix call to dump_bitmap (too many args)

2013-08-22 Thread Yonit Halperin
--- server/red_worker.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/red_worker.c b/server/red_worker.c index 7f1aeda..0c611d0 100644 --- a/server/red_worker.c +++ b/server/red_worker.c @@ -6629,7 +6629,7 @@ static FillBitsType fill_bits(DisplayChannelClient *dcc,

Re: [Spice-devel] [PATCH] miniport/qxl.inf: simplify by having FeatureScore for everyone

2013-08-21 Thread Yonit Halperin
Hi, I don't know much about the inf file, but the patch looks ok, as long as the FeatureScore is ignored on WindowsXp. However, how does this patch fix 728259? IIUC 728259 should be fixed by generating different inf file for different operating systems. Also, why does FeatureScore = FC ?

[Spice-devel] [PATCH spice-server 1/4] spice_timer_queue: don't call timers repeatedly

2013-08-14 Thread Yonit Halperin
For channels that don't run as part of the main loop, we use spice_timer_queue, while for the other channels we use qemu timers support. The callbacks for setting timers are supplied to red_channel via SpiceCoreInterface, and their behavior should be consistent. qemu timers are called only once

[Spice-devel] [PATCH spice-server 2/4] red_channel: add on_input callback for tracing incoming bytes

2013-08-14 Thread Yonit Halperin
The callback will be used in the next patch. --- server/red_channel.c | 7 +++ server/red_channel.h | 2 ++ 2 files changed, 9 insertions(+) diff --git a/server/red_channel.c b/server/red_channel.c index 37b0c1c..bcf5a60 100644 --- a/server/red_channel.c +++ b/server/red_channel.c @@ -251,6

[Spice-devel] [PATCH spice-server 3/4] red_channel: add option to monitor whether a channel client is alive

2013-08-14 Thread Yonit Halperin
rhbz#994175 When a client connection is closed surprisingly (i.e., without a FIN segment), we cannot identify it by a socket error (which is the only way by which we identified disconnections so far). This patch allows a channel client to periodically check the state of the connection and

Re: [Spice-devel] [PATCH v3 00/10] red_worker easy splits

2013-08-13 Thread Yonit Halperin
Looks good. Ack. On 08/13/2013 03:47 AM, Alon Levy wrote: Moving ~500 lines out of red_worker. v2-v3: * ref/unref of red_channel_client_wait_pipe_item_sent inside it, red_worker just calls it. * renames requested. Alon Levy (10): red_worker: mark DRAW_ALL as broken

Re: [Spice-devel] [PATCH 05/10] server: move bit set/clear utilities out of red_worker.h

2013-08-12 Thread Yonit Halperin
Hi, You forgot to add spice_server_utils.h :) Yonit. On 08/12/2013 08:49 AM, Alon Levy wrote: --- server/Makefile.am | 1 + server/red_dispatcher.c | 4 +++- server/red_worker.c | 1 + server/red_worker.h | 18 -- 4 files changed, 5 insertions(+), 19

Re: [Spice-devel] [PATCH 04/10] server: move dump_bitmap to separate file

2013-08-12 Thread Yonit Halperin
Hi, Just a nitpick (below) On 08/12/2013 08:49 AM, Alon Levy wrote: --- server/Makefile.am | 2 + server/red_worker.c | 157 ++-- server/spice_bitmap_utils.c | 152 ++

Re: [Spice-devel] [PATCH v2 6/9] server: move three functions to red_channel

2013-08-12 Thread Yonit Halperin
Hi, red_time.h is missing. On 08/12/2013 11:32 AM, Alon Levy wrote: Three blocking functions, one was split to leave the display channel specific referencing of the DrawablePipeItem being sent inside red_worker, but the rest (most) of the timeout logic was moved to red_channel, including the

Re: [Spice-devel] [PATCH v2 6/9] server: move three functions to red_channel

2013-08-12 Thread Yonit Halperin
Hi, I think red_channel_client_wait_pipe_item_sent should ref and unref the pipe item by itself (using the hold/release-item callbacks). Then, you don't need dcc_wait_pipe_item_sent. Also, I'm not sure why PUSH_TIMEOUT is bigger than DETACH timeout. It would have been logical if the bigger

Re: [Spice-devel] [PATCH v2 6/9] server: move three functions to red_channel

2013-08-12 Thread Yonit Halperin
On 08/12/2013 12:32 PM, Alon Levy wrote: Hi, I think red_channel_client_wait_pipe_item_sent should ref and unref the pipe item by itself (using the hold/release-item callbacks). Then, you don't need dcc_wait_pipe_item_sent. This is a bit scarier then my mechanical change. Since there are two

[Spice-devel] [PATCH 1/3] red_worker: decrease the timeout when flushing commands and waiting for the client.

2013-08-06 Thread Yonit Halperin
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

Re: [Spice-devel] [PATCH 1/3] red_worker: decrease the timeout when flushing commands and waiting for the client.

2013-08-06 Thread Yonit Halperin
should be PATCH 1/1 :) On 08/06/2013 02:51 PM, Yonit Halperin wrote: 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

[Spice-devel] [PATCH spice-gtk] usb-device-manager: fix compilation error when usbredir is disabled

2013-08-06 Thread Yonit Halperin
--- gtk/usb-device-manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gtk/usb-device-manager.c b/gtk/usb-device-manager.c index 0535609..71d3462 100644 --- a/gtk/usb-device-manager.c +++ b/gtk/usb-device-manager.c @@ -329,10 +329,10 @@ static gboolean

Re: [Spice-devel] [PATCH 1/3] display/driver: DrvDeleteDeviceBitmap: log if pdev disabled

2013-07-30 Thread Yonit Halperin
Ack On 07/29/2013 09:14 AM, Alon Levy wrote: Alternative is to leak, and this happens regularely. Perhaps we should be keeping track and destroying these surfaces later (in case no reset happens in the middle, which destroys all off screen surfaces anyway). --- display/driver.c | 5 + 1

Re: [Spice-devel] [qxl-win PATCH] display: add punting where it is missing

2013-07-29 Thread Yonit Halperin
Ack On 07/26/2013 02:43 PM, Alon Levy wrote: --- display/brush.c | 4 +++- display/driver.c | 2 ++ display/pointer.c | 8 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/display/brush.c b/display/brush.c index 0b9400d..fe94f1c 100644 --- a/display/brush.c +++

[Spice-devel] [PATCH spice-server 1/8] red_channel: prevent adding and pushing pipe items after a channel_client has diconnected

2013-07-26 Thread Yonit Halperin
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

[Spice-devel] [PATCH spice-server 2/8] red_channel: add ref count to RedClient

2013-07-26 Thread Yonit Halperin
--- server/red_channel.c | 23 --- server/red_channel.h | 17 +++-- 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/server/red_channel.c b/server/red_channel.c index a35da9b..9b5e314 100644 --- a/server/red_channel.c +++ b/server/red_channel.c @@

[Spice-devel] [PATCH spice-server 6/8] snd_worker: fix memory leak of PlaybackChannel

2013-07-26 Thread Yonit Halperin
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 --- server/snd_worker.c | 9 - 1

[Spice-devel] [PATCH spice-server 8/8] log: improve debug information related to client disconnection

2013-07-26 Thread Yonit Halperin
--- server/red_channel.c | 9 ++--- server/snd_worker.c | 7 --- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/server/red_channel.c b/server/red_channel.c index 9b5e314..89b4092 100644 --- a/server/red_channel.c +++ b/server/red_channel.c @@ -1109,6 +1109,7 @@ static

[Spice-devel] [PATCH spice-server 7/8] snd_worker/snd_disconnect_channel: don't call snd_channel_put if the channel has already been disconnected

2013-07-26 Thread Yonit Halperin
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)

[Spice-devel] [PATCH spice-server 4/8] decouple disconnection of the main channel from client destruction

2013-07-26 Thread Yonit Halperin
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

[Spice-devel] [PATCH spice-server 5/8] reds: s/red_client_disconnect/red_channel_client_shutdown inside callbacks

2013-07-26 Thread Yonit Halperin
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,

Re: [Spice-devel] seamless spice migration : question about password/ticket for target vm

2013-07-22 Thread Yonit Halperin
Hi, On 07/22/2013 08:04 AM, Alexandre DERUMIER wrote: Hi, I'm trying to do migration, and I have a question about password on target vm. If I understand, client try to connect to target vm with same password (temporary ticket) used to connect to source vm. But, we need to configure this

Re: [Spice-devel] [(spice) PATCHv2 RFC 2/2] TIOCOUTQ - SIOCOUTQ and portability ifdefs

2013-07-22 Thread Yonit Halperin
Ack. The ping is sent over the display channel to check an upper limit for the roundtrip time. If the tcp stack is busy, we postpone the ping. So, without the ioctl, when the tcp stack is busy, the roundtrip estimation will probably be much bigger than the real one. However, we currently

Re: [Spice-devel] seamless spice migration : question about password/ticket for target vm

2013-07-22 Thread Yonit Halperin
On 07/22/2013 12:50 PM, Marc-André Lureau wrote: Hi - Mensaje original - Hi, On 07/22/2013 08:04 AM, Alexandre DERUMIER wrote: Hi, I'm trying to do migration, and I have a question about password on target vm. If I understand, client try to connect to target vm with same password

Re: [Spice-devel] [(spice) PATCH RFC 1/2] configure.ac comment typo nit

2013-07-22 Thread Yonit Halperin
Ack On 07/18/2013 11:04 AM, Nahum Shalman wrote: --- configure.ac |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/configure.ac b/configure.ac index edda8e9..a549ed9 100644 --- a/configure.ac +++ b/configure.ac @@ -20,7 +20,7 @@ m4_define([SPICE_AGE], [8]) # Note on

Re: [Spice-devel] [PATCH] xddm/miniport/qxl.inf: bump version and touch date

2013-07-17 Thread Yonit Halperin
ACK. Maybe Arnon knows more about the version scheme. On 07/17/2013 10:15 AM, Alon Levy wrote: We haven't done this for most any change. Perhaps we should do this? No idea what the x.y.z.w convention is, so I made the most minor change right now. But the date is the important bit. ---

Re: [Spice-devel] Information about video detection

2013-07-12 Thread Yonit Halperin
Hi, On 07/02/2013 10:47 AM, Matilde Yanez wrote: 2013/7/2 Yonit Halperin yhalp...@redhat.com mailto:yhalp...@redhat.com On 07/02/2013 10:22 AM, Matilde Yanez wrote: Hi, thanks for answers. 2013/7/2 Yonit Halperin yhalp...@redhat.com mailto:yhalp

Re: [Spice-devel] [server PATCH 1/8] red_worker: use only RCC_FOREACH_SAFE

2013-07-08 Thread Yonit Halperin
Hi, On 07/08/2013 06:32 AM, Uri Lublin wrote: 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,

Re: [Spice-devel] [server PATCH 0/8] use a SAFE version of RING_FOREACH (worker/channel)

2013-07-08 Thread Yonit Halperin
Ack Series, with the comment that the actual risky places that cause the crash, may better use safe red_channel.c routines instead. But it can be done later. On 07/08/2013 06:32 AM, Uri Lublin wrote: If in RING_FOREACH (in == somewhere in the callee-tree) there is a call to read or write, it

Re: [Spice-devel] [PATCH] Use RING_FOREACH_SAFE in red_channel.c functions which are missing it

2013-07-05 Thread Yonit Halperin
Ack. Thanks! We have recently associated this problem with some opened bugs we have. I believe Uri is working on a patch for a similar fix to the red_pipes.* routines in red_worker. On 07/05/2013 03:11 AM, David Gibson wrote: Currently, both red_channel_pipes_add_type() and

[Spice-devel] [PATCH qxl-win] display: apply the fix in fc314927bc48835e to Alpha Bitmaps

2013-07-05 Thread Yonit Halperin
rhbz#968050 In contrast to Microsoft Msdn documentation, the iUniq of a SURFOBJ doesn't always change when the surface changes. However, it seems that the iUniq of the associated color_trans (XLATEOBJ) changes, while its flXlate=XO_TRIVIAL. Since we tried to retrieve the alpha bitmap key only by

Re: [Spice-devel] Information about video detection

2013-07-02 Thread Yonit Halperin
Hi, On 07/01/2013 04:27 AM, Matilde Yanez wrote: Hello, I need some information about video detection, on VM. When I am on web pages, or documents, with images the VM seems to detect video and start the mjpeg_encoder process. Thus, the display is slows down. What vm are you using? We

Re: [Spice-devel] Information about video detection

2013-07-02 Thread Yonit Halperin
On 07/02/2013 10:22 AM, Matilde Yanez wrote: Hi, thanks for answers. 2013/7/2 Yonit Halperin yhalp...@redhat.com mailto:yhalp...@redhat.com Hi, On 07/01/2013 04:27 AM, Matilde Yanez wrote: Hello, I need some information about video detection, on VM. When I

[Spice-devel] Announcing spice-protocol 0.12.6

2013-06-27 Thread Yonit Halperin
Hi All, I'm happy to announce a new spice-protocol release in the stable spice-0.12.x series Changes in spice-protocol-0.12.6 * Add adaptive video streaming support: control playback latency and receive playback reports from the client. * Add agent

Re: [Spice-devel] give me some advise how to improve display effect and decrease client cpu usage

2013-06-27 Thread Yonit Halperin
option for remote-viewer. You can change the size of the cache and see if it improves the performance. Then we can consider implementing caching on disk, and not only memory. Cheers, Yonit. thanks. At 2013-06-19 01:44:29,Yonit Halperin yhalp...@redhat.com wrote: Hi, On 06/18/2013 10:33 AM

[Spice-devel] [PATCH spice-protocol] Release 0.12.6

2013-06-26 Thread Yonit Halperin
--- NEWS | 7 +++ configure.ac | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index a602292..23b83d2 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,10 @@ +Major changes in 0.12.6 +=== +* add adaptive video streaming support: + control

[Spice-devel] [PATCH] red_worker: fix for stuck display_channel over WAN (jpeg_enabled=true)

2013-06-25 Thread Yonit Halperin
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

Re: [Spice-devel] Questions regarding Bug 62033 - Means to detect local-only

2013-06-24 Thread Yonit Halperin
Hi, For the purpose of disabling guest desktop effects, we already have a spice-gtk option which we use for Windows guests, and can be used for Linux guests as well. The Linux agent should support VD_AGENT_DISPLAY_CONFIG for this. However, for local usage, we would also like to disable

Re: [Spice-devel] [qxl-win PATCH 2/2] display: fix deadlock when dbg_level = 15

2013-06-24 Thread Yonit Halperin
On 06/23/2013 06:30 AM, Uri Lublin wrote: On 06/21/2013 04:50 PM, Yonit Halperin wrote: DebugPrintV first locks print_sem, and then locks io_sem. async_io, locks io_sem. In ordr to avoid a deadlock, DebugPrintV MUSTN'T be called when io_sem is locked. Also notice, that locking io_sem during

[Spice-devel] [PATCH server 1/3] spice: silencing most of the ping/pong logging

2013-06-24 Thread Yonit Halperin
Those messages are too frequent and don't contribute much --- server/red_channel.c | 13 ++--- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/server/red_channel.c b/server/red_channel.c index 5662041..c0b1781 100644 --- a/server/red_channel.c +++ b/server/red_channel.c @@

[Spice-devel] [PATCH server 3/3] red_worker: improve stream stats readability and ease of parsing

2013-06-24 Thread Yonit Halperin
also added start/end-bit-rate and avg-quality to the final stream stats. --- server/mjpeg_encoder.c | 1 - server/red_worker.c| 22 +++--- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/server/mjpeg_encoder.c b/server/mjpeg_encoder.c index 92aef27..04b244e

[Spice-devel] [PATCH spice-gtk 1/2] display: video stats logging - improve readability and ease of parsing

2013-06-24 Thread Yonit Halperin
--- gtk/channel-display.c | 19 +++ 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/gtk/channel-display.c b/gtk/channel-display.c index bc6fc08..07f6c1e 100644 --- a/gtk/channel-display.c +++ b/gtk/channel-display.c @@ -1511,6 +1511,7 @@ static void

[Spice-devel] [PATCH server 2/3] mjpeg_encoder: add mjpeg_encoder_get_stats

2013-06-24 Thread Yonit Halperin
--- server/mjpeg_encoder.c | 11 +++ server/mjpeg_encoder.h | 7 +++ 2 files changed, 18 insertions(+) diff --git a/server/mjpeg_encoder.c b/server/mjpeg_encoder.c index 4460322..92aef27 100644 --- a/server/mjpeg_encoder.c +++ b/server/mjpeg_encoder.c @@ -169,6 +169,7 @@ struct

[Spice-devel] [PATCH spice-gtk 2/2] display: disabling adaptive video streaming via env var SPICE_DISABLE_ADAPTIVE_STREAMING

2013-06-24 Thread Yonit Halperin
spice-server supports adaptive video streaming only if the client publishes SPICE_DISPLAY_CAP_STREAM_REPORT. Disabling this feature is useful for debugging and performance comparison. --- gtk/channel-display.c | 24 ++-- 1 file changed, 18 insertions(+), 6 deletions(-) diff

[Spice-devel] [qxl-win PATCH 1/2] display: handle correctly bitmaps with line-size 64K

2013-06-21 Thread Yonit Halperin
rhbz#966835 We do not support copying such bitmaps. But instead of failing operations that involve such bitmaps we either BSODed (in checked builds), or proceeded with the bitmap copying (in free builds) - this lead to an infinite loop allocating QXLDataChunks without any data, just header. ---

[Spice-devel] [qxl-win PATCH 2/2] display: fix deadlock when dbg_level = 15

2013-06-21 Thread Yonit Halperin
DebugPrintV first locks print_sem, and then locks io_sem. async_io, locks io_sem. In ordr to avoid a deadlock, DebugPrintV MUSTN'T be called when io_sem is locked. Also notice, that locking io_sem during DebugPrintV limits our ability to use the log_port for debugging concurrency problems related

Re: [Spice-devel] give me some advise how to improve display effect and decrease client cpu usage

2013-06-18 Thread Yonit Halperin
Hi, On 06/18/2013 10:33 AM, bigclouds wrote: hi,all i have used spice for a long time,it is good, but in pratice it needs improvement, like resource usage, network bandwidth,performance. 1.it eats up much cpu on client side, up to 90% when play video. i am sure whether decompression or

Re: [Spice-devel] video detect and jpeg

2013-05-24 Thread Yonit Halperin
Hi, Video streaming support involves: (1) lossy compression of video frames (using jpeg) (2) lip sync (3) frame rate control Video is detected by identification of high rate updates of the same region over the primary surface. off=turn off video streaming support filter=don't stream as

[Spice-devel] [PATCH spice-server] red_channel: replace an assert upon threads mismatch with a warning

2013-05-24 Thread Yonit Halperin
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

[Spice-devel] [PATCH] main_channel: fix double release of migration target data

2013-05-23 Thread Yonit Halperin
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. --- server/main_channel.c | 2 ++ 1 file changed, 2 insertions(+)

Re: [Spice-devel] [PATCH spice-server 06/10] reds: disconnect the session when an essential channel is disconnected

2013-05-21 Thread Yonit Halperin
(snd_send_data, snd_receive). thanks 在 2013-05-08 23:57:54,Marc-André Lureau marcandre.lur...@gmail.com 写道: On Wed, May 8, 2013 at 5:03 PM, Yonit Halperin yhalp...@redhat.com mailto:yhalp...@redhat.com wrote: I can live with that. Maybe we should add to spice-gtk a user

[Spice-devel] [PATCH spice-gtk v4 0/3] asynced mm-time handling

2013-05-10 Thread Yonit Halperin
. Yonit Halperin (3): emit_main_context macro: handle calls from both coroutine context and main context spice-session: new signal for notifying on a significant change in mm-time channel-display: protect video streaming from temporarily unsynced mm_time (migration related) gtk

[Spice-devel] [PATCH spice-gtk v4 1/3] emit_main_context macro: handle calls from both coroutine context and main context

2013-05-10 Thread Yonit Halperin
--- gtk/coroutine.h | 1 + gtk/spice-channel-priv.h | 12 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/gtk/coroutine.h b/gtk/coroutine.h index 031a97b..a9b3a87 100644 --- a/gtk/coroutine.h +++ b/gtk/coroutine.h @@ -55,6 +55,7 @@ struct coroutine #endif

[Spice-devel] [PATCH spice-gtk v4 2/3] spice-session: new signal for notifying on a significant change in mm-time

2013-05-10 Thread Yonit Halperin
mm-time can change after migration. This can cause video synchronization problems after migration if not handled appropriately (see rhbz#951664). --- gtk/spice-session.c | 42 ++ 1 file changed, 42 insertions(+) diff --git a/gtk/spice-session.c

[Spice-devel] [PATCH spice-gtk v4 3/3] channel-display: protect video streaming from temporarily unsynced mm_time (migration related)

2013-05-10 Thread Yonit Halperin
rhbz#951664 When the src and dst servers have different mm-times, we can hit a case when for a short period of time the session mm-time and the video mm-time are not synced. If the video mm-time is much bigger than the session mm-time, the next stream rendering will be scheduled to (video-mm-time

Re: [Spice-devel] a stupid question, what does 'RED' word mean?

2013-05-09 Thread Yonit Halperin
On 05/09/2013 02:33 AM, bigclouds wrote: many functions start with 'red', what does it mean? thanks That is historical. It stands for Remote Desktop. ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org

Re: [Spice-devel] [PATCH spice-server] red_worker: don't get bit_rate from main_channel_client, if it wasn't initialized

2013-05-09 Thread Yonit Halperin
On 05/08/2013 07:27 PM, Yonit Halperin wrote: When setting an initial video stream bit rate, if the bit rate wasn't calculated by main_channel_client, and we don't have estimation from previos streams, use some default values. --- server/red_worker.c | 42

Re: [Spice-devel] [PATCH spice-gtk v3 1/2] spice-session: new signal for notifying on a significant change in mm-time

2013-05-09 Thread Yonit Halperin
On 05/09/2013 10:46 AM, Marc-André Lureau wrote: On Wed, May 1, 2013 at 4:19 PM, Yonit Halperin yhalp...@redhat.com mailto:yhalp...@redhat.com wrote: mm-time can change after migration. This can cause video synchronization problems after migration if not handled appropriately (see

Re: [Spice-devel] [PATCH spice-gtk v3 1/2] spice-session: new signal for notifying on a significant change in mm-time

2013-05-09 Thread Yonit Halperin
On 05/09/2013 11:16 AM, Marc-André Lureau wrote: On Thu, May 9, 2013 at 5:06 PM, Yonit Halperin yhalp...@redhat.com mailto:yhalp...@redhat.com wrote: On 05/09/2013 10:46 AM, Marc-André Lureau wrote: On Wed, May 1, 2013 at 4:19 PM, Yonit Halperin yhalp...@redhat.com

[Spice-devel] [PATCH spice-server 01/10] main_channel: add routine for checking if a network test had been conducted and completed

2013-05-08 Thread Yonit Halperin
--- server/main_channel.c | 10 +- server/main_channel.h | 6 ++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/server/main_channel.c b/server/main_channel.c index dd927ab..4cf7e19 100644 --- a/server/main_channel.c +++ b/server/main_channel.c @@ -164,6 +164,7 @@ enum

[Spice-devel] [PATCH spice-server 02/10] red_worker: cleanup: add is_low_bandwidth flag to CommonChannelClient

2013-05-08 Thread Yonit Halperin
Replace the mixed calls to display_channel_client_is_low_bandwidth and to main_channel_client_is_low_bandwidth, with one flag in CommonChannelClient that is set upon channel creation. --- server/red_worker.c | 29 +++-- 1 file changed, 15 insertions(+), 14 deletions(-)

[Spice-devel] [PATCH spice-server 03/10] red_worker: fix incorrect is_low_bandwidth after migrating a low bandwidth connection

2013-05-08 Thread Yonit Halperin
rhbz#956345 After a spice session has been migrated, we don't retest the network (user experience considerations). Instead, we obtain the is_low_bandwidth flag from the src-server, via the migration data. Before this patch, if we migrated from server s1 to s2 and then to s3, and if the connection

[Spice-devel] [PATCH spice-server 04/10] reds: move handle_channel_event logic from main_dispatcher to reds

2013-05-08 Thread Yonit Halperin
--- server/main_dispatcher.c | 5 + server/reds.c| 19 ++- server/reds.h| 4 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/server/main_dispatcher.c b/server/main_dispatcher.c index 8402402..92b0791 100644 ---

[Spice-devel] [PATCH spice-server 05/10] reds: fix memory leak when core-base.minor_version 3

2013-05-08 Thread Yonit Halperin
--- server/reds.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/server/reds.c b/server/reds.c index a378f80..f6a1ce9 100644 --- a/server/reds.c +++ b/server/reds.c @@ -196,10 +196,9 @@ static void reds_stream_push_channel_event(RedsStream *s, int event) void

[Spice-devel] [PATCH spice-server 06/10] reds: disconnect the session when an essential channel is disconnected

2013-05-08 Thread Yonit Halperin
--- server/reds.c | 25 + 1 file changed, 25 insertions(+) diff --git a/server/reds.c b/server/reds.c index f6a1ce9..38923b0 100644 --- a/server/reds.c +++ b/server/reds.c @@ -194,12 +194,37 @@ static void reds_stream_push_channel_event(RedsStream *s, int event)

  1   2   3   4   5   6   7   8   >