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

2018-04-10 Thread Jim Fehlig 

On 04/06/2018 06:54 PM, Marek Marczykowski-Górecki wrote:

On Wed, Mar 28, 2018 at 01:42:47PM -0600, Jim Fehlig wrote:

On 03/27/2018 05:55 PM, Marek Marczykowski-Górecki wrote:

diff --git a/tests/virmocklibxl.c b/tests/virmocklibxl.c
index 747f9f8..28281b6 100644
--- a/tests/virmocklibxl.c
+++ b/tests/virmocklibxl.c
@@ -27,6 +27,7 @@
   # include 
   # include 
   # include 
+# include 
   # include 
   # include 
@@ -48,6 +49,24 @@ 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(, 0, sizeof(info));
+
+return 
+/* silence gcc warning */
+return real_libxl_get_version_info(ctx);
+}
+
+VIR_MOCK_STUB_RET_ARGS(libxl_get_free_memory,
+   int, 0,
+   libxl_ctx *, ctx,
+   uint32_t *, memkb);
+


This doesn't compile with Xen >= 4.8

In file included from virmocklibxl.c:26:0:
virmocklibxl.c:66:24: error: conflicting types for 'libxl_get_free_memory'
  VIR_MOCK_STUB_RET_ARGS(libxl_get_free_memory,
 ^
virmock.h:182:13: note: in definition of macro 'VIR_MOCK_STUB_RET_ARGS'
  rettype name(VIR_MOCK_ARGTYPENAMES_UNUSED(__VA_ARGS__)) \
  ^~~~
In file included from virmocklibxl.c:29:0:
/usr/include/libxl.h:1570:5: note: previous declaration of
'libxl_get_free_memory' was here
  int libxl_get_free_memory(libxl_ctx *ctx, uint64_t *memkb);
  ^

Using the uint32_t variant works in the libxl driver since we have
-DLIBXL_API_VERSION=0x040400 in LIBXL_CFLAGS. I worked around the
compilation failure with LIBXL_HAVE_MEMKB_64BITS,


I can't reproduce this problem, either with 4.8 or 4.10. Even more, if I
add alternative mock with uint64_t, under #if LIBXL_HAVE_MEMKB_64BITS, I
get compile failure, because of conflicting types (with
libxl_get_free_memory_0x040700)...

Can you confirm it's really a problem, not some mismatching header
versions on your side?


Perhaps I've also made a mistake rebasing some of these patches. Can you pretty 
please rebase against current master and repost a V7 (adding all the R-B)? If 
you say it passes 'make check' on 4.5 and 4.10, I'll chase down any problems on 
my side. Thanks!


Regards,
Jim

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

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

2018-04-06 Thread Marek Marczykowski-Górecki 
On Wed, Mar 28, 2018 at 01:42:47PM -0600, Jim Fehlig wrote:
> On 03/27/2018 05:55 PM, Marek Marczykowski-Górecki wrote:
> > diff --git a/tests/virmocklibxl.c b/tests/virmocklibxl.c
> > index 747f9f8..28281b6 100644
> > --- a/tests/virmocklibxl.c
> > +++ b/tests/virmocklibxl.c
> > @@ -27,6 +27,7 @@
> >   # include 
> >   # include 
> >   # include 
> > +# include 
> >   # include 
> >   # include 
> > @@ -48,6 +49,24 @@ 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(, 0, sizeof(info));
> > +
> > +return 
> > +/* silence gcc warning */
> > +return real_libxl_get_version_info(ctx);
> > +}
> > +
> > +VIR_MOCK_STUB_RET_ARGS(libxl_get_free_memory,
> > +   int, 0,
> > +   libxl_ctx *, ctx,
> > +   uint32_t *, memkb);
> > +
> 
> This doesn't compile with Xen >= 4.8
> 
> In file included from virmocklibxl.c:26:0:
> virmocklibxl.c:66:24: error: conflicting types for 'libxl_get_free_memory'
>  VIR_MOCK_STUB_RET_ARGS(libxl_get_free_memory,
> ^
> virmock.h:182:13: note: in definition of macro 'VIR_MOCK_STUB_RET_ARGS'
>  rettype name(VIR_MOCK_ARGTYPENAMES_UNUSED(__VA_ARGS__)) \
>  ^~~~
> In file included from virmocklibxl.c:29:0:
> /usr/include/libxl.h:1570:5: note: previous declaration of
> 'libxl_get_free_memory' was here
>  int libxl_get_free_memory(libxl_ctx *ctx, uint64_t *memkb);
>  ^
> 
> Using the uint32_t variant works in the libxl driver since we have
> -DLIBXL_API_VERSION=0x040400 in LIBXL_CFLAGS. I worked around the
> compilation failure with LIBXL_HAVE_MEMKB_64BITS, 

I can't reproduce this problem, either with 4.8 or 4.10. Even more, if I
add alternative mock with uint64_t, under #if LIBXL_HAVE_MEMKB_64BITS, I
get compile failure, because of conflicting types (with
libxl_get_free_memory_0x040700)...

Can you confirm it's really a problem, not some mismatching header
versions on your side?

> but then
> libxlxml2domconfigtest crashed.

I've got test failure, apparently because something have changed with
VGA model in json format. I'll remove it from the test, as it is
unrelated to CPUID.

-- 
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?


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2018-03-28 Thread Jim Fehlig 

On 03/27/2018 05:55 PM, 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 
---
Changes since v6:
  - tests: add libxl_get_free_memory mock needed on Xen 4.5
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   | 30 ++
  5 files changed, 55 insertions(+), 17 deletions(-)

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 = _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, _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, _config) 
< 0)
+   cfg, _config) < 0)
  goto cleanup_dom;
  
  if (cfg->autoballoon && libxlDomainFreeMem(cfg->ctx, _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();
  libxl_domain_config_init();
  
+if (!(cfg = libxlDriverConfigNew()))

+goto cleanup;
+
+cfg->caps = caps;
+
  if (!(log = (xentoollog_logger *)xtl_createlogger_stdiostream(stderr, 
XTL_DEBUG, 0)))
  goto cleanup;
  
-if (libxl_ctx_alloc(, LIBXL_VERSION, 0, log) < 0)

+/* replace logger with stderr one */
+libxl_ctx_free(cfg->ctx);
+
+if (libxl_ctx_alloc(>ctx, LIBXL_VERSION, 0, log) < 0)
  goto cleanup;
  
  if (!(gports = virPortAllocatorNew("vnc", 5900, 6000,

@@ -85,22