Re: [Spice-devel] [PATCH 2/2] Optimise client pipe passing pipe position instead of data

On Wed, Sep 14, 2016 at 06:30:42AM -0400, Frediano Ziglio wrote: > > > > I haven't looked very carefully at the changes, but a few comments > > about readability: > > > > On Mon, Sep 12, 2016 at 04:32:03PM +0100, Frediano Ziglio wrote: > > > This avoid to have the search the data scanning all

Re: [Spice-devel] spice performance tweaking

thanx,I'll stick with the centos packages, I need a very good reason before I start using beta packages. And a nice to have feature is not one of them. Also I dug in to the openvpn tweaks and it seems that all of them are related to udp tunnels. Performance is sadly rather low when you have to

Re: [Spice-devel] spice performance tweaking

> Hello, > I'm trying to improve my spice performance on a kvm host/guest. > It's currently rather slow and I can see screens beeing build up, and delays > when draging windows. > It's being tunneled through openvpn, which is set to use tcp. > tcp required because of the firewall which is

Re: [Spice-devel] [spice-server PATCH 0/4] Fix some coverity issues (1)

> > This patchset fixes a few coverity issues. > There are still some issues to be solved in the future, > (which is why the subject contains (1)). > > Uri Lublin (4): > dcc: do not check the pointer returned by spice_new > dcc_gl_scanout_item_new: allocate item when needed >

Re: [Spice-devel] spice performance tweaking

> 2016-09-13 16:43 GMT+02:00 Christophe Fergeau < cferg...@redhat.com > : > > Hey, > > > On Tue, Sep 13, 2016 at 08:47:18AM +0200, Rob Verduijn wrote: > > > > Hello, > > > > > > > > I'm trying to improve my spice performance on a kvm host/guest. > > > > It's currently rather slow and I can

Re: [Spice-devel] [PATCH] SPICE port draft implementation

Hi Oliver, On Mon, 2016-09-12 at 10:46 +0200, Oliver Gutierrez wrote: > --- >  enums.js|  11 - >  main.js |   7 +++- >  port.js | 125 > >  spice.html  |   1 + >  spice_auto.html |   3 +- >  5 files changed,

[Spice-devel] [PATCH v2] Avoid recursive inclusion of headers

red-common.h included utils.h which included red-common.h Signed-off-by: Frediano Ziglio --- server/utils.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Changes from v1: - minimize changes (Jonathon Jongsma) diff --git a/server/utils.h b/server/utils.h index

[Spice-devel] [PATCH] Removed some not necessary headers inclusions

Signed-off-by: Frediano Ziglio --- server/display-channel.h | 7 +-- server/inputs-channel.h | 1 - server/main-dispatcher.h | 1 - server/red-channel.h | 1 - server/red-parse-qxl.h | 1 - server/red-record-qxl.h | 1 - 6 files changed, 1 insertion(+), 11

[Spice-devel] [spice-server PATCH 1/4] dcc: do not check the pointer returned by spice_new

The rest of the code assumes spice_new does not return NULL Signed-off-by: Uri Lublin --- server/dcc.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/server/dcc.c b/server/dcc.c index 36c9c1d..1a7aac9 100644 --- a/server/dcc.c +++ b/server/dcc.c @@ -572,7 +572,6 @@ static

[Spice-devel] [spice-server PATCH 3/4] dcc_gl_draw_item_new: allocate item when needed

Prevents a leak on early return. Found by coverity. Signed-off-by: Uri Lublin --- server/dcc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/dcc.c b/server/dcc.c index cdd888b..2587d72 100644 --- a/server/dcc.c +++ b/server/dcc.c @@ -591,7 +591,7

[Spice-devel] [spice-server PATCH 4/4] dcc_compress_image: fix a possible overflow when calculating image_size

Both src->stride and src->y are uint32_t Fixed by making one of them uint64_t Found by coverity Signed-off-by: Uri Lublin --- server/dcc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/dcc.c b/server/dcc.c index 2587d72..a56b658 100644 ---

