Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move default location of sockets to /run #888

Closed
totaam opened this issue Jun 12, 2015 · 82 comments
Closed

Move default location of sockets to /run #888

totaam opened this issue Jun 12, 2015 · 82 comments

Comments

@totaam
Copy link
Collaborator

totaam commented Jun 12, 2015

Issue migrated from trac ticket # 888

component: core | priority: minor | resolution: fixed

2015-06-12 13:54:34: jonathan.underwood created the issue


Presently, xpra by default creates its sockets in the user home directory, which can create problems with NFS mounted homes etc. The FHS stipulates/recommends sockets be placed under /run[1]. This would seem to be a much more sensible and compliant default. I can't immediately see any drawback. What do you think?

[1] http://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch03s15.html where it says "System programs that maintain transient UNIX-domain sockets must place them in this directory or an appropriate subdirectory as outlined above."

@totaam
Copy link
Collaborator Author

totaam commented Jun 12, 2015

2015-06-12 14:03:08: antoine changed priority from major to minor

@totaam
Copy link
Collaborator Author

totaam commented Jun 12, 2015

2015-06-12 14:03:08: antoine changed type from defect to enhancement

@totaam
Copy link
Collaborator Author

totaam commented Jun 12, 2015

2015-06-12 14:03:08: antoine changed status from new to assigned

@totaam
Copy link
Collaborator Author

totaam commented Jun 12, 2015

2015-06-12 14:03:08: antoine changed component from android to core

@totaam
Copy link
Collaborator Author

totaam commented Jun 12, 2015

2015-06-12 14:03:08: antoine commented


This was suggested before, there is one HUGE drawback: incompatibility with older versions, unless we start looking in both places which is messy.
And here is another BIG one: this won't work on OSX, and probably some other BSD platforms.

@totaam
Copy link
Collaborator Author

totaam commented Jun 12, 2015

2015-06-12 14:12:45: jonathan.underwood commented


Well, couldn't we try to create the socket under /run if on linux and /run is available, otherwise fallback to creating under $HOME ?

I'd argue that stuff breaking on NFS is also very messy - personally the messiness of looking in both locations for sockets seems the lesser of two evils.

I'm happy to look into generating a patch for this, but would rather not put that work in if you wouldn't accept such a patch.

@totaam
Copy link
Collaborator Author

totaam commented Jun 12, 2015

2015-06-12 14:16:01: antoine changed status from assigned to new

@totaam
Copy link
Collaborator Author

totaam commented Jun 12, 2015

2015-06-12 14:16:01: antoine changed owner from antoine to jonathan.underwood

@totaam
Copy link
Collaborator Author

totaam commented Jun 12, 2015

2015-06-12 14:16:01: antoine commented


Oh, don't get me wrong: I would definitely gladly accept the patch! And we probably want to deal with this anyway as part of the osx file location changes for #641 (see #641#comment:7)

