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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Mar 22, 2018 at 04:48:48PM -0600, Jim Fehlig wrote:
> On 03/22/2018 04:44 PM, Jim Fehlig wrote:
> > 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@xxxxxxxxxxxxxxxxxxxxxx>
> > > ---
> > > 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@entry=0x1, t=t@entry=0,
> >      type=type@entry=XS_READ, iovec=iovec@entry=0x7fffffffce50,
> >      num_vecs=num_vecs@entry=1, len=len@entry=0x0) at xs.c:491
> > #2  0x00007ffff752544a in xs_single (h=0x1, t=t@entry=0,
> >      type=type@entry=XS_READ,
> >      string=string@entry=0x7ffff79a4b70 "/local/domain/0/memory/freemem-slack",
> >      len=len@entry=0x0) at xs.c:551
> > #3  0x00007ffff75257e4 in xs_read (h=<optimized out>, t=t@entry=0,
> >      path=path@entry=0x7ffff79a4b70 "/local/domain/0/memory/freemem-slack",
> >      len=len@entry=0x0) at xs.c:597
> > #4  0x00007ffff7978ca9 in libxl__xs_read (gc=gc@entry=0x7fffffffcf40,
> >      t=t@entry=0,
> >      path=path@entry=0x7ffff79a4b70 "/local/domain/0/memory/freemem-slack")
> >      at libxl_xshelp.c:123
> > #5  0x00007ffff79622ab in libxl__get_free_memory_slack (
> >      gc=gc@entry=0x7fffffffcf40,
> >      free_mem_slack=free_mem_slack@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?
> 
> BTW, I forgot to mention that we currently claim support for Xen >= 4.4. See
> m4/virt-driver-libxl.m4.

Does it work before applying this series?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux