[Spice-devel,(v2)] Make sure the child is able to connect to the X server before exiting the mainline.

Submitted by Jeremy White on July 9, 2014, 7:08 p.m.

Details

Message ID 1404932924-18844-1-git-send-email-jwhite@codeweavers.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Jeremy White July 9, 2014, 7:08 p.m.
This allows the vdagent to be used in the Xsetup phase of an xdm session;
otherwise, it's X11 connection fails because of the greeter display grab.

The issue is that we daemonize before attempting the X connection.
We then immediately exit the main process; xdm then goes on to invoke the
greeter which performs an exclusive grab on the X server, so the child
connection fails.

The change is to have the main process wait for up to 10 seconds for an 'all clear'
message from the client.  This lets us return a correct status on a real X
error, as well as letting our child successfully connect to the X server.

Signed-off-by: Jeremy White <jwhite@codeweavers.com>
---
 src/vdagent.c |   50 ++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 44 insertions(+), 6 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/vdagent.c b/src/vdagent.c
index d7f7aba..905ff5b 100644
--- a/src/vdagent.c
+++ b/src/vdagent.c
@@ -35,6 +35,7 @@ 
 #include <sys/stat.h>
 #include <spice/vd_agent.h>
 #include <glib.h>
+#include <poll.h>
 
 #include "udscs.h"
 #include "vdagentd-proto.h"
@@ -151,9 +152,34 @@  static void quit_handler(int sig)
     quit = 1;
 }
 
-void daemonize(void)
+/* When we daemonize, it is useful to have the main process
+   wait to make sure the X connection worked.  We wait up
+   to 10 seconds to get an 'all clear' from the child
+   before we exit.  If we don't, we're able to exit with a
+   status that indicates an error occured */
+void wait_and_exit(int s)
 {
-    int x, retval = 0;
+    char buf[4];
+    struct pollfd p;
+    p.fd = s;
+    p.events = POLLIN;
+
+    if (poll(&p, 1, 10000) > 0)
+        if (read(s, buf, sizeof(buf)) > 0)
+            exit(0);
+
+    exit(1);
+}
+
+int daemonize(void)
+{
+    int x;
+    int fd[2];
+
+    if (socketpair(PF_LOCAL, SOCK_STREAM, 0, fd)) {
+        syslog(LOG_ERR, "socketpair : %s", strerror(errno));
+        exit(1);
+    }
 
     /* detach from terminal */
     switch (fork()) {
@@ -161,13 +187,17 @@  void daemonize(void)
         close(0); close(1); close(2);
         setsid();
         x = open("/dev/null", O_RDWR); x = dup(x); x = dup(x);
-        break;
+        close(fd[0]);
+        return fd[1];
     case -1:
         syslog(LOG_ERR, "fork: %s", strerror(errno));
-        retval = 1;
+        exit(1);
     default:
-        exit(retval);
+        close(fd[1]);
+        wait_and_exit(fd[0]);
     }
+
+    return 0;
 }
 
 static int file_test(const char *path)
@@ -182,6 +212,7 @@  int main(int argc, char *argv[])
     fd_set readfds, writefds;
     int c, n, nfds, x11_fd;
     int do_daemonize = 1;
+    int parent_socket = 0;
     int x11_sync = 0;
     struct sigaction act;
 
@@ -236,7 +267,7 @@  int main(int argc, char *argv[])
     }
 
     if (do_daemonize)
