Re: [Spice-devel] [vdagent-linux 1/2] randr: remove monitors.xml on auto-configuration

2015-03-17 Thread Fabiano Fidêncio
Hey,

Sorry for coming a bit late to the playground ...

On Wed, Oct 8, 2014 at 3:35 PM, Marc-André Lureau
marcandre.lur...@gmail.com wrote:

 On Wed, Oct 8, 2014 at 3:21 PM, Christophe Fergeau cferg...@redhat.com
 wrote:

 gnome2 is obsolete, not using client monitor config is getting obsolete,
 and removing a file which we don't own is a very heavy hammer,


 A hammer? monitors.xml is a user config, safe to delete, well-known file
 name (in gnomerr API reference)

 We could link with libgnome-desktop and call
 gnome_rr_config_get_intended_filename() instead, I thought that was overkill
 for something that is hardcoded.

 This file is not hidden or protected in any way, it doesn't use a private
 namespace etc.


 especially since this file is used by mutter in GNOME3 as well, so no,
 let's not do that upstream.


 we don't want to restore user config when vdagent config applies.
 Fortunately, with gnome3  drm driver, that code path is not triggered.

 Having a random outcome is and a different output in upstream and downstream
 is not better imho.

I tend to agree with Marc-André here, basically because we do *not*
want to restore user config anyway when vdagent config applies.
+1 for having the patch upstream (and downstream).

Best Regards,
-- 
Fabiano Fidêncio
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [vdagent-linux 1/2] randr: remove monitors.xml on auto-configuration

2014-10-24 Thread Christophe Fergeau
On Wed, Oct 08, 2014 at 12:28:53PM +0200, Marc-André Lureau wrote:
 Hi
 
 On Thu, Oct 2, 2014 at 2:10 PM, Christophe Fergeau cferg...@redhat.com
 wrote:
 
  Hey,
 
  On Wed, Aug 27, 2014 at 07:22:07PM +0200, Marc-André Lureau wrote:
   From: Marc-Andre Lureau marcandre.lur...@redhat.com
  
   GNOME will restore monitors.xml configuration whenever the timestamp
   config  change. The change timestamp is the last user applied
   configuration, whereas the config timestamp is updated when
   the screen is updated or ouput/crtc modes are added/removed.
  
   These condition are triggered by vdagent during monitor config. Since we
   can't control the timestamps (playing with delay will be inherently
   event more racy), the only sane way I can think of is to disable gsd
   behaviour. This can be achieved by deleting the ~/.config/monitors.xml,
   which is the intended configuration to restore, so vdagent will override
   whatever configuration was saved previously.
  
   Somehow, if vdagent would be better integrated with gnome2, it would use
   the gnome-rr and/or org.gnome.SettingsDaemon.XRANDR dbus
   API. Thanksfully, in gnome3, the monitor auto-configuration has been
   merged in.
 
  Actually a bit curious how this relates to
  https://bugzilla.gnome.org/show_bug.cgi?id=706735 which seems to have
  added a hack to avoid similar situations ?
 
 
 There is a similar timestamp check in gsd. However, when enabling monitors
 with vdagent, the timestamps are very close and there is a race between
 vdagent  gsd: you get random results. With the proposed patch, spice
 client = vdagent wins over gsd when it wants to set some config.

You make it sound like it's only an issue when running GNOME2. If you
tested with gnome-settings-daemon in RHEL6, the relevant (if I looked in
the right place!) code seems different from upstream because of a
downstream patch.
I also assume it will not be an issue either when
monitor configuration is not done through the agent but through
qxl/client monitor config?

If all of that is correct, and given that we don't own monitors.xml, I'd
prefer not to have this patch upstream.

Christophe


pgppG7BZsh_SU.pgp
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [vdagent-linux 1/2] randr: remove monitors.xml on auto-configuration

2014-10-08 Thread Marc-André Lureau
Hi

