[libvirt] [PATCH v6 2/9] libxl: pass driver config to libxlMakeDomBuildInfo

Jim Fehlig jfehlig at suse.com
Thu Mar 22 22:44:58 UTC 2018


On 03/21/2018 10:32 AM, Marek Marczykowski-Górecki wrote:
> Preparation for global nestedhvm configuration - libxlMakeDomBuildInfo
> needs access to libxlDriverConfig.
> No functional change.
> 
> Adjusting tests require slightly more mockup functions, because of
> libxlDriverConfigNew() call.
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek at invisiblethingslab.com>
> ---
> Changes since v4:
>   - drop now unneeded parameters
> Changes since v3:
>   - new patch, preparation
> ---
>   src/libxl/libxl_conf.c         | 13 +++++++------
>   src/libxl/libxl_conf.h         |  4 +---
>   src/libxl/libxl_domain.c       |  2 +-
>   tests/libxlxml2domconfigtest.c | 23 ++++++++++++++++-------
>   tests/virmocklibxl.c           | 25 +++++++++++++++++++++++++
>   5 files changed, 50 insertions(+), 17 deletions(-)

I had to rebase this patch to account for commits 7ebc4f2a4c, 4c9c7a5ba2, and 
c391e07eb0.

> 
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 2d2a707..e7727a1 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -271,10 +271,11 @@ libxlMakeChrdevStr(virDomainChrDefPtr def, char **buf)
>   
>   static int
>   libxlMakeDomBuildInfo(virDomainDefPtr def,
> -                      libxl_ctx *ctx,
> +                      libxlDriverConfigPtr cfg,
>                         virCapsPtr caps,
>                         libxl_domain_config *d_config)
>   {
> +    libxl_ctx *ctx = cfg->ctx;
>       libxl_domain_build_info *b_info = &d_config->b_info;
>       int hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM;
>       size_t i;
> @@ -2287,17 +2288,17 @@ libxlDriverNodeGetInfo(libxlDriverPrivatePtr driver, virNodeInfoPtr info)
>   int
>   libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
>                          virDomainDefPtr def,
> -                       const char *channelDir LIBXL_ATTR_UNUSED,
> -                       libxl_ctx *ctx,
> -                       virCapsPtr caps,
> +                       libxlDriverConfigPtr cfg,
>                          libxl_domain_config *d_config)
>   {
> +    virCapsPtr caps = cfg->caps;
> +    libxl_ctx *ctx = cfg->ctx;
>       libxl_domain_config_init(d_config);
>   
>       if (libxlMakeDomCreateInfo(ctx, def, &d_config->c_info) < 0)
>           return -1;
>   
> -    if (libxlMakeDomBuildInfo(def, ctx, caps, d_config) < 0)
> +    if (libxlMakeDomBuildInfo(def, cfg, caps, d_config) < 0)
>           return -1;
>   
>   #ifdef LIBXL_HAVE_VNUMA
> @@ -2329,7 +2330,7 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
>   #endif
>   
>   #ifdef LIBXL_HAVE_DEVICE_CHANNEL
> -    if (libxlMakeChannelList(channelDir, def, d_config) < 0)
> +    if (libxlMakeChannelList(cfg->channelDir, def, d_config) < 0)
>           return -1;
>   #endif
>   
> diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
> index 264df11..ce9db26 100644
> --- a/src/libxl/libxl_conf.h
> +++ b/src/libxl/libxl_conf.h
> @@ -215,9 +215,7 @@ libxlCreateXMLConf(void);
>   int
>   libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
>                          virDomainDefPtr def,
> -                       const char *channelDir LIBXL_ATTR_UNUSED,
> -                       libxl_ctx *ctx,
> -                       virCapsPtr caps,
> +                       libxlDriverConfigPtr cfg,
>                          libxl_domain_config *d_config);
>   
>   static inline void
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index 395c8a9..8879481 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -1253,7 +1253,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver,
>           goto cleanup_dom;
>   
>       if (libxlBuildDomainConfig(driver->reservedGraphicsPorts, vm->def,
> -                               cfg->channelDir, cfg->ctx, cfg->caps, &d_config) < 0)
> +                               cfg, &d_config) < 0)
>           goto cleanup_dom;
>   
>       if (cfg->autoballoon && libxlDomainFreeMem(cfg->ctx, &d_config) < 0)
> diff --git a/tests/libxlxml2domconfigtest.c b/tests/libxlxml2domconfigtest.c
> index bd4c3af..cfbc37c 100644
> --- a/tests/libxlxml2domconfigtest.c
> +++ b/tests/libxlxml2domconfigtest.c
> @@ -56,8 +56,8 @@ testCompareXMLToDomConfig(const char *xmlfile,
>       int ret = -1;
>       libxl_domain_config actualconfig;
>       libxl_domain_config expectconfig;
> +    libxlDriverConfigPtr cfg;
>       xentoollog_logger *log = NULL;
> -    libxl_ctx *ctx = NULL;
>       virPortAllocatorPtr gports = NULL;
>       virDomainXMLOptionPtr xmlopt = NULL;
>       virDomainDefPtr vmdef = NULL;
> @@ -68,10 +68,18 @@ testCompareXMLToDomConfig(const char *xmlfile,
>       libxl_domain_config_init(&actualconfig);
>       libxl_domain_config_init(&expectconfig);
>   
> +    if (!(cfg = libxlDriverConfigNew()))
> +        goto cleanup;
> +
> +    cfg->caps = caps;
> +

Before pushing this series I decided to build test it on several Xen releases I 
had readily available: 4.10, 4.9, 4.7, and 4.5. It works fine on all of those 
except 4.5. libxlxml2domconfigtest segfaults here on the first test

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff3d60244 in pthread_mutex_lock () from /lib64/libpthread.so.0
(gdb) bt
#0  0x00007ffff3d60244 in pthread_mutex_lock () from /lib64/libpthread.so.0
#1  0x00007ffff75250c6 in xs_talkv (h=h at entry=0x1, t=t at entry=0,
     type=type at entry=XS_READ, iovec=iovec at entry=0x7fffffffce50,
     num_vecs=num_vecs at entry=1, len=len at entry=0x0) at xs.c:491
#2  0x00007ffff752544a in xs_single (h=0x1, t=t at entry=0,
     type=type at entry=XS_READ,
     string=string at entry=0x7ffff79a4b70 "/local/domain/0/memory/freemem-slack",
     len=len at entry=0x0) at xs.c:551
