Re: [Xen-devel] [PATCH 06/21] libxl: domain save/restore: run in a separate process

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Campbell writes ("Re: [Xen-devel] [PATCH 06/21] libxl: domain save/restore: 
run in a separate process"):
> > [...]
> > +    pid_t pid = libxl__ev_child_fork(gc, &shs->child, helper_exited);
> > +    if (!pid) {
> > +        if (stream_fd <= 2) {
> > +            stream_fd = dup(stream_fd);
> > +            if (stream_fd < 0) {
> > +                LOGE(ERROR,"dup migration stream fd");
> 
> LOGE uses CTX -- is that safe after fork, and will it go anywhere
> anyway?

>From the doc comment for libxl__ev_child_fork:

 * The child should go on to exec (or exit) soon.  The child may not
 * make any further calls to libxl infrastructure, except for memory
 * allocation and logging.  If the child needs to use xenstore it
 * must open its own xs handle and use it directly, rather than via
 * the libxl event machinery.

So this is fine according to the doc comment.  And the doc comment is,
I think, justified.  The logging might go to some file where we share
the descriptor with the parent, but logs are opened with O_APPEND, so
that's fine.  Likewise if it's going to syslog, the socket will be
fine.

That general-purpose xtl loggers need to work in children isn't
explicitly documented in xentoollog.h but I think it's fine to leave
it that way.

> > +                exit(-1);
> > +            }
> > +        }
> > +        libxl_fd_set_cloexec(CTX, stream_fd, 0);
> > +        *stream_fd_arg = GCSPRINTF("%d", stream_fd);
> > +
> > +        for (i=0; i<num_preserve_fds; i++)
> > +            if (preserve_fds[i] >= 0)
> > +                libxl_fd_set_cloexec(CTX, preserve_fds[i], 0);
> 
> The doc comment says these cannot be 0,1,2. Possibly not worth enforcing
> though.

If this restriction is violated then things are going to go very badly
wrong, so an assertion is probably a good idea.  (This will kill the
migration child, not the parent.)

> Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

Thanks,
Ian.

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