[Libguestfs] [PATCH] appliance: reorder the steps to search for appliance

[Pavel Butsykin]

On 25.04.2017 15:11, Richard W.M. Jones wrote:
> On Tue, Apr 25, 2017 at 02:35:26PM +0300, Pavel Butsykin wrote:
>> The patch changes the order of the steps to search for fixed/supermin
>> appliance in accordance with documentation:
>>
>> "If the fixed appliance is found, libguestfs skips supermin entirely
>>   and just runs qemu with the kernel, initrd and root disk from the
>>   fixed appliance."
>
> Does anyone rely on the path-like behaviour of LIBGUESTFS_PATH?  It
> was a mistake to allow that originally, but we're stuck with it now
> unfortunately.

Sorry, but I don't understand what you mean. What is
"path-like behaviour"? What is the mistake ? The mistake is to use the
location of supermin.d directory for the decision on build appliance.?

I was faced with the problem that if in appliance directory there is
supermin.d then libguestfs is trying to build appliance. Even if
supermin.d is empty, even if libguestfs was built with options
--disable-appliance --disable-daemon. But as it turned out, the
behaviour build_appliance() is contrary to the documentation.

>
>> Signed-off-by: Pavel Butsykin <pbutsykin at virtuozzo.com>
>> ---
>>   lib/appliance.c | 22 +++++++++++-----------
>>   1 file changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/lib/appliance.c b/lib/appliance.c
>> index f12918573..06ee06f71 100644
>> --- a/lib/appliance.c
>> +++ b/lib/appliance.c
>> @@ -137,16 +137,6 @@ build_appliance (guestfs_h *g,
>>     CLEANUP_FREE char *path = NULL;
>>
>>     /* Step (1). */
>> -  r = find_path (g, contains_supermin_appliance, NULL, &supermin_path);
>> -  if (r == -1)
>> -    return -1;
>> -
>> -  if (r == 1)
>> -    /* Step (2): build supermin appliance. */
>> -    return build_supermin_appliance (g, supermin_path,
>> -                                     kernel, initrd, appliance);
>> -
>> -  /* Step (3). */
>>     r = find_path (g, contains_fixed_appliance, NULL, &path);
>>     if (r == -1)
>>       return -1;
>> @@ -162,7 +152,7 @@ build_appliance (guestfs_h *g,
>>       return 0;
>>     }
>>
>> -  /* Step (4). */
>> +  /* Step (2). */
>
> The steps actually refer to the documentation.  See in the same
> file, starting from " * Locate or build the appliance."
>

It seems that I missed it, thanks.

> Rich.
>
>>     r = find_path (g, contains_old_style_appliance, NULL, &path);
>>     if (r == -1)
>>       return -1;
>> @@ -177,6 +167,16 @@ build_appliance (guestfs_h *g,
>>       return 0;
>>     }
>>
>> +  /* Step (3). */
>> +  r = find_path (g, contains_supermin_appliance, NULL, &supermin_path);
>> +  if (r == -1)
>> +    return -1;
>> +
>> +  if (r == 1)
>> +    /* Step (4): build supermin appliance. */
>> +    return build_supermin_appliance (g, supermin_path,
>> +                                     kernel, initrd, appliance);
>> +
>>     error (g, _("cannot find any suitable libguestfs supermin, fixed or old-style appliance on LIBGUESTFS_PATH (search path: %s)"),
>>            g->path);
>>     return -1;
>> --
>> 2.11.0
>>
>> _______________________________________________
>> Libguestfs mailing list
>> Libguestfs at redhat.com
>> https://www.redhat.com/mailman/listinfo/libguestfs
>