Re: [Spice-devel] [PATCH] Removed some not necessary headers inclusions

Hi Frediano, what is the rule ? I prefer to include header if some of its declarations is used. e.g.: i see that you removed  #include from inputs-channel.h but VDAgentMouseState is used in the file Thanks, Pavel On Wed, 2016-09-14 at 11:46 +0100, Frediano Ziglio wrote: > Signed-off-by:

Re: [Spice-devel] [spice-server PATCH 4/4] dcc_compress_image: fix a possible overflow when calculating image_size

> Both src->stride and src->y are uint32_t > Fixed by making one of them uint64_t > > Found by coverity > Does not hurt however the image size cannot be > 32 bit so changing image_size to uint32_t would fix the issue too. But on a 64 bit system does not make such of a difference. >

[Spice-devel] [spice-server PATCH 0/4] Fix some coverity issues (1)

This patchset fixes a few coverity issues. There are still some issues to be solved in the future, (which is why the subject contains (1)). Uri Lublin (4): dcc: do not check the pointer returned by spice_new dcc_gl_scanout_item_new: allocate item when needed dcc_gl_draw_item_new: allocate

[Spice-devel] [spice-server PATCH 2/4] dcc_gl_scanout_item_new: allocate item when needed

Prevents a leak on early return. Found by coverity. Signed-off-by: Uri Lublin --- server/dcc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/dcc.c b/server/dcc.c index 1a7aac9..cdd888b 100644 --- a/server/dcc.c +++ b/server/dcc.c @@ -571,7 +571,7

Re: [Spice-devel] spice performance tweaking

Could you test at least? Would be very helpful. We could then backport some improvements. Frediano > thanx,I'll stick with the centos packages, > I need a very good reason before I start using beta packages. > And a nice to have feature is not one of them. > Also I dug in to the openvpn

Re: [Spice-devel] [PATCH] Removed some not necessary headers inclusions

> > Hi Frediano, > > what is the rule ? I prefer to include header if some of its > declarations is used. > It's quite minimal include. > e.g.: i see that you removed >  #include > from inputs-channel.h but VDAgentMouseState is used in the file > Actually this hunk was in a separate patch

Re: [Spice-devel] [RFC PATCH 2/2] Start writing some documentation on protocol

On Fri, 2016-09-09 at 10:44 +0100, Frediano Ziglio wrote: > Signed-off-by: Frediano Ziglio > --- >  docs/spice_protocol.txt | 48 > >  1 file changed, 48 insertions(+) > > diff --git a/docs/spice_protocol.txt

Re: [Spice-devel] [PATCH 2/2] Optimise client pipe passing pipe position instead of data

On Wed, 2016-09-14 at 06:30 -0400, Frediano Ziglio wrote: > > > > > > I haven't looked very carefully at the changes, but a few comments > > about readability: > > > > On Mon, Sep 12, 2016 at 04:32:03PM +0100, Frediano Ziglio wrote: > > > > > > This avoid to have the search the data scanning

[Spice-devel] [vdagent-linux v1 3/3] build-sys: move user/system to respective dir