#3  0x00007ffff75257e4 in xs_read (h=<optimized out>, t=t at entry=0,
     path=path at entry=0x7ffff79a4b70 "/local/domain/0/memory/freemem-slack",
     len=len at entry=0x0) at xs.c:597
#4  0x00007ffff7978ca9 in libxl__xs_read (gc=gc at entry=0x7fffffffcf40,
     t=t at entry=0,
     path=path at entry=0x7ffff79a4b70 "/local/domain/0/memory/freemem-slack")
     at libxl_xshelp.c:123
#5  0x00007ffff79622ab in libxl__get_free_memory_slack (
     gc=gc at entry=0x7fffffffcf40,
     free_mem_slack=free_mem_slack at entry=0x7fffffffcf3c) at libxl.c:5122
#6  0x00007ffff796238e in libxl_get_free_memory (ctx=<optimized out>,
     memkb=0x7fffffffd014) at libxl.c:5394
#7  0x000000000041313b in libxlDriverConfigNew () at libxl/libxl_conf.c:1690
#8  0x000000000040aff0 in testCompareXMLToDomConfig (
     xmlfile=0x63bd20 
"/home/jfehlig/virt/upstream/libvirt/tests/libxlxml2domconfigdata/basic-pv.xml",
     jsonfile=0x63be10 
"/home/jfehlig/virt/upstream/libvirt/tests/libxlxml2domconfigdata/basic-pv.json") 
at libxlxml2domconfigtest.c:71
#9  0x000000000040b4b1 in testCompareXMLToDomConfigHelper (
     data=0x637c50 <info>) at libxlxml2domconfigtest.c:164
#10 0x000000000040bcd4 in virTestRun (
     title=0x42dcc0 "LibXL XML-2-JSON basic-pv",
     body=0x40b3cb <testCompareXMLToDomConfigHelper>, data=0x637c50 <info>)
     at testutils.c:180

I wonder how much we care about Xen 4.5? According to the Xen wiki [0] it is no 
longer supported upstream. 4.6 gets security support until Oct 2018, so IMO it 
should be the minimum version supported by upstream libvirt. Or maybe 4.7 since 
that is the earliest version still getting general upstream support?

Regardless, I'll wait to push the series until we get this hashed out.

Regards,
Jim

[0] https://wiki.xenproject.org/wiki/Xen_Project_Release_Features

