8:25 a.m.
Refactor libxlDriverConfigNew to remove the need
for mocking virFilePath and add libxlDomainGetEmulatorType
to the mock to remove the need to invoke a binary
for nearly every domain we parse
Ján Tomko (7):
testutilsxen: error out on initialization failure
libxl: conf: move default keepalive settings to libxlDriverConfigNew
libxl: StateInitialize: use g_autofree
libxl: split out DriverConfigInit out of DriverConfigNew
libxl: do not mock virFileMakePath
tests: link the libxl tests with libxltestdriver.la
tests: libxl: do not run the emulator
src/libxl/libxl_capabilities.h | 3 +-
src/libxl/libxl_conf.c | 85 ++++++++++++++++++----------------
src/libxl/libxl_conf.h | 2 +
src/libxl/libxl_driver.c | 7 +--
tests/Makefile.am | 9 ++--
tests/libxlmock.c | 18 +++----
tests/testutilsxen.c | 9 +++-
7 files changed, 75 insertions(+), 58 deletions(-)
--
2.24.1
8:25 a.m.
New subject: [libvirt PATCH 1/7] testutilsxen: error out on initialization failure
libxlDriverConfigNew can possibly fail on wrong
firmware values (unlikely) or on failure to create
the log directory (possible if you're debugging
tests with VIR_FILE_ACCESS)
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
Fixes: 4a4132b4625778cf80acb9c92d06351b44468ac3
---
tests/testutilsxen.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tests/testutilsxen.c b/tests/testutilsxen.c
index b73c79581d..76da33826c 100644
--- a/tests/testutilsxen.c
+++ b/tests/testutilsxen.c
@@ -94,7 +94,8 @@ libxlDriverPrivatePtr testXLInitDriver(void)
return NULL;
}
- driver->config = libxlDriverConfigNew();
+ if (!(driver->config = libxlDriverConfigNew()))
+ return NULL;
driver->config->caps = testXLInitCaps();
--
2.24.1
8:25 a.m.
New subject: [libvirt PATCH 2/7] libxl: conf: move default keepalive settings to libxlDriverConfigNew
These hardcoded defaults do not need to be read from
the file. Move them out of libxlDriverConfigLoadFile.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/libxl/libxl_conf.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 56deca7b7c..9c722342ba 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1763,6 +1763,10 @@ libxlDriverConfigNew(void)
goto error;
cfg->firmwares[cfg->nfirmwares - 1]->name = g_strdup(LIBXL_FIRMWARE_DIR
"/hvmloader");
+ /* defaults for keepalive messages */
+ cfg->keepAliveInterval = 5;
+ cfg->keepAliveCount = 5;
+
return cfg;
error:
@@ -1786,10 +1790,6 @@ int libxlDriverConfigLoadFile(libxlDriverConfigPtr cfg,
{
g_autoptr(virConf) conf = NULL;
- /* defaults for keepalive messages */
- cfg->keepAliveInterval = 5;
- cfg->keepAliveCount = 5;
-
/* Check the file is readable before opening it, otherwise
* libvirt emits an error.
*/
--
2.24.1
8:25 a.m.
New subject: [libvirt PATCH 3/7] libxl: StateInitialize: use g_autofree
Use g_autofree to free the driver config file path.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/libxl/libxl_driver.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 2b06f1be1e..0b88c9f9d9 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -652,7 +652,7 @@ libxlStateInitialize(bool privileged,
void *opaque)
{
libxlDriverConfigPtr cfg;
- char *driverConf = NULL;
+ g_autofree char *driverConf = NULL;
char ebuf[1024];
bool autostart = true;
@@ -706,7 +706,6 @@ libxlStateInitialize(bool privileged,
if (libxlDriverConfigLoadFile(cfg, driverConf) < 0)
goto error;
- VIR_FREE(driverConf);
/* Register the callbacks providing access to libvirt's event loop */
libxl_osevent_register_hooks(cfg->ctx, &libxl_osevent_callbacks,
cfg->ctx);
@@ -822,7 +821,6 @@ libxlStateInitialize(bool privileged,
return VIR_DRV_STATE_INIT_COMPLETE;
error:
- VIR_FREE(driverConf);
libxlStateCleanup();
return VIR_DRV_STATE_INIT_ERROR;
}
--
2.24.1
8:25 a.m.
New subject: [libvirt PATCH 4/7] libxl: split out DriverConfigInit out of DriverConfigNew
Take the parts affected by the host state out of DriverConfigNew
and put them into a separate function.
Adjust all the callers to call both functions.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
src/libxl/libxl_conf.c | 81 ++++++++++++++++++++++------------------
src/libxl/libxl_conf.h | 2 +
src/libxl/libxl_driver.c | 3 ++
tests/testutilsxen.c | 3 ++
4 files changed, 52 insertions(+), 37 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 9c722342ba..907df525c5 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1686,8 +1686,6 @@ libxlDriverConfigPtr
libxlDriverConfigNew(void)
{
libxlDriverConfigPtr cfg;
- char ebuf[1024];
- unsigned int free_mem;
if (libxlConfigInitialize() < 0)
return NULL;
@@ -1705,41 +1703,6 @@ libxlDriverConfigNew(void)
cfg->autoDumpDir = g_strdup(LIBXL_DUMP_DIR);
cfg->channelDir = g_strdup(LIBXL_CHANNEL_DIR);
- if (virFileMakePath(cfg->logDir) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("failed to create log dir '%s': %s"),
- cfg->logDir,
- virStrerror(errno, ebuf, sizeof(ebuf)));
- goto error;
- }
-
- cfg->logger = libxlLoggerNew(cfg->logDir, virLogGetDefaultPriority());
- if (!cfg->logger) {
- VIR_ERROR(_("cannot create logger for libxenlight, disabling
driver"));
- goto error;
- }
-
- if (libxl_ctx_alloc(&cfg->ctx, LIBXL_VERSION, 0, (xentoollog_logger
*)cfg->logger)) {
- VIR_ERROR(_("cannot initialize libxenlight context, probably not "
- "running in a Xen Dom0, disabling driver"));
- goto error;
- }
-
- if ((cfg->verInfo = libxl_get_version_info(cfg->ctx)) == NULL) {
- VIR_ERROR(_("cannot version information from libxenlight, "
- "disabling driver"));
- goto error;
- }
- cfg->version = (cfg->verInfo->xen_version_major * 1000000) +
- (cfg->verInfo->xen_version_minor * 1000);
-
- /* This will fill xenstore info about free and dom0 memory if missing,
- * should be called before starting first domain */
- if (libxl_get_free_memory(cfg->ctx, &free_mem)) {
- VIR_ERROR(_("Unable to configure libxl's memory management
parameters"));
- goto error;
- }
-
#ifdef DEFAULT_LOADER_NVRAM
if (virFirmwareParseList(DEFAULT_LOADER_NVRAM,
&cfg->firmwares,
@@ -1774,6 +1737,50 @@ libxlDriverConfigNew(void)
return NULL;
}
+int
+libxlDriverConfigInit(libxlDriverConfigPtr cfg)
+{
+ char ebuf[1024];
+ unsigned int free_mem;
+
+ if (virFileMakePath(cfg->logDir) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to create log dir '%s': %s"),
+ cfg->logDir,
+ virStrerror(errno, ebuf, sizeof(ebuf)));
+ return -1;
+ }
+
+ cfg->logger = libxlLoggerNew(cfg->logDir, virLogGetDefaultPriority());
+ if (!cfg->logger) {
+ VIR_ERROR(_("cannot create logger for libxenlight, disabling
driver"));
+ return -1;
+ }
+
+ if (libxl_ctx_alloc(&cfg->ctx, LIBXL_VERSION, 0, (xentoollog_logger
*)cfg->logger)) {
+ VIR_ERROR(_("cannot initialize libxenlight context, probably not "
+ "running in a Xen Dom0, disabling driver"));
+ return -1;
+ }
+
+ if ((cfg->verInfo = libxl_get_version_info(cfg->ctx)) == NULL) {
+ VIR_ERROR(_("cannot version information from libxenlight, "
+ "disabling driver"));
+ return -1;
+ }
+ cfg->version = (cfg->verInfo->xen_version_major * 1000000) +
+ (cfg->verInfo->xen_version_minor * 1000);
+
+ /* This will fill xenstore info about free and dom0 memory if missing,
+ * should be called before starting first domain */
+ if (libxl_get_free_memory(cfg->ctx, &free_mem)) {
+ VIR_ERROR(_("Unable to configure libxl's memory management
parameters"));
+ return -1;
+ }
+
+ return 0;
+}
+
libxlDriverConfigPtr
libxlDriverConfigGet(libxlDriverPrivatePtr driver)
{
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index b61c52f1a5..07b3373170 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -164,6 +164,8 @@ struct _libxlSavefileHeader {
libxlDriverConfigPtr
libxlDriverConfigNew(void);
+int
+libxlDriverConfigInit(libxlDriverConfigPtr cfg);
libxlDriverConfigPtr
libxlDriverConfigGet(libxlDriverPrivatePtr driver);
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 0b88c9f9d9..d10529ac5b 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -707,6 +707,9 @@ libxlStateInitialize(bool privileged,
if (libxlDriverConfigLoadFile(cfg, driverConf) < 0)
goto error;
+ if (libxlDriverConfigInit(cfg) < 0)
+ goto error;
+
/* Register the callbacks providing access to libvirt's event loop */
libxl_osevent_register_hooks(cfg->ctx, &libxl_osevent_callbacks,
cfg->ctx);
diff --git a/tests/testutilsxen.c b/tests/testutilsxen.c
index 76da33826c..b1260dcebf 100644
--- a/tests/testutilsxen.c
+++ b/tests/testutilsxen.c
@@ -97,6 +97,9 @@ libxlDriverPrivatePtr testXLInitDriver(void)
if (!(driver->config = libxlDriverConfigNew()))
return NULL;
+ if (libxlDriverConfigInit(driver->config) < 0)
+ return NULL;
+
driver->config->caps = testXLInitCaps();
driver->xmlopt = libxlCreateXMLConf(driver);
--
2.24.1
8:25 a.m.
New subject: [libvirt PATCH 5/7] libxl: do not mock virFileMakePath
Point the logDir to abs_builddir instead.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tests/libxlmock.c | 11 -----------
tests/testutilsxen.c | 3 +++
2 files changed, 3 insertions(+), 11 deletions(-)
diff --git a/tests/libxlmock.c b/tests/libxlmock.c
index b995b34cc5..60e6b78129 100644
--- a/tests/libxlmock.c
+++ b/tests/libxlmock.c
@@ -94,17 +94,6 @@ 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/")) {
- g_snprintf((char*)path, strlen(path), ".");
- return 0;
- }
- return real_virFileMakePath(path);
-}
-
VIR_MOCK_IMPL_RET_ARGS(__xstat, int,
int, ver,
const char *, path,
diff --git a/tests/testutilsxen.c b/tests/testutilsxen.c
index b1260dcebf..d50c3003da 100644
--- a/tests/testutilsxen.c
+++ b/tests/testutilsxen.c
@@ -97,6 +97,9 @@ libxlDriverPrivatePtr testXLInitDriver(void)
if (!(driver->config = libxlDriverConfigNew()))
return NULL;
+ g_free(driver->config->logDir);
+ driver->config->logDir = g_strdup(abs_builddir);
+
if (libxlDriverConfigInit(driver->config) < 0)
return NULL;
--
2.24.1
8:25 a.m.
New subject: [libvirt PATCH 6/7] tests: link the libxl tests with libxltestdriver.la
This lets us mock functions from the libxl driver.
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
---
tests/Makefile.am | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index ed5255b62d..761c989e86 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -505,17 +505,20 @@ libxltestdriver_la_LIBADD = $(libxl_LDADDS)
xlconfigtest_SOURCES = \
xlconfigtest.c testutilsxen.c testutilsxen.h \
testutils.c testutils.h
-xlconfigtest_LDADD =$(libxl_LDADDS)
+xlconfigtest_LDADD = libxltestdriver.la \
+ $(libxl_LDADDS)
xmconfigtest_SOURCES = \
xmconfigtest.c testutilsxen.c testutilsxen.h \
testutils.c testutils.h
-xmconfigtest_LDADD = $(libxl_LDADDS)
+xmconfigtest_LDADD = libxltestdriver.la \
+ $(libxl_LDADDS)
libxlxml2domconfigtest_SOURCES = \
libxlxml2domconfigtest.c testutilsxen.c testutilsxen.h \
testutils.c testutils.h
-libxlxml2domconfigtest_LDADD = $(libxl_LDADDS) $(LIBXML_LIBS)
+libxlxml2domconfigtest_LDADD = libxltestdriver.la \
+ $(libxl_LDADDS) $(LIBXML_LIBS)
libxlmock_la_SOURCES = \
libxlmock.c
--
2.24.1
8:25 a.m.
New subject: [libvirt PATCH 7/7] tests: libxl: do not run the emulator
Ever since commit c5a00350 the libxl parser invokes the emulator
to probe which device model to use.
Commit b90c4b5 introduced a workaround that used a stable path
which was very likely to result in the answer matching the default.
However the test is still affected by the host state and the binary
gets invoked if present.
Mock the libxlDomainGetEmulatorType function to stop wasting CPU
cycles every time a 'make check' is run on a system with xen installed.
For example xlconfigtest gets faster by 90 %
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
Fixes: b90c4b5f505698d600303c5b4f03f5d229b329dd
---
src/libxl/libxl_capabilities.h | 3 ++-
tests/libxlmock.c | 7 +++++++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/libxl/libxl_capabilities.h b/src/libxl/libxl_capabilities.h
index 9a7c8bf636..9efb836429 100644
--- a/src/libxl/libxl_capabilities.h
+++ b/src/libxl/libxl_capabilities.h
@@ -50,4 +50,5 @@ libxlMakeDomainCapabilities(virDomainCapsPtr domCaps,
size_t nfirmwares);
int
-libxlDomainGetEmulatorType(const virDomainDef *def);
+libxlDomainGetEmulatorType(const virDomainDef *def)
+ G_GNUC_NO_INLINE;
diff --git a/tests/libxlmock.c b/tests/libxlmock.c
index 60e6b78129..a36ca135f6 100644
--- a/tests/libxlmock.c
+++ b/tests/libxlmock.c
@@ -30,6 +30,7 @@
# include "virfile.h"
# include "virsocket.h"
+# include "libxl/libxl_capabilities.h"
VIR_MOCK_IMPL_RET_VOID(xs_daemon_open,
struct xs_handle *)
@@ -123,4 +124,10 @@ VIR_MOCK_IMPL_RET_ARGS(stat, int,
return real_stat(path, sb);
}
+int
+libxlDomainGetEmulatorType(const virDomainDef *def G_GNUC_UNUSED)
+{
+ return LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
+}
+
#endif /* WITH_LIBXL && WITH_YAJL */
--
2.24.1
5:41 p.m.
New subject: [libvirt PATCH 7/7] tests: libxl: do not run the emulator
On 2/22/20 7:25 AM, Ján Tomko wrote:
Ever since commit c5a00350 the libxl parser invokes the emulator
to probe which device model to use.
Commit b90c4b5 introduced a workaround that used a stable path
which was very likely to result in the answer matching the default.
However the test is still affected by the host state and the binary
gets invoked if present.
Mock the libxlDomainGetEmulatorType function to stop wasting CPU
cycles every time a 'make check' is run on a system with xen installed.
For example xlconfigtest gets faster by 90 %
Signed-off-by: Ján Tomko <jtomko(a)redhat.com>
Fixes: b90c4b5f505698d600303c5b4f03f5d229b329dd
---
src/libxl/libxl_capabilities.h | 3 ++-
tests/libxlmock.c | 7 +++++++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/libxl/libxl_capabilities.h b/src/libxl/libxl_capabilities.h
index 9a7c8bf636..9efb836429 100644
--- a/src/libxl/libxl_capabilities.h
+++ b/src/libxl/libxl_capabilities.h
@@ -50,4 +50,5 @@ libxlMakeDomainCapabilities(virDomainCapsPtr domCaps,
size_t nfirmwares);
int
-libxlDomainGetEmulatorType(const virDomainDef *def);
+libxlDomainGetEmulatorType(const virDomainDef *def)
+ G_GNUC_NO_INLINE;
Ah, I think the lack of this change caused me grief in the past while fiddling
with the mocking.
Regards,
Jim
diff --git a/tests/libxlmock.c b/tests/libxlmock.c
index 60e6b78129..a36ca135f6 100644
--- a/tests/libxlmock.c
+++ b/tests/libxlmock.c
@@ -30,6 +30,7 @@
# include "virfile.h"
# include "virsocket.h"
+# include "libxl/libxl_capabilities.h"
VIR_MOCK_IMPL_RET_VOID(xs_daemon_open,
struct xs_handle *)
@@ -123,4 +124,10 @@ VIR_MOCK_IMPL_RET_ARGS(stat, int,
return real_stat(path, sb);
}
+int
+libxlDomainGetEmulatorType(const virDomainDef *def G_GNUC_UNUSED)
+{
+ return LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
+}
+
#endif /* WITH_LIBXL && WITH_YAJL */
5:42 p.m.
On 2/22/20 7:25 AM, Ján Tomko wrote:
Refactor libxlDriverConfigNew to remove the need
for mocking virFilePath and add libxlDomainGetEmulatorType
to the mock to remove the need to invoke a binary
for nearly every domain we parse
Ján Tomko (7):
testutilsxen: error out on initialization failure
libxl: conf: move default keepalive settings to libxlDriverConfigNew
libxl: StateInitialize: use g_autofree
libxl: split out DriverConfigInit out of DriverConfigNew
libxl: do not mock virFileMakePath
tests: link the libxl tests with libxltestdriver.la
tests: libxl: do not run the emulator
src/libxl/libxl_capabilities.h | 3 +-
src/libxl/libxl_conf.c | 85 ++++++++++++++++++----------------
src/libxl/libxl_conf.h | 2 +
src/libxl/libxl_driver.c | 7 +--
tests/Makefile.am | 9 ++--
tests/libxlmock.c | 18 +++----
tests/testutilsxen.c | 9 +++-
7 files changed, 75 insertions(+), 58 deletions(-)
For the series
Reviewed-by: Jim Fehlig <jfehlig(a)suse.com>
Regards,
Jim
1733
days inactive
1735
days old
9 comments
2 participants
participants (2)
-
Jim Fehlig -
Ján Tomko