[PATCH v2] kexec-tools: Perform run-time linking of libxenctrl.so
Daniel Kiper
daniel.kiper at oracle.com
Mon Dec 18 05:43:52 PST 2017
On Thu, Dec 14, 2017 at 04:48:01PM -0600, Eric DeVolder wrote:
> When kexec is utilized in a Xen environment, it has an explicit
> run-time dependency on libxenctrl.so. This dependency occurs
> during the configure stage and when building kexec-tools.
>
> When kexec is utilized in a non-Xen environment (either bare
> metal or KVM), the configure and build of kexec-tools omits
> any reference to libxenctrl.so.
>
> Thus today it is not currently possible to configure and build
> a *single* kexec that will work in *both* Xen and non-Xen
> environments, unless the libxenctrl.so is *always* present.
>
> For example, a kexec configured for Xen in a Xen environment:
>
> # ldd build/sbin/kexec
> linux-vdso.so.1 => (0x00007ffdeba5c000)
> libxenctrl.so.4.4 => /usr/lib64/libxenctrl.so.4.4 (0x00000038d8000000)
> libz.so.1 => /lib64/libz.so.1 (0x00000038d6c00000)
> libc.so.6 => /lib64/libc.so.6 (0x00000038d6000000)
> libdl.so.2 => /lib64/libdl.so.2 (0x00000038d6400000)
> libpthread.so.0 => /lib64/libpthread.so.0 (0x00000038d6800000)
> /lib64/ld-linux-x86-64.so.2 (0x000055e9f8c6c000)
> # build/sbin/kexec -v
> kexec-tools 2.0.16
>
> However, the *same* kexec executable fails in a non-Xen environment:
>
> # copy xen kexec to .
> # ldd ./kexec
> linux-vdso.so.1 => (0x00007fffa9da7000)
> libxenctrl.so.4.4 => not found
> liblzma.so.0 => /usr/lib64/liblzma.so.0 (0x0000003014e00000)
> libz.so.1 => /lib64/libz.so.1 (0x000000300ea00000)
> libc.so.6 => /lib64/libc.so.6 (0x000000300de00000)
> libpthread.so.0 => /lib64/libpthread.so.0 (0x000000300e200000)
> /lib64/ld-linux-x86-64.so.2 (0x0000558cc786c000)
> # ./kexec -v
> ./kexec: error while loading shared libraries:
> libxenctrl.so.4.4: cannot open shared object file: No such file or directory
>
> At Oracle we "workaround" this by having two kexec-tools packages,
> one for Xen and another for non-Xen environments. At Oracle, the
> desire is to offer a single kexec-tools package that works in either
> environment. To achieve this, kexec-tools would either have to ship
> with libxenctrl.so (which we have deemed as unacceptable), or we can
> make kexec perform run-time linking against libxenctrl.so.
>
> This patch is one possible way to alleviate the explicit run-time
> dependency on libxenctrl.so. This implementation utilizes a set of
> macros to wrap calls into libxenctrl.so so that the library can
> instead be dlopen() and obtain the function via dlsym() and then
> make the call. The advantage of this implementation is that it
> requires few changes to the existing kexec-tools code. The dis-
> advantage is that it uses macros to remap libxenctrl functions
> and do work under the hood.
>
> Another possible implementation worth considering is the approach
> taken by libvmi. Reference the following file:
>
> https://github.com/libvmi/libvmi/blob/master/libvmi/driver/xen/libxc_wrapper.h
>
> The libxc_wrapper_t structure definition that starts at line ~33
> has members that are function pointers into libxenctrl.so. This
> structure is populated once and then later referenced/dereferenced
> by the callers of libxenctrl.so members. The advantage of this
> implementation is it is more explicit in managing the use of
> libxenctrl.so and its versions, but the disadvantage is it would
> require touching more of the kexec-tools code.
>
> The following is a list libxenctrl members utilized by kexec:
>
> Functions:
> xc_interface_open
> xc_kexec_get_range
> xc_interface_close
> xc_kexec_get_range
> xc_interface_open
> xc_get_max_cpus
> xc_kexec_get_range
> xc_version
> xc_kexec_exec
> xc_kexec_status
> xc_kexec_unload
> xc_hypercall_buffer_array_create
> xc__hypercall_buffer_array_alloc
> xc_hypercall_buffer_array_destroy
> xc_kexec_load
> xc_get_machine_memory_map
>
> Data:
> xc__hypercall_buffer_HYPERCALL_BUFFER_NULL
>
> These were identified by configuring and building kexec-tools
> with Xen support, but omitting the -lxenctrl from the LDFLAGS
> in the Makefile for an x86_64 build.
>
> The above libxenctrl members were referenced via these source
> files.
>
> kexec/crashdump-xen.c
> kexec/kexec-xen.c
> kexec/arch/i386/kexec-x86-common.c
> kexec/arch/i386/crashdump-x86.c
>
> This patch provides a wrapper around the calls to the above
> functions in libxenctrl.so. Every libxenctrl call must pass a
> xc_interface which it obtains from xc_interface_open().
> So the existing code is already structured in a manner that
> facilitates graceful dlopen()'ing of the libxenctrl.so and
> the subsequent dlsym() of the required member.
>
> The patch creates a wrapper function around xc_interface_open()
> and xc_interface_close() to perform the dlopen() and dlclose().
>
> For the remaining xc_ functions, this patch defines a macro
> of the same name which performs the dlsym() and then invokes
> the function. See the _xc_call() macro for details.
>
> There was one data item in libxenctrl.so that presented a
> unique problem, HYPERCALL_BUFFER_NULL. It was only utilized
> once, as
>
> set_xen_guest_handle(xen_segs[s].buf.h, HYPERCALL_BUFFER_NULL);
>
> I tried a variety of techniques but could not find a general
> macro-type solution without modifying xenctrl.h. So the
> solution was to declare a local HYPERCALL_BUFFER_NULL, and
> this appears to work. I admit I am not familiar with libxenctrl
> to state if this is a satisfactory workaround, so feedback
> here welcome. I can state that this allows kexec to load/unload/kexec
> on Xen and non-Xen environments that I've tested without issue.
>
> With this patch applied, kexec-tools can be built with Xen
> support and yet there is no explicit run-time dependency on
> libxenctrl.so. Thus it can also be deployed in non-Xen
> environments where libxenctrl.so is not installed.
>
> # ldd build/sbin/kexec
> linux-vdso.so.1 => (0x00007fff7dbcd000)
> liblzma.so.0 => /usr/lib64/liblzma.so.0 (0x00000038d9000000)
> libz.so.1 => /lib64/libz.so.1 (0x00000038d6c00000)
> libdl.so.2 => /lib64/libdl.so.2 (0x00000038d6400000)
> libc.so.6 => /lib64/libc.so.6 (0x00000038d6000000)
> libpthread.so.0 => /lib64/libpthread.so.0 (0x00000038d6800000)
> /lib64/ld-linux-x86-64.so.2 (0x0000562dc0c14000)
> # build/sbin/kexec -v
> kexec-tools 2.0.16
>
> Currently this feature is enabled with the following:
>
> ./configure --with-xen-dl --with-xen=no
>
> This is a bit clunky. I welcome feedback such as better names
> and/or usage of --with, as well as if we might make this feature
> the default.
I would do it in a bit different way. If somebody specifies --with-xen
then kexec-tools should build as usual. However, if somebody specifies
with-xen-dl itself or --with-xen-dl and --with-xen then --with-xen-dl
should have a precedence.
> Signed-off-by: Eric DeVolder <eric.devolder at oracle.com>
> ---
> v1: 29nov2017
> - Daniel Kiper suggested Debian's libxen package of libraries,
> but I did not find similar package on most other systems.
>
> v2: 14dec2017
> - Reposted to kexec and xen-devel mailing lists
> ---
> configure.ac | 18 ++++++++++++++
> kexec/Makefile | 1 +
> kexec/arch/i386/crashdump-x86.c | 4 +---
> kexec/arch/i386/kexec-x86-common.c | 4 +---
> kexec/crashdump-xen.c | 4 +---
> kexec/kexec-xen.c | 43 +++++++++++++++++++++++++++++++++-
> kexec/kexec-xen.h | 48 ++++++++++++++++++++++++++++++++++++++
> 7 files changed, 112 insertions(+), 10 deletions(-)
> create mode 100644 kexec/kexec-xen.h
>
> diff --git a/configure.ac b/configure.ac
> index 208dc0a..4fc8aa0 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -92,6 +92,9 @@ AC_ARG_WITH([lzma], AC_HELP_STRING([--without-lzma],[disable lzma support]),
> AC_ARG_WITH([xen], AC_HELP_STRING([--without-xen],
> [disable extended xen support]), [ with_xen="$withval"], [ with_xen=yes ] )
>
> +AC_ARG_WITH([xen-dl], AC_HELP_STRING([--without-xen-dl],
> + [link libxenctrl.so at run-time rather than build-time]), [ with_xen_dl="$withval"], [ with_xen_dl=no ] )
> +
> AC_ARG_WITH([booke],
> AC_HELP_STRING([--with-booke],[build for booke]),
> AC_DEFINE(CONFIG_BOOKE,1,
> @@ -174,6 +177,21 @@ if test "$with_xen" = yes ; then
> AC_MSG_NOTICE([The kexec_status call is not available]))
> fi
> fi
> +if test "$with_xen_dl" = yes ; then
> + if test "$with_xen" = yes ; then
> + AC_MSG_ERROR([Options --with-xen and --with-xen-dl are mutually exclusive])
> + fi
> + AC_DEFINE(CONFIG_LIBXENCTRL_DL, 1, [Define to 1 to link libxenctrl.so at run-time rather than build-time])
> + AC_CHECK_HEADER(dlfcn.h, , AC_MSG_ERROR([Dynamic library linking not available]))
> + AC_CHECK_LIB(dl, dlopen, , AC_MSG_ERROR([Dynamic library linking not available]))
Please call AC_CHECK_LIB() from AC_CHECK_HEADER(). You can find more details
if you take a look at zlib, lzma and even xenctrl config in configure.ac.
> + AC_CHECK_HEADER(xenctrl.h,
> + AC_DEFINE(HAVE_LIBXENCTRL, 1, [Define to 1 to enable run-time linking of libxenctrl.so]),
> + AC_MSG_ERROR([Xen support not available]))
> +dnl NOTE: Explicitly *NOT* performing AC_CHECK_LIB(xenctrl) as only need the header file to build
> + AC_CHECK_HEADER(xenctrl.h,
> + AC_DEFINE(HAVE_KEXEC_CMD_STATUS, 1, [Define to 1 so kexec_status call is available]),
> + AC_MSG_NOTICE([The kexec_status call is not available]))
> +fi
>
> dnl ---Sanity checks
> if test "$CC" = "no"; then AC_MSG_ERROR([cc not found]); fi
> diff --git a/kexec/Makefile b/kexec/Makefile
> index 2b4fb3d..8871731 100644
> --- a/kexec/Makefile
> +++ b/kexec/Makefile
> @@ -36,6 +36,7 @@ dist += kexec/Makefile \
> kexec/kexec-elf-boot.h \
> kexec/kexec-elf.h kexec/kexec-sha256.h \
> kexec/kexec-zlib.h kexec/kexec-lzma.h \
> + kexec/kexec-xen.h \
> kexec/kexec-syscall.h kexec/kexec.h kexec/kexec.8
>
> dist += kexec/proc_iomem.c
> diff --git a/kexec/arch/i386/crashdump-x86.c b/kexec/arch/i386/crashdump-x86.c
> index 69a063a..a948d9f 100644
> --- a/kexec/arch/i386/crashdump-x86.c
> +++ b/kexec/arch/i386/crashdump-x86.c
> @@ -44,9 +44,7 @@
> #include "kexec-x86.h"
> #include "crashdump-x86.h"
>
> -#ifdef HAVE_LIBXENCTRL
> -#include <xenctrl.h>
> -#endif /* HAVE_LIBXENCTRL */
> +#include "../../kexec-xen.h"
>
> #include "x86-linux-setup.h"
>
> diff --git a/kexec/arch/i386/kexec-x86-common.c b/kexec/arch/i386/kexec-x86-common.c
> index be03618..b44c8b7 100644
> --- a/kexec/arch/i386/kexec-x86-common.c
> +++ b/kexec/arch/i386/kexec-x86-common.c
> @@ -40,9 +40,7 @@
> #include "../../crashdump.h"
> #include "kexec-x86.h"
>
> -#ifdef HAVE_LIBXENCTRL
> -#include <xenctrl.h>
> -#endif /* HAVE_LIBXENCTRL */
> +#include "../../kexec-xen.h"
>
> /* Used below but not present in (older?) xenctrl.h */
> #ifndef E820_PMEM
> diff --git a/kexec/crashdump-xen.c b/kexec/crashdump-xen.c
> index 60594f6..2e4cbdc 100644
> --- a/kexec/crashdump-xen.c
> +++ b/kexec/crashdump-xen.c
> @@ -18,9 +18,7 @@
>
> #include "config.h"
>
> -#ifdef HAVE_LIBXENCTRL
> -#include <xenctrl.h>
> -#endif
> +#include "kexec-xen.h"
>
> struct crash_note_info {
> unsigned long base;
> diff --git a/kexec/kexec-xen.c b/kexec/kexec-xen.c
> index 2b448d3..2b01fee 100644
> --- a/kexec/kexec-xen.c
> +++ b/kexec/kexec-xen.c
> @@ -10,10 +10,51 @@
> #include "config.h"
>
> #ifdef HAVE_LIBXENCTRL
> -#include <xenctrl.h>
> +#include "kexec-xen.h"
>
> #include "crashdump.h"
>
> +#ifdef CONFIG_LIBXENCTRL_DL
> +void *xc_dlhandle = NULL;
Just "void *xc_dlhandle;". Compiler will do work for you.
> +xc_hypercall_buffer_t XC__HYPERCALL_BUFFER_NAME(HYPERCALL_BUFFER_NULL);
static?
> +xc_interface *_xc_interface_open(xentoollog_logger *logger,
I would prefer __xc_interface_open() instead of _xc_interface_open()
(two underscores at the beginning of the function). Same applies to
the functions/variables below.
> + xentoollog_logger *dombuild_logger,
> + unsigned open_flags)
> +{
> + xc_interface *xch = NULL;
xc_interface *xch;
> + if (NULL == xc_dlhandle)
if (!xc_dlhandle)
> + xc_dlhandle = dlopen("libxenctrl.so", RTLD_NOW | RTLD_NODELETE);
if (!xc_dlhandle)
return NULL;
> + if (xc_dlhandle) {
... then you do not need this condition.
> + typedef xc_interface *(*func_t)(xentoollog_logger *logger,
> + xentoollog_logger *dombuild_logger,
> + unsigned open_flags);
Please define type(s) just behind the includes.
> + func_t func = (func_t)dlsym(xc_dlhandle, "xc_interface_open");
> + xch = func(logger, dombuild_logger, open_flags);
> + }
> +
> + return xch;
> +}
> +
> +int _xc_interface_close(xc_interface *xch)
> +{
> + int rc = -1;
> +
> +/*
> + if (xc_dlhandle) {
> +*/
> + typedef int (*func_t)(xc_interface *xch);
> + func_t func = (func_t)dlsym(xc_dlhandle, "xc_interface_close");
> + rc = func(xch);
> +/*
If you are not sure please provide the comment why it is
commented out or drop these lines entirely.
> + xc_dlhandle = NULL;
> + }
> +*/
> + return rc;
> +}
> +#endif /* CONFIG_LIBXENCTRL_DL */
> +
> int xen_kexec_load(struct kexec_info *info)
> {
> uint32_t nr_segments = info->nr_segments;
> diff --git a/kexec/kexec-xen.h b/kexec/kexec-xen.h
> new file mode 100644
> index 0000000..6d2046e
> --- /dev/null
> +++ b/kexec/kexec-xen.h
> @@ -0,0 +1,48 @@
> +#ifndef KEXEC_XEN_H
> +#define KEXEC_XEN_H
> +
> +#ifdef HAVE_LIBXENCTRL
> +#include <xenctrl.h>
> +
> +#ifdef CONFIG_LIBXENCTRL_DL
> +#include <dlfcn.h>
> +
> +/* The handle from dlopen(), needed by dlsym(), dlclose() */
> +extern void *xc_dlhandle;
> +
> +/* Wrappers around xc_interface_open/close() to insert dlopen/dlclose() */
> +xc_interface *_xc_interface_open(xentoollog_logger *logger,
> + xentoollog_logger *dombuild_logger,
> + unsigned open_flags);
> +int _xc_interface_close(xc_interface *xch);
> +
> +/* GCC expression statements for evaluating dlsym() */
> +#define _xc_call(DTYPE, NAME, ARGS...) ({ DTYPE value; typedef DTYPE (*func_t)(xc_interface *, ...); func_t func = dlsym(xc_dlhandle, #NAME); value = func(ARGS); value; } )
Too long line. Please try to not exceed 80 characters.
> +#define _xc_data(DTYPE, NAME) ({ DTYPE *value = (DTYPE *)dlsym(xc_dlhandle, #NAME); value; } )
I would export e.g. __xc_dlsym(const char *symbol) which calls
dlsym(xc_dlhandle, symbol). Then you can make xc_dlhandle static.
Hmmm... xc_dlhandle -> xc_dl_handle -> xc_dlh or even xdlh?
> +/* The wrappers around utilized xenctrl.h functions */
> +#define xc_interface_open(A,B,C) _xc_interface_open(A,B,C)
Lack of space after commas... And please use lowercase letters here.
> +#define xc_interface_close(A) _xc_interface_close(A)
> +#define xc_version(ARGS...) _xc_call(int, xc_version, ARGS)
> +#define xc_get_max_cpus(ARGS...) _xc_call(int, xc_get_max_cpus, ARGS)
> +#define xc_get_machine_memory_map(ARGS...) _xc_call(int, xc_get_machine_memory_map, ARGS)
> +#define xc_kexec_get_range(ARGS...) _xc_call(int, xc_kexec_get_range, ARGS)
> +#define xc_kexec_load(ARGS...) _xc_call(int, xc_kexec_load, ARGS)
> +#define xc_kexec_unload(ARGS...) _xc_call(int, xc_kexec_unload, ARGS)
> +#define xc_kexec_status(ARGS...) _xc_call(int, xc_kexec_status, ARGS)
> +#define xc_kexec_exec(ARGS...) _xc_call(int, xc_kexec_exec, ARGS)
> +#define xc_hypercall_buffer_array_create(ARGS...) _xc_call(xc_hypercall_buffer_array_t *, xc_hypercall_buffer_array_create, ARGS)
> +#define xc__hypercall_buffer_alloc(ARGS...) _xc_call(void *, xc__hypercall_buffer_alloc, ARGS)
> +#define xc__hypercall_buffer_free(ARGS...) _xc_call(void , xc__hypercall_buffer_free, ARGS)
> +#define xc__hypercall_buffer_alloc_pages(ARGS...) _xc_call(void *, xc__hypercall_buffer_alloc_pages, ARGS)
> +#define xc__hypercall_buffer_free_pages(ARGS...) _xc_call(void , xc__hypercall_buffer_free_pages, ARGS)
> +#define xc__hypercall_buffer_array_alloc(ARGS...) _xc_call(void *, xc__hypercall_buffer_array_alloc, ARGS)
> +#define xc__hypercall_buffer_array_get(ARGS...) _xc_call(void *, xc__hypercall_buffer_array_get, ARGS)
> +#define xc_hypercall_buffer_array_destroy(ARGS...) _xc_call(void *, xc_hypercall_buffer_array_destroy, ARGS)
Could you try to make this more readable? Alignment, line wraps
if needed, etc. And of course too long lines in many places...
Really kexec-tools use all of them?
Daniel
More information about the kexec
mailing list