On Tue, Apr 25, 2017 at 02:35:26PM +0300, Pavel Butsykin wrote: 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. The steps actually refer to the documentation. See in the same file, starting from " * Locate or build the appliance." 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

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. Signed-off-by: Pavel Butsykin <pbutsykin(a)virtuozzo.com&gt; --- generator/actions_properties.ml | 23 +++++++++++ lib/appliance.c | 84 +++++++++++++++++++++-------------------- lib/guestfs-internal.h | 1 + lib/handle.c | 23 +++++++++++ m4/guestfs_appliance.m4 | 7 ++++ 5 files changed, 97 insertions(+), 41 deletions(-) diff --git a/generator/actions_properties.ml b/generator/actions_properties.ml index 87144b14f..8f172b517 100644 --- a/generator/actions_properties.ml +++ b/generator/actions_properties.ml @@ -608,6 +608,29 @@ The environment variable C<XDG_RUNTIME_DIR> controls the default value: If C<XDG_RUNTIME_DIR> is set, then that is the default. Else F</tmp> is the default." }; + { defaults with + name = "set_appliance"; added = (1, 37, 11); + style = RErr, [Bool "appliance"], []; + fish_alias = ["appliance"]; config_only = true; + blocking = false; + shortdesc = "set enable appliance flag"; + longdesc = "\ +This 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. + +The default is true if the libguestfs is configured with --enable-appliance, +otherwise false." }; + + { defaults with + name = "get_appliance"; added = (1, 37, 11); + style = RBool "appliance", [], []; + blocking = false; + shortdesc = "get enable appliance flag"; + longdesc = "\ +This returns the enable appliance flag." }; + ] let daemon_functions = [ diff --git a/lib/appliance.c b/lib/appliance.c index f12918573..28e03e92f 100644 --- a/lib/appliance.c +++ b/lib/appliance.c @@ -132,53 +132,55 @@ build_appliance (guestfs_h *g, char **initrd, char **appliance) { - int r; - CLEANUP_FREE char *supermin_path = NULL; - 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); + if (g->enable_appliance) { + CLEANUP_FREE char *supermin_path = NULL; + /* Step (1). */ + int r = find_path (g, contains_supermin_appliance, NULL, &supermin_path); + if (r == -1) + return -1; - /* Step (3). */ - r = find_path (g, contains_fixed_appliance, NULL, &path); - if (r == -1) - return -1; + if (r == 1) + /* Step (2): build supermin appliance. */ + return build_supermin_appliance (g, supermin_path, + kernel, initrd, appliance); + } else { + CLEANUP_FREE char *path = NULL; + /* Step (3). */ + int r = find_path (g, contains_fixed_appliance, NULL, &path); + if (r == -1) + return -1; - if (r == 1) { - const size_t len = strlen (path); - *kernel = safe_malloc (g, len + 6 /* "kernel" */ + 2); - *initrd = safe_malloc (g, len + 6 /* "initrd" */ + 2); - *appliance = safe_malloc (g, len + 4 /* "root" */ + 2); - sprintf (*kernel, "%s/kernel", path); - sprintf (*initrd, "%s/initrd", path); - sprintf (*appliance, "%s/root", path); - return 0; - } + if (r == 1) { + const size_t len = strlen (path); + *kernel = safe_malloc (g, len + 6 /* "kernel" */ + 2); + *initrd = safe_malloc (g, len + 6 /* "initrd" */ + 2); + *appliance = safe_malloc (g, len + 4 /* "root" */ + 2); + sprintf (*kernel, "%s/kernel", path); + sprintf (*initrd, "%s/initrd", path); + sprintf (*appliance, "%s/root", path); + return 0; + } - /* Step (4). */ - r = find_path (g, contains_old_style_appliance, NULL, &path); - if (r == -1) - return -1; + /* Step (4). */ + r = find_path (g, contains_old_style_appliance, NULL, &path); + if (r == -1) + return -1; - if (r == 1) { - const size_t len = strlen (path); - *kernel = safe_malloc (g, len + strlen (kernel_name) + 2); - *initrd = safe_malloc (g, len + strlen (initrd_name) + 2); - sprintf (*kernel, "%s/%s", path, kernel_name); - sprintf (*initrd, "%s/%s", path, initrd_name); - *appliance = NULL; - return 0; + if (r == 1) { + const size_t len = strlen (path); + *kernel = safe_malloc (g, len + strlen (kernel_name) + 2); + *initrd = safe_malloc (g, len + strlen (initrd_name) + 2); + sprintf (*kernel, "%s/%s", path, kernel_name); + sprintf (*initrd, "%s/%s", path, initrd_name); + *appliance = NULL; + return 0; + } } - error (g, _("cannot find any suitable libguestfs supermin, fixed or old-style appliance on LIBGUESTFS_PATH (search path: %s)"), - g->path); + error (g, _("cannot find any suitable %s (search path: %s)"), + g->enable_appliance ? "libguestfs supermin" : + "fixed or old-style appliance on LIBGUESTFS_PATH", + g->path); return -1; } diff --git a/lib/guestfs-internal.h b/lib/guestfs-internal.h index 5e9d97c07..2b80f1ab9 100644 --- a/lib/guestfs-internal.h +++ b/lib/guestfs-internal.h @@ -414,6 +414,7 @@ struct guestfs_h { bool selinux; /* selinux enabled? */ bool pgroup; /* Create process group for children? */ bool close_on_exit; /* Is this handle on the atexit list? */ + bool enable_appliance; /* Enable the build supermin appliance */ int smp; /* If > 1, -smp flag passed to hv. */ int memsize; /* Size of RAM (megabytes). */ diff --git a/lib/handle.c b/lib/handle.c index 91f5f755d..b2b18b186 100644 --- a/lib/handle.c +++ b/lib/handle.c @@ -205,6 +205,16 @@ parse_environment (guestfs_h *g, guestfs_set_verbose (g, b); } + str = do_getenv (data, "LIBGUESTFS_ENABLE_APPLIANCE"); + if (str) { + b = guestfs_int_is_true (str); + if (b == -1) { + error (g, _("%s=%s: non-boolean value"), "LIBGUESTFS_ENABLE_APPLIANCE", str); + return -1; + } + guestfs_set_appliance (g, b); + } + str = do_getenv (data, "LIBGUESTFS_TMPDIR"); if (str && STRNEQ (str, "")) { if (guestfs_set_tmpdir (g, str) == -1) @@ -927,3 +937,16 @@ guestfs_impl_get_smp (guestfs_h *g) { return g->smp; } + +int +guestfs_impl_set_appliance (guestfs_h *g, int v) +{ + g->enable_appliance = !!v; + return 0; +} + +int +guestfs_impl_get_appliance (guestfs_h *g) +{ + return g->enable_appliance; +} diff --git a/m4/guestfs_appliance.m4 b/m4/guestfs_appliance.m4 index 890b1999c..0e0768656 100644 --- a/m4/guestfs_appliance.m4 +++ b/m4/guestfs_appliance.m4 @@ -28,6 +28,13 @@ AM_CONDITIONAL([ENABLE_APPLIANCE],[test "x$ENABLE_APPLIANCE" = "xyes"]) AC_MSG_RESULT([$ENABLE_APPLIANCE]) AC_SUBST([ENABLE_APPLIANCE]) +LIBGUESTFS_ENABLE_APPLIANC=0 +if test "x$ENABLE_APPLIANCE" = "xyes"; then +LIBGUESTFS_ENABLE_APPLIANCE=1 +fi +AC_DEFINE_UNQUOTED([LIBGUESTFS_ENABLE_APPLIANCE], [$LIBGUESTFS_ENABLE_APPLIANCE], + [Define to 1 or 0, depending on the build configuration.]) + if test "x$enable_daemon" != "xyes" && test "x$ENABLE_APPLIANCE" = "xyes" ; then AC_MSG_FAILURE([conflicting ./configure arguments: if you --disable-daemon then you have to --disable-appliance as well, since the appliance contains -- 2.11.0