Re: [Spice-devel] spice-gtk updates for Fedora 25 need to be added to bodhi

[Hans de Goede]

Hi,

On 10/10/2016 02:38 PM, Marc-André Lureau wrote:

Hi

- Original Message -

Hi,

On 10/10/2016 11:49 AM, Marc-André Lureau wrote:

Hi

- Original Message -

Hi All,

I noticed that there have been a few spice-gtk builds fixing
the bugs I was hitting, thank you for that.

I had to install them manually from koji, since they are
not yet listed in bodhi. Can you please create bodhi
updates for these ?  :

https://bodhi.fedoraproject.org/updates/?packages=spice-gtk


f24:
https://bodhi.fedoraproject.org/updates/FEDORA-2016-fea866fa85
f25:
https://bodhi.fedoraproject.org/updates/FEDORA-2016-c1bf3f1059

Is that fine?


Yes those are fine.


Btw, since you are experienced Fedora packager, could you provide some
hints on solving the errors in
https://taskotron.fedoraproject.org/artifacts/all/886bb18a-8c8d-11e6-be50-525400120b80/task_output/spice-gtk-0.33-1.fc25.log


You should be able to reproduce those by doing:

fedpkg local
rpmlint *.src.rpm x86_64

in a fedpkg spice-gtk clone.

Then go through them 1:1, fix them rinse-repeat until all (most?) are fixed.


Yes, that I know of, I was wondering if you (or someone on the list) could help 
with the actual errors.


Well some of them are quite trivial to fix:

spice-gtk.i686: W: obsolete-not-provided spice-gtk-python

Can be fixed by adding:

Provides: spice-gtk-python = %{version}-%{release} to the list
of main Requires / Provides.

And this one:

spice-gtk3-devel.armv7hl: W: obsolete-not-provided spice-gtk-devel

Can be fixed by adding a similar provides to the spice-gtk3-devel subpkg, etc.

Likewise all the:

spice-gtk-tools.i686: W: no-manual-page-for-binary spicy-screenshot

Errors can be fixed by actually adding man-pages for those binaries.

Note these 2:

spice-gtk.src: W: invalid-url URL: http://spice-space.org/page/Spice-Gtk 
spice-gtk.src: W: invalid-url Source0: 
http://www.spice-space.org/download/gtk/spice-gtk-0.33.tar.bz2 

Seem to be false positives caused by the system doing the checks not having 
network access.

So I would start with fixing all the trivial ones and then see from there. TBH 
I do not
think you can fix them all, rpmlint is not  very bright tool.

Regards,

Hans




___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] spice-gtk updates for Fedora 25 need to be added to bodhi

[Marc-André Lureau]

Hi

- Original Message -
> Hi,
> 
> On 10/10/2016 02:38 PM, Marc-André Lureau wrote:
> > Hi
> >
> > - Original Message -
> >> Hi,
> >>
> >> On 10/10/2016 11:49 AM, Marc-André Lureau wrote:
> >>> Hi
> >>>
> >>> - Original Message -
>  Hi All,
> 
>  I noticed that there have been a few spice-gtk builds fixing
>  the bugs I was hitting, thank you for that.
> 
>  I had to install them manually from koji, since they are
>  not yet listed in bodhi. Can you please create bodhi
>  updates for these ?  :
> 
>  https://bodhi.fedoraproject.org/updates/?packages=spice-gtk
> >>>
> >>> f24:
> >>> https://bodhi.fedoraproject.org/updates/FEDORA-2016-fea866fa85
> >>> f25:
> >>> https://bodhi.fedoraproject.org/updates/FEDORA-2016-c1bf3f1059
> >>>
> >>> Is that fine?
> >>
> >> Yes those are fine.
> >>
> >>> Btw, since you are experienced Fedora packager, could you provide some
> >>> hints on solving the errors in
> >>> https://taskotron.fedoraproject.org/artifacts/all/886bb18a-8c8d-11e6-be50-525400120b80/task_output/spice-gtk-0.33-1.fc25.log
> >>
> >> You should be able to reproduce those by doing:
> >>
> >> fedpkg local
> >> rpmlint *.src.rpm x86_64
> >>
> >> in a fedpkg spice-gtk clone.
> >>
> >> Then go through them 1:1, fix them rinse-repeat until all (most?) are
> >> fixed.
> >
> > Yes, that I know of, I was wondering if you (or someone on the list) could
> > help with the actual errors.
> 
> Well some of them are quite trivial to fix:
> 
> spice-gtk.i686: W: obsolete-not-provided spice-gtk-python
> 
> Can be fixed by adding:
> 
> Provides: spice-gtk-python = %{version}-%{release} to the list
> of main Requires / Provides.
> 
> And this one:
> 
> spice-gtk3-devel.armv7hl: W: obsolete-not-provided spice-gtk-devel
> 
> Can be fixed by adding a similar provides to the spice-gtk3-devel subpkg,
> etc.
> 
> Likewise all the:
> 
> spice-gtk-tools.i686: W: no-manual-page-for-binary spicy-screenshot
> 
> Errors can be fixed by actually adding man-pages for those binaries.
> 
> Note these 2:
> 
> spice-gtk.src: W: invalid-url URL: http://spice-space.org/page/Spice-Gtk
> 
> spice-gtk.src: W: invalid-url Source0:
> http://www.spice-space.org/download/gtk/spice-gtk-0.33.tar.bz2  error [Errno -2] Name or service not known>
> 
> Seem to be false positives caused by the system doing the checks not having
> network access.
> 
> So I would start with fixing all the trivial ones and then see from there.
> TBH I do not
> think you can fix them all, rpmlint is not  very bright tool.

Sure, but I would rather start with E: "errors" :)

thanks anyway
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] spice-gtk updates for Fedora 25 need to be added to bodhi

[Hans de Goede]

Hi,

On 10/10/2016 11:49 AM, Marc-André Lureau wrote:

Hi

- Original Message -

Hi All,

I noticed that there have been a few spice-gtk builds fixing
the bugs I was hitting, thank you for that.

I had to install them manually from koji, since they are
not yet listed in bodhi. Can you please create bodhi
updates for these ?  :

https://bodhi.fedoraproject.org/updates/?packages=spice-gtk


f24:
https://bodhi.fedoraproject.org/updates/FEDORA-2016-fea866fa85
f25:
https://bodhi.fedoraproject.org/updates/FEDORA-2016-c1bf3f1059

Is that fine?


Yes those are fine.


Btw, since you are experienced Fedora packager, could you provide some hints on 
solving the errors in 
https://taskotron.fedoraproject.org/artifacts/all/886bb18a-8c8d-11e6-be50-525400120b80/task_output/spice-gtk-0.33-1.fc25.log


You should be able to reproduce those by doing:

fedpkg local
rpmlint *.src.rpm x86_64

in a fedpkg spice-gtk clone.

Then go through them 1:1, fix them rinse-repeat until all (most?) are fixed.

Unfortunately AFAIK there is no way yet to filter out some of the 
false-positives.

Regards,

Hans
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-gtk v3] main: Send monitor config only when it changes

[Marc-André Lureau]

Hi

- Original Message -
> When the guest receives the monitor configuration message, it replies
> (through spice-server) by destroying the primary surface, which makes
> the SpiceDisplay disabled if its "resize-guest" property is used.

> This change of the display state (disabled/enabled) leads to sending
> a new monitor config message (containing the disabled display) after
> a second. Then spice-server sends the monitor configuration message,
> spice-gtk replies back and we may end up with a loop of monitor
> configuration messages even if no real change of monitor configuration
> happens.

It's hard follow, could you explain step by step what you mean.
> To avoid the loop, the new monitor configuration messages are only sent
> if they are different from the current monitor configuration.
> 
> All the issues are visible only with Wayland & QXL driver on the guest
> 
> Fixes:
> https://bugzilla.redhat.com/show_bug.cgi?id=1266484
> https://bugs.freedesktop.org/show_bug.cgi?id=94950
> ---
> v3: rebased, fixed typos

In v2, Jonathon raised a valid concern: "However, it's theoretically possible 
that the last sent configuration may not have been successful." For ex, vdagent 
wasn't yet started (whatever handles the monitor config change).

Imho, it should be fine for the client to send the same monitor config, the 
server should however not notify of changes if none happened.

