Re: [Xen-devel] [PATCH v2] xl: track child processes for the benefit of libxl

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2012-05-18 at 19:25 +0100, Ian Jackson wrote:
> Each time xl forks, it needs to record the pid, so that its exit
> status can be preserved if it happens that libxl's event loop reaped
> it.  Consequently we also have a new wrapper for waitpid, which in
> that case returns the previously-reaped status.
> 
> When we get a console ready callback from libxl, check to see if we
> have spawned but not reaped a previous console client, and if so reap
> it now.  (This is necessary to prevent improper use of the xlchild
> struct, but has the happy consequence of checking the exit status of
> the first console client in the pygrub case.)
> 
> Refactor the two calls to libxl_ctx_alloc into a new function
> xl_ctx_alloc which also sets the child reaped handler callback.
> 
> All of this has the effect of suppressing a message
>    unknown child [nnnn] unexpected exited status zero
> which would sometimes (depending on a race) appear with `xl create -c'
> and pygrub.
> 
> Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Reported-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> Cc: Roger Pau Monne <roger.pau@xxxxxxxxxx>
> 
> Changes since v1:
>  * Replace macro-based iteration over children with enum-based version.

I find this a lot easier to follow, thanks!

> +pid_t xl_waitpid(xlchildnum child, int *status, int flags)
> +{
> +    xlchild *ch = &children[child];
> +    pid_t got = ch->pid;
> +    assert(got);
> +    if (ch->reaped) {
> +        *status = ch->status;
> +        ch->pid = 0;
> +        return got;
> +    }
> +    for (;;) {
> +        got = waitpid(ch->pid, status, flags);

Is it always the case that xl has at most one child?

Because if not then we may reap the wrong one here.

I believe we do never have more than one child, the corner case would be
if we have a console child and a daemon child simultaneously, but I
don't think that can happen (likewise migration child and either of the
others).

> @@ -108,8 +110,32 @@ struct cmd_spec *cmdtable_lookup(const char *s);
>  
>  extern libxl_ctx *ctx;
>  extern xentoollog_logger_stdiostream *logger;
> -pid_t xl_fork(libxl_ctx *ctx); /* like fork, but prints and dies if it fails 
> */
> -void postfork(void);
> +
> +void xl_ctx_alloc(void);
> +
> +/* child processes */
> +
> +typedef struct {
> +    /* every struct like this must be in XLCHILD_LIST */

This comment is obsolete now.

[...]



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