| Message ID | 1294352941-16508-2-git-send-email-ajax@redhat.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
diff --git a/dix/globals.c b/dix/globals.c index 0a6b170..0ee83c0 100644 --- a/dix/globals.c +++ b/dix/globals.c @@ -126,6 +126,7 @@ int defaultColorVisualClass = -1; int monitorResolution = 0; char *display; +int displayfd; char *ConnectionInfo; CARD32 TimeOutValue = DEFAULT_TIMEOUT * MILLI_PER_SECOND; diff --git a/doc/man/Xserver.man b/doc/man/Xserver.man index b725949..5e599be 100644 --- a/doc/man/Xserver.man +++ b/doc/man/Xserver.man @@ -121,6 +121,13 @@ Not obeyed by all servers. .B \-core causes the server to generate a core dump on fatal errors. .TP 8 +.B \-displayfd \fIfd\fP +specifies a file descriptor in the launching process. Rather than specify +a display number, the X server will attempt to listen on successively higher +display numbers, and upon finding a free one, will write the port number back +on this file descriptor as a newline-terminated string. The \-pn option is +ignored when using \-displayfd. +.TP 8 .B \-deferglyphs \fIwhichfonts\fP specifies the types of fonts for which the server should attempt to use deferred glyph loading. \fIwhichfonts\fP can be all (all fonts), diff --git a/include/opaque.h b/include/opaque.h index 5c70717..f23203f 100644 --- a/include/opaque.h +++ b/include/opaque.h @@ -50,6 +50,7 @@ extern _X_EXPORT int ScreenSaverAllowExposures; extern _X_EXPORT int defaultScreenSaverBlanking; extern _X_EXPORT int defaultScreenSaverAllowExposures; extern _X_EXPORT char *display; +extern _X_EXPORT int displayfd; extern _X_EXPORT int defaultBackingStore; extern _X_EXPORT Bool disableBackingStore; diff --git a/os/connection.c b/os/connection.c index 5580fab..8fb8c36 100644 --- a/os/connection.c +++ b/os/connection.c @@ -145,6 +145,7 @@ Bool AnyClientsWriteBlocked; /* true if some client blocked on write */ static Bool RunFromSmartParent; /* send SIGUSR1 to parent process */ Bool RunFromSigStopParent; /* send SIGSTOP to our own process; Upstart (or equivalent) will send SIGCONT back. */ +static char dynamic_display[7]; Bool PartialNetwork; /* continue even if unable to bind all addrs */ static Pid_t ParentProcess; @@ -356,11 +357,25 @@ NotifyParentProcess(void) kill (ParentProcess, SIGUSR1); } } + if (dynamic_display[0]) + write(displayfd, dynamic_display, strlen(dynamic_display)); if (RunFromSigStopParent) raise (SIGSTOP); #endif } +static Bool +TryCreateSocket(int num, int *partial) +{ + char port[20]; + + sprintf(port, "%d", num); + + return _XSERVTransMakeAllCOTSServerListeners(port, partial, + &ListenTransCount, + &ListenTransConns); +} + /***************** * CreateWellKnownSockets * At initialization, create the sockets to listen on for new clients. @@ -371,7 +386,6 @@ CreateWellKnownSockets(void) { int i; int partial; - char port[20]; FD_ZERO(&AllSockets); FD_ZERO(&AllClients); @@ -386,32 +400,44 @@ CreateWellKnownSockets(void) FD_ZERO (&WellKnownConnections); - sprintf (port, "%d", atoi (display)); - - if ((_XSERVTransMakeAllCOTSServerListeners (port, &partial, - &ListenTransCount, &ListenTransConns) >= 0) && - (ListenTransCount >= 1)) + if (display) { - if (!PartialNetwork && partial) - { - FatalError ("Failed to establish all listening sockets"); - } - else + if (TryCreateSocket(atoi(display), &partial) && + (ListenTransCount >= 1)) + if (!PartialNetwork && partial) + FatalError ("Failed to establish all listening sockets"); + } + else /* -displayfd */ + { + Bool found = 0; + for (i = 0; i < 65535 - 1024; i++) { - ListenTransFds = malloc(ListenTransCount * sizeof (int)); - - for (i = 0; i < ListenTransCount; i++) + if (!TryCreateSocket(i, &partial) && !partial) { - int fd = _XSERVTransGetConnectionNumber (ListenTransConns[i]); - - ListenTransFds[i] = fd; - FD_SET (fd, &WellKnownConnections); - - if (!_XSERVTransIsLocal (ListenTransConns[i])) - { - DefineSelf (fd); - } + found = 1; + break; } + else + CloseWellKnownConnections(); + } + if (!found) + FatalError("Failed to find a socket to listen on"); + sprintf(dynamic_display, "%d\n", i); + display = dynamic_display; + } + + ListenTransFds = malloc (ListenTransCount * sizeof (int)); + + for (i = 0; i < ListenTransCount; i++) + { + int fd = _XSERVTransGetConnectionNumber (ListenTransConns[i]); + + ListenTransFds[i] = fd; + FD_SET (fd, &WellKnownConnections); + + if (!_XSERVTransIsLocal (ListenTransConns[i])) + { + DefineSelf (fd); } } diff --git a/os/utils.c b/os/utils.c index 18fd911..eb2fafe 100644 --- a/os/utils.c +++ b/os/utils.c @@ -671,6 +671,17 @@ ProcessCommandLine(int argc, char *argv[]) else UseMsg(); } + else if (strcmp(argv[i], "-displayfd") == 0) + { + if (++i < argc) + { + displayfd = atoi(argv[i]); + display = NULL; + nolock = TRUE; + } + else + UseMsg(); + } #ifdef DPMSExtension else if ( strcmp( argv[i], "dpms") == 0) /* ignored for compatibility */ ;
On 01/ 6/11 02:28 PM, Adam Jackson wrote: > This option specifies a file descriptor in the launching process. X > will scan for an available display number and write that number back to > the launching process, at the same time as SIGUSR1 generation. This > means display managers don't need to guess at available display numbers. > As a consequence, if X fails to start when using -displayfd, it's not > because the display was in use, so there's no point in retrying the X > launch on a higher display number. Seems like a good idea. In a similar vein, so does this if anyone wants to code it up: https://bugs.freedesktop.org/show_bug.cgi?id=32479 Please add a -minvt option > @@ -356,11 +357,25 @@ NotifyParentProcess(void) > kill (ParentProcess, SIGUSR1); > } > } > + if (dynamic_display[0]) > + write(displayfd, dynamic_display, strlen(dynamic_display)); Should the fd be closed after the write()? Seems like it's left open, unused for the life of the process, including being passed to forked children (xkbcomp).
On Thu, Jan 6, 2011 at 17:28:59 -0500, Adam Jackson wrote: > diff --git a/os/connection.c b/os/connection.c > index 5580fab..8fb8c36 100644 > --- a/os/connection.c > +++ b/os/connection.c > @@ -145,6 +145,7 @@ Bool AnyClientsWriteBlocked; /* true if some client blocked on write */ > static Bool RunFromSmartParent; /* send SIGUSR1 to parent process */ > Bool RunFromSigStopParent; /* send SIGSTOP to our own process; Upstart (or > equivalent) will send SIGCONT back. */ > +static char dynamic_display[7]; > Bool PartialNetwork; /* continue even if unable to bind all addrs */ > static Pid_t ParentProcess; > > @@ -356,11 +357,25 @@ NotifyParentProcess(void) > kill (ParentProcess, SIGUSR1); > } > } > + if (dynamic_display[0]) > + write(displayfd, dynamic_display, strlen(dynamic_display)); > if (RunFromSigStopParent) > raise (SIGSTOP); > #endif > } > > +static Bool > +TryCreateSocket(int num, int *partial) > +{ > + char port[20]; > + > + sprintf(port, "%d", num); > + > + return _XSERVTransMakeAllCOTSServerListeners(port, partial, > + &ListenTransCount, > + &ListenTransConns); Cast from int to Bool, _XSERVTransMakeAllCOTSServerListeners seems to return 0 on success, I guess this should return _XSERVTransMakeAllCOTSServerListeners() >= 0? > +} > + > /***************** > * CreateWellKnownSockets > * At initialization, create the sockets to listen on for new clients. > @@ -371,7 +386,6 @@ CreateWellKnownSockets(void) > { > int i; > int partial; > - char port[20]; > > FD_ZERO(&AllSockets); > FD_ZERO(&AllClients); > @@ -386,32 +400,44 @@ CreateWellKnownSockets(void) > > FD_ZERO (&WellKnownConnections); > > - sprintf (port, "%d", atoi (display)); > - > - if ((_XSERVTransMakeAllCOTSServerListeners (port, &partial, > - &ListenTransCount, &ListenTransConns) >= 0) && > - (ListenTransCount >= 1)) > + if (display) > { > - if (!PartialNetwork && partial) > - { > - FatalError ("Failed to establish all listening sockets"); > - } > - else > + if (TryCreateSocket(atoi(display), &partial) && > + (ListenTransCount >= 1)) > + if (!PartialNetwork && partial) > + FatalError ("Failed to establish all listening sockets"); > + } > + else /* -displayfd */ > + { > + Bool found = 0; > + for (i = 0; i < 65535 - 1024; i++) Should this be 65535 - 6000? > { > - ListenTransFds = malloc(ListenTransCount * sizeof (int)); > - > - for (i = 0; i < ListenTransCount; i++) > + if (!TryCreateSocket(i, &partial) && !partial) > { > - int fd = _XSERVTransGetConnectionNumber (ListenTransConns[i]); > - > - ListenTransFds[i] = fd; > - FD_SET (fd, &WellKnownConnections); > - > - if (!_XSERVTransIsLocal (ListenTransConns[i])) > - { > - DefineSelf (fd); > - } > + found = 1; > + break; > } > + else > + CloseWellKnownConnections(); > + } > + if (!found) > + FatalError("Failed to find a socket to listen on"); > + sprintf(dynamic_display, "%d\n", i); > + display = dynamic_display; > + } > + > + ListenTransFds = malloc (ListenTransCount * sizeof (int)); > + > + for (i = 0; i < ListenTransCount; i++) > + { > + int fd = _XSERVTransGetConnectionNumber (ListenTransConns[i]); > + > + ListenTransFds[i] = fd; > + FD_SET (fd, &WellKnownConnections); > + > + if (!_XSERVTransIsLocal (ListenTransConns[i])) > + { > + DefineSelf (fd); > } > } > Cheers, Julien
On 01/ 6/11 03:02 PM, Julien Cristau wrote: > On Thu, Jan 6, 2011 at 17:28:59 -0500, Adam Jackson wrote: >> + for (i = 0; i < 65535 - 1024; i++) > > Should this be 65535 - 6000? Or 65535 - X_TCP_PORT ?
On Fri, 2011-01-07 at 00:02 +0100, Julien Cristau wrote: > On Thu, Jan 6, 2011 at 17:28:59 -0500, Adam Jackson wrote: > > +static Bool > > +TryCreateSocket(int num, int *partial) > > +{ > > + char port[20]; > > + > > + sprintf(port, "%d", num); > > + > > + return _XSERVTransMakeAllCOTSServerListeners(port, partial, > > + &ListenTransCount, > > + &ListenTransConns); > > Cast from int to Bool, _XSERVTransMakeAllCOTSServerListeners seems to > return 0 on success, I guess this should return > _XSERVTransMakeAllCOTSServerListeners() >= 0? Good point, though the rest of the code seems to work correctly with that inversion. > > + else /* -displayfd */ > > + { > > + Bool found = 0; > > + for (i = 0; i < 65535 - 1024; i++) > > Should this be 65535 - 6000? Yeah. I was trying to avoid binding to reserved ports but clearly that's not what that does, and apparently asking for a display number that would wrap around into the 1025..5999 range doesn't work anyway. - ajax
This option specifies a file descriptor in the launching process. X will scan for an available display number and write that number back to the launching process, at the same time as SIGUSR1 generation. This means display managers don't need to guess at available display numbers. As a consequence, if X fails to start when using -displayfd, it's not because the display was in use, so there's no point in retrying the X launch on a higher display number. Signed-off-by: Adam Jackson <ajax@redhat.com> --- dix/globals.c | 1 + doc/man/Xserver.man | 7 +++++ include/opaque.h | 1 + os/connection.c | 72 ++++++++++++++++++++++++++++++++++---------------- os/utils.c | 11 ++++++++ 5 files changed, 69 insertions(+), 23 deletions(-)