On Thu, Oct 2, 2014 at 2:10 PM, Christophe Fergeau cferg...@redhat.com
wrote:

 Hey,

 On Wed, Aug 27, 2014 at 07:22:07PM +0200, Marc-André Lureau wrote:
  From: Marc-Andre Lureau marcandre.lur...@redhat.com
 
  GNOME will restore monitors.xml configuration whenever the timestamp
  config  change. The change timestamp is the last user applied
  configuration, whereas the config timestamp is updated when
  the screen is updated or ouput/crtc modes are added/removed.
 
  These condition are triggered by vdagent during monitor config. Since we
  can't control the timestamps (playing with delay will be inherently
  event more racy), the only sane way I can think of is to disable gsd
  behaviour. This can be achieved by deleting the ~/.config/monitors.xml,
  which is the intended configuration to restore, so vdagent will override
  whatever configuration was saved previously.
 
  Somehow, if vdagent would be better integrated with gnome2, it would use
  the gnome-rr and/or org.gnome.SettingsDaemon.XRANDR dbus
  API. Thanksfully, in gnome3, the monitor auto-configuration has been
  merged in.

 Actually a bit curious how this relates to
 https://bugzilla.gnome.org/show_bug.cgi?id=706735 which seems to have
 added a hack to avoid similar situations ?


There is a similar timestamp check in gsd. However, when enabling monitors
with vdagent, the timestamps are very close and there is a race between
vdagent  gsd: you get random results. With the proposed patch, spice
client = vdagent wins over gsd when it wants to set some config.


-- 
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [vdagent-linux 1/2] randr: remove monitors.xml on auto-configuration

2014-10-08 Thread Marc-André Lureau
On Wed, Oct 8, 2014 at 1:11 PM, Christophe Fergeau cferg...@redhat.com
wrote:

 On Wed, Oct 08, 2014 at 12:28:53PM +0200, Marc-André Lureau wrote:
  Hi
 
  On Thu, Oct 2, 2014 at 2:10 PM, Christophe Fergeau cferg...@redhat.com
  wrote:
 
   Hey,
  
   On Wed, Aug 27, 2014 at 07:22:07PM +0200, Marc-André Lureau wrote:
From: Marc-Andre Lureau marcandre.lur...@redhat.com
   
GNOME will restore monitors.xml configuration whenever the timestamp
config  change. The change timestamp is the last user applied
configuration, whereas the config timestamp is updated when
the screen is updated or ouput/crtc modes are added/removed.
   
These condition are triggered by vdagent during monitor config.
 Since we
can't control the timestamps (playing with delay will be inherently
event more racy), the only sane way I can think of is to disable gsd
behaviour. This can be achieved by deleting the
 ~/.config/monitors.xml,
which is the intended configuration to restore, so vdagent will
 override
whatever configuration was saved previously.
   
Somehow, if vdagent would be better integrated with gnome2, it would
 use
the gnome-rr and/or org.gnome.SettingsDaemon.XRANDR dbus
API. Thanksfully, in gnome3, the monitor auto-configuration has been
merged in.
  
   Actually a bit curious how this relates to
   https://bugzilla.gnome.org/show_bug.cgi?id=706735 which seems to have
   added a hack to avoid similar situations ?
  
  
  There is a similar timestamp check in gsd. However, when enabling
 monitors
  with vdagent, the timestamps are very close and there is a race between
  vdagent  gsd: you get random results. With the proposed patch, spice
  client = vdagent wins over gsd when it wants to set some config.

 You make it sound like it's only an issue when running GNOME2. If you


I already mentionned recent gnome3 auto-config has been merged in (last
year)  and don't reach the issue (no related code).

tested with gnome-settings-daemon in RHEL6, the relevant (if I looked in
 the right place!) code seems different from upstream because of a
 downstream patch.


You are correct, showing that this part of gsd code doesn't have a clean
solution and needs various workarounds.


 I also assume it will not be an issue either when
 monitor configuration is not done through the agent but through
 qxl/client monitor config?


What do you mean? Monitor are only enabled through vdagent here.



 If all of that is correct, and given that we don't own monitors.xml, I'd
 prefer not to have this patch upstream.


This patch does not harm, we don't want gsd to race with vdagent: vdagent
should win over for auto-conf/resize to work properly.

