GNOME.org
 

[gdm-list] Reworked patch for multi-seat and multi-display support

Hi Ray,

Thanks for your comments at
http://mail.gnome.org/archives/gdm-list/2009-May/msg00007.html

I've finished the tasks below and now the new patch is ready for review.

1 Use separate .disp file for display type, remove display-types.conf
  install .disp files to /etc/ConsoleKit/displays.d/,
  each .disp file represent a display type.
2 Only use Command in .disp file, remove Arguments/UseAuth/Chooser/Priority
3 Remove priority code
4 When adding/removing displays, CK should emit signal instead of
  calling GDM method directly
  For org.freedesktop.ConsoleKit.Seat
     add signal  SessionToAdd and SessionToRemove
     add method  ManageSeat
  For org.freedesktop.ConsoleKit.Manager
     remove method CreateStaticSessions
     add method GetUnmanagedSeats
5 Use a{sv} for dbus method CreateDisplay, since
  task #2 is done, there are only two parameters need to be send with
  signal, this work is not necessary
6 Rewrite ck-dynamic to reflect change in task #4
7 Rewrite ck_display_type_new()
8 Remove method org.freedesktop.ConsoleKit.Manager.ListCreatedSessions.
  Let ck-dynamic do the GetSeats() and filtering dynamic displays itself
9 make ck_seat_add_display(), ck_seat_set_display_type() and ck_seat_get_displ
  return a gbooolean
10 Remove unused create_display_with_parameters()
11 Comments should appear before the lines they're commenting on, not after.
12 Reverse Name and Version back
13 Figure out X11_DIR from pkg-config

The new patches links are:
* https://bugs.freedesktop.org/attachment.cgi?id=26451
  ConsoleKit part based on branch multi-seat by commit
  25145f35ab527a2219604e1710eb65e2178f6978
* http://bugzilla.gnome.org/attachment.cgi?id=135995&action=view
  GDM part based on branch display-configuration by commit
  15b783fb68e53c98d283f565eca305a3f919322d

If you need separated patches for above tasks, I can offer them one by one.

There is only one task left - represent seat and display in
configuration files. I ever asked it before, but seems you miss that
email, please see the attached email and let me know your idea.

BTW, my request for accessing CK code is pending
(https://bugs.freedesktop.org/show_bug.cgi?id=21732). It is hardy for
review and rework if I can commit the changes to the branches directly.
Could you help me do that?

References
-----------
[1] Design document (updated):
http://wiki.genunix.org/wiki/index.php/design_for_newgdm_consolekit_multiseat_multidisplay

[2] Test report:
http://wiki.genunix.org/wiki/index.php/testing_for_newgdm_consolekit_multiseat_multidisplay

[3] ConsoleKit bug (updated):
https://bugs.freedesktop.org/show_bug.cgi?id=19333

[4] GDM bug(updated):
http://bugzilla.gnome.org/show_bug.cgi?id=536355


Thanks,
Halton.
--- Begin Message --- 
Hi Ray,

When I'm trying to redesign how to represent seat and display in
configuration files based on your comments. I can not work out a new one
without understand how to change. Please see my comments in line.

For your reference, I put a two static local displays seat based on
current design. 

01-two-static.seat
(each file represent one seat)
-------------
[Seat Entry]
Name=
Version=1.0
Description=start two static local displays, one is :0 on vt7 and another one is :1 on vt8
Hidden=
DisplayType=Local
Devices=
DisplayNumbers=0;1
IsConsoles=true;true
TtyDevices=7;8

display-types.conf
(all display types are in this file)
-------------
[Local]
Command=/usr/bin/Xorg
Arguments=-br -verbose -nolisten tcp
Chooser=false
UseAuth=true
Priority=0
[...]

On Mon, 2009-05-11 at 15:23 -0400, Ray Strode wrote:
> > +DisplayNumbers=0
> > +# Display numbers list, separated by a separator character,
> typically ';'
> > +# or ','. If want to automatically choose next available display
> number,
> > +# indicate -1
> I wouldn't allow ';' and ','.  I would just pick one (and I would pick
> ';').
> The only reason gkeyfile let's you pick, is because when the icon
> theme spec
> got written no one noticed that desktop files used ; instead of , so
> it ended
> up getting the wrong one (and when gkeyfile got written we wanted it
> to work
> for the icon theme files too)
> 
> So if I want to configure my system to start two displays at boot up,
> with
> any display number, I don't care, is it:
> 
> DisplayNumbers=-1;-1;
> 
> ?  That's a little clunky.
Yes, same question with Brian. How do you think it should look like?
> 
> > +
> > +TtyDevices=
> > +# Values list to indicate the tty device number, separated by a
> separator
> > +# character, typically ';' or ','.
> > +# Zero or negative number or no value means choose next available
> vt number
> So TtyDevices and DisplayNumbers have to always be in sync?  This
> seems very fragile to changes.
> 
> I think we need a separate [Display Entry] that gets tied to a
> particular seat. Then in the .seat file we'd have
> 
> Displays=localdisplay1;localdisplay2;
> 
> or some such.  The display entry file would have what display number
> to use and what tty to use
> (and the absense of those keys would mean "figure it out
> automatically")
I think you're talking about how to represent {$display,$vt,
$is_console}. Are you suggesting put {$display,$vt,$is_console} in
another configuration file? How to?

