On 03/22/2018 04:59 PM, Marek Marczykowski-Górecki wrote:
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(a)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@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?
Sorry, forgot to mention that too. Yes, it works before applying the series.
Regards,
Jim