-- 
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [vdagent-linux 1/2] randr: remove monitors.xml on auto-configuration

2014-10-08 Thread Christophe Fergeau
On Wed, Oct 08, 2014 at 02:04:38PM +0200, Marc-André Lureau wrote:
 On Wed, Oct 8, 2014 at 1:11 PM, Christophe Fergeau cferg...@redhat.com
 wrote:
 
  You make it sound like it's only an issue when running GNOME2. If you
 
 
 I already mentionned recent gnome3 auto-config has been merged in (last
 year)  and don't reach the issue (no related code).
 
 tested with gnome-settings-daemon in RHEL6, the relevant (if I looked in
  the right place!) code seems different from upstream because of a
  downstream patch.
 
 
 You are correct, showing that this part of gsd code doesn't have a clean
 solution and needs various workarounds.
 
 
  I also assume it will not be an issue either when
  monitor configuration is not done through the agent but through
  qxl/client monitor config?
 
 
 What do you mean? Monitor are only enabled through vdagent here.
 
 
 
  If all of that is correct, and given that we don't own monitors.xml, I'd
  prefer not to have this patch upstream.
 
 
 This patch does not harm, we don't want gsd to race with vdagent: vdagent
 should win over for auto-conf/resize to work properly.

gnome2 is obsolete, not using client monitor config is getting obsolete,
and removing a file which we don't own is a very heavy hammer,
especially since this file is used by mutter in GNOME3 as well, so no,
let's not do that upstream.

Christophe


pgpXUWDgHU7py.pgp
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [vdagent-linux 1/2] randr: remove monitors.xml on auto-configuration

2014-10-08 Thread Marc-André Lureau
On Wed, Oct 8, 2014 at 3:21 PM, Christophe Fergeau cferg...@redhat.com
wrote:

 gnome2 is obsolete, not using client monitor config is getting obsolete,
 and removing a file which we don't own is a very heavy hammer,


A hammer? monitors.xml is a user config, safe to delete, well-known file
name (in gnomerr API reference)

We could link with libgnome-desktop and call
gnome_rr_config_get_intended_filename() instead, I thought that was
overkill for something that is hardcoded.

This file is not hidden or protected in any way, it doesn't use a private
namespace etc.


 especially since this file is used by mutter in GNOME3 as well, so no,
 let's not do that upstream.


we don't want to restore user config when vdagent config applies.
Fortunately, with gnome3  drm driver, that code path is not triggered.

Having a random outcome is and a different output in upstream and
downstream is not better imho.

-- 
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [vdagent-linux 1/2] randr: remove monitors.xml on auto-configuration

2014-10-03 Thread Christophe Fergeau
Hey,

On Wed, Aug 27, 2014 at 07:22:07PM +0200, Marc-André Lureau wrote:
 From: Marc-Andre Lureau marcandre.lur...@redhat.com
 
 GNOME will restore monitors.xml configuration whenever the timestamp
 config  change. The change timestamp is the last user applied
 configuration, whereas the config timestamp is updated when
 the screen is updated or ouput/crtc modes are added/removed.
 
 These condition are triggered by vdagent during monitor config. Since we
 can't control the timestamps (playing with delay will be inherently
 event more racy), the only sane way I can think of is to disable gsd
 behaviour. This can be achieved by deleting the ~/.config/monitors.xml,
 which is the intended configuration to restore, so vdagent will override
 whatever configuration was saved previously.
 
 Somehow, if vdagent would be better integrated with gnome2, it would use
 the gnome-rr and/or org.gnome.SettingsDaemon.XRANDR dbus
 API. Thanksfully, in gnome3, the monitor auto-configuration has been
 merged in.

Actually a bit curious how this relates to
https://bugzilla.gnome.org/show_bug.cgi?id=706735 which seems to have
added a hack to avoid similar situations ?

Christophe


pgpVVq_v_vGtw.pgp
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


Re: [Spice-devel] [vdagent-linux 1/2] randr: remove monitors.xml on auto-configuration

2014-09-08 Thread Marc-André Lureau
ping