From: Marc-André Lureau Signed-off-by: Victor Toso --- Makefile.am | 59 +--- src/{ => vdagent}/vdagent-audio.c| 0 src/{ => vdagent}/vdagent-audio.h| 0 src/{ =>

[Spice-devel] [vdagent-linux v1 0/3] Reorganize vdagent Makefile and directory

Hi, Besides moving agents to their respective folders, some organization is done in the Makefile. I guess we could have Makefile to recursively build it later on. I picked this three patches from vdagent-gtk branch from Marc-André at: https://github.com/elmarco/vdagent-gtk I only changed the

[Spice-devel] [vdagent-linux v1 2/3] build-sys: get rid of noinst_HEADERS

From: Marc-André Lureau Regular headers file should be listed in SOURCES Signed-off-by: Victor Toso --- Makefile.am | 26 -- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/Makefile.am b/Makefile.am

[Spice-devel] [PATCH v2 4/9] Replace a couple Rings with GList

Make RedsState::mig_target_clients into a GList to improve encapsulation and maintainability. Also RedsMigTargetClient::pending_links. With GList, a type implementation can be ignorant of whether they're contained within a list or not. --- Changes in v2: - don't change memory allocator -- use

[Spice-devel] [PATCH v2 2/9] Add CursorChannelPrivate struct

Encapsulate private data of CursorChannel in a private struct. This isn't very useful at the moment, but it will help prepare the way for porting the RedChannel heirarchy to GObject. --- Changes in v2: - use priv[1] to avoid memory leak server/cursor-channel.c | 57

[Spice-devel] [PATCH v2 0/9] Backported patches from Refactory (Sept 17)

Basically the same as the last patch series, but with updates. See individual patches for details. Jonathon Jongsma (9): Add DisplayChannelPrivate struct Add CursorChannelPrivate struct Rename display_channel_set_monitors_config_to_primary() Replace a couple Rings with GList

[Spice-devel] [PATCH v2 6/9] Change Drawable->pipes from Ring to GList

This improves the readability of the code and keeps things encapsulated better. --- Changes in v2: - changed some loops from while to for - moved some declarations within loop scope server/dcc.c | 12 +--- server/dcc.h | 1 - server/display-channel.c | 34

[Spice-devel] [PATCH v2 7/9] Make glz_dictionary_list a GList

Removing more intrusive RingItems from various structures and improving readibility. --- no changes server/image-encoders.c | 17 +++-- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/server/image-encoders.c b/server/image-encoders.c index 5759230..39aca6c 100644 ---

[Spice-devel] [PATCH v2 8/9] Change RedCharDevice::write_queue to GQueue

Change a couple more Rings to GQueue --- Changes in v2: - use GQueue, not GList server/char-device.c | 79 +--- server/char-device.h | 1 - 2 files changed, 32 insertions(+), 48 deletions(-) diff --git a/server/char-device.c

[Spice-devel] [PATCH v2 3/9] Rename display_channel_set_monitors_config_to_primary()

Since this function is a DisplayChannel method, use a name consistent with naming conventions. Acked-by: Pavel Grunt --- no changes server/display-channel.c | 2 +- server/display-channel.h | 2 +- server/red-worker.c | 2 +- 3 files changed, 3 insertions(+), 3

[Spice-devel] [PATCH v2 5/9] RedChannelClient: store pipe items in a GQueue

Instead of using a Ring (and having a ring item link in every pipe item), store them in a GQueue. This also necesitated changing RedCharDeviceVDIPort->priv->read_bufs to a GList as well. Also Optimise client pipe by passing pipe position instead of data. This avoids having the search the data

[Spice-devel] [PATCH v2 1/9] Add DisplayChannelPrivate struct

Move all of the DisplayChannel data memembers into a private struct to encapsulate things better. This necessitated a few new 'public' methods and a small bit of refactoring to avoid poking into DisplayChannel internals from too many places. The DisplayChannel and the DisplayChannelClient are

Re: [Spice-devel] [vdagent-linux v1 0/3] Reorganize vdagent Makefile and directory

On Wed, 2016-09-14 at 18:51 +0200, Victor Toso wrote: > Hi, > > Besides moving agents to their respective folders, some organization > is > done in the Makefile. I guess we could have Makefile to recursively > build it later on. > > I picked this three patches from vdagent-gtk branch from

Re: [Spice-devel] spice performance tweaking

2016-09-14 9:53 GMT+02:00 Christophe Fergeau : > On Tue, Sep 13, 2016 at 08:04:04PM +0200, Rob Verduijn wrote: > > > > gets an error if I add that to the config. > > It says this option is included since 1.3.3 and centos has qemu-kvm 1.5.3 > > Did I forget something to

Re: [Spice-devel] spice performance tweaking

On Wed, Sep 14, 2016 at 12:20:23PM +0200, Rob Verduijn wrote: > 2016-09-14 9:53 GMT+02:00 Christophe Fergeau : > > > On Tue, Sep 13, 2016 at 08:04:04PM +0200, Rob Verduijn wrote: > > > > > > gets an error if I add that to the config. > > > It says this option is included

Re: [Spice-devel] [PATCH 2/2] Optimise client pipe passing pipe position instead of data

> > I haven't looked very carefully at the changes, but a few comments > about readability: > > On Mon, Sep 12, 2016 at 04:32:03PM +0100, Frediano Ziglio wrote: > > This avoid to have the search the data scanning all the > > queue changing the order of these operations from O(n) to O(1). > > >

Re: [Spice-devel] [PATCH] Avoid recursive inclusion of headers

> > On Mon, 2016-09-12 at 12:53 +0100, Frediano Ziglio wrote: > > red-common.h included utils.h which included red-common.h > > > > Signed-off-by: Frediano Ziglio > > --- > >  server/main-channel-client.c | 1 + > >  server/red-channel-client.c  | 1 + > >  server/red-common.h 

Re: [Spice-devel] spice performance tweaking

Ok local only as the article says. Still nice to relieve my local vm's a bit. Cheers Rob Verduijn 2016-09-14 12:28 GMT+02:00 Christophe Fergeau : > On Wed, Sep 14, 2016 at 12:20:23PM +0200, Rob Verduijn wrote: > > 2016-09-14 9:53 GMT+02:00 Christophe Fergeau

Re: [Spice-devel] spice performance tweaking

On Wed, Sep 14, 2016 at 12:34:04PM +0200, Rob Verduijn wrote: > Found the url : > https://www.spice-space.org/spice-user-manual.html > rhel7.3beta currently has > libvirt 2.0.0 > mesa-libGL 11.2.2 > mesa-libGLU 9.0.0 > spice-server 0.12.4 > qemu-kvm 1.5.3 > qemu-guest-agent 2.5.0 > > > If this

Re: [Spice-devel] [PATCH spice-gtk] widget: Inform about transfer failure

On Wed, Sep 14, 2016 at 11:33:16AM +0200, Pavel Grunt wrote: > Hi Christophe, > > On Wed, 2016-09-14 at 11:24 +0200, Christophe Fergeau wrote: > > On Wed, Sep 14, 2016 at 10:03:31AM +0200, Pavel Grunt wrote: > > Call spice_main_file_copy_finish to get result of the transfer > > --- > >  

Re: [Spice-devel] [PATCH spice v2 1/2] agent-filter: Use enum as return value

Acked-by: Christophe Fergeau On Tue, Sep 13, 2016 at 11:29:16PM +0200, Pavel Grunt wrote: > Also remove unused AGENT_MSG_FILTER_END > --- > v2 per Christophe's comments: > - removed AGENT_MSG_FILTER_END > - avoid 'default' and explicitly check for the enum value > --- >

Re: [Spice-devel] [PATCH spice v2 2/2] reds: Simplify vdi_port_read_buf_process

On Tue, Sep 13, 2016 at 11:29:17PM +0200, Pavel Grunt wrote: > Reuse and handle the return value from agent_msg_filter_process_data Not overly convinced code is better this way (agent_msg_filter_process_data is simpler, the 2 callers are more complicated ;) Acked-by: Christophe Fergeau

Re: [Spice-devel] [PATCH spice v2 1/2] agent-filter: Use enum as return value

> > Also remove unused AGENT_MSG_FILTER_END > --- > v2 per Christophe's comments: > - removed AGENT_MSG_FILTER_END > - avoid 'default' and explicitly check for the enum value > --- > server/agent-msg-filter.c | 4 ++-- > server/agent-msg-filter.h | 11 +-- > server/reds.c

Re: [Spice-devel] [PATCH spice v2 2/2] reds: Simplify vdi_port_read_buf_process

> > Reuse and handle the return value from agent_msg_filter_process_data > --- > v2 per Frediano's comments: > - clarified the commit message > - documented return values > - added fall through comment > --- > server/reds.c | 69 > +++ >

Re: [Spice-devel] [PATCH spice v2 1/2] agent-filter: Use enum as return value

On Wed, 2016-09-14 at 05:53 -0400, Frediano Ziglio wrote: > > > > Also remove unused AGENT_MSG_FILTER_END > > --- > > v2 per Christophe's comments: > >  - removed AGENT_MSG_FILTER_END > >  - avoid 'default' and explicitly check for the enum value > > --- > >  server/agent-msg-filter.c |  4 ++-- >

Re: [Spice-devel] [PATCH 1/2] Change GList in GQueue

> > Hey, > > On Mon, Sep 12, 2016 at 04:32:02PM +0100, Frediano Ziglio wrote: > > This patch is intended to be merged into "RedChannelClient: store pipe > > items in a GList". > > > > GQueue improve GList for pipe usage as tail inserting/removal/access > > are O(1) instead of O(n). > > Also

Re: [Spice-devel] spice performance tweaking

On Tue, Sep 13, 2016 at 08:04:04PM +0200, Rob Verduijn wrote: > > gets an error if I add that to the config. > It says this option is included since 1.3.3 and centos has qemu-kvm 1.5.3 > Did I forget something to enable this ? gl enable is not going to give you remote for now. The "Since 1.3.3"

Re: [Spice-devel] [PATCH spice-gtk] widget: Inform about transfer failure

On Wed, Sep 14, 2016 at 10:03:31AM +0200, Pavel Grunt wrote: > Call spice_main_file_copy_finish to get result of the transfer > --- > src/spice-widget.c | 18 +- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/src/spice-widget.c b/src/spice-widget.c > index

Re: [Spice-devel] [PATCH spice 2/2] reds: Refactor agent message forwarding

On Mon, Sep 12, 2016 at 08:38:32AM -0400, Frediano Ziglio wrote: > Don't we have an enum for this return instead of using int ? > > enum { > AGENT_MSG_FILTER_OK, > AGENT_MSG_FILTER_DISCARD, > AGENT_MSG_FILTER_PROTO_ERROR, > AGENT_MSG_FILTER_MONITORS_CONFIG, >

Re: [Spice-devel] [PATCH spice-gtk] widget: Inform about transfer failure

Hi Christophe, On Wed, 2016-09-14 at 11:24 +0200, Christophe Fergeau wrote: > On Wed, Sep 14, 2016 at 10:03:31AM +0200, Pavel Grunt wrote: > Call spice_main_file_copy_finish to get result of the transfer > --- >  src/spice-widget.c | 18 +- >  1 file changed, 17 insertions(+), 1

[Spice-devel] [PATCH spice-gtk] widget: Inform about transfer failure

Call spice_main_file_copy_finish to get result of the transfer --- src/spice-widget.c | 18 +- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/spice-widget.c b/src/spice-widget.c index 9c8f7d1..cc89f22 100644 --- a/src/spice-widget.c +++ b/src/spice-widget.c @@

Re: [Spice-devel] spice performance tweaking

For which distro is that package ? Centos 7.2 ? rhel7.3beta or fedora24 ? Rob Verduijn 2016-09-14 15:59 GMT+02:00 Frediano Ziglio : > Could you test at least? Would be very helpful. We could then backport > some improvements. > > Frediano > > > thanx,I'll stick with the

Re: [Spice-devel] spice performance tweaking

Or rather, Could you specify what you would like to see tested ? Setup of the host / guest ? specific guest definitions ? Rob Verduijn 2016-09-14 15:59 GMT+02:00 Frediano Ziglio : > Could you test at least? Would be very helpful. We could then backport > some improvements.