Re: [Xen-devel] [PATCH 1 of 3 RESEND] libxl: Look for bootloader in libexec path

[George Dunlap <george.dunlap@xxxxxxxxxxxxx>]

On 09/05/12 14:39, Ian Campbell wrote:
> I think since libxl uses execvp the first two are handled by path
> expansion, the next two are what you are adding (so has priority over
> $PATH in your patch, which I think is OK) and the last one is irrelevant
> in this context (xend uses the same function for other contexts).
>
> However in order to not differ on the first two execvp needs to be given
> the opportunity to search the path, which means that you need to check
> of your newly constructed bootloader exists, and if it doesn't then try
> and continue with the relative path not the absolute one.
>
> Also since this patch comes before the rename+symlink patch as it stands
> this patch will temporarily break people who just have "pygrub" without
> a path in their config.
Ah, I didn't realize that xl would already search the path (via execvp).

I think it should be pretty straightforward to add a check for 
existence, and if not fall back to letting execvp search the path.
If I do that, arguably this patch would still come first in the series?  
Not that it matters much, really.
 -George

> Ian.
>
> > Signed-off-by: George Dunlap<george.dunlap@xxxxxxxxxxxxx>
> >
> > diff -r 8a86d841e6d4 -r e43157a86933 tools/libxl/libxl_bootloader.c
> > --- a/tools/libxl/libxl_bootloader.c    Tue May 08 13:36:24 2012 +0200
> > +++ b/tools/libxl/libxl_bootloader.c    Wed May 09 11:38:50 2012 +0100
> > @@ -333,6 +333,7 @@ int libxl_run_bootloader(libxl_ctx *ctx,
> >
> >       char tempdir_template[] = "/var/run/libxl/bl.XXXXXX";
> >       char *tempdir;
> > +    const char *bootloader = NULL;
> >
> >       char *dom_console_xs_path;
> >       char dom_console_slave_tty_path[PATH_MAX];
> > @@ -397,6 +398,13 @@ int libxl_run_bootloader(libxl_ctx *ctx,
> >           goto out_close;
> >       }
> >
> > +    bootloader = libxl__abs_path(gc, info->u.pv.bootloader,
> > +                                 libxl__libexec_path());
> > +    if ( bootloader == NULL ) {
> > +        rc = ERROR_NOMEM;
> > +        goto out_close;
> > +    }
> > +
> >       /*
> >        * We need to present the bootloader's tty as a pty slave that xenconsole
> >        * can access.  Since the bootloader itself needs a pty slave,
> > @@ -417,7 +425,7 @@ int libxl_run_bootloader(libxl_ctx *ctx,
> >       dom_console_xs_path = libxl__sprintf(gc, "%s/console/tty", 
> > libxl__xs_get_dompath(gc, domid));
> >       libxl__xs_write(gc, XBT_NULL, dom_console_xs_path, "%s", 
> > dom_console_slave_tty_path);
> >
> > -    pid = fork_exec_bootloader(&bootloader_fd, info->u.pv.bootloader, args);
> > +    pid = fork_exec_bootloader(&bootloader_fd, bootloader, args);
> >       if (pid<  0) {
> >           goto out_close;
> >       }
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@xxxxxxxxxxxxx
> > http://lists.xen.org/xen-devel

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