I'm just wary of changes that can require fixes after fixes... Hopefully this won't be the case (unlike #172#comment:38 onwards say..)

@totaam
Copy link
Collaborator Author

totaam commented Jun 29, 2015

2015-06-29 07:50:47: antoine commented


Also:

  • /var/run/user/$UID/xpra vs /var/run/xpra...
    The first one makes sense by default (just have to make sure $UID is substituted after we connect via SSH and not before), but the second one can be useful when sharing sessions.. so I will probably add both?
    It looks like centos 6+ supports tmpfiles.d, so we can ship a file to create /var/run/xpra.
    It might make sense to also add an xpra group to facilitate mmap-group shared session access at the same time.
  • this will need updating:
        group.add_option("--log-file", action="store",
                      dest="log_file", default=defaults.log_file,
                      help="When daemonizing, this is where the log messages will go. Default: '%default'."
                      + " If a relative filename is specified the it is relative to --socket-dir,"
                      + " the value of '$DISPLAY' will be substituted with the actual display used"
                      )

I think it makes sense to add log-dir and sockets-dir and keep the current default socket-dir where it is. For the next LTS release, we can then change the default socket-dir to /var/run/user/$UID/xpra (or /var/run/xpra - whatever), this will break older clients connecting to that new server unless they use socket-dir=/var/run/user/$UID/xpra...

Shouldn't the run-xpra script also be moved somewhere else? .local/bin?
(PITA, this is going to take some time as the path is hardcoded in the ssh command line...)

@totaam
Copy link
Collaborator Author

totaam commented Jun 29, 2015

2015-06-29 07:52:05: antoine uploaded file move-socket.patch (26.6 KiB)

work in progress patch: deals with a bit more than just sockets

@totaam
Copy link
Collaborator Author

totaam commented Jun 29, 2015

2015-06-29 09:52:14: jonathan.underwood commented


Hi Antoine,

I think /var/run/user/$UID/xpra is strongly preferred over /var/run/xpra as the former automatically gives you access control - it would prevent users connecting to other users xpra sessions.

One thing I have been trying to figure out though with regard to /var/run/user/$UID/xpra, which is automatically deleted (by systemd) when all user sessions end, is whether xpra registers as a user session or not. Or indeed, whether it's necessary.

@totaam
Copy link
Collaborator Author

totaam commented Jun 30, 2015

2015-06-30 03:59:22: antoine commented


it would prevent users connecting to other users xpra sessions.
[[BR]]
That's why I mentioned it with the mmap group option, which is used for sharing sessions.

[[BR]]

which is automatically deleted (by systemd) when all user sessions end, is whether xpra registers as a user session or not
[[BR]]
I fear the added complexity here, and the difficulties in packaging and supporting all distros.

@totaam
Copy link
Collaborator Author

totaam commented Jun 30, 2015

2015-06-30 08:07:24: jonathan.underwood commented


Replying to [comment:6 antoine]:

it would prevent users connecting to other users xpra sessions.
[[BR]]
That's why I mentioned it with the mmap group option, which is used for sharing sessions.

Ah, yes, sorry, I should read more carefully.

[[BR]]

which is automatically deleted (by systemd) when all user sessions end, is whether xpra registers as a user session or not
[[BR]]
I fear the added complexity here, and the difficulties in packaging and supporting all distros.

Well I may well be misunderstanding things wrt sessions and deletion of that directory. Unfortunately it's not very well documented what the expectations are of /run/user/$PID. So, please don't be discourgaed by what I wrote above.

Also, as a sidenote, I think we should be considering /run/user/$PID rather than /var/run/user/$PID, no?

@totaam
Copy link
Collaborator Author

totaam commented Jun 30, 2015

2015-06-30 08:13:12: antoine commented


I think we should be considering /run/user/$PID rather than /var/run/user/$PID, no?
[[BR]]
Yes, that's what I've used in the work in progress.

[[BR]]

it's not very well documented what the expectations are of /run/user/$PID
[[BR]]
/run/user/$UID I think ;)

@totaam
Copy link
Collaborator Author

totaam commented Jun 30, 2015

2015-06-30 08:20:14: jonathan.underwood commented


Replying to [comment:8 antoine]:

I think we should be considering /run/user/$PID rather than /var/run/user/$PID, no?
[[BR]]
Yes, that's what I've used in the work in progress.

Ah good. I expect to have time to try your patch at the weekend.

[[BR]]

it's not very well documented what the expectations are of /run/user/$PID
[[BR]]
/run/user/$UID I think ;)

Too early, not enough coffee :).

@totaam
Copy link
Collaborator Author

totaam commented Jul 1, 2015

2015-07-01 18:23:59: antoine uploaded file move-socket-v2.patch (31.2 KiB)

updated patch, almost usable

@totaam
Copy link
Collaborator Author

totaam commented Jul 1, 2015

2015-07-01 18:27:24: antoine changed status from new to assigned

@totaam
Copy link
Collaborator Author

totaam commented Jul 1, 2015

2015-07-01 18:27:24: antoine changed owner from jonathan.underwood to antoine

@totaam
Copy link
Collaborator Author

totaam commented Jul 1, 2015

2015-07-01 18:27:24: antoine commented


Still lots of work to do: the code is ugly, needs more tidying up, then later: man page updates, wiki updates, etc..

I would also like to get the basic win32 named pipe server + client code going, as this may overlap.

In the process of changing this code, I also discovered a big omission which was trivial to implement:

xpra attach socket:/path/to/the/socket

Why have we never supported this until now? It will make integration with things like [/wiki/Usage/Docker docker] much easier.

@totaam
Copy link
Collaborator Author

totaam commented Jul 2, 2015

2015-07-02 07:35:39: antoine uploaded file move-socket-v3.patch (54.2 KiB)

latest patch

@totaam
Copy link
Collaborator Author

totaam commented Jul 2, 2015

2015-07-02 12:33:31: antoine uploaded file move-socket-v4.patch (74.3 KiB)

more cleanups

@totaam
Copy link
Collaborator Author

totaam commented Jul 2, 2015

2015-07-02 13:38:31: antoine commented


Mostly done in this massive changeset r9802 (+fixup in r9812), see commit message for details.

Still TODO / verify:

  • authentication modules were modified, re-test them
  • creating all the directories correctly?
  • tmpfiles.d and packaging
  • longer term, changing the defaults so that ~/.xpra is no longer the first entry in the socket-dirs list (the value which gets used unless one specifies socket-dir)
  • I think having an xpra showconfig subcommand to dump the config options would be useful as there are just too many sources for the settings: runtime probing, application default config, system defaults, per user defaults - and some of those files can have multiple locations!
  • osx packaging (OSX .pkg creation scrips #641) and the win32 named pipe code (ms windows shadow server improvements #389) may require further changes - best to deal with those in the same release

Things to test (on Linux - win32 and osx are much less important to test right now):

  • dead sockets still get cleaned up:
$ xpra start --socket-dir=/tmp
$ xpra list
Found the following xpra sessions:
/home/antoine/.xpra:
	LIVE session at :11
$ killall -9 xpra
$ xpra list
Found the following xpra sessions:
/home/antoine/.xpra:
	UNKNOWN session at :11
Re-probing unknown sessions: {'/home/antoine/.xpra': ['/home/antoine/.xpra/desktop-11']}
	DEAD session at :11 (cleaned up)
  • specifying socket-dir only searches / uses the specified dir:
$ xpra start :10 --socket-dir=/tmp
$ xpra start :11 --socket-dir=~/.xpra
$ xpra list
Found the following xpra sessions:
/home/antoine/.xpra:
	LIVE session at :10
$ xpra list --socket-dir=/tmp
Found the following xpra sessions:
/tmp:
	LIVE session at :11
  • sockets can be found if the dir they are placed in is in socket-dirs list, and we can specify it in two different ways (: separated string or multiple arguments):
$ xpra start :10 --socket-dir=/tmp
$ xpra list --socket-dirs=/var/tmp:/tmp
Found the following xpra sessions:
/tmp:
	LIVE session at :10
$ xpra list --socket-dirs=/var/tmp --socket-dirs=/tmp
Found the following xpra sessions:
/tmp:
	LIVE session at :10
  • ssh connections:
  • we should end up passing socket-dir to the remote end, so it will use whatever is set in the config for socket-dirs:
$ xpra start :10 --socket-dir=/tmp
$ xpra version ssh:localhost:10 --socket-dir=/tmp
0.16.0
  • we do not pass socket-dirs:
$ xpra start :10 --socket-dir=/tmp
$ xpra version ssh:localhost:10 --socket-dirs=/tmp
xpra initialization error: cannot find any live servers to connect to

@totaam
Copy link
Collaborator Author

totaam commented Jul 2, 2015

2015-07-02 16:06:54: antoine commented


r9809 also adds the subcommand: xpra showconfig, which is useful for verifying the options that actually get used when there are so many places we can get them from!

@totaam
Copy link
Collaborator Author

totaam commented Jul 6, 2015

2015-07-06 06:39:18: antoine changed status from assigned to new

@totaam
Copy link
Collaborator Author

totaam commented Jul 6, 2015

2015-07-06 06:39:18: antoine changed owner from antoine to jonathan.underwood

@totaam
Copy link
Collaborator Author

totaam commented Jul 6, 2015

2015-07-06 06:39:18: antoine commented


As of r9840, we use /var/run instead of /run by default, that's because it is more backwards compatible.
For reference, here's what I found from a common set of distros:

CentOS5.x/var/run
CentOS6.x/var/run
CentOS7.x/var/run -> ../run
fedora-21/var/run
fedora-22/var/run
fedora-rawhide/var/run
jessie/var/run
precise/var/run -> /run
stretch/var/run
trusty/var/run -> /run
utopic/var/run -> /run
vivid/var/run -> /run
wheezy/var/run

And:

CentOS7.x/run
fedora-21/run
fedora-22/run
fedora-rawhide/run
jessie/run -> /var/run
precise/run
stretch/run -> /var/run
trusty/run
utopic/run
vivid/run
wheezy/run -> /var/run

So centos 5.x and 6.x don't have /run at all, as for the others.. it depends, some link back to it, others from it.

The xpra server log and Xorg log files, where should they go? (taking into account the fact that some sessions can be shared if this matters at all)

If /var/run/user/$UID/xpra does not exist, I believe we create it with 0700 permissions.
/var/run/xpra should be created by the tmpfiles.d hooks added in r9842 (as 0770, group "xpra"): we ship the config file, install it via setup.py and package it for both rpm and debs. (whether it gets used is another matter)
The specfile also adds an "xpra" group.

Tested with:

$ sudo systemd-tmpfiles --create /usr/lib/tmpfiles.d/xpra.conf 
$ sudo ls -ald /var/run/xpra/
drwxrwx---. 2 root xpra 40 Jul  6 12:34 /var/run/xpra/

I think this will do for this release?
@jonathan.underwood: does that work for you? If you cannot test, please re-assign to @afarr so he is aware of the change.

@totaam
Copy link
Collaborator Author

totaam commented Jul 6, 2015

2015-07-06 11:37:49: jonathan.underwood commented


I have 9845 built on Fedora 22 and am commencing testing now. But, presently I am afraid that's the only platform I have available to test on.

@totaam
Copy link
Collaborator Author

totaam commented Jul 6, 2015

2015-07-06 11:42:07: jonathan.underwood commented


First issue (starting withouyt $HOME/.xpra present):

$ xpra start :101 --start-child=xterm
xpra main error:
Traceback (most recent call last):
  File "/usr/lib64/python2.7/site-packages/xpra/scripts/main.py", line 103, in main
    return run_mode(script_file, err, options, args, mode, defaults)
  File "/usr/lib64/python2.7/site-packages/xpra/scripts/main.py", line 840, in run_mode
    return run_server(error_cb, options, mode, script_file, args)
  File "/usr/lib64/python2.7/site-packages/xpra/scripts/server.py", line 806, in run_server
    logfd = open_log_file(log_filename0)
  File "/usr/lib64/python2.7/site-packages/xpra/scripts/server.py", line 442, in open_log_file
    return os.open(logpath, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o666)
OSError: [Errno 2] No such file or directory: '~/.xpra/:101.log'

@totaam
Copy link
Collaborator Author

totaam commented Jul 6, 2015

2015-07-06 12:19:34: antoine commented


@jonathan.underwood: thanks, that's fixed in r9848, I'm still not sure that "~/.xpra" is the right place for log files (our own, and the Xvfb) - but this will do for now I guess.

@totaam
Copy link
Collaborator Author

totaam commented Jul 7, 2015

2015-07-07 14:10:58: jonathan.underwood commented


Attaching GDB just shows the process exited with return code 1, but there's no backtrace. I am thinking it was killed during gnome logout as part of the session management. While investigating this further I noticed the following for a running xpra:

$ ps  xao pid,ppid,pgid,sid,comm

...
 3179     1  3178  3178 xpra
 3183  3179  3183  3183 Xvfb
 3225  3179  3178  3178 xterm
...

which struck me as strange - should the Xvfb process not have ppid,pgid,sid=3179,3178,3178 ?

Aside, other applications in the gnome session have ppid=1401, pgid=1359, sid=1359, so at least we can see the xpra related processes have different ppid,pgid,sid which I think is correct if we don't want the session manager to kill xpra on logout. However, I haven't yet worked out why, nonetheless, xpra is still being killed on logout :).

