Richard W.M. Jones
2017-Apr-28 14:29 UTC
Re: [Libguestfs] [PATCH] add enable_appliance flag to specify the appliance build
On Fri, Apr 28, 2017 at 05:03:58PM +0300, Pavel Butsykin wrote:> On 27.04.2017 18:28, Richard W.M. Jones wrote: > > > >On Thu, Apr 27, 2017 at 03:55:10PM +0300, Pavel Butsykin wrote: > >>The flag determines the behavior of working with the appliance, and specifically > >>the choice to use the build supermin appliance or the fixed appliance. If the > >>flag is false then the libguestfs will only use the fixed appliance, otherwise > >>the build supermin appliance. > >> > >>This patch is aimed at solving the problem with undefined behavior when > >>searching for the appliance. The libguestfs configured with --enable-appliance > >>will only use the build supermin appliance, with --disable-appliance only use > >>the fixed appliance. > >> > > > >It has to be said I like this approach even less :-/ > > > > Any arguments? What exactly is wrong?Specifically it exposes a whole new API which just serves to adjust some internal issue in libguestfs (which as I said in the other reply is really a bug in libguestfs). Adding new APIs has a high bar because we have to support them forever. I really think the solution here is to fix the path search so it works like you'd expect a path search to work, ie: LIBGUESTFS_PATH=/a:/b:/c means look in /a for any appliance, then look in /b, then look in /c. (Whereas currently it means something like "go round and round in circles looking for different sorts of appliance".) Then change the distro so it doesn't install two appliances into a single location, which only ever worked by random chance (if indeed it ever did work). Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
Pavel Butsykin
2017-May-02 12:37 UTC
[Libguestfs] [PATCH] add enable_appliance flag to specify the appliance build
On 28.04.2017 17:29, Richard W.M. Jones wrote:> On Fri, Apr 28, 2017 at 05:03:58PM +0300, Pavel Butsykin wrote: >> On 27.04.2017 18:28, Richard W.M. Jones wrote: >>> >>> On Thu, Apr 27, 2017 at 03:55:10PM +0300, Pavel Butsykin wrote: >>>> The flag determines the behavior of working with the appliance, and specifically >>>> the choice to use the build supermin appliance or the fixed appliance. If the >>>> flag is false then the libguestfs will only use the fixed appliance, otherwise >>>> the build supermin appliance. >>>> >>>> This patch is aimed at solving the problem with undefined behavior when >>>> searching for the appliance. The libguestfs configured with --enable-appliance >>>> will only use the build supermin appliance, with --disable-appliance only use >>>> the fixed appliance. >>>> >>> >>> It has to be said I like this approach even less :-/ >>> >> >> Any arguments? What exactly is wrong? > > Specifically it exposes a whole new API which just serves to adjust > some internal issue in libguestfs (which as I said in the other reply > is really a bug in libguestfs). Adding new APIs has a high bar > because we have to support them forever.Thank you for the explanation.> I really think the solution here is to fix the path search so it > works like you'd expect a path search to work, ie: > > LIBGUESTFS_PATH=/a:/b:/c > > means look in /a for any appliance, then look in /b, then look in /c. > (Whereas currently it means something like "go round and round in > circles looking for different sorts of appliance".) > > Then change the distro so it doesn't install two appliances into a > single location, which only ever worked by random chance (if indeed it > ever did work).I agree that this will solve my problem, although the error is theoretically possible but in practice this is unrealistic. Given the relative simplicity of the changes, it makes sense to make such a fix, thanks.> Rich. >
Reasonably Related Threads
- Re: [PATCH] add enable_appliance flag to specify the appliance build
- Re: [PATCH] appliance: reorder the steps to search for appliance
- Re: [PATCH] appliance: reorder the steps to search for appliance
- Re: [PATCH] appliance: reorder the steps to search for appliance
- Re: [PATCH] appliance: reorder the steps to search for appliance