Re: [Xen-devel] [PATCH 23/24] libxl: child processes cleanups

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2012-04-16 at 18:18 +0100, Ian Jackson wrote:
> Abolish libxl_fork.  Its only callers were in xl.  Its functionality
> is now moved elsewhere, as follows:
> 
> * The "logging version of fork", which is what it was billed as, is now
>   xl_fork, which also dies on failure.
> 
> * Closing the xenstore handle in the child is now done in
>   libxl__ev_child_fork, which is the only remaining place where fork
>   is called in libxl.
> 
> * We provide a new function libxl__ev_child_xenstore_reopen for
>   in-libxl children to make the ctx useable for xenstore again.
> 
> * Consequently libxl__spawn_record_pid now no longer needs to mess
>   about with its own xenstore handle.  As a bonus it can now just use
>   libxl__xs_write.
> 
> Also, since we have now converted all the forkers in libxl, we can
> always honour the fork_replacement childproc hook - so do so.
> 
> Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> ---
>  tools/libxl/libxl_exec.c     |   35 ++++++++++++++++-------------------
>  tools/libxl/libxl_fork.c     |   25 ++++++++++++++++++++++++-
>  tools/libxl/libxl_internal.h |    5 +++++
>  tools/libxl/libxl_utils.c    |   21 ---------------------
>  tools/libxl/libxl_utils.h    |    3 +--
>  tools/libxl/xl.c             |   12 ++++++++++++
>  tools/libxl/xl.h             |    1 +
>  tools/libxl/xl_cmdimpl.c     |    5 ++---
>  8 files changed, 61 insertions(+), 46 deletions(-)
> 
> diff --git a/tools/libxl/libxl_exec.c b/tools/libxl/libxl_exec.c
> index d44b702..d681736 100644
> --- a/tools/libxl/libxl_exec.c
> +++ b/tools/libxl/libxl_exec.c
>  
> @@ -302,7 +291,15 @@ int libxl__spawn_spawn(libxl__egc *egc, 
> libxl__spawn_state *ss)
>  
>      /* we are now the middle process */
>  
> -    child = fork();
> +    pid_t (*fork_replacement)(void*) =
> +        CTX->childproc_hooks
> +        ? CTX->childproc_hooks->fork_replacement
> +        : 0;
> +    child =
> +        fork_replacement
> +        ? fork_replacement(CTX->childproc_user)
> +        : fork();

A helper function or macro would be useful here?

> +
>      if (child == -1)
>          exit(255);
>      if (!child) {
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index fb4dee8..3e90ac4 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -627,6 +627,11 @@ static inline void libxl__ev_child_init(libxl__ev_child 
> *childw_out)
>  static inline int libxl__ev_child_inuse(libxl__ev_child *childw_out)
>                  { return childw_out->pid >= 0; }
>  
> +/* Useable (only) in the child to once more make the ctx useable for
> + * xenstore operations.

Specifically "the child" is the middle child of a spawn? Otherwise the
constraint must be something like "before any threads are created in the
new process", or something like that?

>   logs failure in the form "what: <error
> + * message>". */
> +_hidden int libxl__ev_child_xenstore_reopen(libxl__gc *gc, const char *what);
> +
>  
>  /*
>   * Other event-handling support provided by the libxl event core to


Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel