[libvirt PATCH v2 0/1] tests: Introduce virhostdevmock

Changes from [v1]: * move function to a separate mock library; * load mock library for all tests that use virHostdevManager. [v1] https://www.redhat.com/archives/libvir-list/2020-May/msg00020.html Andrea Bolognani (1): tests: Introduce virhostdevmock tests/Makefile.am | 7 +++++++ tests/qemuhotplugtest.c | 1 + tests/virhostdevmock.c | 29 +++++++++++++++++++++++++++++ tests/virhostdevtest.c | 4 +++- 4 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 tests/virhostdevmock.c -- 2.25.4

We need this for all tests that use virHostdevManager, because during creation of this object for unprivileged connections like those used in the test suite we would end up writing inside the user's home directory. That's bad manners in general, but when running the test suite inside a purposefully constrained environment such as the one exposed by pbuilder, it turns into an outright test failure: Could not initialize HostdevManager - operation failed: Failed to create state dir '/nonexistent/.cache/libvirt/hostdevmgr' Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- tests/Makefile.am | 7 +++++++ tests/qemuhotplugtest.c | 1 + tests/virhostdevmock.c | 29 +++++++++++++++++++++++++++++ tests/virhostdevtest.c | 4 +++- 4 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 tests/virhostdevmock.c diff --git a/tests/Makefile.am b/tests/Makefile.am index ada5b8fc57..fc516376b4 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -212,6 +212,7 @@ test_libraries = libshunload.la \ libvirnetdaemonmock.la \ libvirnetserverclientmock.la \ libvircgroupmock.la \ + libvirhostdevmock.la \ libvirpcimock.la \ libvirnetdevmock.la \ libvirrandommock.la \ @@ -1226,6 +1227,12 @@ libvirfilecachemock_la_SOURCES = \ libvirfilecachemock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) libvirfilecachemock_la_LIBADD = $(MOCKLIBS_LIBS) +libvirhostdevmock_la_SOURCES = \ + virhostdevmock.c \ + $(NULL) +libvirhostdevmock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) +libvirhostdevmock_la_LIBADD = $(MOCKLIBS_LIBS) + if WITH_LINUX vircaps2xmltest_SOURCES = \ vircaps2xmltest.c testutils.h testutils.c virfilewrapper.h virfilewrapper.c diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 9a215ab303..cf87de187f 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -900,6 +900,7 @@ mymain(void) } VIR_TEST_MAIN_PRELOAD(mymain, + VIR_TEST_MOCK("virhostdev"), VIR_TEST_MOCK("virpci"), VIR_TEST_MOCK("domaincaps"), VIR_TEST_MOCK("virprocess"), diff --git a/tests/virhostdevmock.c b/tests/virhostdevmock.c new file mode 100644 index 0000000000..8658d9affd --- /dev/null +++ b/tests/virhostdevmock.c @@ -0,0 +1,29 @@ +/* + * Copyright (C) 2020 Red Hat, Inc. + * SPDX-License-Identifier: LGPL-2.1-or-later + */ + +#include <config.h> + +#include "virutil.h" +#include "virmock.h" + +static char *(*real_virGetUserRuntimeDirectory)(void); + +static void +init_syms(void) +{ + if (real_virGetUserRuntimeDirectory) + return; + + VIR_MOCK_REAL_INIT(virGetUserRuntimeDirectory); +} + +char * +virGetUserRuntimeDirectory(void) +{ + init_syms(); + + return g_build_filename(g_getenv("LIBVIRT_FAKE_ROOT_DIR"), + "user-runtime-directory", NULL); +} diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c index b6260bd9c1..b0bad683a8 100644 --- a/tests/virhostdevtest.c +++ b/tests/virhostdevtest.c @@ -628,7 +628,9 @@ mymain(void) return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -VIR_TEST_MAIN_PRELOAD(mymain, VIR_TEST_MOCK("virpci")) +VIR_TEST_MAIN_PRELOAD(mymain, + VIR_TEST_MOCK("virhostdev"), + VIR_TEST_MOCK("virpci")) #else int main(void) -- 2.25.4

On 5/6/20 2:54 PM, Andrea Bolognani wrote:
We need this for all tests that use virHostdevManager, because during creation of this object for unprivileged connections like those used in the test suite we would end up writing inside the user's home directory.
That's bad manners in general, but when running the test suite inside a purposefully constrained environment such as the one exposed by pbuilder, it turns into an outright test failure:
Could not initialize HostdevManager - operation failed: Failed to create state dir '/nonexistent/.cache/libvirt/hostdevmgr'
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- tests/Makefile.am | 7 +++++++ tests/qemuhotplugtest.c | 1 + tests/virhostdevmock.c | 29 +++++++++++++++++++++++++++++ tests/virhostdevtest.c | 4 +++- 4 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 tests/virhostdevmock.c
diff --git a/tests/Makefile.am b/tests/Makefile.am index ada5b8fc57..fc516376b4 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -212,6 +212,7 @@ test_libraries = libshunload.la \ libvirnetdaemonmock.la \ libvirnetserverclientmock.la \ libvircgroupmock.la \ + libvirhostdevmock.la \ libvirpcimock.la \ libvirnetdevmock.la \ libvirrandommock.la \ @@ -1226,6 +1227,12 @@ libvirfilecachemock_la_SOURCES = \ libvirfilecachemock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) libvirfilecachemock_la_LIBADD = $(MOCKLIBS_LIBS)
+libvirhostdevmock_la_SOURCES = \ + virhostdevmock.c \ + $(NULL) +libvirhostdevmock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) +libvirhostdevmock_la_LIBADD = $(MOCKLIBS_LIBS) + if WITH_LINUX vircaps2xmltest_SOURCES = \ vircaps2xmltest.c testutils.h testutils.c virfilewrapper.h virfilewrapper.c diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 9a215ab303..cf87de187f 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -900,6 +900,7 @@ mymain(void) }
VIR_TEST_MAIN_PRELOAD(mymain, + VIR_TEST_MOCK("virhostdev"), VIR_TEST_MOCK("virpci"), VIR_TEST_MOCK("domaincaps"), VIR_TEST_MOCK("virprocess"), diff --git a/tests/virhostdevmock.c b/tests/virhostdevmock.c new file mode 100644 index 0000000000..8658d9affd --- /dev/null +++ b/tests/virhostdevmock.c @@ -0,0 +1,29 @@ +/* + * Copyright (C) 2020 Red Hat, Inc. + * SPDX-License-Identifier: LGPL-2.1-or-later + */ + +#include <config.h> + +#include "virutil.h" +#include "virmock.h" + +static char *(*real_virGetUserRuntimeDirectory)(void);
This is not needed, you are not calling the original function anywhere. Moreover, the original function must be marked as attribute no-inline. Squash this in: diff --git i/src/util/virutil.h w/src/util/virutil.h index ee23f0c1f4..49b4bf440f 100644 --- i/src/util/virutil.h +++ w/src/util/virutil.h @@ -99,7 +99,7 @@ char *virGetUserDirectory(void); char *virGetUserDirectoryByUID(uid_t uid); char *virGetUserConfigDirectory(void); char *virGetUserCacheDirectory(void); -char *virGetUserRuntimeDirectory(void); +char *virGetUserRuntimeDirectory(void) G_GNUC_NO_INLINE; char *virGetUserShell(uid_t uid); char *virGetUserName(uid_t uid) G_GNUC_NO_INLINE; char *virGetGroupName(gid_t gid) G_GNUC_NO_INLINE; diff --git i/tests/virhostdevmock.c w/tests/virhostdevmock.c index 8658d9affd..9b0e4dc2b0 100644 --- i/tests/virhostdevmock.c +++ w/tests/virhostdevmock.c @@ -6,24 +6,10 @@ #include <config.h> #include "virutil.h" -#include "virmock.h" - -static char *(*real_virGetUserRuntimeDirectory)(void); - -static void -init_syms(void) -{ - if (real_virGetUserRuntimeDirectory) - return; - - VIR_MOCK_REAL_INIT(virGetUserRuntimeDirectory); -} char * virGetUserRuntimeDirectory(void) { - init_syms(); - return g_build_filename(g_getenv("LIBVIRT_FAKE_ROOT_DIR"), "user-runtime-directory", NULL); } Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal

On Wed, 2020-05-06 at 15:55 +0200, Michal Privoznik wrote:
On 5/6/20 2:54 PM, Andrea Bolognani wrote:
+++ b/tests/virhostdevmock.c @@ -0,0 +1,29 @@ +/* + * Copyright (C) 2020 Red Hat, Inc. + * SPDX-License-Identifier: LGPL-2.1-or-later + */ + +#include <config.h> + +#include "virutil.h" +#include "virmock.h" + +static char *(*real_virGetUserRuntimeDirectory)(void);
This is not needed, you are not calling the original function anywhere. Moreover, the original function must be marked as attribute no-inline.
Good point! I've squashed your changes in and pushed. Thanks :) -- Andrea Bolognani / Red Hat / Virtualization
participants (2)
-
Andrea Bolognani
-
Michal Privoznik