> v2:
> https://lists.freedesktop.org/archives/spice-devel/2015-October/022784.html
> ---
>  src/channel-main.c | 59
>  +++---
>  1 file changed, 38 insertions(+), 21 deletions(-)
> 
> diff --git a/src/channel-main.c b/src/channel-main.c
> index 990a06a..1a46819 100644
> --- a/src/channel-main.c
> +++ b/src/channel-main.c
> @@ -105,7 +105,8 @@ struct _SpiceMainChannelPrivate  {
>  guint   agent_msg_pos;
>  uint8_t agent_msg_size;
>  uint32_tagent_caps[VD_AGENT_CAPS_SIZE];
> -SpiceDisplayConfig  display[MAX_DISPLAY];
> +SpiceDisplayConfig  display_current[MAX_DISPLAY]; /* current
> configuration of spice-widgets */
> +SpiceDisplayConfig  display_new[MAX_DISPLAY]; /* new requested
> configuration */
>  ginttimer_id;
>  GQueue  *agent_msg_queue;
>  GHashTable  *file_xfer_tasks;
> @@ -1098,11 +1099,11 @@ gboolean
> spice_main_send_monitor_config(SpiceMainChannel *channel)
>  
>  if (spice_main_agent_test_capability(channel,
>   VD_AGENT_CAP_SPARSE_MONITORS_CONFIG)) {
> -monitors = SPICE_N_ELEMENTS(c->display);
> +monitors = SPICE_N_ELEMENTS(c->display_new);
>  } else {
>  monitors = 0;
> -for (i = 0; i < SPICE_N_ELEMENTS(c->display); i++) {
> -if (c->display[i].display_state == DISPLAY_ENABLED)
> +for (i = 0; i < SPICE_N_ELEMENTS(c->display_new); i++) {
> +if (c->display_new[i].display_state == DISPLAY_ENABLED)
>  monitors += 1;
>  }
>  }
> @@ -1117,24 +1118,25 @@ gboolean
> spice_main_send_monitor_config(SpiceMainChannel *channel)
>  
>  CHANNEL_DEBUG(channel, "sending new monitors config to guest");
>  j = 0;
> -for (i = 0; i < SPICE_N_ELEMENTS(c->display); i++) {
> -if (c->display[i].display_state != DISPLAY_ENABLED) {
> +for (i = 0; i < SPICE_N_ELEMENTS(c->display_new); i++) {
> +if (c->display_new[i].display_state != DISPLAY_ENABLED) {
>  if (spice_main_agent_test_capability(channel,
>   VD_AGENT_CAP_SPARSE_MONITORS_CONFIG))
>  j++;
>  continue;
>  }
>  mon->monitors[j].depth  = c->display_color_depth ?
>  c->display_color_depth : 32;
> -mon->monitors[j].width  = c->display[i].width;
> -mon->monitors[j].height = c->display[i].height;
> -mon->monitors[j].x = c->display[i].x;
> -mon->monitors[j].y = c->display[i].y;
> +mon->monitors[j].width  = c->display_new[i].width;
> +mon->monitors[j].height = c->display_new[i].height;
> +mon->monitors[j].x = c->display_new[i].x;
> +mon->monitors[j].y = c->display_new[i].y;
>  CHANNEL_DEBUG(channel, "monitor #%d: %ux%u+%d+%d @ %u bpp", j,
>mon->monitors[j].width, mon->monitors[j].height,
>mon->monitors[j].x, mon->monitors[j].y,
>mon->monitors[j].depth);
>  j++;
>  }
> +memcpy(c->display_current, c->display_new, MAX_DISPLAY *
> sizeof(SpiceDisplayConfig));
>  
>  if (c->disable_display_align == FALSE)
>  monitors_align(mon->monitors, mon->num_of_monitors);
> @@ -1469,13 +1471,23 @@ static gboolean
> any_display_has_dimensions(SpiceMainChannel *channel)
>  c = channel->priv;
>  
>  for (i = 0; i < MAX_DISPLAY; i++) {
> -

Re: [Spice-devel] spice-gtk updates for Fedora 25 need to be added to bodhi

[Marc-André Lureau]

Hi

- Original Message -
> Hi,
> 
> On 10/10/2016 11:49 AM, Marc-André Lureau wrote:
> > Hi
> >
> > - Original Message -
> >> Hi All,
> >>
> >> I noticed that there have been a few spice-gtk builds fixing
> >> the bugs I was hitting, thank you for that.
> >>
> >> I had to install them manually from koji, since they are
> >> not yet listed in bodhi. Can you please create bodhi
> >> updates for these ?  :
> >>
> >> https://bodhi.fedoraproject.org/updates/?packages=spice-gtk
> >
> > f24:
> > https://bodhi.fedoraproject.org/updates/FEDORA-2016-fea866fa85
> > f25:
> > https://bodhi.fedoraproject.org/updates/FEDORA-2016-c1bf3f1059
> >
> > Is that fine?
> 
> Yes those are fine.
> 
> > Btw, since you are experienced Fedora packager, could you provide some
> > hints on solving the errors in
> > https://taskotron.fedoraproject.org/artifacts/all/886bb18a-8c8d-11e6-be50-525400120b80/task_output/spice-gtk-0.33-1.fc25.log
> 
> You should be able to reproduce those by doing:
> 
> fedpkg local
> rpmlint *.src.rpm x86_64
> 
> in a fedpkg spice-gtk clone.
> 
> Then go through them 1:1, fix them rinse-repeat until all (most?) are fixed.

Yes, that I know of, I was wondering if you (or someone on the list) could help 
with the actual errors.
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

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

[Oliver Gutierrez]

THanks :) No problem. Just were asking :)

On Mon, Oct 10, 2016 at 1:55 PM, Pavel Grunt  wrote:

> On Mon, 2016-10-10 at 13:14 +0200, Oliver Gutierrez wrote:
> > This patch has not been pushed yet. Is there any problem?
>
> Sorry for the delay Oliver
> It is pushed
> Pavel
>
>
> > On Wed, Oct 5, 2016 at 8:06 AM, Pavel Grunt 
> > wrote:
> > > Hi Oliver,
> > >
> > > it looks good to me :)
> > >
> > > Acked-by: Pavel Grunt 
> > >
> > > Thanks,
> > > Pavel
> > >
> > > On Mon, 2016-10-03 at 14:09 +0200, Oliver Gutierrez wrote:
> > > > ---
> > > >  enums.js|  9 ++
> > > >  main.js |  3 ++
> > > >  port.js | 85
> > > > +
> > > >  spice.html  | 13 +
> > > >  spice_auto.html | 13 +
> > > >  spicemsg.js | 18 
> > > >  utils.js|  7 +
> > > >  7 files changed, 148 insertions(+)
> > > >  create mode 100644 port.js
> > > >
> > > > diff --git a/enums.js b/enums.js
> > > > index 301fea0..b6e013c 100644
> > > > --- a/enums.js
> > > > +++ b/enums.js
> > > > @@ -166,6 +166,15 @@ var SPICE_MSG_PLAYBACK_VOLUME   =
> > > 105;
> > > >  var SPICE_MSG_PLAYBACK_MUTE = 106;
> > > >  var SPICE_MSG_PLAYBACK_LATENCY  = 107;
> > > >
> > > > +var SPICE_MSG_SPICEVMC_DATA = 101;
> > > > +var SPICE_MSG_PORT_INIT = 201;
> > > > +var SPICE_MSG_PORT_EVENT= 202;
> > > > +var SPICE_MSG_END_PORT  = 203;
> > > > +
> > > > +var SPICE_MSGC_SPICEVMC_DATA= 101;
> > > > +var SPICE_MSGC_PORT_EVENT   = 201;
> > > > +var SPICE_MSGC_END_PORT = 202;
> > > > +
> > > >  var SPICE_PLAYBACK_CAP_CELT_0_5_1   = 0;
> > > >  var SPICE_PLAYBACK_CAP_VOLUME   = 1;
> > > >  var SPICE_PLAYBACK_CAP_LATENCY  = 2;
> > > > diff --git a/main.js b/main.js
> > > > index 874a038..2d8a1ff 100644
> > > > --- a/main.js
> > > > +++ b/main.js
> > > > @@ -59,6 +59,7 @@ function SpiceMainConn()
> > > >  this.file_xfer_tasks = {};
> > > >  this.file_xfer_task_id = 0;
> > > >  this.file_xfer_read_queue = [];
> > > > +this.ports = [];
> > > >  }
> > > >
> > > >  SpiceMainConn.prototype = Object.create(SpiceConn.prototype);
> > > > @@ -154,6 +155,8 @@
> > > SpiceMainConn.prototype.process_channel_message
> > > > = function(msg)
> > > >  this.cursor = new SpiceCursorConn(conn);
> > > >  else if (chans.channels[i].type ==
> > > > SPICE_CHANNEL_PLAYBACK)
> > > >  this.cursor = new SpicePlaybackConn(conn);
> > > > +else if (chans.channels[i].type ==
> > > SPICE_CHANNEL_PORT)
> > > > +this.ports.push(new SpicePortConn(conn));
> > > >  else
> > > >  {
> > > >  if (! ("extra_channels" in this))
> > > > diff --git a/port.js b/port.js
> > > > new file mode 100644
> > > > index 000..ee22073
> > > > --- /dev/null
> > > > +++ b/port.js
> > > > @@ -0,0 +1,85 @@
> > > > +"use strict";
> > > > +/*
> > > > +   Copyright (C) 2016 by Oliver Gutierrez 
> > > > + Miroslav Chodil 
> > > > +
> > > > +   This file is part of spice-html5.
> > > > +
> > > > +   spice-html5 is free software: you can redistribute it and/or
> > > > modify
> > > > +   it under the terms of the GNU Lesser General Public License
> > > as
> > > > published by
> > > > +   the Free Software Foundation, either version 3 of the
> > > License,
> > > > or
> > > > +   (at your option) any later version.
> > > > +
> > > > +   spice-html5 is distributed in the hope that it will be
> > > useful,
> > > > +   but WITHOUT ANY WARRANTY; without even the implied warranty
> > > of
> > > > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
> > > the
> > > > +   GNU Lesser General Public License for more details.
> > > > +
> > > > +   You should have received a copy of the GNU Lesser General
> > > Public
> > > > License
> > > > +   along with spice-html5.  If not, see  > > ense
> > > > s/>.
> > > > +*/
> > > > +
> > > > +/*-
> > > 
> > > > ---
> > > > +**  SpicePortConn
> > > > +**  Drive the Spice Port Channel
> > > > +**-
> > > 
> > > > -*/
> > > > +function SpicePortConn()
> > > > +{
> > > > +DEBUG > 0 && console.log('SPICE port: created SPICE port
> > > > channel. Args:', arguments);
> > > > +SpiceConn.apply(this, arguments);
> > > > +this.port_name = null;
> > > > +}
> > > > +
> > > > +SpicePortConn.prototype = Object.create(SpiceConn.prototype);
> > > > +
> > > > +SpicePortConn.prototype.process_channel_message = function(msg)
> > > > +{
> > > > +if (msg.type == SPICE_MSG_PORT_INIT)
> > > > +{
> > > > +if 

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

[Oliver Gutierrez]

This patch has not been pushed yet. Is there any problem?

On Wed, Oct 5, 2016 at 8:06 AM, Pavel Grunt  wrote:

> Hi Oliver,
>
> it looks good to me :)
>
> Acked-by: Pavel Grunt 
>
> Thanks,
> Pavel
>
> On Mon, 2016-10-03 at 14:09 +0200, Oliver Gutierrez wrote:
> > ---
> >  enums.js|  9 ++
> >  main.js |  3 ++
> >  port.js | 85
> > +
> >  spice.html  | 13 +
> >  spice_auto.html | 13 +
> >  spicemsg.js | 18 
> >  utils.js|  7 +
> >  7 files changed, 148 insertions(+)
> >  create mode 100644 port.js
> >
> > diff --git a/enums.js b/enums.js
> > index 301fea0..b6e013c 100644
> > --- a/enums.js
> > +++ b/enums.js
> > @@ -166,6 +166,15 @@ var SPICE_MSG_PLAYBACK_VOLUME   = 105;
> >  var SPICE_MSG_PLAYBACK_MUTE = 106;
> >  var SPICE_MSG_PLAYBACK_LATENCY  = 107;
> >
> > +var SPICE_MSG_SPICEVMC_DATA = 101;
> > +var SPICE_MSG_PORT_INIT = 201;
> > +var SPICE_MSG_PORT_EVENT= 202;
> > +var SPICE_MSG_END_PORT  = 203;
> > +
> > +var SPICE_MSGC_SPICEVMC_DATA= 101;
> > +var SPICE_MSGC_PORT_EVENT   = 201;
> > +var SPICE_MSGC_END_PORT = 202;
> > +
> >  var SPICE_PLAYBACK_CAP_CELT_0_5_1   = 0;
> >  var SPICE_PLAYBACK_CAP_VOLUME   = 1;
> >  var SPICE_PLAYBACK_CAP_LATENCY  = 2;
> > diff --git a/main.js b/main.js
> > index 874a038..2d8a1ff 100644
> > --- a/main.js
> > +++ b/main.js
> > @@ -59,6 +59,7 @@ function SpiceMainConn()
> >  this.file_xfer_tasks = {};
> >  this.file_xfer_task_id = 0;
> >  this.file_xfer_read_queue = [];
> > +this.ports = [];
> >  }
> >
> >  SpiceMainConn.prototype = Object.create(SpiceConn.prototype);
> > @@ -154,6 +155,8 @@ SpiceMainConn.prototype.process_channel_message
> > = function(msg)
> >  this.cursor = new SpiceCursorConn(conn);
> >  else if (chans.channels[i].type ==
> > SPICE_CHANNEL_PLAYBACK)
> >  this.cursor = new SpicePlaybackConn(conn);
> > +else if (chans.channels[i].type == SPICE_CHANNEL_PORT)
> > +this.ports.push(new SpicePortConn(conn));
> >  else
> >  {
> >  if (! ("extra_channels" in this))
> > diff --git a/port.js b/port.js
> > new file mode 100644
> > index 000..ee22073
> > --- /dev/null
> > +++ b/port.js
> > @@ -0,0 +1,85 @@
> > +"use strict";
> > +/*
> > +   Copyright (C) 2016 by Oliver Gutierrez 
> > + Miroslav Chodil 
> > +
> > +   This file is part of spice-html5.
> > +
> > +   spice-html5 is free software: you can redistribute it and/or
> > modify
> > +   it under the terms of the GNU Lesser General Public License as
> > published by
> > +   the Free Software Foundation, either version 3 of the License,
> > or
> > +   (at your option) any later version.
> > +
> > +   spice-html5 is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +   GNU Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > License
> > +   along with spice-html5.  If not, see  > s/>.
> > +*/
> > +
> > +/*-
> > ---
> > +**  SpicePortConn
> > +**  Drive the Spice Port Channel
> > +**-
> > -*/
> > +function SpicePortConn()
> > +{
> > +DEBUG > 0 && console.log('SPICE port: created SPICE port
> > channel. Args:', arguments);
> > +SpiceConn.apply(this, arguments);
> > +this.port_name = null;
> > +}
> > +
> > +SpicePortConn.prototype = Object.create(SpiceConn.prototype);
> > +
> > +SpicePortConn.prototype.process_channel_message = function(msg)
> > +{
> > +if (msg.type == SPICE_MSG_PORT_INIT)
> > +{
> > +if (this.port_name === null)
> > +{
> > +var m = new SpiceMsgPortInit(msg.data);
> > +this.portName = arraybuffer_to_str(new
> > Uint8Array(m.name));
> > +this.portOpened = m.opened
> > +DEBUG > 0 && console.log('SPICE port: Port',
> > this.portName, 'initialized');
> > +return true;
> > +}
> > +
> > +DEBUG > 0 && console.log('SPICE port: Port',
> > this.port_name, 'is already initialized.');
> > +}
> > +else if (msg.type == SPICE_MSG_PORT_EVENT)
> > +{
> > +DEBUG > 0 && console.log('SPICE port: Port event received
> > for', this.portName, msg);
> > +var event = new CustomEvent('spice-port-event', {
> > +detail: {
> > +channel: this,
> > +

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

[Pavel Grunt]

On Mon, 2016-10-10 at 13:14 +0200, Oliver Gutierrez wrote:
> This patch has not been pushed yet. Is there any problem?

Sorry for the delay Oliver
It is pushed
Pavel


> On Wed, Oct 5, 2016 at 8:06 AM, Pavel Grunt 
> wrote:
> > Hi Oliver,
> > 
> > it looks good to me :)
> > 
> > Acked-by: Pavel Grunt 
> > 
> > Thanks,
> > Pavel
> > 
> > On Mon, 2016-10-03 at 14:09 +0200, Oliver Gutierrez wrote:
> > > ---
> > >  enums.js|  9 ++
> > >  main.js |  3 ++
> > >  port.js | 85
> > > +
> > >  spice.html  | 13 +
> > >  spice_auto.html | 13 +
> > >  spicemsg.js | 18 
> > >  utils.js|  7 +
> > >  7 files changed, 148 insertions(+)
> > >  create mode 100644 port.js
> > >
> > > diff --git a/enums.js b/enums.js
> > > index 301fea0..b6e013c 100644
> > > --- a/enums.js
> > > +++ b/enums.js
> > > @@ -166,6 +166,15 @@ var SPICE_MSG_PLAYBACK_VOLUME   =
> > 105;
> > >  var SPICE_MSG_PLAYBACK_MUTE = 106;
> > >  var SPICE_MSG_PLAYBACK_LATENCY  = 107;
> > >  
> > > +var SPICE_MSG_SPICEVMC_DATA = 101;
> > > +var SPICE_MSG_PORT_INIT = 201;
> > > +var SPICE_MSG_PORT_EVENT= 202;
> > > +var SPICE_MSG_END_PORT  = 203;
> > > +
> > > +var SPICE_MSGC_SPICEVMC_DATA= 101;
> > > +var SPICE_MSGC_PORT_EVENT   = 201;
> > > +var SPICE_MSGC_END_PORT = 202;
> > > +
> > >  var SPICE_PLAYBACK_CAP_CELT_0_5_1   = 0;
> > >  var SPICE_PLAYBACK_CAP_VOLUME   = 1;
> > >  var SPICE_PLAYBACK_CAP_LATENCY  = 2;
> > > diff --git a/main.js b/main.js
> > > index 874a038..2d8a1ff 100644
> > > --- a/main.js
> > > +++ b/main.js
> > > @@ -59,6 +59,7 @@ function SpiceMainConn()
> > >  this.file_xfer_tasks = {};
> > >  this.file_xfer_task_id = 0;
> > >  this.file_xfer_read_queue = [];
> > > +this.ports = [];
> > >  }
> > >  
> > >  SpiceMainConn.prototype = Object.create(SpiceConn.prototype);
> > > @@ -154,6 +155,8 @@
> > SpiceMainConn.prototype.process_channel_message
> > > = function(msg)
> > >  this.cursor = new SpiceCursorConn(conn);
> > >  else if (chans.channels[i].type ==
> > > SPICE_CHANNEL_PLAYBACK)
> > >  this.cursor = new SpicePlaybackConn(conn);
> > > +else if (chans.channels[i].type ==
> > SPICE_CHANNEL_PORT)
> > > +this.ports.push(new SpicePortConn(conn));
> > >  else
> > >  {
> > >  if (! ("extra_channels" in this))
> > > diff --git a/port.js b/port.js
> > > new file mode 100644
> > > index 000..ee22073
> > > --- /dev/null
> > > +++ b/port.js
> > > @@ -0,0 +1,85 @@
> > > +"use strict";
> > > +/*
> > > +   Copyright (C) 2016 by Oliver Gutierrez 
> > > + Miroslav Chodil 
> > > +
> > > +   This file is part of spice-html5.
> > > +
> > > +   spice-html5 is free software: you can redistribute it and/or
> > > modify
> > > +   it under the terms of the GNU Lesser General Public License
> > as
> > > published by
> > > +   the Free Software Foundation, either version 3 of the
> > License,
> > > or
> > > +   (at your option) any later version.
> > > +
> > > +   spice-html5 is distributed in the hope that it will be
> > useful,
> > > +   but WITHOUT ANY WARRANTY; without even the implied warranty
> > of
> > > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
> > the
> > > +   GNU Lesser General Public License for more details.
> > > +
> > > +   You should have received a copy of the GNU Lesser General
> > Public
> > > License
> > > +   along with spice-html5.  If not, see  > ense
> > > s/>.
> > > +*/
> > > +
> > > +/*-
> > 
> > > ---
> > > +**  SpicePortConn
> > > +**  Drive the Spice Port Channel
> > > +**-
> > 
> > > -*/
> > > +function SpicePortConn()
> > > +{
> > > +DEBUG > 0 && console.log('SPICE port: created SPICE port
> > > channel. Args:', arguments);
> > > +SpiceConn.apply(this, arguments);
> > > +this.port_name = null;
> > > +}
> > > +
> > > +SpicePortConn.prototype = Object.create(SpiceConn.prototype);
> > > +
> > > +SpicePortConn.prototype.process_channel_message = function(msg)
> > > +{
> > > +if (msg.type == SPICE_MSG_PORT_INIT)
> > > +{
> > > +if (this.port_name === null)
> > > +{
> > > +var m = new SpiceMsgPortInit(msg.data);
> > > +this.portName = arraybuffer_to_str(new
> > > Uint8Array(m.name));
> > > +this.portOpened = m.opened
> > > +DEBUG > 0 && console.log('SPICE port: Port',
> > > this.portName, 'initialized');
> > > +return true;
> > > +}
> > > +
> > > 

[Spice-devel] [PATCH spice-gtk v3] main: Send monitor config only when it changes

[Pavel Grunt]

When the guest receives the monitor configuration message, it replies
(through spice-server) by destroying the primary surface, which makes
the SpiceDisplay disabled if its "resize-guest" property is used.
This change of the display state (disabled/enabled) leads to sending
a new monitor config message (containing the disabled display) after
a second. Then spice-server sends the monitor configuration message,
spice-gtk replies back and we may end up with a loop of monitor
configuration messages even if no real change of monitor configuration
happens.

To avoid the loop, the new monitor configuration messages are only sent
if they are different from the current monitor configuration.

All the issues are visible only with Wayland & QXL driver on the guest

Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=1266484
https://bugs.freedesktop.org/show_bug.cgi?id=94950
---
v3: rebased, fixed typos
v2: https://lists.freedesktop.org/archives/spice-devel/2015-October/022784.html
---
 src/channel-main.c | 59 +++---
 1 file changed, 38 insertions(+), 21 deletions(-)

diff --git a/src/channel-main.c b/src/channel-main.c
index 990a06a..1a46819 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -105,7 +105,8 @@ struct _SpiceMainChannelPrivate  {
 guint   agent_msg_pos;
 uint8_t agent_msg_size;
 uint32_tagent_caps[VD_AGENT_CAPS_SIZE];
-SpiceDisplayConfig  display[MAX_DISPLAY];
+SpiceDisplayConfig  display_current[MAX_DISPLAY]; /* current 
configuration of spice-widgets */
+SpiceDisplayConfig  display_new[MAX_DISPLAY]; /* new requested 
configuration */
 ginttimer_id;
 GQueue  *agent_msg_queue;
 GHashTable  *file_xfer_tasks;
@@ -1098,11 +1099,11 @@ gboolean 
spice_main_send_monitor_config(SpiceMainChannel *channel)
 
 if (spice_main_agent_test_capability(channel,
  VD_AGENT_CAP_SPARSE_MONITORS_CONFIG)) {
-monitors = SPICE_N_ELEMENTS(c->display);
+monitors = SPICE_N_ELEMENTS(c->display_new);
 } else {
 monitors = 0;
-for (i = 0; i < SPICE_N_ELEMENTS(c->display); i++) {
-if (c->display[i].display_state == DISPLAY_ENABLED)
+for (i = 0; i < SPICE_N_ELEMENTS(c->display_new); i++) {
+if (c->display_new[i].display_state == DISPLAY_ENABLED)
 monitors += 1;
 }
 }
@@ -1117,24 +1118,25 @@ gboolean 
spice_main_send_monitor_config(SpiceMainChannel *channel)
 
 CHANNEL_DEBUG(channel, "sending new monitors config to guest");
 j = 0;
-for (i = 0; i < SPICE_N_ELEMENTS(c->display); i++) {
-if (c->display[i].display_state != DISPLAY_ENABLED) {
+for (i = 0; i < SPICE_N_ELEMENTS(c->display_new); i++) {
+if (c->display_new[i].display_state != DISPLAY_ENABLED) {
 if (spice_main_agent_test_capability(channel,
  VD_AGENT_CAP_SPARSE_MONITORS_CONFIG))
 j++;
 continue;
 }
 mon->monitors[j].depth  = c->display_color_depth ? 
c->display_color_depth : 32;
-mon->monitors[j].width  = c->display[i].width;
-mon->monitors[j].height = c->display[i].height;
-mon->monitors[j].x = c->display[i].x;
-mon->monitors[j].y = c->display[i].y;
+mon->monitors[j].width  = c->display_new[i].width;
+mon->monitors[j].height = c->display_new[i].height;
+mon->monitors[j].x = c->display_new[i].x;
+mon->monitors[j].y = c->display_new[i].y;
 CHANNEL_DEBUG(channel, "monitor #%d: %ux%u+%d+%d @ %u bpp", j,
   mon->monitors[j].width, mon->monitors[j].height,
   mon->monitors[j].x, mon->monitors[j].y,
   mon->monitors[j].depth);
 j++;
 }
+memcpy(c->display_current, c->display_new, MAX_DISPLAY * 
sizeof(SpiceDisplayConfig));
 
 if (c->disable_display_align == FALSE)
 monitors_align(mon->monitors, mon->num_of_monitors);
@@ -1469,13 +1471,23 @@ static gboolean 
any_display_has_dimensions(SpiceMainChannel *channel)
 c = channel->priv;
 
 for (i = 0; i < MAX_DISPLAY; i++) {
-if (c->display[i].width > 0 && c->display[i].height > 0)
+if (c->display_new[i].width > 0 && c->display_new[i].height > 0)
 return TRUE;
 }
 
 return FALSE;
 }
 
+static gboolean spice_monitor_configuration_changed(SpiceMainChannel *channel)
+{
+SpiceMainChannelPrivate *c;
+
+g_return_val_if_fail(SPICE_IS_MAIN_CHANNEL(channel), FALSE);
+c = channel->priv;
+
+return memcmp(c->display_current, c->display_new, MAX_DISPLAY * 
sizeof(SpiceDisplayConfig)) != 0;
+}
+
 /* main context*/
 static gboolean timer_set_display(gpointer data)
 {
@@ -1488,6 +1500,11 @@ static gboolean timer_set_display(gpointer data)
 if 

Re: [Spice-devel] [PATCH spice-gtk v3] main: Send monitor config only when it changes

[Pavel Grunt]

Hi Marc-André,

On Mon, 2016-10-10 at 08:37 -0400, Marc-André Lureau wrote:
> Hi
> 
> - Original Message -
> > When the guest receives the monitor configuration message, it
> > replies
> > (through spice-server) by destroying the primary surface, which
> > makes
> > the SpiceDisplay disabled if its "resize-guest" property is used.
> > This change of the display state (disabled/enabled) leads to
> > sending
> > a new monitor config message (containing the disabled display)
> > after
> > a second. Then spice-server sends the monitor configuration
> > message,
> > spice-gtk replies back and we may end up with a loop of monitor
> > configuration messages even if no real change of monitor
> > configuration
> > happens.
> 
> It's hard follow, could you explain step by step what you mean.
> > To avoid the loop, the new monitor configuration messages are only
> > sent
> > if they are different from the current monitor configuration.
> > 
> > All the issues are visible only with Wayland & QXL driver on the
> > guest
> > 
> > Fixes:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1266484
> > https://bugs.freedesktop.org/show_bug.cgi?id=94950
> > ---
> > v3: rebased, fixed typos
> 
> In v2, Jonathon raised a valid concern: "However, it's theoretically
> possible that the last sent configuration may not have been
> successful." For ex, vdagent wasn't yet started (whatever handles
> the monitor config change).

The configuration messages are sent only when agent is running.

It is happening all the time that the configuration message was
technically not successful (linux guest supports only width of
multiple of 8) and in fact it is the reason of this flickering bug. 

The problem is that client has no info that the message was
unsuccessful.

> 
> Imho, it should be fine for the client to send the same monitor
> config
How should client know that it should resend the message ? Would it
require a new message ?

> , the server should however not notify of changes if none happened.
(the change has happened - the primary surface was destroyed)

iow some component should just ignore the size request - it can be
client, server or driver (eg. Windows driver ignores that, QXL +Xorg
driver ignores that).

Imho server does a correct job - it gets info from the driver that the
primary surface was destroyed and it forwards the message to the
client.

I think this patch is just a workaround of the real root cause. It is
only GNOME on Wayland having the issue (iirc QXL was blacklisted by
GNOME - maybe it is a part of the problem).

The other option is to stop using QXL for kms and the patch can be
dropped.

Jonathon and You are right that there is a risk of introducing a
regression - we don't have tests for this area

Pavel
> 
> > v2:
> > https://lists.freedesktop.org/archives/spice-devel/2015-October/02
> > 2784.html
> > ---
> >  src/channel-main.c | 59
> >  +++---
> >  1 file changed, 38 insertions(+), 21 deletions(-)
> > 
> > diff --git a/src/channel-main.c b/src/channel-main.c
> > index 990a06a..1a46819 100644
> > --- a/src/channel-main.c
> > +++ b/src/channel-main.c
> > @@ -105,7 +105,8 @@ struct _SpiceMainChannelPrivate  {
> >  guint   agent_msg_pos;
> >  uint8_t agent_msg_size;
> >  uint32_tagent_caps[VD_AGENT_CAPS_SIZE];
> > -SpiceDisplayConfig  display[MAX_DISPLAY];
> > +SpiceDisplayConfig  display_current[MAX_DISPLAY]; /*
> > current
> > configuration of spice-widgets */
> > +SpiceDisplayConfig  display_new[MAX_DISPLAY]; /* new
> > requested
> > configuration */
> >  ginttimer_id;
> >  GQueue  *agent_msg_queue;
> >  GHashTable  *file_xfer_tasks;
> > @@ -1098,11 +1099,11 @@ gboolean
> > spice_main_send_monitor_config(SpiceMainChannel *channel)
> >  
> >  if (spice_main_agent_test_capability(channel,
> >   VD_AGENT_CAP_SPARSE_MONITORS
> > _CONFIG)) {
> > -monitors = SPICE_N_ELEMENTS(c->display);
> > +monitors = SPICE_N_ELEMENTS(c->display_new);
> >  } else {
> >  monitors = 0;
> > -for (i = 0; i < SPICE_N_ELEMENTS(c->display); i++) {
> > -if (c->display[i].display_state == DISPLAY_ENABLED)
> > +for (i = 0; i < SPICE_N_ELEMENTS(c->display_new); i++) {
> > +if (c->display_new[i].display_state ==
> > DISPLAY_ENABLED)
> >  monitors += 1;
> >  }
> >  }
> > @@ -1117,24 +1118,25 @@ gboolean
> > spice_main_send_monitor_config(SpiceMainChannel *channel)
> >  
> >  CHANNEL_DEBUG(channel, "sending new monitors config to
> > guest");
> >  j = 0;
> > -for (i = 0; i < SPICE_N_ELEMENTS(c->display); i++) {
> > -if (c->display[i].display_state != DISPLAY_ENABLED) {
> > +for (i = 0; i < SPICE_N_ELEMENTS(c->display_new); i++) {
> > +if 

Re: [Spice-devel] [PATCH spice-server v2 7/8] Convert RedChannel heirarchy to GObject

[Frediano Ziglio]

> Subject: [Spice-devel] [PATCH spice-server v2 7/8] Convert RedChannel 
> heirarchy to GObject
> 

Small spell, it's "hierarchy".

> From: Jonathon Jongsma 
> 
> FIXME: this commit appears to introduce a vdagent-related crash in
> vdi_port_read_one_msg_from_device(). sin->st is NULL.

If this is still true I would fix it.

> ---
>  server/Makefile.am  |   2 +
>  server/common-graphics-channel.c| 109 --
>  server/common-graphics-channel.h|  39 ++-
>  server/cursor-channel.c |  80 +++--
>  server/cursor-channel.h |  31 +-
>  server/dcc-send.c   |  11 +-
>  server/dcc.c|   1 +
>  server/dcc.h|   2 +-
>  server/display-channel-private.h|  76 +
>  server/display-channel.c| 261 +++
>  server/display-channel.h| 127 +++
>  server/dummy-channel-client.c   |  17 +-
>  server/dummy-channel.c  |  58 
>  server/dummy-channel.h  |  61 
>  server/inputs-channel.c | 263 +--
>  server/inputs-channel.h |  30 +-
>  server/main-channel-client.c|  65 ++--
>  server/main-channel-client.h|   5 +-
>  server/main-channel.c   | 196 +++
>  server/main-channel.h   |  44 ++-
>  server/red-channel-client-private.h |  19 ++
>  server/red-channel-client.c | 179 ++
>  server/red-channel-client.h |   6 +-
>  server/red-channel.c| 645
>  
>  server/red-channel.h| 180 +-
>  server/red-parse-qxl.h  |   2 +
>  server/red-qxl.c|  21 +-
>  server/red-replay-qxl.c |   2 +-
>  server/red-worker.c |  27 +-
>  server/red-worker.h |   2 -
>  server/reds-private.h   |   3 +-
>  server/reds.c   |  66 ++--
>  server/smartcard.c  | 127 +--
>  server/sound.c  |  43 ++-
>  server/spicevmc.c   | 429 ++--
>  server/stream.c |   4 +-
>  server/stream.h |   3 -
>  37 files changed, 2192 insertions(+), 1044 deletions(-)
>  create mode 100644 server/display-channel-private.h
>  create mode 100644 server/dummy-channel.c
>  create mode 100644 server/dummy-channel.h

I imagined this DummyChannel* would be viral... already in
my TODO list.

> 
> diff --git a/server/Makefile.am b/server/Makefile.am
> index 036abcd..c809330 100644
> --- a/server/Makefile.am
> +++ b/server/Makefile.am
> @@ -102,6 +102,8 @@ libserver_la_SOURCES =\
>   red-channel-client.c\
>   red-channel-client.h\
>   red-channel-client-private.h\
> + dummy-channel.c \
> + dummy-channel.h \
>   dummy-channel-client.c  \
>   dummy-channel-client.h  \
>   red-common.h\
> diff --git a/server/common-graphics-channel.c
> b/server/common-graphics-channel.c
> index 6871540..d8ee9dd 100644
> --- a/server/common-graphics-channel.c
> +++ b/server/common-graphics-channel.c
> @@ -29,6 +29,10 @@
>  
>  #define CHANNEL_RECEIVE_BUF_SIZE 1024
>  
> +G_DEFINE_ABSTRACT_TYPE(CommonGraphicsChannel, common_graphics_channel,
> RED_TYPE_CHANNEL)
> +
> +#define GRAPHICS_CHANNEL_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE((o),
> TYPE_COMMON_GRAPHICS_CHANNEL, CommonGraphicsChannelPrivate))
> +
>  struct CommonGraphicsChannelPrivate
>  {
>  QXLInstance *qxl;
> @@ -44,7 +48,7 @@ struct CommonGraphicsChannelPrivate
>  static uint8_t *common_alloc_recv_buf(RedChannelClient *rcc, uint16_t type,
>  uint32_t size)
>  {
>  RedChannel *channel = red_channel_client_get_channel(rcc);
> -CommonGraphicsChannel *common = SPICE_CONTAINEROF(channel,
> CommonGraphicsChannel, base);
> +CommonGraphicsChannel *common = COMMON_GRAPHICS_CHANNEL(channel);
>  
>  /* SPICE_MSGC_MIGRATE_DATA is the only client message whose size is
>  dynamic */
>  if (type == SPICE_MSGC_MIGRATE_DATA) {
> @@ -66,6 +70,48 @@ static void common_release_recv_buf(RedChannelClient *rcc,
> uint16_t type, uint32
>  }
>  }
>  
> +
> +enum {
> +PROP0,
> +PROP_QXL
> +};
> +

I remember I had some patches to remove this field... in my TODO list...

> +static void
> +common_graphics_channel_get_property(GObject *object,
> +   guint property_id,
> +   GValue *value,
> +   GParamSpec *pspec)
> +{
> +CommonGraphicsChannel *self = COMMON_GRAPHICS_CHANNEL(object);
> +
> +switch (property_id)
> +{
> +case PROP_QXL:
> +g_value_set_pointer(value, self->priv->qxl);
> +break;
> +  

Re: [Spice-devel] Function definition style

[Marc-André Lureau]

Hi

- Original Message -
> Hi,
> 
> On Mon, Oct 10, 2016 at 12:54:26PM -0400, Frediano Ziglio wrote:
> > Hi,
> >   I noted that in recent patches we started using this style:
> > 
> > static void
> > function_name(type name)
> > {
> > }
> > 
> > instead of the "classic" (in our code)
> > 
> > static void function_name(type name)
> > {
> > }
> > 
> > Personally I like the first and I don't complain (and other people
> > seems to not complain too) however sometimes it does not fit as the
> > other style is used.
> >
> > Do we agree we can use both styles or we just didn't pay much
> > attention?
> 
> The later? I'm often writing functions as the first style approach which
> is not common style in spice*. For the same reason, I don't pay much
> attention of that on reviews.
> 
> I would not mind to keep both styles.. or we should really write a hook
> to start checking for code style because this is quite common mistake...

In spice-gtk we use both style, I don't mind, but I have a slight preference 
for the first.

declarations are however almost always

static void function_name(type name);

in short, I like glib/gtk style best for no very rationale reasons.
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] Function definition style

[Frediano Ziglio]

Hi,
  I noted that in recent patches we started using this style:

static void
function_name(type name)
{
}

instead of the "classic" (in our code)

static void function_name(type name)
{
}

Personally I like the first and I don't complain (and other people
seems to not complain too) however sometimes it does not fit as the
other style is used.

Do we agree we can use both styles or we just didn't pay much
attention?

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] Function definition style

[Victor Toso]

Hi,

On Mon, Oct 10, 2016 at 12:54:26PM -0400, Frediano Ziglio wrote:
> Hi,
>   I noted that in recent patches we started using this style:
> 
> static void
> function_name(type name)
> {
> }
> 
> instead of the "classic" (in our code)
> 
> static void function_name(type name)
> {
> }
> 
> Personally I like the first and I don't complain (and other people
> seems to not complain too) however sometimes it does not fit as the
> other style is used.
>
> Do we agree we can use both styles or we just didn't pay much
> attention?

The later? I'm often writing functions as the first style approach which
is not common style in spice*. For the same reason, I don't pay much
attention of that on reviews.

I would not mind to keep both styles.. or we should really write a hook
to start checking for code style because this is quite common mistake...

Cheers

> 
> Frediano
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-server] style: Specify function definition indentation

[Frediano Ziglio]

Signed-off-by: Frediano Ziglio 
---
 docs/spice_style.txt | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/docs/spice_style.txt b/docs/spice_style.txt
index 026a354..cd874df 100644
--- a/docs/spice_style.txt
+++ b/docs/spice_style.txt
@@ -198,7 +198,14 @@ void function(type1 arg1, type2 arg2,
   type3 arg3)
 
 +
-* New line is acceptable only in arguments list
+* New line is acceptable in arguments list and before function name, like
++
+[source,c]
+
+void
+function(type1 arg1, type2 arg2,
+ type3 arg3)
+
 
 Branching indentation
 -
-- 
2.7.4

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH] !fixup Use macros for casting Channel types

[Jonathon Jongsma]

Convert a couple of additional casts
---
One additional fixup on top of Frediano's with some additional cases mentioned
by Pavel

 server/inputs-channel.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/server/inputs-channel.c b/server/inputs-channel.c
index 83c1360..85ca155 100644
--- a/server/inputs-channel.c
+++ b/server/inputs-channel.c
@@ -259,7 +259,7 @@ static void inputs_channel_send_item(RedChannelClient *rcc, 
RedPipeItem *base)
 red_channel_client_init_send_data(rcc, 
SPICE_MSG_INPUTS_MOUSE_MOTION_ACK, base);
 break;
 case RED_PIPE_ITEM_MIGRATE_DATA:
-
((InputsChannel*)red_channel_client_get_channel(rcc))->src_during_migrate = 
FALSE;
+
INPUTS_CHANNEL(red_channel_client_get_channel(rcc))->src_during_migrate = FALSE;
 inputs_channel_client_send_migrate_data(rcc, m, base);
 break;
 default:
@@ -272,7 +272,7 @@ static void inputs_channel_send_item(RedChannelClient *rcc, 
RedPipeItem *base)
 static int inputs_channel_handle_parsed(RedChannelClient *rcc, uint32_t size, 
uint16_t type,
 void *message)
 {
-InputsChannel *inputs_channel = (InputsChannel 
*)red_channel_client_get_channel(rcc);
+InputsChannel *inputs_channel = 
INPUTS_CHANNEL(red_channel_client_get_channel(rcc));
 InputsChannelClient *icc = INPUTS_CHANNEL_CLIENT(rcc);
 uint32_t i;
 RedsState *reds = red_channel_get_server(_channel->base);
@@ -458,13 +458,13 @@ static void inputs_channel_on_disconnect(RedChannelClient 
*rcc)
 if (!rcc) {
 return;
 }
-inputs_release_keys((InputsChannel*)red_channel_client_get_channel(rcc));
+inputs_release_keys(INPUTS_CHANNEL(red_channel_client_get_channel(rcc)));
 }
 
 static void inputs_pipe_add_init(RedChannelClient *rcc)
 {
 RedInputsInitPipeItem *item = spice_malloc(sizeof(RedInputsInitPipeItem));
-InputsChannel *inputs = 
(InputsChannel*)red_channel_client_get_channel(rcc);
+InputsChannel *inputs = 
INPUTS_CHANNEL(red_channel_client_get_channel(rcc));
 
 red_pipe_item_init(>base, RED_PIPE_ITEM_INPUTS_INIT);
 item->modifiers = kbd_get_leds(inputs_channel_get_keyboard(inputs));
@@ -511,7 +511,7 @@ static void inputs_connect(RedChannel *channel, RedClient 
*client,
 
 static void inputs_migrate(RedChannelClient *rcc)
 {
-InputsChannel *inputs = 
(InputsChannel*)red_channel_client_get_channel(rcc);
+InputsChannel *inputs = 
INPUTS_CHANNEL(red_channel_client_get_channel(rcc));
 inputs->src_during_migrate = TRUE;
 red_channel_client_default_migrate(rcc);
 }
@@ -548,7 +548,7 @@ static int 
inputs_channel_handle_migrate_data(RedChannelClient *rcc,
   void *message)
 {
 InputsChannelClient *icc = INPUTS_CHANNEL_CLIENT(rcc);
-InputsChannel *inputs = 
(InputsChannel*)red_channel_client_get_channel(rcc);
+InputsChannel *inputs = 
INPUTS_CHANNEL(red_channel_client_get_channel(rcc));
 SpiceMigrateDataHeader *header;
 SpiceMigrateDataInputs *mig_data;
 
@@ -580,7 +580,7 @@ InputsChannel* inputs_channel_new(RedsState *reds)
 channel_cbs.handle_migrate_data = inputs_channel_handle_migrate_data;
 channel_cbs.handle_migrate_flush_mark = 
inputs_channel_handle_migrate_flush_mark;
 
-inputs = (InputsChannel *)red_channel_create_parser(
+inputs = INPUTS_CHANNEL(red_channel_create_parser(
 sizeof(InputsChannel),
 reds,
 reds_get_core_interface(reds),
@@ -589,7 +589,7 @@ InputsChannel* inputs_channel_new(RedsState *reds)
 
spice_get_client_channel_parser(SPICE_CHANNEL_INPUTS, NULL),
 inputs_channel_handle_parsed,
 _cbs,
-SPICE_MIGRATE_NEED_FLUSH | 
SPICE_MIGRATE_NEED_DATA_TRANSFER);
+SPICE_MIGRATE_NEED_FLUSH | 
SPICE_MIGRATE_NEED_DATA_TRANSFER));
 
 if (!inputs) {
 spice_error("failed to allocate Inputs Channel");
-- 
2.7.4

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-gtk v3] main: Send monitor config only when it changes

[Christophe Fergeau]

Hey,

On Mon, Oct 10, 2016 at 05:12:53PM +0200, Pavel Grunt wrote:
> On Mon, 2016-10-10 at 17:03 +0200, Christophe Fergeau wrote:
> > Hey,
> > 
> > On Mon, Oct 10, 2016 at 04:09:11PM +0200, Pavel Grunt wrote:
> > > The configuration messages are sent only when agent is running.
> > > 
> > > It is happening all the time that the configuration message was
> > > technically not successful (linux guest supports only width of
> > > multiple of 8) and in fact it is the reason of this flickering
> > > bug. 
> > 
> > "linux guests supports only width of multiple of 8", I assume you
> > mean
> > when trying to set the resolution through direct use of KMS? Do you
> > know
> > why this seems to be working fine when the Xorg QXL driver is in
> > use?
> 
> I believe the gnome-settings-deamon and mutter are involved in this.

In both cases, mutter is involved, though things are going through 2
different code paths, meta-monitor-manager-xrandr.c VS
meta-monitor-manager-kms.c (and in the latter case some DRM ioctl
failures can indeed be seen, I did not check yet what the xrandr code is
doing).

> gsd has problems getting edid for qxl under wayland, but I don't know
> why

Oh, edid has been removed even from the QXL xorg driver a while ago, I
would not be surprised if we did not have any EDID information anywhere
(
see 
https://cgit.freedesktop.org/xorg/driver/xf86-video-qxl/commit/?id=96e6be278896ea6ecb43984d7e6fe8eea3b75ab1
)

> > 
> > What is destroying the primary surface guest-side though? I guess
> > this
> > happens in the kernel KMS driver as part of the attempted resolution
> > change?
>
> yes

I think if the resolution change is going to fail, then the primary
surface should not be destroyed. Dunno how unrelated the 2 events are...
This would fix the blinking, but I suspect we'd keep attempting to
change the resolution and fail.


> > 
> > > The other option is to stop using QXL for kms and the patch can be
> > > dropped.
> > 
> > I guess you mean stop using a QXL device when wayland is in use? I
> > don't
> > think we can realistically do that.
> >
> No I meant to stop using qxl for fedora24+ and start using virtio-gpu
> instead

We could recommend not using qxl for f24+, but it's going to be hard to
prevent it totally, and you cannot change it for existing VMs which are
upgraded from f23 to f24 for example. I'd rather than we fix this :)

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-server v2 5/8] Add CommonGraphicsChannelPrivate struct

[Frediano Ziglio]

From: Jonathon Jongsma 

Encapsulate private data for CommonGraphicsChannel and prepare for
GObject conversion.
---
 server/common-graphics-channel.c | 34 --
 server/common-graphics-channel.h | 14 ++
 server/cursor-channel-client.c   |  2 +-
 server/cursor-channel.c  |  8 +---
 server/dcc-send.c|  2 +-
 server/dcc.c |  8 
 server/display-channel.c |  6 +++---
 server/red-worker.c  |  7 ---
 8 files changed, 56 insertions(+), 25 deletions(-)

diff --git a/server/common-graphics-channel.c b/server/common-graphics-channel.c
index bcf7279..6871540 100644
--- a/server/common-graphics-channel.c
+++ b/server/common-graphics-channel.c
@@ -27,6 +27,20 @@
 #include "dcc.h"
 #include "main-channel-client.h"
 
+#define CHANNEL_RECEIVE_BUF_SIZE 1024
+
+struct CommonGraphicsChannelPrivate
+{
+QXLInstance *qxl;
+uint8_t recv_buf[CHANNEL_RECEIVE_BUF_SIZE];
+uint32_t id_alloc; // bitfield. TODO - use this instead of shift scheme.
+int during_target_migrate; /* TRUE when the client that is associated with 
the channel
+  is during migration. Turned off when the vm 
is started.
+  The flag is used to avoid sending messages 
that are artifacts
+  of the transition from stopped vm to loaded 
vm (e.g., recreation
+  of the primary surface) */
+};
+
 static uint8_t *common_alloc_recv_buf(RedChannelClient *rcc, uint16_t type, 
uint32_t size)
 {
 RedChannel *channel = red_channel_client_get_channel(rcc);
@@ -41,7 +55,7 @@ static uint8_t *common_alloc_recv_buf(RedChannelClient *rcc, 
uint16_t type, uint
 spice_critical("unexpected message size %u (max is %d)", size, 
CHANNEL_RECEIVE_BUF_SIZE);
 return NULL;
 }
-return common->recv_buf;
+return common->priv->recv_buf;
 }
 
 static void common_release_recv_buf(RedChannelClient *rcc, uint16_t type, 
uint32_t size,
@@ -123,7 +137,23 @@ CommonGraphicsChannel* 
common_graphics_channel_new(RedsState *server,
 spice_return_val_if_fail(channel, NULL);
 
 common = COMMON_GRAPHICS_CHANNEL(channel);
-common->qxl = qxl;
+common->priv = g_new0(CommonGraphicsChannelPrivate, 1);
+common->priv->qxl = qxl;
 return common;
 }
 
+void common_graphics_channel_set_during_target_migrate(CommonGraphicsChannel 
*self, gboolean value)
+{
+self->priv->during_target_migrate = value;
+}
+
+gboolean 
common_graphics_channel_get_during_target_migrate(CommonGraphicsChannel *self)
+{
+return self->priv->during_target_migrate;
+}
+
+QXLInstance* common_graphics_channel_get_qxl(CommonGraphicsChannel *self)
+{
+return self->priv->qxl;
+}
+
diff --git a/server/common-graphics-channel.h b/server/common-graphics-channel.h
index 7b2aeff..c6c3f48 100644
--- a/server/common-graphics-channel.h
+++ b/server/common-graphics-channel.h
@@ -25,21 +25,19 @@ int common_channel_config_socket(RedChannelClient *rcc);
 
 #define COMMON_CLIENT_TIMEOUT (NSEC_PER_SEC * 30)
 
-#define CHANNEL_RECEIVE_BUF_SIZE 1024
+typedef struct CommonGraphicsChannelPrivate CommonGraphicsChannelPrivate;
 typedef struct CommonGraphicsChannel {
 RedChannel base; // Must be the first thing
 
-QXLInstance *qxl;
-uint8_t recv_buf[CHANNEL_RECEIVE_BUF_SIZE];
-int during_target_migrate; /* TRUE when the client that is associated with 
the channel
-  is during migration. Turned off when the vm 
is started.
-  The flag is used to avoid sending messages 
that are artifacts
-  of the transition from stopped vm to loaded 
vm (e.g., recreation
-  of the primary surface) */
+CommonGraphicsChannelPrivate *priv;
 } CommonGraphicsChannel;
 
 #define COMMON_GRAPHICS_CHANNEL(Channel) ((CommonGraphicsChannel*)(Channel))
 
+void common_graphics_channel_set_during_target_migrate(CommonGraphicsChannel 
*self, gboolean value);
+gboolean 
common_graphics_channel_get_during_target_migrate(CommonGraphicsChannel *self);
+QXLInstance* common_graphics_channel_get_qxl(CommonGraphicsChannel *self);
+
 enum {
 RED_PIPE_ITEM_TYPE_VERB = RED_PIPE_ITEM_TYPE_CHANNEL_BASE,
 RED_PIPE_ITEM_TYPE_INVAL_ONE,
diff --git a/server/cursor-channel-client.c b/server/cursor-channel-client.c
index 90778ed..b7ab2e5 100644
--- a/server/cursor-channel-client.c
+++ b/server/cursor-channel-client.c
@@ -124,7 +124,7 @@ CursorChannelClient* 
cursor_channel_client_new(CursorChannel *cursor, RedClient
  "common-caps", common_caps_array,
  "caps", caps_array,
  NULL);
-COMMON_GRAPHICS_CHANNEL(cursor)->during_target_migrate = mig_target;
+
common_graphics_channel_set_during_target_migrate(COMMON_GRAPHICS_CHANNEL(cursor),
 

[Spice-devel] [PATCH spice-server v2 2/8] fixup! Move CommonGraphicsChannel to a new file

[Frediano Ziglio]

From: Pavel Grunt 

---
 server/Makefile.am  | 4 ++--
 server/red-worker.c | 6 ++
 server/red-worker.h | 1 -
 3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/server/Makefile.am b/server/Makefile.am
index 3382946..036abcd 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -77,8 +77,8 @@ libserver_la_SOURCES =\
cache-item.h\
char-device.c   \
char-device.h   \
-   common-graphics-channel.c   \
-   common-graphics-channel.h   \
+   common-graphics-channel.c   \
+   common-graphics-channel.h   \
demarshallers.h \
event-loop.c\
glz-encoder.c   \
diff --git a/server/red-worker.c b/server/red-worker.c
index 085f4f3..2dfa8e4 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -1365,8 +1365,7 @@ RedWorker* red_worker_new(QXLInstance *qxl,
 worker->cursor_channel = cursor_channel_new(reds, qxl,
 >core);
 channel = RED_CHANNEL(worker->cursor_channel);
-red_channel_set_stat_node(channel, stat_add_node(reds, worker->stat,
- "cursor_channel", TRUE));
+red_channel_set_stat_node(channel, stat_add_node(reds, worker->stat, 
"cursor_channel", TRUE));
 red_channel_register_client_cbs(channel, client_cursor_cbs, dispatcher);
 reds_register_channel(reds, channel);
 
@@ -1377,8 +1376,7 @@ RedWorker* red_worker_new(QXLInstance *qxl,
   init_info.n_surfaces);
 
 channel = RED_CHANNEL(worker->display_channel);
-red_channel_set_stat_node(channel, stat_add_node(reds, worker->stat,
- "display_channel", TRUE));
+red_channel_set_stat_node(channel, stat_add_node(reds, worker->stat, 
"display_channel", TRUE));
 red_channel_register_client_cbs(channel, client_display_cbs, dispatcher);
 red_channel_set_cap(channel, SPICE_DISPLAY_CAP_MONITORS_CONFIG);
 red_channel_set_cap(channel, SPICE_DISPLAY_CAP_PREF_COMPRESSION);
diff --git a/server/red-worker.h b/server/red-worker.h
index 370240f..dc2ff24 100644
--- a/server/red-worker.h
+++ b/server/red-worker.h
@@ -29,7 +29,6 @@ RedWorker* red_worker_new(QXLInstance *qxl,
   const ClientCbs *client_cursor_cbs,
   const ClientCbs *client_display_cbs);
 bool   red_worker_run(RedWorker *worker);
-SpiceCoreInterfaceInternal* red_worker_get_core_interface(RedWorker *worker);
 
 void red_drawable_unref(RedDrawable *red_drawable);
 
-- 
2.7.4

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-server v2 1/8] Move CommonGraphicsChannel to a new file

[Frediano Ziglio]

From: Jonathon Jongsma 

Move out of red-worker.c. This requires a little bit of minor
refactoring to avoid accessing some RedWorker internals in the
constructor function, etc.
---
 server/Makefile.am   |   2 +
 server/common-graphics-channel.c | 129 +++
 server/common-graphics-channel.h |  89 +++
 server/cursor-channel-client.c   |   1 +
 server/cursor-channel.c  |  11 ++--
 server/cursor-channel.h  |   5 +-
 server/display-channel.c |   9 +--
 server/display-channel.h |   6 +-
 server/red-worker.c  | 117 +++
 server/red-worker.h  |  64 +--
 10 files changed, 248 insertions(+), 185 deletions(-)
 create mode 100644 server/common-graphics-channel.c
 create mode 100644 server/common-graphics-channel.h

diff --git a/server/Makefile.am b/server/Makefile.am
index f217399..3382946 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -77,6 +77,8 @@ libserver_la_SOURCES =\
cache-item.h\
char-device.c   \
char-device.h   \
+   common-graphics-channel.c   \
+   common-graphics-channel.h   \
demarshallers.h \
event-loop.c\
glz-encoder.c   \
diff --git a/server/common-graphics-channel.c b/server/common-graphics-channel.c
new file mode 100644
index 000..625162c
--- /dev/null
+++ b/server/common-graphics-channel.c
@@ -0,0 +1,129 @@
+/*
+   Copyright (C) 2009-2016 Red Hat, Inc.
+
+   This library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   This library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with this library; if not, see .
+*/
+#ifdef HAVE_CONFIG_H
+#include 
+#endif
+
+#include 
+#include 
+#include 
+#include 
+
+#include "common-graphics-channel.h"
+#include "dcc.h"
+#include "main-channel-client.h"
+
+static uint8_t *common_alloc_recv_buf(RedChannelClient *rcc, uint16_t type, 
uint32_t size)
+{
+RedChannel *channel = red_channel_client_get_channel(rcc);
+CommonGraphicsChannel *common = SPICE_CONTAINEROF(channel, 
CommonGraphicsChannel, base);
+
+/* SPICE_MSGC_MIGRATE_DATA is the only client message whose size is 
dynamic */
+if (type == SPICE_MSGC_MIGRATE_DATA) {
+return spice_malloc(size);
+}
+
+if (size > CHANNEL_RECEIVE_BUF_SIZE) {
+spice_critical("unexpected message size %u (max is %d)", size, 
CHANNEL_RECEIVE_BUF_SIZE);
+return NULL;
+}
+return common->recv_buf;
+}
+
+static void common_release_recv_buf(RedChannelClient *rcc, uint16_t type, 
uint32_t size,
+uint8_t* msg)
+{
+if (type == SPICE_MSGC_MIGRATE_DATA) {
+free(msg);
+}
+}
+
+int common_channel_config_socket(RedChannelClient *rcc)
+{
+RedClient *client = red_channel_client_get_client(rcc);
+MainChannelClient *mcc = red_client_get_main(client);
+RedsStream *stream = red_channel_client_get_stream(rcc);
+int flags;
+int delay_val;
+gboolean is_low_bandwidth;
+
+if ((flags = fcntl(stream->socket, F_GETFL)) == -1) {
+spice_warning("accept failed, %s", strerror(errno));
+return FALSE;
+}
+
+if (fcntl(stream->socket, F_SETFL, flags | O_NONBLOCK) == -1) {
+spice_warning("accept failed, %s", strerror(errno));
+return FALSE;
+}
+
+// TODO - this should be dynamic, not one time at channel creation
+is_low_bandwidth = main_channel_client_is_low_bandwidth(mcc);
+delay_val = is_low_bandwidth ? 0 : 1;
+/* FIXME: Using Nagle's Algorithm can lead to apparent delays, depending
+ * on the delayed ack timeout on the other side.
+ * Instead of using Nagle's, we need to implement message buffering on
+ * the application level.
+ * see: http://www.stuartcheshire.org/papers/NagleDelayedAck/
+ */
+if (setsockopt(stream->socket, IPPROTO_TCP, TCP_NODELAY, _val,
+   sizeof(delay_val)) == -1) {
+if (errno != ENOTSUP) {
+spice_warning("setsockopt failed, %s", strerror(errno));
+}
+}
+// TODO: move wide/narrow ack setting to red_channel.
+red_channel_client_ack_set_client_window(rcc,
+is_low_bandwidth ?
+   

[Spice-devel] [PATCH spice-server v2 3/8] Use macros for casting Channel types

[Frediano Ziglio]

From: Jonathon Jongsma 

In preparation for converting RedChannel to GObject, switch to using
RED_CHANNEL()-type macros for casting. For now they just do a regular
cast, but it helps reduce the size of the GObject patch to make it
easier to review.
---
 server/common-graphics-channel.c | 2 +-
 server/cursor-channel.c  | 6 +++---
 server/cursor-channel.h  | 1 +
 server/display-channel.c | 4 ++--
 server/display-channel.h | 2 ++
 server/inputs-channel-client.c   | 2 +-
 server/inputs-channel.c  | 2 +-
 server/inputs-channel.h  | 1 +
 server/main-channel-client.c | 2 +-
 server/main-channel.c| 2 +-
 server/main-channel.h| 2 ++
 11 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/server/common-graphics-channel.c b/server/common-graphics-channel.c
index 625162c..bcf7279 100644
--- a/server/common-graphics-channel.c
+++ b/server/common-graphics-channel.c
@@ -122,7 +122,7 @@ CommonGraphicsChannel* 
common_graphics_channel_new(RedsState *server,
 migration_flags);
 spice_return_val_if_fail(channel, NULL);
 
-common = (CommonGraphicsChannel *)channel;
+common = COMMON_GRAPHICS_CHANNEL(channel);
 common->qxl = qxl;
 return common;
 }
diff --git a/server/cursor-channel.c b/server/cursor-channel.c
index c73745f..85a7fe5 100644
--- a/server/cursor-channel.c
+++ b/server/cursor-channel.c
@@ -171,7 +171,7 @@ static void cursor_fill(CursorChannelClient *ccc, 
SpiceCursor *red_cursor,
 
 void cursor_channel_disconnect(CursorChannel *cursor_channel)
 {
-RedChannel *channel = (RedChannel *)cursor_channel;
+RedChannel *channel = RED_CHANNEL(cursor_channel);
 
 if (!channel || !red_channel_is_connected(channel)) {
 return;
@@ -199,7 +199,7 @@ static void red_marshall_cursor_init(CursorChannelClient 
*ccc, SpiceMarshaller *
 AddBufInfo info;
 
 spice_assert(rcc);
-cursor_channel = (CursorChannel*)red_channel_client_get_channel(rcc);
+cursor_channel = CURSOR_CHANNEL(red_channel_client_get_channel(rcc));
 
 red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_INIT, NULL);
 msg.visible = cursor_channel->priv->cursor_visible;
@@ -325,7 +325,7 @@ CursorChannel* cursor_channel_new(RedsState *server, 
QXLInstance *qxl,
   SPICE_CHANNEL_CURSOR, 0,
   , 
red_channel_client_handle_message);
 
-cursor_channel = (CursorChannel *)channel;
+cursor_channel = CURSOR_CHANNEL(channel);
 cursor_channel->priv->cursor_visible = TRUE;
 cursor_channel->priv->mouse_mode = SPICE_MOUSE_MODE_SERVER;
 
diff --git a/server/cursor-channel.h b/server/cursor-channel.h
index a3ddaa3..0f2b43d 100644
--- a/server/cursor-channel.h
+++ b/server/cursor-channel.h
@@ -20,6 +20,7 @@
 
 #include "common-graphics-channel.h"
 
+#define CURSOR_CHANNEL(channel) ((CursorChannel*)channel)
 /**
  * This type it's a RedChannel class which implement cursor (mouse)
  * movements.
diff --git a/server/display-channel.c b/server/display-channel.c
index 93744a4..b9e2b93 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -1924,10 +1924,10 @@ DisplayChannel* display_channel_new(RedsState *reds,
 };
 
 spice_info("create display channel");
-display = (DisplayChannel *)common_graphics_channel_new(
+display = DISPLAY_CHANNEL(common_graphics_channel_new(
 reds, qxl, core, sizeof(*display), SPICE_CHANNEL_DISPLAY,
 SPICE_MIGRATE_NEED_FLUSH | SPICE_MIGRATE_NEED_DATA_TRANSFER,
-, dcc_handle_message);
+, dcc_handle_message));
 spice_return_val_if_fail(display, NULL);
 display->priv->pub = display;
 
diff --git a/server/display-channel.h b/server/display-channel.h
index 936768e..998970e 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -45,6 +45,8 @@
 #include "image-encoders.h"
 #include "common-graphics-channel.h"
 
+#define DISPLAY_CHANNEL(channel) ((DisplayChannel*)channel)
+
 typedef struct DependItem {
 Drawable *drawable;
 RingItem ring_item;
diff --git a/server/inputs-channel-client.c b/server/inputs-channel-client.c
index 76de382..4ab2457 100644
--- a/server/inputs-channel-client.c
+++ b/server/inputs-channel-client.c
@@ -110,7 +110,7 @@ void 
inputs_channel_client_handle_migrate_data(InputsChannelClient *icc,
 
 void inputs_channel_client_on_mouse_motion(InputsChannelClient *icc)
 {
-InputsChannel *inputs_channel = (InputsChannel 
*)red_channel_client_get_channel(RED_CHANNEL_CLIENT(icc));
+InputsChannel *inputs_channel = 
INPUTS_CHANNEL(red_channel_client_get_channel(RED_CHANNEL_CLIENT(icc)));
 
 if (++icc->priv->motion_count % SPICE_INPUT_MOTION_ACK_BUNCH == 0 &&
 !inputs_channel_is_src_during_migrate(inputs_channel)) {
diff --git a/server/inputs-channel.c b/server/inputs-channel.c
index 840d5e9..83c1360 100644
--- a/server/inputs-channel.c
+++ 

[Spice-devel] [PATCH spice-server v2 4/8] fixup! Use macros for casting Channel types

[Frediano Ziglio]

Use parenthesis to enclose macro parameters.
More CURSOR_CHANNEL to implementation, not used externally.

Signed-off-by: Frediano Ziglio 
---
 server/cursor-channel.c  | 2 ++
 server/cursor-channel.h  | 1 -
 server/display-channel.h | 2 +-
 server/inputs-channel.h  | 3 ++-
 server/main-channel.h| 2 +-
 5 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/server/cursor-channel.c b/server/cursor-channel.c
index 85a7fe5..e84b593 100644
--- a/server/cursor-channel.c
+++ b/server/cursor-channel.c
@@ -27,6 +27,8 @@
 #include "cursor-channel-client.h"
 #include "reds.h"
 
+#define CURSOR_CHANNEL(channel) ((CursorChannel*)(channel))
+
 typedef struct CursorChannelClient CursorChannelClient;
 
 enum {
diff --git a/server/cursor-channel.h b/server/cursor-channel.h
index 0f2b43d..a3ddaa3 100644
--- a/server/cursor-channel.h
+++ b/server/cursor-channel.h
@@ -20,7 +20,6 @@
 
 #include "common-graphics-channel.h"
 
-#define CURSOR_CHANNEL(channel) ((CursorChannel*)channel)
 /**
  * This type it's a RedChannel class which implement cursor (mouse)
  * movements.
diff --git a/server/display-channel.h b/server/display-channel.h
index 998970e..3762e54 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -45,7 +45,7 @@
 #include "image-encoders.h"
 #include "common-graphics-channel.h"
 
-#define DISPLAY_CHANNEL(channel) ((DisplayChannel*)channel)
+#define DISPLAY_CHANNEL(channel) ((DisplayChannel*)(channel))
 
 typedef struct DependItem {
 Drawable *drawable;
diff --git a/server/inputs-channel.h b/server/inputs-channel.h
index 7001094..ae84eed 100644
--- a/server/inputs-channel.h
+++ b/server/inputs-channel.h
@@ -26,7 +26,8 @@
 
 #include "red-channel.h"
 
-#define INPUTS_CHANNEL(channel) ((InputsChannel*)channel)
+#define INPUTS_CHANNEL(channel) ((InputsChannel*)(channel))
+
 typedef struct InputsChannel InputsChannel;
 
 InputsChannel* inputs_channel_new(RedsState *reds);
diff --git a/server/main-channel.h b/server/main-channel.h
index caea014..e0858d0 100644
--- a/server/main-channel.h
+++ b/server/main-channel.h
@@ -25,7 +25,7 @@
 #include "red-channel.h"
 #include "main-channel-client.h"
 
-#define MAIN_CHANNEL(channel) ((MainChannel*)channel)
+#define MAIN_CHANNEL(channel) ((MainChannel*)(channel))
 
 // TODO: Defines used to calculate receive buffer size, and also by reds.c
 // other options: is to make a reds_main_consts.h, to duplicate defines.
-- 
2.7.4

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-server v2 0/8] Updated refactory patchset

[Frediano Ziglio]

Added some fixup:
- proposals from Pavel;
- split long line to follow our style.

Frediano Ziglio (3):
  fixup! Use macros for casting Channel types
  fixup! Add CommonGraphicsChannelPrivate struct
  fixup! Convert RedChannel heirarchy to GObject

Jonathon Jongsma (4):
  Move CommonGraphicsChannel to a new file
  Use macros for casting Channel types
  Add CommonGraphicsChannelPrivate struct
  Convert RedChannel heirarchy to GObject

Pavel Grunt (1):
  fixup! Move CommonGraphicsChannel to a new file

 server/Makefile.am  |   4 +
 server/common-graphics-channel.c| 203 +++
 server/common-graphics-channel.h| 107 ++
 server/cursor-channel-client.c  |   3 +-
 server/cursor-channel.c |  94 +++---
 server/cursor-channel.h |  38 ++-
 server/dcc-send.c   |  13 +-
 server/dcc.c|  10 +-
 server/dcc.h|   2 +-
 server/display-channel-private.h|  76 +
 server/display-channel.c| 273 +++
 server/display-channel.h| 136 
 server/dummy-channel-client.c   |  17 +-
 server/dummy-channel.c  |  59 
 server/dummy-channel.h  |  63 
 server/inputs-channel-client.c  |   2 +-
 server/inputs-channel.c | 272 +--
 server/inputs-channel.h |  32 ++
 server/main-channel-client.c|  66 ++--
 server/main-channel-client.h|   5 +-
 server/main-channel.c   | 203 +++
 server/main-channel.h   |  46 ++-
 server/red-channel-client-private.h |  19 ++
 server/red-channel-client.c | 179 ++
 server/red-channel-client.h |   6 +-
 server/red-channel.c| 649 
 server/red-channel.h| 183 +-
 server/red-parse-qxl.h  |   2 +
 server/red-qxl.c|  21 +-
 server/red-replay-qxl.c |   2 +-
 server/red-worker.c | 152 ++---
 server/red-worker.h |  65 
 server/reds-private.h   |   3 +-
 server/reds.c   |  66 ++--
 server/smartcard.c  | 138 ++--
 server/sound.c  |  44 ++-
 server/spicevmc.c   | 450 +++--
 server/stream.c |   4 +-
 server/stream.h |   3 -
 39 files changed, 2512 insertions(+), 1198 deletions(-)
 create mode 100644 server/common-graphics-channel.c
 create mode 100644 server/common-graphics-channel.h
 create mode 100644 server/display-channel-private.h
 create mode 100644 server/dummy-channel.c
 create mode 100644 server/dummy-channel.h

-- 
2.7.4

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH spice-server v2 8/8] fixup! Convert RedChannel heirarchy to GObject

[Frediano Ziglio]

Avoid lines with more than 100 characters

Signed-off-by: Frediano Ziglio 
---
 server/common-graphics-channel.c |   3 +-
 server/common-graphics-channel.h |  15 +++--
 server/cursor-channel.c  |   3 +-
 server/cursor-channel.h  |   6 +-
 server/display-channel.c |   3 +-
 server/display-channel.h |   9 ++-
 server/dummy-channel.c   |   3 +-
 server/dummy-channel.h   |   6 +-
 server/inputs-channel.c  |  27 +---
 server/inputs-channel.h  |   6 +-
 server/main-channel-client.c |   3 +-
 server/main-channel.c|  21 --
 server/main-channel.h|   6 +-
 server/red-channel.c | 140 ---
 server/red-channel.h |   9 ++-
 server/smartcard.c   |  25 ---
 server/sound.c   |   3 +-
 server/spicevmc.c|  33 ++---
 18 files changed, 192 insertions(+), 129 deletions(-)

diff --git a/server/common-graphics-channel.c b/server/common-graphics-channel.c
index d8ee9dd..3f00b05 100644
--- a/server/common-graphics-channel.c
+++ b/server/common-graphics-channel.c
@@ -31,7 +31,8 @@
 
 G_DEFINE_ABSTRACT_TYPE(CommonGraphicsChannel, common_graphics_channel, 
RED_TYPE_CHANNEL)
 
-#define GRAPHICS_CHANNEL_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE((o), 
TYPE_COMMON_GRAPHICS_CHANNEL, CommonGraphicsChannelPrivate))
+#define GRAPHICS_CHANNEL_PRIVATE(o) \
+(G_TYPE_INSTANCE_GET_PRIVATE((o), TYPE_COMMON_GRAPHICS_CHANNEL, 
CommonGraphicsChannelPrivate))
 
 struct CommonGraphicsChannelPrivate
 {
diff --git a/server/common-graphics-channel.h b/server/common-graphics-channel.h
index 6961375..a8c3f3d 100644
--- a/server/common-graphics-channel.h
+++ b/server/common-graphics-channel.h
@@ -31,11 +31,16 @@ int common_channel_config_socket(RedChannelClient *rcc);
 
 #define TYPE_COMMON_GRAPHICS_CHANNEL common_graphics_channel_get_type()
 
-#define COMMON_GRAPHICS_CHANNEL(obj) (G_TYPE_CHECK_INSTANCE_CAST((obj), 
TYPE_COMMON_GRAPHICS_CHANNEL, CommonGraphicsChannel))
-#define COMMON_GRAPHICS_CHANNEL_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST((klass), 
TYPE_COMMON_GRAPHICS_CHANNEL, CommonGraphicsChannelClass))
-#define COMMON_IS_GRAPHICS_CHANNEL(obj) (G_TYPE_CHECK_INSTANCE_TYPE((obj), 
TYPE_COMMON_GRAPHICS_CHANNEL))
-#define COMMON_IS_GRAPHICS_CHANNEL_CLASS(klass) 
(G_TYPE_CHECK_CLASS_TYPE((klass), TYPE_COMMON_GRAPHICS_CHANNEL))
-#define COMMON_GRAPHICS_CHANNEL_GET_CLASS(obj) 
(G_TYPE_INSTANCE_GET_CLASS((obj), TYPE_COMMON_GRAPHICS_CHANNEL, 
CommonGraphicsChannelClass))
+#define COMMON_GRAPHICS_CHANNEL(obj) \
+(G_TYPE_CHECK_INSTANCE_CAST((obj), TYPE_COMMON_GRAPHICS_CHANNEL, 
CommonGraphicsChannel))
+#define COMMON_GRAPHICS_CHANNEL_CLASS(klass) \
+(G_TYPE_CHECK_CLASS_CAST((klass), TYPE_COMMON_GRAPHICS_CHANNEL, 
CommonGraphicsChannelClass))
+#define COMMON_IS_GRAPHICS_CHANNEL(obj) \
+(G_TYPE_CHECK_INSTANCE_TYPE((obj), TYPE_COMMON_GRAPHICS_CHANNEL))
+#define COMMON_IS_GRAPHICS_CHANNEL_CLASS(klass) \
+(G_TYPE_CHECK_CLASS_TYPE((klass), TYPE_COMMON_GRAPHICS_CHANNEL))
+#define COMMON_GRAPHICS_CHANNEL_GET_CLASS(obj) \
+(G_TYPE_INSTANCE_GET_CLASS((obj), TYPE_COMMON_GRAPHICS_CHANNEL, 
CommonGraphicsChannelClass))
 
 typedef struct CommonGraphicsChannel CommonGraphicsChannel;
 typedef struct CommonGraphicsChannelClass CommonGraphicsChannelClass;
diff --git a/server/cursor-channel.c b/server/cursor-channel.c
index cf19c7f..4640e5f 100644
--- a/server/cursor-channel.c
+++ b/server/cursor-channel.c
@@ -51,7 +51,8 @@ typedef struct RedCursorPipeItem {
 
 G_DEFINE_TYPE(CursorChannel, cursor_channel, TYPE_COMMON_GRAPHICS_CHANNEL)
 
-#define CURSOR_CHANNEL_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE((o), 
TYPE_CURSOR_CHANNEL, CursorChannelPrivate))
+#define CURSOR_CHANNEL_PRIVATE(o) \
+(G_TYPE_INSTANCE_GET_PRIVATE((o), TYPE_CURSOR_CHANNEL, 
CursorChannelPrivate))
 
 struct CursorChannelPrivate
 {
diff --git a/server/cursor-channel.h b/server/cursor-channel.h
index 8b3bc17..73c3773 100644
--- a/server/cursor-channel.h
+++ b/server/cursor-channel.h
@@ -26,10 +26,12 @@ G_BEGIN_DECLS
 #define TYPE_CURSOR_CHANNEL cursor_channel_get_type()
 
 #define CURSOR_CHANNEL(obj) (G_TYPE_CHECK_INSTANCE_CAST((obj), 
TYPE_CURSOR_CHANNEL, CursorChannel))
-#define CURSOR_CHANNEL_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST((klass), 
TYPE_CURSOR_CHANNEL, CursorChannelClass))
+#define CURSOR_CHANNEL_CLASS(klass) \
+(G_TYPE_CHECK_CLASS_CAST((klass), TYPE_CURSOR_CHANNEL, CursorChannelClass))
 #define IS_CURSOR_CHANNEL(obj) (G_TYPE_CHECK_INSTANCE_TYPE((obj), 
TYPE_CURSOR_CHANNEL))
 #define IS_CURSOR_CHANNEL_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE((klass), 
TYPE_CURSOR_CHANNEL))
-#define CURSOR_CHANNEL_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS((obj), 
TYPE_CURSOR_CHANNEL, CursorChannelClass))
+#define CURSOR_CHANNEL_GET_CLASS(obj) \
+(G_TYPE_INSTANCE_GET_CLASS((obj), TYPE_CURSOR_CHANNEL, CursorChannelClass))
 
 /**
  * This type it's a RedChannel class which 

[Spice-devel] [PATCH spice-server v2 6/8] fixup! Add CommonGraphicsChannelPrivate struct

[Frediano Ziglio]

Avoid lines with more than 100 characters

Signed-off-by: Frediano Ziglio 
---
 server/dcc.c| 3 ++-
 server/red-worker.c | 9 ++---
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/server/dcc.c b/server/dcc.c
index 021c241..3519d2e 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -284,7 +284,8 @@ void dcc_create_surface(DisplayChannelClient *dcc, int 
surface_id)
 flags = is_primary_surface(DCC_TO_DC(dcc), surface_id) ? 
SPICE_SURFACE_FLAGS_PRIMARY : 0;
 
 /* don't send redundant create surface commands to client */
-if (!dcc || 
common_graphics_channel_get_during_target_migrate(COMMON_GRAPHICS_CHANNEL(display))
 ||
+if (!dcc ||
+
common_graphics_channel_get_during_target_migrate(COMMON_GRAPHICS_CHANNEL(display))
 ||
 dcc->priv->surface_client_created[surface_id]) {
 return;
 }
diff --git a/server/red-worker.c b/server/red-worker.c
index 8da154a..678856b 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -533,8 +533,9 @@ static void dev_create_primary_surface(RedWorker *worker, 
uint32_t surface_id,
line_0, surface.flags & 
QXL_SURF_FLAG_KEEP_DATA, TRUE);
 display_channel_set_monitors_config_to_primary(display);
 
+CommonGraphicsChannel *common = 
COMMON_GRAPHICS_CHANNEL(worker->display_channel);
 if (display_is_connected(worker) &&
-
!common_graphics_channel_get_during_target_migrate(COMMON_GRAPHICS_CHANNEL(worker->display_channel)))
 {
+!common_graphics_channel_get_during_target_migrate(common)) {
 /* guest created primary, so it will (hopefully) send a monitors_config
  * now, don't send our own temporary one */
 if (!worker->driver_cap_monitors_config) {
@@ -639,10 +640,12 @@ static void handle_dev_start(void *opaque, void *payload)
 
 spice_assert(!worker->running);
 if (worker->cursor_channel) {
-
common_graphics_channel_set_during_target_migrate(COMMON_GRAPHICS_CHANNEL(worker->cursor_channel),
 FALSE);
+CommonGraphicsChannel *common = 
COMMON_GRAPHICS_CHANNEL(worker->cursor_channel);
+common_graphics_channel_set_during_target_migrate(common, FALSE);
 }
 if (worker->display_channel) {
-
common_graphics_channel_set_during_target_migrate(COMMON_GRAPHICS_CHANNEL(worker->display_channel),
 FALSE);
+CommonGraphicsChannel *common = 
COMMON_GRAPHICS_CHANNEL(worker->display_channel);
+common_graphics_channel_set_during_target_migrate(common, FALSE);
 display_channel_wait_for_migrate_data(worker->display_channel);
 }
 worker->running = TRUE;
-- 
2.7.4

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server v2 5/8] Add CommonGraphicsChannelPrivate struct

[Frediano Ziglio]

> 
> From: Jonathon Jongsma 
> 
> Encapsulate private data for CommonGraphicsChannel and prepare for
> GObject conversion.

This object has all the fields with accessors... not a really private
I would say. Is not possible to implement in a different way using
this structure in CursorChannelPrivate and DisplayChannelPrivate ?

> ---
>  server/common-graphics-channel.c | 34 --
>  server/common-graphics-channel.h | 14 ++
>  server/cursor-channel-client.c   |  2 +-
>  server/cursor-channel.c  |  8 +---
>  server/dcc-send.c|  2 +-
>  server/dcc.c |  8 
>  server/display-channel.c |  6 +++---
>  server/red-worker.c  |  7 ---
>  8 files changed, 56 insertions(+), 25 deletions(-)
> 
> diff --git a/server/common-graphics-channel.c
> b/server/common-graphics-channel.c
> index bcf7279..6871540 100644
> --- a/server/common-graphics-channel.c
> +++ b/server/common-graphics-channel.c
> @@ -27,6 +27,20 @@
>  #include "dcc.h"
>  #include "main-channel-client.h"
>  
> +#define CHANNEL_RECEIVE_BUF_SIZE 1024
> +
> +struct CommonGraphicsChannelPrivate
> +{
> +QXLInstance *qxl;
> +uint8_t recv_buf[CHANNEL_RECEIVE_BUF_SIZE];
> +uint32_t id_alloc; // bitfield. TODO - use this instead of shift scheme.

No reason to introduced again this unused field.

> +int during_target_migrate; /* TRUE when the client that is associated
> with the channel
> +  is during migration. Turned off when the
> vm is started.
> +  The flag is used to avoid sending messages
> that are artifacts
> +  of the transition from stopped vm to
> loaded vm (e.g., recreation
> +  of the primary surface) */
> +};
> +
>  static uint8_t *common_alloc_recv_buf(RedChannelClient *rcc, uint16_t type,
>  uint32_t size)
>  {
>  RedChannel *channel = red_channel_client_get_channel(rcc);
> @@ -41,7 +55,7 @@ static uint8_t *common_alloc_recv_buf(RedChannelClient
> *rcc, uint16_t type, uint
>  spice_critical("unexpected message size %u (max is %d)", size,
>  CHANNEL_RECEIVE_BUF_SIZE);
>  return NULL;
>  }
> -return common->recv_buf;
> +return common->priv->recv_buf;
>  }
>  
>  static void common_release_recv_buf(RedChannelClient *rcc, uint16_t type,
>  uint32_t size,
> @@ -123,7 +137,23 @@ CommonGraphicsChannel*
> common_graphics_channel_new(RedsState *server,
>  spice_return_val_if_fail(channel, NULL);
>  
>  common = COMMON_GRAPHICS_CHANNEL(channel);
> -common->qxl = qxl;
> +common->priv = g_new0(CommonGraphicsChannelPrivate, 1);
> +common->priv->qxl = qxl;
>  return common;
>  }
>  

new and no free, leak... but only till next patch so it's not a big
issue.

> +void common_graphics_channel_set_during_target_migrate(CommonGraphicsChannel
> *self, gboolean value)
> +{
> +self->priv->during_target_migrate = value;
> +}
> +
> +gboolean
> common_graphics_channel_get_during_target_migrate(CommonGraphicsChannel
> *self)
> +{
> +return self->priv->during_target_migrate;
> +}
> +

This field should really not be in this "private" structure

> +QXLInstance* common_graphics_channel_get_qxl(CommonGraphicsChannel *self)
> +{
> +return self->priv->qxl;
> +}
> +
> diff --git a/server/common-graphics-channel.h
> b/server/common-graphics-channel.h
> index 7b2aeff..c6c3f48 100644
> --- a/server/common-graphics-channel.h
> +++ b/server/common-graphics-channel.h
> @@ -25,21 +25,19 @@ int common_channel_config_socket(RedChannelClient *rcc);
>  
>  #define COMMON_CLIENT_TIMEOUT (NSEC_PER_SEC * 30)
>  
> -#define CHANNEL_RECEIVE_BUF_SIZE 1024
> +typedef struct CommonGraphicsChannelPrivate CommonGraphicsChannelPrivate;
>  typedef struct CommonGraphicsChannel {
>  RedChannel base; // Must be the first thing
>  
> -QXLInstance *qxl;
> -uint8_t recv_buf[CHANNEL_RECEIVE_BUF_SIZE];
> -int during_target_migrate; /* TRUE when the client that is associated
> with the channel
> -  is during migration. Turned off when the
> vm is started.
> -  The flag is used to avoid sending messages
> that are artifacts
> -  of the transition from stopped vm to
> loaded vm (e.g., recreation
> -  of the primary surface) */
> +CommonGraphicsChannelPrivate *priv;
>  } CommonGraphicsChannel;
>  
>  #define COMMON_GRAPHICS_CHANNEL(Channel) ((CommonGraphicsChannel*)(Channel))
>  
> +void common_graphics_channel_set_during_target_migrate(CommonGraphicsChannel
> *self, gboolean value);
> +gboolean
> common_graphics_channel_get_during_target_migrate(CommonGraphicsChannel
> *self);
> +QXLInstance* common_graphics_channel_get_qxl(CommonGraphicsChannel *self);
> +
>  enum {
>  RED_PIPE_ITEM_TYPE_VERB = 

Re: [Spice-devel] [PATCH 1/4] Move CommonGraphicsChannel to a new file

[Pavel Grunt]

Hi,

On Fri, 2016-10-07 at 17:01 -0500, Jonathon Jongsma wrote:
> Move out of red-worker.c. This requires a little bit of minor
> refactoring to avoid accessing some RedWorker internals in the
> constructor function, etc.
> ---
>  server/Makefile.am   |   2 +
>  server/common-graphics-channel.c | 129
> +++
>  server/common-graphics-channel.h |  89 +++
>  server/cursor-channel-client.c   |   1 +
>  server/cursor-channel.c  |  11 ++--
>  server/cursor-channel.h  |   5 +-
>  server/display-channel.c |   9 +--
>  server/display-channel.h |   6 +-
>  server/red-worker.c  | 117 +++-
> ---
>  server/red-worker.h  |  64 +--
>  10 files changed, 248 insertions(+), 185 deletions(-)
>  create mode 100644 server/common-graphics-channel.c
>  create mode 100644 server/common-graphics-channel.h
> 
> diff --git a/server/Makefile.am b/server/Makefile.am
> index f217399..3382946 100644
> --- a/server/Makefile.am
> +++ b/server/Makefile.am
> @@ -77,6 +77,8 @@ libserver_la_SOURCES =  
> \
>   cache-item.h\
>   char-device.c   \
>   char-device.h   \
> + common-graphics-channel.c   \
> + common-graphics-channel.h   \
wrong indent - tab size should be 8
>   demarshallers.h \
>   event-loop.c\
>   glz-encoder.c   \
> diff --git a/server/common-graphics-channel.c b/server/common-
> graphics-channel.c
> new file mode 100644
> index 000..625162c
> --- /dev/null
> +++ b/server/common-graphics-channel.c
> @@ -0,0 +1,129 @@
> +/*
> +   Copyright (C) 2009-2016 Red Hat, Inc.
> +
> +   This library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later
> version.
> +
> +   This library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with this library; if not, see  /licenses/>.
> +*/
> +#ifdef HAVE_CONFIG_H
> +#include 
> +#endif
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "common-graphics-channel.h"
> +#include "dcc.h"
> +#include "main-channel-client.h"
> +
> +static uint8_t *common_alloc_recv_buf(RedChannelClient *rcc,
> uint16_t type, uint32_t size)
> +{
> +RedChannel *channel = red_channel_client_get_channel(rcc);
> +CommonGraphicsChannel *common = SPICE_CONTAINEROF(channel,
> CommonGraphicsChannel, base);
> +
> +/* SPICE_MSGC_MIGRATE_DATA is the only client message whose
> size is dynamic */
> +if (type == SPICE_MSGC_MIGRATE_DATA) {
> +return spice_malloc(size);
> +}
> +
> +if (size > CHANNEL_RECEIVE_BUF_SIZE) {
> +spice_critical("unexpected message size %u (max is %d)",
> size, CHANNEL_RECEIVE_BUF_SIZE);
> +return NULL;
> +}
> +return common->recv_buf;
> +}
> +
> +static void common_release_recv_buf(RedChannelClient *rcc, uint16_t
> type, uint32_t size,
> +uint8_t* msg)
> +{
> +if (type == SPICE_MSGC_MIGRATE_DATA) {
> +free(msg);
> +}
> +}
> +
> +int common_channel_config_socket(RedChannelClient *rcc)
> +{
> +RedClient *client = red_channel_client_get_client(rcc);
> +MainChannelClient *mcc = red_client_get_main(client);
> +RedsStream *stream = red_channel_client_get_stream(rcc);
> +int flags;
> +int delay_val;
> +gboolean is_low_bandwidth;
> +
> +if ((flags = fcntl(stream->socket, F_GETFL)) == -1) {
> +spice_warning("accept failed, %s", strerror(errno));
> +return FALSE;
> +}
> +
> +if (fcntl(stream->socket, F_SETFL, flags | O_NONBLOCK) == -1) {
> +spice_warning("accept failed, %s", strerror(errno));
> +return FALSE;
> +}
> +
> +// TODO - this should be dynamic, not one time at channel
> creation
> +is_low_bandwidth = main_channel_client_is_low_bandwidth(mcc);
> +delay_val = is_low_bandwidth ? 0 : 1;
> +/* FIXME: Using Nagle's Algorithm can lead to apparent delays,
> depending
> + * on the delayed ack timeout on the other side.
> + * Instead of using Nagle's, we need to implement message
> buffering on
> + * the application level.
> + * see: http://www.stuartcheshire.org/papers/NagleDelayedAck/
> + */
> +if (setsockopt(stream->socket, IPPROTO_TCP, TCP_NODELAY,
> _val,
> + 

[Spice-devel] spice-gtk updates for Fedora 25 need to be added to bodhi

[Hans de Goede]

Hi All,

I noticed that there have been a few spice-gtk builds fixing
the bugs I was hitting, thank you for that.

I had to install them manually from koji, since they are
not yet listed in bodhi. Can you please create bodhi
updates for these ?  :

https://bodhi.fedoraproject.org/updates/?packages=spice-gtk

Regards,

Hans

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server] style: Specify function definition indentation

[Jonathon Jongsma]

Acked-by: Jonathon Jongsma 


On Mon, 2016-10-10 at 19:27 +0100, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio 
> ---
>  docs/spice_style.txt | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/spice_style.txt b/docs/spice_style.txt
> index 026a354..cd874df 100644
> --- a/docs/spice_style.txt
> +++ b/docs/spice_style.txt
> @@ -198,7 +198,14 @@ void function(type1 arg1, type2 arg2,
>    type3 arg3)
>  
>  +
> -* New line is acceptable only in arguments list
> +* New line is acceptable in arguments list and before function name,
> like
> ++
> +[source,c]
> +
> +void
> +function(type1 arg1, type2 arg2,
> + type3 arg3)
> +
>  
>  Branching indentation
>  -
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] Function definition style

[Jonathon Jongsma]

On Mon, 2016-10-10 at 14:11 -0400, Marc-André Lureau wrote:
> Hi
> 
> - Original Message -
> > 
> > Hi,
> > 
> > On Mon, Oct 10, 2016 at 12:54:26PM -0400, Frediano Ziglio wrote:
> > > 
> > > Hi,
> > >   I noted that in recent patches we started using this style:
> > > 
> > > static void
> > > function_name(type name)
> > > {
> > > }
> > > 
> > > instead of the "classic" (in our code)
> > > 
> > > static void function_name(type name)
> > > {
> > > }
> > > 
> > > Personally I like the first and I don't complain (and other
> > > people
> > > seems to not complain too) however sometimes it does not fit as
> > > the
> > > other style is used.
> > > 
> > > Do we agree we can use both styles or we just didn't pay much
> > > attention?
> > 
> > The later? I'm often writing functions as the first style approach
> > which
> > is not common style in spice*. For the same reason, I don't pay
> > much
> > attention of that on reviews.
> > 
> > I would not mind to keep both styles.. or we should really write a
> > hook
> > to start checking for code style because this is quite common
> > mistake...
> 
> In spice-gtk we use both style, I don't mind, but I have a slight
> preference for the first.
> 
> declarations are however almost always
> 
> static void function_name(type name);
> 
> in short, I like glib/gtk style best for no very rationale reasons.
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel


To be honest, I don't really have much of a preference. I guess the
first style is a little better when you want to keep the lines from
getting too long. Perhaps it would be better to choose one and try to
stick with it, but I don't mind too much if we have both in the code
base.

Jonathon
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server v2 5/8] Add CommonGraphicsChannelPrivate struct

[Jonathon Jongsma]

On Mon, 2016-10-10 at 12:20 -0400, Frediano Ziglio wrote:
> > 
> > 
> > From: Jonathon Jongsma 
> > 
> > Encapsulate private data for CommonGraphicsChannel and prepare for
> > GObject conversion.
> 
> This object has all the fields with accessors... not a really private
> I would say. Is not possible to implement in a different way using
> this structure in CursorChannelPrivate and DisplayChannelPrivate ?

Possibly, but I'm not sure that I see the advantage of doing so. Also
note that the DisplayChannelClient currently uses some of these
variables, so we still need to access/modify this data from outside of
the DisplayChannel implementation. 


> 
> > 
> > ---
> >  server/common-graphics-channel.c | 34
> > --
> >  server/common-graphics-channel.h | 14 ++
> >  server/cursor-channel-client.c   |  2 +-
> >  server/cursor-channel.c  |  8 +---
> >  server/dcc-send.c|  2 +-
> >  server/dcc.c |  8 
> >  server/display-channel.c |  6 +++---
> >  server/red-worker.c  |  7 ---
> >  8 files changed, 56 insertions(+), 25 deletions(-)
> > 
> > diff --git a/server/common-graphics-channel.c
> > b/server/common-graphics-channel.c
> > index bcf7279..6871540 100644
> > --- a/server/common-graphics-channel.c
> > +++ b/server/common-graphics-channel.c
> > @@ -27,6 +27,20 @@
> >  #include "dcc.h"
> >  #include "main-channel-client.h"
> >  
> > +#define CHANNEL_RECEIVE_BUF_SIZE 1024
> > +
> > +struct CommonGraphicsChannelPrivate
> > +{
> > +QXLInstance *qxl;
> > +uint8_t recv_buf[CHANNEL_RECEIVE_BUF_SIZE];
> > +uint32_t id_alloc; // bitfield. TODO - use this instead of
> > shift scheme.
> 
> No reason to introduced again this unused field.

Oops, must have slipped back in during the rebase.

> 
> > @@ -123,7 +137,23 @@ CommonGraphicsChannel*
> > common_graphics_channel_new(RedsState *server,
> >  spice_return_val_if_fail(channel, NULL);
> >  
> >  common = COMMON_GRAPHICS_CHANNEL(channel);
> > -common->qxl = qxl;
> > +common->priv = g_new0(CommonGraphicsChannelPrivate, 1);
> > +common->priv->qxl = qxl;
> >  return common;
> >  }
> >  
> 
> new and no free, leak... but only till next patch so it's not a big
> issue.

Hmm, yeah. We could do the 1-element array trick again, but that would
require us to have the private struct definition available in the
header. Not sure it's worth it.


> 
> > 
> > +void
> > common_graphics_channel_set_during_target_migrate(CommonGraphicsCha
> > nnel
> > *self, gboolean value)
> > +{
> > +self->priv->during_target_migrate = value;
> > +}
> > +
> > +gboolean
> > common_graphics_channel_get_during_target_migrate(CommonGraphicsCha
> > nnel
> > *self)
> > +{
> > +return self->priv->during_target_migrate;
> > +}
> > +
> 
> This field should really not be in this "private" structure

Can you expand on this comment? Where do you think it should be?


Jonathon
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-server v2 7/8] Convert RedChannel heirarchy to GObject

[Jonathon Jongsma]

On Mon, 2016-10-10 at 12:48 -0400, Frediano Ziglio wrote:
> > 
> > Subject: [Spice-devel] [PATCH spice-server v2 7/8] Convert
> > RedChannel  heirarchy to GObject
> > 
> 
> Small spell, it's "hierarchy".
> 
> > 
> > From: Jonathon Jongsma 
> > 
> > FIXME: this commit appears to introduce a vdagent-related crash in
> > vdi_port_read_one_msg_from_device(). sin->st is NULL.
> 
> If this is still true I would fix it.

I added this comment a long time ago when I was running into a crash
while testing the refactory branch. I bisected and ended up at this
commit, but didn't have time to fix the crash right away, so I added
the comment to remind myself. However, I have not seen the crash at all
recently.

My theory is that perhaps this crash was caused by a bad rebase that
introduced an error that we've subsequently fixed due to reviews and
additional rebasing.


> 
> > @@ -381,7 +367,7 @@ void cursor_channel_process_cmd(CursorChannel
> > *cursor,
> > RedCursorCmd *cursor_cmd)
> >  
> >  void cursor_channel_reset(CursorChannel *cursor)
> >  {
> > -RedChannel *channel = >common.base;
> > +RedChannel *channel = RED_CHANNEL(cursor);
> >  
> >  spice_return_if_fail(cursor);
> >  
> > @@ -395,9 +381,9 @@ void cursor_channel_reset(CursorChannel
> > *cursor)
> >  if
> >  (!common_graphics_channel_get_during_target_migrate(COMMON
> > _GRAPHICS_CHANNEL(cursor)))
> >  {
> >  red_pipes_add_verb(channel, SPICE_MSG_CURSOR_RESET);
> >  }
> > -if (!red_channel_wait_all_sent(>common.base,
> > +if (!red_channel_wait_all_sent(RED_CHANNEL(cursor),
> > COMMON_CLIENT_TIMEOUT)) {
> > -red_channel_apply_clients(channel,
> > +red_channel_apply_clients(RED_CHANNEL(cursor),
> 
> Why not using channel variable ??
> 


Strange. Not sure. Will change.

> > 
> >    red_channel_client_disconnec
> > t_if_pending_send);
> >  }
> >  }
> > @@ -407,7 +393,7 @@ static void
> > cursor_channel_init_client(CursorChannel
> > *cursor, CursorChannelClien
> >  {
> >  spice_return_if_fail(cursor);
> >  
> > -if (!red_channel_is_connected(>common.base)
> > +if (!red_channel_is_connected(RED_CHANNEL(cursor))
> >  ||
> > common_graphics_channel_get_during_target_migrate(COMMON_GRAPHICS_C
> > HANNEL(cursor)))
> >  || {
> >  spice_debug("during_target_migrate: skip init");
> >  return;
> > @@ -420,7 +406,7 @@ static void
> > cursor_channel_init_client(CursorChannel
> > *cursor, CursorChannelClien
> >  red_channel_pipes_add_type(RED_CHANNEL(cursor),
> >  RED_PIPE_ITEM_TYPE_CURSOR_INIT);
> >  }
> >  
> > -void cursor_channel_init(CursorChannel *cursor)
> > +void cursor_channel_do_init(CursorChannel *cursor)
> >  {
> >  cursor_channel_init_client(cursor, NULL);
> >  }
> > @@ -454,3 +440,25 @@ void cursor_channel_connect(CursorChannel
> > *cursor,
> > RedClient *client, RedsStream
> >  
> >  cursor_channel_init_client(cursor, ccc);
> >  }
> > +
> > +static void
> > +cursor_channel_class_init(CursorChannelClass *klass)
> > +{
> > +RedChannelClass *channel_class = RED_CHANNEL_CLASS(klass);
> > +
> > +g_type_class_add_private(klass, sizeof(CursorChannelPrivate));
> > +
> > +channel_class->parser =
> > spice_get_client_channel_parser(SPICE_CHANNEL_CURSOR, NULL);
> > +channel_class->handle_parsed =
> > red_channel_client_handle_message;
> > +
> > +channel_class->on_disconnect
> > =  cursor_channel_client_on_disconnect;
> > +channel_class->send_item = cursor_channel_send_item;
> > +}
> > +
> > +static void
> > +cursor_channel_init(CursorChannel *self)
> > +{
> > +self->priv = CURSOR_CHANNEL_PRIVATE(self);
> > +self->priv->cursor_visible = TRUE;
> > +self->priv->mouse_mode = SPICE_MOUSE_MODE_SERVER;
> > +}
> > diff --git a/server/cursor-channel.h b/server/cursor-channel.h
> > index a3ddaa3..8b3bc17 100644
> > --- a/server/cursor-channel.h
> > +++ b/server/cursor-channel.h
> > @@ -19,6 +19,17 @@
> >  # define CURSOR_CHANNEL_H_
> >  
> >  #include "common-graphics-channel.h"
> > +#include "red-parse-qxl.h"
> > +
> > +G_BEGIN_DECLS
> > +
> > +#define TYPE_CURSOR_CHANNEL cursor_channel_get_type()
> > +
> > +#define CURSOR_CHANNEL(obj) (G_TYPE_CHECK_INSTANCE_CAST((obj),
> > TYPE_CURSOR_CHANNEL, CursorChannel))
> > +#define CURSOR_CHANNEL_CLASS(klass)
> > (G_TYPE_CHECK_CLASS_CAST((klass),
> > TYPE_CURSOR_CHANNEL, CursorChannelClass))
> > +#define IS_CURSOR_CHANNEL(obj) (G_TYPE_CHECK_INSTANCE_TYPE((obj),
> > TYPE_CURSOR_CHANNEL))
> > +#define IS_CURSOR_CHANNEL_CLASS(klass)
> > (G_TYPE_CHECK_CLASS_TYPE((klass),
> > TYPE_CURSOR_CHANNEL))
> > +#define CURSOR_CHANNEL_GET_CLASS(obj)
> > (G_TYPE_INSTANCE_GET_CLASS((obj),
> > TYPE_CURSOR_CHANNEL, CursorChannelClass))
> >  
> >  /**
> >   * This type it's a RedChannel class which implement cursor
> > (mouse)
> > @@ -26,6 +37,22 @@
> > 

Re: [Spice-devel] [PATCH 2/4] Use macros for casting Channel types

[Pavel Grunt]

On Fri, 2016-10-07 at 17:01 -0500, Jonathon Jongsma wrote:
> In preparation for converting RedChannel to GObject, switch to using
> RED_CHANNEL()-type macros for casting. For now they just do a
> regular
> cast, but it helps reduce the size of the GObject patch to make it
> easier to review.
I agree

git grep "Channel \?\*)"

gives few more occurrences

Pavel

> ---
>  server/common-graphics-channel.c | 2 +-
>  server/cursor-channel.c  | 6 +++---
>  server/cursor-channel.h  | 1 +
>  server/display-channel.c | 4 ++--
>  server/display-channel.h | 2 ++
>  server/inputs-channel-client.c   | 2 +-
>  server/inputs-channel.c  | 2 +-
>  server/inputs-channel.h  | 1 +
>  server/main-channel-client.c | 2 +-
>  server/main-channel.c| 2 +-
>  server/main-channel.h| 2 ++
>  11 files changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/server/common-graphics-channel.c b/server/common-
> graphics-channel.c
> index 625162c..bcf7279 100644
> --- a/server/common-graphics-channel.c
> +++ b/server/common-graphics-channel.c
> @@ -122,7 +122,7 @@ CommonGraphicsChannel*
> common_graphics_channel_new(RedsState *server,
>  migration_flags);
>  spice_return_val_if_fail(channel, NULL);
>  
> -common = (CommonGraphicsChannel *)channel;
> +common = COMMON_GRAPHICS_CHANNEL(channel);
>  common->qxl = qxl;
>  return common;
>  }
> diff --git a/server/cursor-channel.c b/server/cursor-channel.c
> index c73745f..85a7fe5 100644
> --- a/server/cursor-channel.c
> +++ b/server/cursor-channel.c
> @@ -171,7 +171,7 @@ static void cursor_fill(CursorChannelClient
> *ccc, SpiceCursor *red_cursor,
>  
>  void cursor_channel_disconnect(CursorChannel *cursor_channel)
>  {
> -RedChannel *channel = (RedChannel *)cursor_channel;
> +RedChannel *channel = RED_CHANNEL(cursor_channel);
>  
>  if (!channel || !red_channel_is_connected(channel)) {
>  return;
> @@ -199,7 +199,7 @@ static void
> red_marshall_cursor_init(CursorChannelClient *ccc, SpiceMarshaller *
>  AddBufInfo info;
>  
>  spice_assert(rcc);
> -cursor_channel =
> (CursorChannel*)red_channel_client_get_channel(rcc);
> +cursor_channel =
> CURSOR_CHANNEL(red_channel_client_get_channel(rcc));
>  
>  red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_INIT,
> NULL);
>  msg.visible = cursor_channel->priv->cursor_visible;
> @@ -325,7 +325,7 @@ CursorChannel* cursor_channel_new(RedsState
> *server, QXLInstance *qxl,
>    SPICE_CHANNEL_CURSOR, 0,
>    ,
> red_channel_client_handle_message);
>  
> -cursor_channel = (CursorChannel *)channel;
> +cursor_channel = CURSOR_CHANNEL(channel);
>  cursor_channel->priv->cursor_visible = TRUE;
>  cursor_channel->priv->mouse_mode = SPICE_MOUSE_MODE_SERVER;
>  
> diff --git a/server/cursor-channel.h b/server/cursor-channel.h
> index a3ddaa3..0f2b43d 100644
> --- a/server/cursor-channel.h
> +++ b/server/cursor-channel.h
> @@ -20,6 +20,7 @@
>  
>  #include "common-graphics-channel.h"
>  
> +#define CURSOR_CHANNEL(channel) ((CursorChannel*)channel)
>  /**
>   * This type it's a RedChannel class which implement cursor (mouse)
>   * movements.
> diff --git a/server/display-channel.c b/server/display-channel.c
> index 93744a4..b9e2b93 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -1924,10 +1924,10 @@ DisplayChannel*
> display_channel_new(RedsState *reds,
>  };
>  
>  spice_info("create display channel");
> -display = (DisplayChannel *)common_graphics_channel_new(
> +display = DISPLAY_CHANNEL(common_graphics_channel_new(
>  reds, qxl, core, sizeof(*display), SPICE_CHANNEL_DISPLAY,
>  SPICE_MIGRATE_NEED_FLUSH |
> SPICE_MIGRATE_NEED_DATA_TRANSFER,
> -, dcc_handle_message);
> +, dcc_handle_message));
>  spice_return_val_if_fail(display, NULL);
>  display->priv->pub = display;
>  
> diff --git a/server/display-channel.h b/server/display-channel.h
> index 936768e..998970e 100644
> --- a/server/display-channel.h
> +++ b/server/display-channel.h
> @@ -45,6 +45,8 @@
>  #include "image-encoders.h"
>  #include "common-graphics-channel.h"
>  
> +#define DISPLAY_CHANNEL(channel) ((DisplayChannel*)channel)
> +
>  typedef struct DependItem {
>  Drawable *drawable;
>  RingItem ring_item;
> diff --git a/server/inputs-channel-client.c b/server/inputs-channel-
> client.c
> index 76de382..4ab2457 100644
> --- a/server/inputs-channel-client.c
> +++ b/server/inputs-channel-client.c
> @@ -110,7 +110,7 @@ void
> inputs_channel_client_handle_migrate_data(InputsChannelClient *icc,
>  
>  void inputs_channel_client_on_mouse_motion(InputsChannelClient
> *icc)
>  {
> -InputsChannel *inputs_channel = (InputsChannel
> *)red_channel_client_get_channel(RED_CHANNEL_CLIENT(icc));
> +InputsChannel *inputs_channel =
> 

Re: [Spice-devel] spice-gtk updates for Fedora 25 need to be added to bodhi

[Marc-André Lureau]

Hi

- Original Message -
> Hi All,
> 
> I noticed that there have been a few spice-gtk builds fixing
> the bugs I was hitting, thank you for that.
> 
> I had to install them manually from koji, since they are
> not yet listed in bodhi. Can you please create bodhi
> updates for these ?  :
> 
> https://bodhi.fedoraproject.org/updates/?packages=spice-gtk

f24:
https://bodhi.fedoraproject.org/updates/FEDORA-2016-fea866fa85
f25:
https://bodhi.fedoraproject.org/updates/FEDORA-2016-c1bf3f1059

Is that fine?

Btw, since you are experienced Fedora packager, could you provide some hints on 
solving the errors in 
https://taskotron.fedoraproject.org/artifacts/all/886bb18a-8c8d-11e6-be50-525400120b80/task_output/spice-gtk-0.33-1.fc25.log
 
Thanks
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-gtk v3] main: Send monitor config only when it changes

[Christophe Fergeau]

Hey,

On Mon, Oct 10, 2016 at 04:09:11PM +0200, Pavel Grunt wrote:
> The configuration messages are sent only when agent is running.
> 
> It is happening all the time that the configuration message was
> technically not successful (linux guest supports only width of
> multiple of 8) and in fact it is the reason of this flickering bug. 

"linux guests supports only width of multiple of 8", I assume you mean
when trying to set the resolution through direct use of KMS? Do you know
why this seems to be working fine when the Xorg QXL driver is in use?

> > , the server should however not notify of changes if none happened.
> (the change has happened - the primary surface was destroyed)
> 
> iow some component should just ignore the size request - it can be
> client, server or driver (eg. Windows driver ignores that, QXL +Xorg
> driver ignores that).
> 
> Imho server does a correct job - it gets info from the driver that the
> primary surface was destroyed and it forwards the message to the
> client.

What is destroying the primary surface guest-side though? I guess this
happens in the kernel KMS driver as part of the attempted resolution
change?

> The other option is to stop using QXL for kms and the patch can be
> dropped.

I guess you mean stop using a QXL device when wayland is in use? I don't
think we can realistically do that.

Christophe


signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Re: [Spice-devel] [PATCH spice-gtk v3] main: Send monitor config only when it changes

[Pavel Grunt]

On Mon, 2016-10-10 at 17:03 +0200, Christophe Fergeau wrote:
> Hey,
> 
> On Mon, Oct 10, 2016 at 04:09:11PM +0200, Pavel Grunt wrote:
> > The configuration messages are sent only when agent is running.
> > 
> > It is happening all the time that the configuration message was
> > technically not successful (linux guest supports only width of
> > multiple of 8) and in fact it is the reason of this flickering
> > bug. 
> 
> "linux guests supports only width of multiple of 8", I assume you
> mean
> when trying to set the resolution through direct use of KMS? Do you
> know
> why this seems to be working fine when the Xorg QXL driver is in
> use?

I believe the gnome-settings-deamon and mutter are involved in this.
gsd has problems getting edid for qxl under wayland, but I don't know
why
> 
> > > , the server should however not notify of changes if none
> > > happened.
> > 
> > (the change has happened - the primary surface was destroyed)
> > 
> > iow some component should just ignore the size request - it can be
> > client, server or driver (eg. Windows driver ignores that, QXL
> > +Xorg
> > driver ignores that).
> > 
> > Imho server does a correct job - it gets info from the driver that
> > the
> > primary surface was destroyed and it forwards the message to the
> > client.
> 
> What is destroying the primary surface guest-side though? I guess
> this
> happens in the kernel KMS driver as part of the attempted resolution
> change?
yes
> 
> > The other option is to stop using QXL for kms and the patch can be
> > dropped.
> 
> I guess you mean stop using a QXL device when wayland is in use? I
> don't
> think we can realistically do that.
No I meant to stop using qxl for fedora24+ and start using virtio-gpu
instead

Pavel

> 
> Christophe
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel