Not mocking this function results in files being created in the
user's home directory when running the test and in the build
failing altogether inside a constrained environment such as the
one used by pbuilder:
Could not initialize HostdevManager - operation failed: Failed
to create state dir '/nonexistent/.cache/libvirt/hostdevmgr'
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
tests/qemuhotplugmock.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/tests/qemuhotplugmock.c b/tests/qemuhotplugmock.c
index d2324913cf..27c81670a3 100644
--- a/tests/qemuhotplugmock.c
+++ b/tests/qemuhotplugmock.c
@@ -27,6 +27,7 @@
static int (*real_virGetDeviceID)(const char *path, int *maj, int *min);
static bool (*real_virFileExists)(const char *path);
+static char *(*real_virGetUserRuntimeDirectory)(void);
static void
init_syms(void)
@@ -36,6 +37,7 @@ init_syms(void)
VIR_MOCK_REAL_INIT(virGetDeviceID);
VIR_MOCK_REAL_INIT(virFileExists);
+ VIR_MOCK_REAL_INIT(virGetUserRuntimeDirectory);
}
unsigned long long
@@ -106,3 +108,10 @@ void
qemuProcessKillManagedPRDaemon(virDomainObjPtr vm G_GNUC_UNUSED)
{
}
+
+char *
+virGetUserRuntimeDirectory(void)
+{
+ return g_build_filename(g_getenv("LIBVIRT_FAKE_ROOT_DIR"),
+ "user-runtime-directory", NULL);
+}
--
2.25.4
On 5/1/20 2:30 PM, Andrea Bolognani wrote:
> Not mocking this function results in files being created in the
> user's home directory when running the test and in the build
> failing altogether inside a constrained environment such as the
> one used by pbuilder:
>
> Could not initialize HostdevManager - operation failed: Failed
> to create state dir '/nonexistent/.cache/libvirt/hostdevmgr'
>
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
> tests/qemuhotplugmock.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/tests/qemuhotplugmock.c b/tests/qemuhotplugmock.c
> index d2324913cf..27c81670a3 100644
> --- a/tests/qemuhotplugmock.c
> +++ b/tests/qemuhotplugmock.c
> @@ -27,6 +27,7 @@
>
> static int (*real_virGetDeviceID)(const char *path, int *maj, int *min);
> static bool (*real_virFileExists)(const char *path);
> +static char *(*real_virGetUserRuntimeDirectory)(void);
>
> static void
> init_syms(void)
> @@ -36,6 +37,7 @@ init_syms(void)
>
> VIR_MOCK_REAL_INIT(virGetDeviceID);
> VIR_MOCK_REAL_INIT(virFileExists);
> + VIR_MOCK_REAL_INIT(virGetUserRuntimeDirectory);
> }
>
> unsigned long long
> @@ -106,3 +108,10 @@ void
> qemuProcessKillManagedPRDaemon(virDomainObjPtr vm G_GNUC_UNUSED)
> {
> }
> +
> +char *
> +virGetUserRuntimeDirectory(void)
> +{
> + return g_build_filename(g_getenv("LIBVIRT_FAKE_ROOT_DIR"),
> + "user-runtime-directory", NULL);
> +}
>
While the qemuhotplugtest is (currently) the only one that actually uses
the runtimedir, this function is called basically from every qemu* test
because (almost?) every qemu* test calls virQEMUDriverConfigNew().
Therefore I think we need to fix qemuTestDriverInit() too so that after
it call ConfigNew() the path is replaced with some known value.
Michal
On Mon, 2020-05-04 at 13:44 +0200, Michal Privoznik wrote:
> On 5/1/20 2:30 PM, Andrea Bolognani wrote:
> > +char *
> > +virGetUserRuntimeDirectory(void)
> > +{
> > + return g_build_filename(g_getenv("LIBVIRT_FAKE_ROOT_DIR"),
> > + "user-runtime-directory", NULL);
> > +}
>
> While the qemuhotplugtest is (currently) the only one that actually uses
> the runtimedir, this function is called basically from every qemu* test
> because (almost?) every qemu* test calls virQEMUDriverConfigNew().
> Therefore I think we need to fix qemuTestDriverInit() too so that after
> it call ConfigNew() the path is replaced with some known value.
Good point! I'll look into making this more generic.
--
Andrea Bolognani / Red Hat / Virtualization
On Mon, 2020-05-04 at 18:21 +0200, Andrea Bolognani wrote:
> On Mon, 2020-05-04 at 13:44 +0200, Michal Privoznik wrote:
> > On 5/1/20 2:30 PM, Andrea Bolognani wrote:
> > > +char *
> > > +virGetUserRuntimeDirectory(void)
> > > +{
> > > + return g_build_filename(g_getenv("LIBVIRT_FAKE_ROOT_DIR"),
> > > + "user-runtime-directory", NULL);
> > > +}
> >
> > While the qemuhotplugtest is (currently) the only one that actually uses
> > the runtimedir, this function is called basically from every qemu* test
> > because (almost?) every qemu* test calls virQEMUDriverConfigNew().
> > Therefore I think we need to fix qemuTestDriverInit() too so that after
> > it call ConfigNew() the path is replaced with some known value.
>
> Good point! I'll look into making this more generic.
Unfortunately it's not possible to fix this the way you suggested,
because virHostdevManager attempts to create its state directory
during object allocation and so we can't just change it after the
fact.
I have, however, made the mock more generic and applied it to other
tests that use virHostdevManager[1]. That, along with Dan's patch
that poisons $HOME and $XDG_RUNTIME_DIR in the test suite, should be
enough to ensure more issues like this one don't sneak back in.
[1] https://www.redhat.com/archives/libvir-list/2020-May/msg00212.html
[2] https://www.redhat.com/archives/libvir-list/2020-May/msg00058.html
--
Andrea Bolognani / Red Hat / Virtualization
On Fri, May 01, 2020 at 02:30:16PM +0200, Andrea Bolognani wrote:
> Not mocking this function results in files being created in the
> user's home directory when running the test and in the build
> failing altogether inside a constrained environment such as the
> one used by pbuilder:
What's pbuilder ? I wonder if we can make it so that in our
containers $HOME points to a non-writable directory such that
we can see this failure (and others) in our GitLab CI ?
>
> Could not initialize HostdevManager - operation failed: Failed
> to create state dir '/nonexistent/.cache/libvirt/hostdevmgr'
>
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
> tests/qemuhotplugmock.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/tests/qemuhotplugmock.c b/tests/qemuhotplugmock.c
> index d2324913cf..27c81670a3 100644
> --- a/tests/qemuhotplugmock.c
> +++ b/tests/qemuhotplugmock.c
> @@ -27,6 +27,7 @@
>
> static int (*real_virGetDeviceID)(const char *path, int *maj, int *min);
> static bool (*real_virFileExists)(const char *path);
> +static char *(*real_virGetUserRuntimeDirectory)(void);
>
> static void
> init_syms(void)
> @@ -36,6 +37,7 @@ init_syms(void)
>
> VIR_MOCK_REAL_INIT(virGetDeviceID);
> VIR_MOCK_REAL_INIT(virFileExists);
> + VIR_MOCK_REAL_INIT(virGetUserRuntimeDirectory);
> }
>
> unsigned long long
> @@ -106,3 +108,10 @@ void
> qemuProcessKillManagedPRDaemon(virDomainObjPtr vm G_GNUC_UNUSED)
> {
> }
> +
> +char *
> +virGetUserRuntimeDirectory(void)
> +{
> + return g_build_filename(g_getenv("LIBVIRT_FAKE_ROOT_DIR"),
> + "user-runtime-directory", NULL);
> +}
> --
> 2.25.4
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Fri, 2020-05-01 at 14:09 +0100, Daniel P. Berrangé wrote: > On Fri, May 01, 2020 at 02:30:16PM +0200, Andrea Bolognani wrote: > > Not mocking this function results in files being created in the > > user's home directory when running the test and in the build > > failing altogether inside a constrained environment such as the > > one used by pbuilder: > > What's pbuilder ? It's a tool used in Debian to verify that packages can be built successfully in a minimal environment that only contains the build dependencies that have been explicitly declared and where access to the network and the filesystem outside of the working directory is forbidden. I think the equivalent in the Fedora world is mock, but I haven't used the latter in person so I might be off-base :) > I wonder if we can make it so that in our > containers $HOME points to a non-writable directory such that > we can see this failure (and others) in our GitLab CI ? Yeah, I was thinking the same, and it definitely sounds like a good idea. -- Andrea Bolognani / Red Hat / Virtualization
On Fri, May 01, 2020 at 03:22:22PM +0200, Andrea Bolognani wrote: > On Fri, 2020-05-01 at 14:09 +0100, Daniel P. Berrangé wrote: > > On Fri, May 01, 2020 at 02:30:16PM +0200, Andrea Bolognani wrote: > > > Not mocking this function results in files being created in the > > > user's home directory when running the test and in the build > > > failing altogether inside a constrained environment such as the > > > one used by pbuilder: > > > > What's pbuilder ? > > It's a tool used in Debian to verify that packages can be built > successfully in a minimal environment that only contains the build > dependencies that have been explicitly declared and where access to > the network and the filesystem outside of the working directory is > forbidden. > > I think the equivalent in the Fedora world is mock, but I haven't > used the latter in person so I might be off-base :) > > > I wonder if we can make it so that in our > > containers $HOME points to a non-writable directory such that > > we can see this failure (and others) in our GitLab CI ? > > Yeah, I was thinking the same, and it definitely sounds like a good > idea. Turns out this can be done very nicely even outside of CI: https://www.redhat.com/archives/libvir-list/2020-May/msg00058.html Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
© 2016 - 2024 Red Hat, Inc.