-        daemonize();
+        parent_socket = daemonize();
 
 reconnect:
     if (version_mismatch) {
@@ -275,6 +306,13 @@  reconnect:
     vdagent_file_xfers = vdagent_file_xfers_create(client, fx_dir,
                                                    fx_open_dir, debug);
 
+    if (parent_socket) {
+        if (write(parent_socket, "OK", 2) != 2)
+            syslog(LOG_WARNING, "Parent already gone.");
+        close(parent_socket);
+        parent_socket = 0;
+    }
+
     while (client && !quit) {
         FD_ZERO(&readfds);
         FD_ZERO(&writefds);

Comments

On 07/09/2014 10:08 PM, Jeremy White wrote:
> This allows the vdagent to be used in the Xsetup phase of an xdm session;
> otherwise, it's X11 connection fails because of the greeter display grab.
> 
> The issue is that we daemonize before attempting the X connection.
> We then immediately exit the main process; xdm then goes on to invoke the
> greeter which performs an exclusive grab on the X server, so the child
> connection fails.
> 
> The change is to have the main process wait for up to 10 seconds for an 'all clear'
> message from the client.  This lets us return a correct status on a real X
> error, as well as letting our child successfully connect to the X server.
> 
> Signed-off-by: Jeremy White <jwhite@codeweavers.com>
> ---

Pushed together with the previous two v1 patches.

>  src/vdagent.c |   50 ++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 44 insertions(+), 6 deletions(-)
> 
> diff --git a/src/vdagent.c b/src/vdagent.c
> index d7f7aba..905ff5b 100644
> --- a/src/vdagent.c
> +++ b/src/vdagent.c
> @@ -35,6 +35,7 @@
>  #include <sys/stat.h>
>  #include <spice/vd_agent.h>
>  #include <glib.h>
> +#include <poll.h>
>  
>  #include "udscs.h"
>  #include "vdagentd-proto.h"
> @@ -151,9 +152,34 @@ static void quit_handler(int sig)
>      quit = 1;
>  }
>  
> -void daemonize(void)
> +/* When we daemonize, it is useful to have the main process
> +   wait to make sure the X connection worked.  We wait up
> +   to 10 seconds to get an 'all clear' from the child
> +   before we exit.  If we don't, we're able to exit with a
> +   status that indicates an error occured */
> +void wait_and_exit(int s)
>  {
> -    int x, retval = 0;
> +    char buf[4];
> +    struct pollfd p;
> +    p.fd = s;
> +    p.events = POLLIN;
> +
> +    if (poll(&p, 1, 10000) > 0)
> +        if (read(s, buf, sizeof(buf)) > 0)
> +            exit(0);
> +
> +    exit(1);
> +}
> +
> +int daemonize(void)
> +{
> +    int x;
> +    int fd[2];
> +
> +    if (socketpair(PF_LOCAL, SOCK_STREAM, 0, fd)) {
> +        syslog(LOG_ERR, "socketpair : %s", strerror(errno));
> +        exit(1);
> +    }
>  
>      /* detach from terminal */
>      switch (fork()) {
> @@ -161,13 +187,17 @@ void daemonize(void)
>          close(0); close(1); close(2);
>          setsid();
>          x = open("/dev/null", O_RDWR); x = dup(x); x = dup(x);
> -        break;
> +        close(fd[0]);
> +        return fd[1];
>      case -1:
>          syslog(LOG_ERR, "fork: %s", strerror(errno));
> -        retval = 1;
> +        exit(1);
>      default:
> -        exit(retval);
> +        close(fd[1]);
> +        wait_and_exit(fd[0]);
>      }
> +
> +    return 0;
>  }
>  
>  static int file_test(const char *path)
> @@ -182,6 +212,7 @@ int main(int argc, char *argv[])
>      fd_set readfds, writefds;
>      int c, n, nfds, x11_fd;
>      int do_daemonize = 1;
> +    int parent_socket = 0;
>      int x11_sync = 0;
>      struct sigaction act;
>  
> @@ -236,7 +267,7 @@ int main(int argc, char *argv[])
>      }
>  
>      if (do_daemonize)
> -        daemonize();
> +        parent_socket = daemonize();
>  
>  reconnect:
>      if (version_mismatch) {
> @@ -275,6 +306,13 @@ reconnect:
>      vdagent_file_xfers = vdagent_file_xfers_create(client, fx_dir,
>                                                     fx_open_dir, debug);
>  
> +    if (parent_socket) {
> +        if (write(parent_socket, "OK", 2) != 2)
> +            syslog(LOG_WARNING, "Parent already gone.");
> +        close(parent_socket);
> +        parent_socket = 0;
> +    }
> +
>      while (client && !quit) {
>          FD_ZERO(&readfds);
>          FD_ZERO(&writefds);
>