10:37 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.
Christian Ehrhardt (2):
test: let qemuhotplugtest report details of init fails
test: qemuhotplugtest mock virFileMakePath
tests/qemuhotplugmock.c | 18 ++++++++++++++++++
tests/qemuhotplugtest.c | 5 +++++
2 files changed, 23 insertions(+)
--
2.25.0
10:37 a.m.
New subject: [libvirt] [PATCH 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.
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..94440791d5 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
11:25 a.m.
New subject: [libvirt] [PATCH 1/2] test: let qemuhotplugtest report details of init fails
On 1/15/20 1:37 PM, 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.
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..94440791d5 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());
Suggestion/nit: make 'virGetLastErrorMessage()' with the same identation of
the string above it:
+ VIR_TEST_VERBOSE("Could not initialize HostdevManager -
%s\n",
+ virGetLastErrorMessage());
LGTM regardless, so
Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
+ return EXIT_FAILURE;
+ }
#define DO_TEST(file, ACTION, dev, fial, kep, ...) \
10:37 a.m.
New subject: [libvirt] [PATCH 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.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com>
---
tests/qemuhotplugmock.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/tests/qemuhotplugmock.c b/tests/qemuhotplugmock.c
index 43a9d79051..bd5eb3ceb6 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,20 @@ 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
11:32 a.m.
New subject: [libvirt] [PATCH 2/2] test: qemuhotplugtest mock virFileMakePath
On 1/15/20 1:37 PM, Christian Ehrhardt wrote:
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.
Signed-off-by: Christian Ehrhardt <christian.ehrhardt(a)canonical.com>
---
tests/qemuhotplugmock.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/tests/qemuhotplugmock.c b/tests/qemuhotplugmock.c
index 43a9d79051..bd5eb3ceb6 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,20 @@ 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;
+ }
+ }
make syntax-check is not happy with the curly brackets here:
Whitespace after non-keyword:
../tests/qemuhotplugmock.c:46: if (!g_file_test (home, G_FILE_TEST_EXISTS)) {
Curly brackets around single-line body:
../tests/qemuhotplugmock.c:46-48:
if (!g_file_test (home, G_FILE_TEST_EXISTS)) {
return 0;
}
build-aux/syntax-check.mk: incorrect formatting
make: *** [../build-aux/syntax-check.mk:2173: spacing-check] Error 1
make: *** Waiting for unfinished jobs....
$
This fixes it:
$ git diff
diff --git a/tests/qemuhotplugmock.c b/tests/qemuhotplugmock.c
index bd5eb3ceb6..fdad585e33 100644
--- a/tests/qemuhotplugmock.c
+++ b/tests/qemuhotplugmock.c
@@ -43,9 +43,8 @@ VIR_MOCK_IMPL_RET_ARGS(virFileMakePath, int,
/* 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)) {
+ if (!g_file_test(home, G_FILE_TEST_EXISTS))
return 0;
- }
}
return real_virFileMakePath(path);
}
With this fixed:
Reviewed-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
+ return real_virFileMakePath(path);
+}
1773
days inactive
1773
days old
4 comments
2 participants
participants (2)
-
Christian Ehrhardt -
Daniel Henrique Barboza