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

[George Dunlap <george.dunlap@eu.citrix.com>]

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@eu.citrix.com>
>>
>> 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
>