Re: [PATCH v2 3/3] tools/xl: reject bootloader=pygrub in case pygrub is disabled

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.08.2023 15:36, Juergen Gross wrote:
> On 08.08.23 15:32, Jan Beulich wrote:
>> On 08.08.2023 15:22, Juergen Gross wrote:
>>> --- a/tools/xl/xl_parse.c
>>> +++ b/tools/xl/xl_parse.c
>>> @@ -1692,6 +1692,15 @@ void parse_config_data(const char *config_source,
>>>       xlu_cfg_get_defbool(config, "acpi", &b_info->acpi, 0);
>>>   
>>>       xlu_cfg_replace_string (config, "bootloader", &b_info->bootloader, 0);
>>> +#ifndef HAVE_PYGRUB
>>> +    if (b_info->bootloader &&
>>> +        (!strcmp(b_info->bootloader, "pygrub") ||
>>> +    !strcmp(b_info->bootloader, "/usr/bin/pygrub"))) {
>>
>> And no other path combinations can occur? strstr() is perhaps too lax,
>> but what about finding the last slash (if any) and comparing the rest
>> of the string (the full string when there's no slash) against "pygrub"?
> 
> "pygrub" is the preferred variant, "/usr/bin/pygrub" seems to be the
> legacy variant, which will result in a warning to use "pygrub" only
> (in case pygrub is enabled, of course).
> 
> I don't think we should test for other non-standard paths.
> 
>>
>>> +        fprintf(stderr, "ERROR: this instance of Xen has been built 
>>> without support of \"pygrub\".\n");
>>
>> The other question (I'm sorry for my ignorance here) is whether pygrub
>> could come from anywhere else.
> 
> It would be possible to use that in case it is e.g. installed in
> /usr/local/bin/pygrub (assuming the check above isn't modified).

Well, okay, I'll leave this to Anthony then.

Jan