On Wed, Aug 27, 2014 at 7:22 PM, Marc-André Lureau 
marcandre.lur...@redhat.com wrote:

 From: Marc-Andre Lureau marcandre.lur...@redhat.com

 GNOME will restore monitors.xml configuration whenever the timestamp
 config  change. The change timestamp is the last user applied
 configuration, whereas the config timestamp is updated when
 the screen is updated or ouput/crtc modes are added/removed.

 These condition are triggered by vdagent during monitor config. Since we
 can't control the timestamps (playing with delay will be inherently
 event more racy), the only sane way I can think of is to disable gsd
 behaviour. This can be achieved by deleting the ~/.config/monitors.xml,
 which is the intended configuration to restore, so vdagent will override
 whatever configuration was saved previously.

 Somehow, if vdagent would be better integrated with gnome2, it would use
 the gnome-rr and/or org.gnome.SettingsDaemon.XRANDR dbus
 API. Thanksfully, in gnome3, the monitor auto-configuration has been
 merged in.

 https://bugzilla.redhat.com/show_bug.cgi?id=1086657
 ---
  src/vdagent-x11-randr.c | 5 +
  1 file changed, 5 insertions(+)

 diff --git a/src/vdagent-x11-randr.c b/src/vdagent-x11-randr.c
 index 46367bc..5faaee6 100644
 --- a/src/vdagent-x11-randr.c
 +++ b/src/vdagent-x11-randr.c
 @@ -21,6 +21,7 @@
  along with this program.  If not, see http://www.gnu.org/licenses/.
  */

 +#include glib.h
  #include string.h
  #include syslog.h
  #include stdlib.h
 @@ -745,6 +746,10 @@ void vdagent_x11_set_monitor_config(struct
 vdagent_x11 *x11,
  goto exit;
  }

 +gchar *config = g_build_filename (g_get_user_config_dir (),
 monitors.xml, NULL);
 +g_unlink(config);
 +g_free(config);
 +
  for (i = mon_config-num_of_monitors; i  x11-randr.res-noutput;
 i++)
  xrandr_disable_output(x11, i);

 --
 1.9.3

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




-- 
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel


[Spice-devel] [vdagent-linux 1/2] randr: remove monitors.xml on auto-configuration

2014-08-27 Thread Marc-André Lureau
From: Marc-Andre Lureau marcandre.lur...@redhat.com

GNOME will restore monitors.xml configuration whenever the timestamp
config  change. The change timestamp is the last user applied
configuration, whereas the config timestamp is updated when
the screen is updated or ouput/crtc modes are added/removed.

These condition are triggered by vdagent during monitor config. Since we
can't control the timestamps (playing with delay will be inherently
event more racy), the only sane way I can think of is to disable gsd
behaviour. This can be achieved by deleting the ~/.config/monitors.xml,
which is the intended configuration to restore, so vdagent will override
whatever configuration was saved previously.

Somehow, if vdagent would be better integrated with gnome2, it would use
the gnome-rr and/or org.gnome.SettingsDaemon.XRANDR dbus
API. Thanksfully, in gnome3, the monitor auto-configuration has been
merged in.

https://bugzilla.redhat.com/show_bug.cgi?id=1086657
---
 src/vdagent-x11-randr.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/vdagent-x11-randr.c b/src/vdagent-x11-randr.c
index 46367bc..5faaee6 100644
--- a/src/vdagent-x11-randr.c
+++ b/src/vdagent-x11-randr.c
@@ -21,6 +21,7 @@
 along with this program.  If not, see http://www.gnu.org/licenses/.
 */
 
+#include glib.h
 #include string.h
 #include syslog.h
 #include stdlib.h
@@ -745,6 +746,10 @@ void vdagent_x11_set_monitor_config(struct vdagent_x11 
*x11,
 goto exit;
 }
 
+gchar *config = g_build_filename (g_get_user_config_dir (), 
monitors.xml, NULL);
+g_unlink(config);
+g_free(config);
+
 for (i = mon_config-num_of_monitors; i  x11-randr.res-noutput; i++)
 xrandr_disable_output(x11, i);
 
-- 
1.9.3

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