> 
> DisplayType could get moved to the display entry, too.  Or maybe we
> requires all displays at one seat to have the same type, not sure.
> 
> > +
> > +IsConsoles=true
> > +# Values list to indicate whether the give DisplayNumbers are on
> console
> > +# or not , separated by a separator character, typically ';' or
> ','.
> That shouldn't be plural.  Maybe "RequiresConsole" would be better?
> If we end up with Displays= above, then we'll need to make sure that
> all the display types in the list
> match in the "RequiresConsole" respect.
> 
> > diff --git a/data/display-types.conf.in b/data/display-types.conf.in
> > new file mode 100644
> > index 0000000..4c4246d
> > --- /dev/null
> > +++ b/data/display-types.conf.in
> > @@ -0,0 +1,27 @@
> > +[Local]
> > +Command= X11_DIR@/Xorg
> > +Arguments=-br -verbose -nolisten tcp
> > +Chooser=false
> > +UseAuth=true
> > +Priority=
> Why is Command and Arguments two separate things?  I'd do this as
> 
> Command= X11_DIR@/Xorg $display -br -verbose -nolisten tcp $vt
> 
> Then $display and $vt would get substituted with whatever display is
> specified in the seat display configuration.
I do not think we need specify $display and $vt here, it is more
complex.

ConsoleKit need parse the Command sting and replace sting start from
'$'. What if there is typo like $disp1ay.

Under current design, when ConsoleKit get DisplayNumber and TtyDevice
from .seat file, it will pass them to GDM.

As for Arguments, my purpose is to clearly tell user he could specify
more argument with that key. Combine them to one string is also okay. 

Either way is okay, I do not have strong point to prefer which one. But
current implementation is based on Command/Arguments way, can we keep on
use that because it is working?

> 
> > +
> > +[RemoteMachine]
> > +Command= X11_DIR@/Xorg
> > +Arguments=-br -verbose -indirect
> > +Chooser=true
> > +UseAuth=true
> > +Priority=
> > +
> > +[LocalVNC]
> > +Command= X11_DIR@/Xvnc
> > +Arguments=-query localhost
> > +Chooser=false
> > +UseAuth=true
> > +Priority=
> > +
> > +[Headless]
> > +Command= X11_DIR@/Xvfb
> > +Chooser=false
> > +Arguments=
> > +UseAuth=true
> > +Priority=
> 
> I'm a little worried that these guys are all getting stuffed into one
> file.
> Who "owns" the file, the distribution or the sysadmin?  If the distro
> decides
> to change which command line arguments are passed to the X server is
> it going
> to have to tell the sysadmin to update the file?

This file is original shipped with distros, sysadmin could change the
content as wish. When re-installation, this file should not be updated
without permission of sysadm. I think most package manager do same thing
with configuration files under /etc.

I do can not say which way is better. Again, if you insist on one file
per display_type, I can do that change.
> 
> If you did each entry as a separate file, like you did for seat
> configuration,
> this will be a lot more managable.  E.g.,
> 
> [Display Type]
> Name=Local
> Description=Configuration to use for local displays
> Command=Xorg $display -br -verbose -nolisten tcp $vt
> Chooser=false
> UseAuth=true

It is good that you could give out the example you think is right for
the above two local display case. That will help me understand better.
Could you do that? 

Thanks,
Halton.

_______________________________________________
gdm-list mailing list
gdm-list gnome org
http://mail.gnome.org/mailman/listinfo/gdm-list

--- End Message ---
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]