@totaam
Copy link
Collaborator Author

totaam commented Jul 7, 2015

2015-07-07 14:25:22: jonathan.underwood commented


I guess it's actually correct, now I think of it:

 3179 ?        Sl     0:00 /usr/bin/python /usr/bin/xpra start :105 --start-child=xterm
 3183 ?        Ssl    0:00  \_ Xvfb-for-Xpra-:105 +extension Composite -screen 0 5760x2560x24+32 -nolisten tcp -noreset -auth /run/user/1000/gdm/Xauthority :105
 3225 ?        S      0:00  \_ xterm
 3228 pts/1    Ss+    0:00      \_ bash

@totaam
Copy link
Collaborator Author

totaam commented Jul 7, 2015

2015-07-07 14:35:30: antoine commented


You may have more luck with Xorg? (I know some distros do funny things with Xvfb and session management - or maybe it's part of the Xvfb code even)

You could also run the server with -d all to see why it decides to shutdown. If the Xvfb is killed, it should exit with an error message.

@totaam
Copy link
Collaborator Author

totaam commented Jul 7, 2015

2015-07-07 14:37:48: jonathan.underwood commented


Right, but the XVfb server remains, it's only the xpra process and decendents which are killed.

I'll try with Xdummy/Xorg and see what happens.

@totaam
Copy link
Collaborator Author

totaam commented Jul 7, 2015

2015-07-07 15:02:30: jonathan.underwood commented


Hmmmm. Another small issue. It used to be the case that you could specify --with-Xdummy --with-Xdummy_wrapper when running setup.py build to force selection of the Xdummy wrapper rather than Xvfb without having the Xorg detection stuff kick in. Subsequently, running a python setup.py --skip-build install (without --with-Xdummy --with-Xdummy_wrapper) would install an xpra.conf file with used Xdummy.

Currently, unless --with-Xdummy --with-Xdummy_wrapper is specified both with build and install, the Xorg detection stuff kicks in during install, and a xpra.conf using Xvfb is installed.

Logically, I expect xpra.conf to be created during build, and whatever is present during install is just installed if we're using --skip-build. I'm not sure when this crept in.

@totaam
Copy link
Collaborator Author

totaam commented Jul 7, 2015

2015-07-07 15:11:12: jonathan.underwood commented


Anyway, I can confirm that xpra is killed on gnome logout for both Xvfb and Xorg/dummy. In both cases the Xvfb/Xorg process remains.

@totaam
Copy link
Collaborator Author

totaam commented Jul 7, 2015

2015-07-07 15:25:27: antoine commented


The Xdummy issues would have to go in separate ticket, I'm not sure I'll get around to it anytime soon - the distutils overrides are a mess.

If xpra is killed but the vfb remains, then it is probably killed forcibly - as in, not by sending a SIGINT, but a real crash. Anything in the server log at all with -d all?

@totaam
Copy link
Collaborator Author

totaam commented Jul 7, 2015

2015-07-07 15:30:32: jonathan.underwood commented


Replying to [comment:47 antoine]:

The Xdummy issues would have to go in separate ticket, I'm not sure I'll get around to it anytime soon - the distutils overrides are a mess.

OK, ticket #908 filed.

@totaam
Copy link
Collaborator Author

totaam commented Jul 7, 2015

2015-07-07 15:40:47: jonathan.underwood commented


Replying to [comment:47 antoine]:

If xpra is killed but the vfb remains, then it is probably killed forcibly - as in, not by sending a SIGINT, but a real crash. Anything in the server log at all with -d all?

Nothing at all in the server log even with -d all.

I am pretty sure this is because systemd-logind is killing all processes in the cgroup associated with the gnome session[1] upon logout. It seems we somehow need to create a new session for the xpra process. Quite why logind isn't also killing the Xvfb/Xorg is another question. Clearly my understanding isn't great here. :)

