2:28 a.m.
After debugging for a while and eventually finding the root cause we
discussed this on IRC today:
<cpaelzer> danpb: BTW the test crash in 6.0 that I'm tracing (asked yesterday)
is virHostdevManagerInitialize failing
<cpaelzer> the test goes on with mgr set to 0x0 and that is the segfault
later on
<cpaelzer> virSetError was helpful as "Could not initialize HostdevManager
operation failed: Failed to create state dir
'/sbuild-nonexistent/.cache/libvirt/hostdevmgr'" sounds like a
very testbed-specific issue that I might be able to solve when
invoking the tests
<danpb> hmm, seems like we're missing a mock for mkdir()
<danpb> i guess we should mock virFileMakePath in this case
This series got v6.0.0 building in Ubuntu, so I thought it is worth to
submit it for review.
Updates in v2:
- improve indent of virGetLastErrorMessage
- drop superfluous curly brackets
- added the Reviewed-by of Daniel Henrique Barboza (Thanks !)
Christian Ehrhardt (2):
test: let qemuhotplugtest report details of init fails
test: qemuhotplugtest mock virFileMakePath
tests/qemuhotplugmock.c | 17 +++++++++++++++++
tests/qemuhotplugtest.c | 5 +++++
2 files changed, 22 insertions(+)
--
2.25.0
2:28 a.m.
New subject: [libvirt] [PATCH v2 1/2] test: let qemuhotplugtest report details of init fails
If virHostdevManagerGetDefault in qemuhotplugtest fails it works
for quite a while to later segfault when accessing
mgr->activePCIHostdevs.
Report the error details and break on a failed init to see the
real issue right away.
Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
Signed-off-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com>
---
tests/qemuhotplugtest.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index a60c8d1c93..6a3e61c54b 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -634,6 +634,11 @@ mymain(void)
return EXIT_FAILURE;
driver.hostdevMgr = virHostdevManagerGetDefault();
+ if (driver.hostdevMgr == NULL) {
+ VIR_TEST_VERBOSE("Could not initialize HostdevManager - %s\n",
+ virGetLastErrorMessage());
+ return EXIT_FAILURE;
+ }
#define DO_TEST(file, ACTION, dev, fial, kep, ...) \
--
2.25.0
10:32 a.m.
New subject: [libvirt] [PATCH v2 1/2] test: let qemuhotplugtest report details of init fails
On Thu, 2020-01-16 at 09:28 +0100, Christian Ehrhardt wrote:
If virHostdevManagerGetDefault in qemuhotplugtest fails it works
for quite a while to later segfault when accessing
mgr->activePCIHostdevs.
Report the error details and break on a failed init to see the
real issue right away.
Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
Signed-off-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com>
---
tests/qemuhotplugtest.c | 5 +++++
1 file changed, 5 insertions(+)
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
2:54 a.m.
New subject: [libvirt] [PATCH v2 1/2] test: let qemuhotplugtest report details of init fails
On Fri, Jan 17, 2020 at 6:32 PM Andrea Bolognani <abologna(a)redhat.com>
wrote:
On Thu, 2020-01-16 at 09:28 +0100, Christian Ehrhardt wrote:
> If virHostdevManagerGetDefault in qemuhotplugtest fails it works
> for quite a while to later segfault when accessing
> mgr->activePCIHostdevs.
>
> Report the error details and break on a failed init to see the
> real issue right away.
>
> Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
> Signed-off-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com>
> ---
> tests/qemuhotplugtest.c | 5 +++++
> 1 file changed, 5 insertions(+)
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
Thanks for the reviews Daniel and Andrea, I have pushed this (just this -
not the 2/2 which is still up for discussion) to master now.
--
Andrea Bolognani / Red Hat / Virtualization
--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd
2:28 a.m.
New subject: [libvirt] [PATCH v2 2/2] test: qemuhotplugtest mock virFileMakePath
In certain build environments (e.g. Debian and Ubuntu) $HOME is set
to a non existing path intentionally. That breaks and crashes
qemuhotplugtest by failing the init in virHostdevManagerGetDefault.
Avoid that issue by mocking the virFileMakePath behavior if it is
passed a path that matches $HOME and doesn't exists. That fixes
qemuhotplugtest in Debian/Ubuntu builds of v6.0.0.
Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
Signed-off-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com>
---
tests/qemuhotplugmock.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/tests/qemuhotplugmock.c b/tests/qemuhotplugmock.c
index 43a9d79051..06a28742d4 100644
--- a/tests/qemuhotplugmock.c
+++ b/tests/qemuhotplugmock.c
@@ -18,6 +18,7 @@
#include <config.h>
+#include "virmock.h"
#include "qemu/qemu_hotplug.h"
#include "conf/domain_conf.h"
@@ -31,3 +32,19 @@ qemuDomainGetUnplugTimeout(virDomainObjPtr vm G_GNUC_UNUSED)
return 200;
return 100;
}
+
+VIR_MOCK_IMPL_RET_ARGS(virFileMakePath, int,
+ const char *, path)
+{
+ const char *home;
+
+ VIR_MOCK_REAL_INIT(virFileMakePath);
+
+ /* ignore non-existing homes (e.g. in build environments) */
+ home = getenv("HOME");
+ if (strstr(path, home)) {
+ if (!g_file_test (home, G_FILE_TEST_EXISTS))
+ return 0;
+ }
+ return real_virFileMakePath(path);
+}
--
2.25.0
10:47 a.m.
New subject: [libvirt] [PATCH v2 2/2] test: qemuhotplugtest mock virFileMakePath
On Thu, 2020-01-16 at 09:28 +0100, Christian Ehrhardt wrote:
+VIR_MOCK_IMPL_RET_ARGS(virFileMakePath, int,
+ const char *, path)
+{
+ const char *home;
+
+ VIR_MOCK_REAL_INIT(virFileMakePath);
+
+ /* ignore non-existing homes (e.g. in build environments) */
+ home = getenv("HOME");
+ if (strstr(path, home)) {
+ if (!g_file_test (home, G_FILE_TEST_EXISTS))
+ return 0;
+ }
+ return real_virFileMakePath(path);
+}
This doesn't look like the correct fix: what will happen is that,
instead of creating the directory the library code expects, we will
not create it and lie to the caller about this fact.
What we should do instead is create the directory, but make sure it
is prefixed with LIBVIRT_FAKE_ROOT_DIR, in a similar way to what's
done in virpcimock. Of course file access will need to be mocked in
the same way for the tests to work...
--
Andrea Bolognani / Red Hat / Virtualization
3:01 a.m.
New subject: [libvirt] [PATCH v2 2/2] test: qemuhotplugtest mock virFileMakePath
On Fri, Jan 17, 2020 at 6:47 PM Andrea Bolognani <abologna(a)redhat.com>
wrote:
On Thu, 2020-01-16 at 09:28 +0100, Christian Ehrhardt wrote:
> +VIR_MOCK_IMPL_RET_ARGS(virFileMakePath, int,
> + const char *, path)
> +{
> + const char *home;
> +
> + VIR_MOCK_REAL_INIT(virFileMakePath);
> +
> + /* ignore non-existing homes (e.g. in build environments) */
> + home = getenv("HOME");
> + if (strstr(path, home)) {
> + if (!g_file_test (home, G_FILE_TEST_EXISTS))
> + return 0;
> + }
> + return real_virFileMakePath(path);
> +}
This doesn't look like the correct fix: what will happen is that,
instead of creating the directory the library code expects, we will
not create it and lie to the caller about this fact.
Not creating and lying was exactly what I had in mind for this particular
case as it was the least invasive change to achieve what was needed for the
test.
What we should do instead is create the directory, but make sure it
is prefixed with LIBVIRT_FAKE_ROOT_DIR, in a similar way to
what's
done in virpcimock. Of course file access will need to be mocked in
the same way for the tests to work...
I was initially adding all sorts of virFileWrapperAddPrefix but it failed
me.
Trying again in the style of virpcimock seems like a good idea, but I have
to be honest due to a business trip and some other tasks I won't get to it
soon.
If that build error with non-existing $HOME bothers anyone else feel free
to beat me to it with a v3 of this.
--
Andrea Bolognani / Red Hat / Virtualization
--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd
1766
days inactive
1772
days old
6 comments
2 participants
participants (2)
-
Andrea Bolognani -
Christian Ehrhardt