On Wed, 2010-07-21 at 08:11 +0100, Yu Zhiguo wrote:
> Hi Ian,
>
> Yu Zhiguo wrote:
> > execute command by execvp() so can search command in PATH.
> >
>
> It is trivial, but can you ack this fix?
> Before this fix, when create guest, we must use absolute path
> for bootloader, e.g. bootloader = "/usr/bin/pygrub".
> If not in absolute path now, xl create will block in:
>
> pid = fork_exec_bootloader(&bootloader_fd, (char *)info->u.pv.bootloader,
> args);
> ...
> while (1) {
> fifo_fd = open(fifo, O_RDONLY); ------------> here
>
> because pygrub cannot be executed so no data will be outputted into this
> fifo.
Hmm, perhaps we need some more error handling from fork_exec_bootloader,
probably in addition to switching to execvp()? Perhaps a waitpid(..,..,
WNOHANG) sometime before the fifo open to check the child hasn't gone
away (although I'm not sure how oen makes this non-racey)?
Alternatively, maybe simply opening the FIFO O_NDELAY rather than using
open+fcntl is sufficient stop stop us blocking here? I guess that would
be safe since this is the read end of the FIFO. We would catch the
bootloader failure (to exec or otherwise) later on in
bootloader_interact (which either works already due to select returning
EBADF or probably needs fixing to handle the bootloader failing
regardless).
Also, xend special cases the bare word "pygrub" and searches a specific
list of likely install locations, perhaps libxl should duplicate that
behaviour? (I think I prefer search $PATH to this but maybe consistency
with previous behaviour trumps that?)
Ian.
>
> Yu
>
> > Signed-off-by: Yu Zhiguo <yuzg@xxxxxxxxxxxxxx>
> >
> > diff -r 12f0618400de -r da4c3756920e tools/libxl/libxl_exec.c
> > --- a/tools/libxl/libxl_exec.c Fri Jul 16 13:54:44 2010 +0100
> > +++ b/tools/libxl/libxl_exec.c Tue Jul 20 02:14:44 2010 +0800
> > @@ -53,7 +53,7 @@
> > /* in case our caller set it to IGN. subprocesses are entitled
> > * to assume they got DFL. */
> >
> > - execv(arg0, args);
> > + execvp(arg0, args);
> > _exit(-1);
> > }
> >
> >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxxxxxxxx
> > http://lists.xensource.com/xen-devel
> >
> >
> >
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|