[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 09/21] Pull signal setup code out into separate method



On Fri, Oct 23, 2009 at 02:05:38PM +0100, Daniel P. Berrange wrote:
> * daemon/libvirtd.c: Introduce a daemonSetupSignals() method
>   and put all signal handling code there
> * daemon/libvirtd.h: Add sigread/sigwrite to qemud_server type
> ---
>  daemon/libvirtd.c |  128 +++++++++++++++++++++++++++++++---------------------
>  daemon/libvirtd.h |    1 +
>  2 files changed, 77 insertions(+), 52 deletions(-)
> 
> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> index ab0c88c..4e268a2 100644
> --- a/daemon/libvirtd.c
> +++ b/daemon/libvirtd.c
> @@ -782,7 +782,7 @@ static void virshErrorHandler(void *opaque ATTRIBUTE_UNUSED, virErrorPtr err ATT
>       * took care of reporting the error */
>  }
>  
> -static struct qemud_server *qemudInitialize(int sigread) {
> +static struct qemud_server *qemudInitialize(void) {
>      struct qemud_server *server;
>  
>      if (VIR_ALLOC(server) < 0) {
> @@ -790,21 +790,26 @@ static struct qemud_server *qemudInitialize(int sigread) {
>          return NULL;
>      }
>  
> +    server->privileged = geteuid() == 0 ? 1 : 0;
> +    server->sigread = server->sigwrite = -1;
> +
>      if (virMutexInit(&server->lock) < 0) {
>          VIR_ERROR("%s", _("cannot initialize mutex"));
>          VIR_FREE(server);
> +        return NULL;
>      }
>      if (virCondInit(&server->job) < 0) {
>          VIR_ERROR("%s", _("cannot initialize condition variable"));
>          virMutexDestroy(&server->lock);
>          VIR_FREE(server);
> +        return NULL;
>      }
>  
> -    server->privileged = geteuid() == 0 ? 1 : 0;
> -    server->sigread = sigread;
> -
>      if (virEventInit() < 0) {
>          VIR_ERROR0(_("Failed to initialize event system"));
> +        virMutexDestroy(&server->lock);
> +        if (virCondDestroy(&server->job) < 0)
> +        {}

  Hum ... but is there really anything we can do there ?

>          VIR_FREE(server);
>          return NULL;
>      }
> @@ -2292,7 +2297,10 @@ cleanup:
>  static void qemudCleanup(struct qemud_server *server) {
>      struct qemud_socket *sock;
>  
> -    close(server->sigread);
> +    if (server->sigread != -1)
> +        close(server->sigread);
> +    if (server->sigwrite != -1)
> +        close(server->sigwrite);
>  
>      sock = server->sockets;
>      while (sock) {
> @@ -2787,6 +2795,58 @@ qemudSetupPrivs (void)
>  #define qemudSetupPrivs() 0
>  #endif
>  
> +static int
> +daemonSetupSignals(struct qemud_server *server)
> +{
> +    struct sigaction sig_action;
> +    int sigpipe[2];
> +
> +    if (pipe(sigpipe) < 0)
> +        return -1;
> +
> +    if (virSetNonBlock(sigpipe[0]) < 0 ||
> +        virSetNonBlock(sigpipe[1]) < 0 ||
> +        virSetCloseExec(sigpipe[0]) < 0 ||
> +        virSetCloseExec(sigpipe[1]) < 0) {
> +        char ebuf[1024];
> +        VIR_ERROR(_("Failed to create pipe: %s"),
> +                  virStrerror(errno, ebuf, sizeof ebuf));
> +        goto error;
> +    }
> +
> +    sig_action.sa_sigaction = sig_handler;
> +    sig_action.sa_flags = SA_SIGINFO;
> +    sigemptyset(&sig_action.sa_mask);
> +
> +    sigaction(SIGHUP, &sig_action, NULL);
> +    sigaction(SIGINT, &sig_action, NULL);
> +    sigaction(SIGQUIT, &sig_action, NULL);
> +    sigaction(SIGTERM, &sig_action, NULL);
> +    sigaction(SIGCHLD, &sig_action, NULL);
> +
> +    sig_action.sa_handler = SIG_IGN;
> +    sigaction(SIGPIPE, &sig_action, NULL);
> +
> +    if (virEventAddHandleImpl(sigpipe[0],
> +                              VIR_EVENT_HANDLE_READABLE,
> +                              qemudDispatchSignalEvent,
> +                              server, NULL) < 0) {
> +        VIR_ERROR0(_("Failed to register callback for signal pipe"));
> +        goto error;
> +    }
> +
> +    server->sigread = sigpipe[0];
> +    server->sigwrite = sigpipe[1];
> +    sigwrite = sigpipe[1];
> +
> +    return 0;
> +
> +error:
> +    close(sigpipe[0]);
> +    close(sigpipe[1]);
> +    return -1;
> +}
> +

 ACK, this is something not trivial to understand isolating it is a good
idea, and with a comment it would be perfect :-)


Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]