[1] https://dvdhrm.wordpress.com/2013/08/24/session-management-on-linux/

@totaam
Copy link
Collaborator Author

totaam commented Jul 7, 2015

2015-07-07 15:43:41: jonathan.underwood commented


As an aside, I notice that if I use xpra list after the xpra session is killed, if I am using Xvfb the lock file in /tmp/ is successfuly removed by the xpra list command cleaning up process. But if Xorg/dummy is used, the lock file remains. In both cases the Xvfb/Xorg process remains. I am not sure which behaviour is the correct one regarding the lock file, but it seems to be inconsistent presently.

@totaam
Copy link
Collaborator Author

totaam commented Jul 21, 2015

2015-07-21 05:45:30: totaam commented


I don't think the logind stuff belongs in here, can we close this? (and by that I mean, re-assign to 'afarr')

@totaam
Copy link
Collaborator Author

totaam commented Jul 27, 2015

2015-07-27 13:46:07: jonathan.underwood commented


Yes, agreed - the xpra being killed on logout issue is separate.

@totaam
Copy link
Collaborator Author

totaam commented Jul 27, 2015

2015-07-27 13:48:29: antoine changed owner from jonathan.underwood to afarr

@totaam
Copy link
Collaborator Author

totaam commented Jul 27, 2015

