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

Jim Fehlig <jfehlig@xxxxxxxx> · Tue, 17 Apr 2018 14:00:25 -0600

On 04/11/2018 07:03 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 <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
Reviewed-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
---
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           | 31 +++++++++++++++++++++++++++++++
  5 files changed, 56 insertions(+), 17 deletions(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index ae369bc..2565f64 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -271,11 +271,12 @@ libxlMakeChrdevStr(virDomainChrDefPtr def, char **buf)
  
  static int
  libxlMakeDomBuildInfo(virDomainDefPtr def,
-                      libxl_ctx *ctx,
+                      libxlDriverConfigPtr cfg,
                        virCapsPtr caps,
                        libxl_domain_config *d_config)
  {
      virDomainClockDef clock = def->clock;
+    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;
@@ -2346,17 +2347,17 @@ libxlDriverNodeGetInfo(libxlDriverPrivatePtr driver, virNodeInfoPtr info)
  int
  libxlBuildDomainConfig(virPortAllocatorRangePtr 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
@@ -2388,7 +2389,7 @@ libxlBuildDomainConfig(virPortAllocatorRangePtr 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 0e85dff..633ebf5 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -215,9 +215,7 @@ libxlCreateXMLConf(void);
  int
  libxlBuildDomainConfig(virPortAllocatorRangePtr 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 ef9a902..0614589 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1261,7 +1261,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 6eec4c7..9d280e9 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;
      virPortAllocatorRangePtr 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;
+
      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 = virPortAllocatorRangeNew("vnc", 5900, 6000)))
@@ -84,22 +92,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;
@@ -122,10 +130,11 @@ testCompareXMLToDomConfig(const char *xmlfile,
      virDomainDefFree(vmdef);
      virPortAllocatorRangeFree(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 9735956..50ae258 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>
  # include <sys/socket.h>
@@ -49,6 +50,25 @@ 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));
+
+    /* silence gcc warning about unused function */
+    if (0)
+        real_libxl_get_version_info(ctx);
+    return &info;
+}
+
+VIR_MOCK_STUB_RET_ARGS(libxl_get_free_memory,
+                       int, 0,
+                       libxl_ctx *, ctx,
+                       uint32_t *, memkb);
+

This still fails for me on Xen 4.8 through 4.10

virmocklibxl.c:67: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);
     ^~~~~~~~~~~~~~~~~~~~~

Your response in the V6 thread about "conflicting types (with
libxl_get_free_memory_0x040700)" was a good hint. Things work fine for me on Xen 
4.4 through 4.10 with the following squashed in

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 2c0d1311c..8c4b6c220 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -528,6 +528,7 @@ libxlxml2domconfigtest_LDADD = $(libxl_LDADDS) $(LIBXML_LIBS)

 virmocklibxl_la_SOURCES = \
        virmocklibxl.c
+virmocklibxl_la_CFLAGS = $(LIBXL_CFLAGS)
 virmocklibxl_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
 virmocklibxl_la_LIBADD = $(MOCKLIBS_LIBS)

But I'm still confused as to how this works for you. Any idea about that? :-)

Regards,
Jim

  VIR_MOCK_STUB_RET_ARGS(xc_interface_close,
                         int, 0,
                         xc_interface *, handle)
@@ -75,6 +95,17 @@ VIR_MOCK_STUB_RET_ARGS(bind,
                         const struct sockaddr *, addr,
                         socklen_t, addrlen)
  
+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,


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







