From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH 1 of 3 RESEND] libxl: Look for bootloader in libexec path Date: Wed, 9 May 2012 15:01:01 +0100 Message-ID: <4FAA789D.4080905@eu.citrix.com> References: <1336570788.25514.117.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1336570788.25514.117.camel@zakaz.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org 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 >> >> 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@lists.xen.org >> http://lists.xen.org/xen-devel >