2015-07-27 13:48:29: antoine commented


@afarr: please close as an ACK. I don't think you will be particularly interested in this change for your environment, but worth knowing about - and feel free to test.

@totaam
Copy link
Collaborator Author

totaam commented Aug 5, 2015

2015-08-05 01:02:38: afarr changed status from new to closed

@totaam
Copy link
Collaborator Author

totaam commented Aug 5, 2015

2015-08-05 01:02:38: afarr set resolution to fixed

@totaam
Copy link
Collaborator Author

totaam commented Aug 5, 2015

2015-08-05 01:02:38: afarr commented


Ok... I'm not sure what the usefulness of moving the default socket location to /run is (though, I do see errors with permissions when I try to set socket-dir = /var/run/xpra in my xpra.conf with the 0.15.4 server).

Doesn't sound like it's something of much use in our environment, but with 0.16.0 r10216 fedora 21 server, when I set socket-dir = /run/user/1000/xpra in the xpra.conf everything behaves well (even with a 0.15.4 10055 osx client) and xpra list is also giving me the expected:

Found the following xpra sessions:
/run/user/1000/xpra:
	LIVE session at :13

Seems to be working as expected, and maybe we'll find a use for it.

Closing.

@totaam totaam closed this as completed Aug 5, 2015
@totaam
Copy link
Collaborator Author

totaam commented Aug 13, 2015

2015-08-13 11:06:18: antoine commented


(fixup in r10288)

@totaam
Copy link
Collaborator Author

totaam commented Aug 24, 2015

2015-08-24 13:00:36: antoine commented


Follow up: #963

@totaam
Copy link
Collaborator Author

totaam commented Dec 26, 2015

2015-12-26 17:39:32: antoine commented


Please see #963#comment:1

@totaam
Copy link
Collaborator Author

totaam commented Dec 29, 2015

2015-12-29 10:07:21: antoine commented


See also #1066.

@totaam
Copy link
Collaborator Author

totaam commented Oct 9, 2016

2016-10-09 09:13:51: antoine commented


As of r14071 the default setting for "log-dir" is "auto" so we try to use "/var/run/user/$UID/" and fallback to "~/.xpra/" or even "/tmp".

@totaam
Copy link
Collaborator Author

totaam commented Oct 9, 2016

2016-10-09 11:40:49: antoine commented


As of r14078, failures to create the sockets in "$XDG_RUNTIME_DIR/xpra" are no longer fatal, so we add this directory to the default list of socket-dirs on all distributions with a "/var/run/user" directory. And when this fails, we still have the other default locations:

  • "~/.xpra" backwards compatible default
  • "/var/run/xpra" for group sharing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant