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

[Pavel Butsykin]

On 27.04.2017 17:43, Richard W.M. Jones wrote:
> On Thu, Apr 27, 2017 at 03:54:44PM +0300, Pavel Butsykin wrote:
> [...]
>>> (2) If the fixed appliance was located somewhere else, you could do:
>>>
>>>    LIBGUESTFS_PATH=/path/to/somewhere/else:/usr/lib64/guestfs
>>>    export LIBGUESTFS_PATH
>>>
>>> and then it would look for the fixed appliance in
>>> /path/to/somewhere/else and use it.  If the fixed appliance exists
>>> there, then it would never check /usr/lib64/guestfs.
>>
>> It's a bit wrong, the first step is search supermin.d in all specified
>> directories:
>>    /* 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);
>
> OK, I see, that is indeed a bug.  Can you suggest a patch which fixes
> this so that directories are searched correctly?
>
In fact, it's hard for me to understand this is a bug or not, because
it's not clear what behavior is expected. The first patch was aimed at
bringing consistency between documentation and code. But of course, it
doesn't solve the real problem. Changing the priority doesn't change
the overall situation, the only difference is the type of appliance
through which still someone could accidentally stumble. The fix for
this "bug", specifically the search for all appliances in each
directory separately, in fact it doesn't fix the real problem,
similarly the supermin.d can fall into the new directory and libguestfs
can pick up another's appliance.

> However I still think that putting all of the appliances into a single
> directory and expecting libguestfs to know which one you meant it to
> choose first is the wrong idea.
>
I agree, to lay certain order in which the libguestfs chooses
appliances is astrange idea, but that's how it is now implemented. And
it's worth to fix. We can't guarantee that the same directory cannot
contain two different appliances. Some distributions can install fixed
appliance from packges 
(http://euscan.gentooexperimental.org/files/gentoo/app-emulation/libguestfs-appliance-1.28.1.ebuild)
and here is a possible different situation. The user will have trouble
understanding why after installation "package A" is broken "package B".
But we can minimally to protect ourselves from unforeseen errors. Now I
see two unrelated issues:

1. To search for the supermin appliance using only the directory name,
which of course is not enough. If the directory is empty then the
libguestfs exits with the following error:
supermin: ext2fs_namei: parent directory not found: /usr/lib: File not 
found by ext2_lookup
supermin: failure: ext2fs_namei: parent directory not found
libguestfs: error: /usr/bin/supermin exited with error status 1, see 
debug messages above

Where of course the real reason is not visible. So I agree with your
proposal to add more files to check, I'll send a patch.

2. If the directory contains two different types of appliances, it is
not obvious how the libguestfs will work (without looking at code).
This can lead to different breakdowns (mismatch between the version of
the daemon, a different set of libraries, etc). Despite the fact that
there is support for two different types of appliances at build that
could help to solve the issue. In my opinion, if the build is
configured with options "--disable-appliance --disable-daemon"
that pursued a clear goal to use the fixed appliance. It's hard for me
to imagine when it's necessary to build with the included appliance and
then to use the fixed appliance, or vice versa. For debugging purposes
we can always set environment variable. The solution I introduced in
the patch "add enable_appliance flag to specify the appliance build",
it's a simple change nevertheless it will allow to solve the issue with
unexpected errors.

> Rich.
>