>       if (!(log = (xentoollog_logger *)xtl_createlogger_stdiostream(stderr, XTL_DEBUG, 0)))
>           goto cleanup;
>   
> -    if (libxl_ctx_alloc(&ctx, LIBXL_VERSION, 0, log) < 0)
> +    /* replace logger with stderr one */
> +    libxl_ctx_free(cfg->ctx);
> +
> +    if (libxl_ctx_alloc(&cfg->ctx, LIBXL_VERSION, 0, log) < 0)
>           goto cleanup;
>   
>       if (!(gports = virPortAllocatorNew("vnc", 5900, 6000,
> @@ -85,22 +93,22 @@ testCompareXMLToDomConfig(const char *xmlfile,
>                                           NULL, VIR_DOMAIN_XML_INACTIVE)))
>           goto cleanup;
>   
> -    if (libxlBuildDomainConfig(gports, vmdef, NULL, ctx, caps, &actualconfig) < 0)
> +    if (libxlBuildDomainConfig(gports, vmdef, cfg, &actualconfig) < 0)
>           goto cleanup;
>   
> -    if (!(actualjson = libxl_domain_config_to_json(ctx, &actualconfig))) {
> +    if (!(actualjson = libxl_domain_config_to_json(cfg->ctx, &actualconfig))) {
>           virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                          "Failed to retrieve JSON doc for libxl_domain_config");
>           goto cleanup;
>       }
>   
>       virTestLoadFile(jsonfile, &tempjson);
> -    if (libxl_domain_config_from_json(ctx, &expectconfig, tempjson) != 0) {
> +    if (libxl_domain_config_from_json(cfg->ctx, &expectconfig, tempjson) != 0) {
>           virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                          "Failed to create libxl_domain_config from JSON doc");
>           goto cleanup;
>       }
> -    if (!(expectjson = libxl_domain_config_to_json(ctx, &expectconfig))) {
> +    if (!(expectjson = libxl_domain_config_to_json(cfg->ctx, &expectconfig))) {
>           virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                          "Failed to retrieve JSON doc for libxl_domain_config");
>           goto cleanup;
> @@ -118,10 +126,11 @@ testCompareXMLToDomConfig(const char *xmlfile,
>       virDomainDefFree(vmdef);
>       virObjectUnref(gports);
>       virObjectUnref(xmlopt);
> -    libxl_ctx_free(ctx);
>       libxl_domain_config_dispose(&actualconfig);
>       libxl_domain_config_dispose(&expectconfig);
>       xtl_logger_destroy(log);
> +    cfg->caps = NULL;
> +    virObjectUnref(cfg);
>       return ret;
>   }
>   
> diff --git a/tests/virmocklibxl.c b/tests/virmocklibxl.c
> index 747f9f8..133064b 100644
> --- a/tests/virmocklibxl.c
> +++ b/tests/virmocklibxl.c
> @@ -27,6 +27,7 @@
>   # include <sys/stat.h>
>   # include <unistd.h>
>   # include <libxl.h>
> +# include <util/virfile.h>
>   # include <xenstore.h>
>   # include <xenctrl.h>
>   
> @@ -48,6 +49,19 @@ VIR_MOCK_IMPL_RET_ARGS(xc_interface_open,
>   }
>   
>   
> +VIR_MOCK_IMPL_RET_ARGS(libxl_get_version_info,
> +                       const libxl_version_info*,
> +                       libxl_ctx *, ctx)
> +{
> +    static libxl_version_info info;
> +
> +    memset(&info, 0, sizeof(info));
> +
> +    return &info;
> +    /* silence gcc warning */
> +    return real_libxl_get_version_info(ctx);
> +}
> +
>   VIR_MOCK_STUB_RET_ARGS(xc_interface_close,
>                          int, 0,
>                          xc_interface *, handle)
> @@ -68,6 +82,17 @@ VIR_MOCK_STUB_RET_ARGS(xc_sharing_used_frames,
>   VIR_MOCK_STUB_VOID_ARGS(xs_daemon_close,
>                           struct xs_handle *, handle)
>   
> +VIR_MOCK_IMPL_RET_ARGS(virFileMakePath, int,
> +                       const char *, path)
> +{
> +    /* replace log path with a writable directory */
> +    if (strstr(path, "/log/")) {
> +        snprintf((char*)path, strlen(path), ".");
> +        return 0;
> +    }
> +    return real_virFileMakePath(path);
> +}
> +
>   VIR_MOCK_IMPL_RET_ARGS(__xstat, int,
>                          int, ver,
>                          const char *, path,
> 




More information about the libvir-list mailing list