4:10 p.m.
The set of changes fixes qemuxml2argvtest and closes issue #58.
Changes since v1:
- Simplified lookup of symbols in main executable (Michal)
- Reduced timeout to 90s for qemuxml2argvtest (Andrea)
- Two new patches to fix qemucapsprobe
Pipeline results: https://gitlab.com/roolebo/libvirt/-/pipelines/220225729
Roman Bolshakov (6):
tests: Fix opendir mocks on macOS
tests: Fix mock chaining on macOS
qemuxml2argvtest: Increase timeout on macOS
ci: Run test suite on macOS
tests: Delay mock creation
qemucapsprobemock: Fix lookup of qemu functions
ci/cirrus/build.yml | 3 +--
tests/meson.build | 49 +++++++++++++++++++++++++-----------------
tests/virfilewrapper.c | 4 ++++
tests/virmock.h | 13 +++++++++++
tests/virpcimock.c | 5 +++++
5 files changed, 52 insertions(+), 22 deletions(-)
--
2.29.2
4:10 p.m.
New subject: [PATCH v2 1/6] tests: Fix opendir mocks on macOS
opendir() mocks need to search for decorated function with $INODE64
suffix, like stat mocks.
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Signed-off-by: Roman Bolshakov <r.bolshakov(a)yadro.com>
---
tests/virfilewrapper.c | 4 ++++
tests/virpcimock.c | 4 ++++
2 files changed, 8 insertions(+)
diff --git a/tests/virfilewrapper.c b/tests/virfilewrapper.c
index 0500a3617e..ca2356b5c9 100644
--- a/tests/virfilewrapper.c
+++ b/tests/virfilewrapper.c
@@ -56,7 +56,11 @@ static void init_syms(void)
VIR_MOCK_REAL_INIT(access);
VIR_MOCK_REAL_INIT(mkdir);
VIR_MOCK_REAL_INIT(open);
+# ifdef __APPLE__
+ VIR_MOCK_REAL_INIT_ALIASED(opendir, "opendir$INODE64");
+# else
VIR_MOCK_REAL_INIT(opendir);
+# endif
VIR_MOCK_REAL_INIT(execv);
VIR_MOCK_REAL_INIT(execve);
}
diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index 5cd688c825..686f894e99 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -935,7 +935,11 @@ init_syms(void)
VIR_MOCK_REAL_INIT(__open_2);
# endif /* ! __GLIBC__ */
VIR_MOCK_REAL_INIT(close);
+# ifdef __APPLE__
+ VIR_MOCK_REAL_INIT_ALIASED(opendir, "opendir$INODE64");
+# else
VIR_MOCK_REAL_INIT(opendir);
+# endif
VIR_MOCK_REAL_INIT(virFileCanonicalizePath);
}
--
2.29.2
4:10 p.m.
New subject: [PATCH v2 2/6] tests: Fix mock chaining on macOS
Some tests in qemuxml2argvtest need opendir() from virpcimock, others
need opendir() from virfilewrapper.
But as of now, only opendir() from virpcimock has an effect.
real_opendir in virpcimock has a pointer to opendir$INODE64 in
libsystem_kernel.dylib instead of pointing to opendir$INODE64 in
qemuxml2argvtest (from virfilewrapper). And because the second one is
never used, tests that rely on prefixes added by virFileWrapperAddPrefix
fail.
That can be fixed if dlsym(3) is asked explicitly to search symbols in
main executable with RTLD_MAIN_ONLY before going to other dylibs.
Existing RTLD_NEXT handle results into libsystem_kernel.dylib being
searched before main executable.
Signed-off-by: Roman Bolshakov <r.bolshakov(a)yadro.com>
---
tests/virmock.h | 13 +++++++++++++
tests/virpcimock.c | 1 +
2 files changed, 14 insertions(+)
diff --git a/tests/virmock.h b/tests/virmock.h
index dea5feb80f..5250e733a1 100644
--- a/tests/virmock.h
+++ b/tests/virmock.h
@@ -284,9 +284,21 @@
static void (*real_##name)(void); \
void wrap_##name(void)
+#if defined(VIR_MOCK_LOOKUP_MAIN) && defined(__APPLE__)
+# define VIR_MOCK_REAL_INIT_MAIN(name, alias) \
+ do { \
+ if (real_##name == NULL) { \
+ real_##name = dlsym(RTLD_MAIN_ONLY, alias); \
+ } \
+ } while (0)
+#else
+# define VIR_MOCK_REAL_INIT_MAIN(name, alias) \
+ do {} while (0)
+#endif
#define VIR_MOCK_REAL_INIT(name) \
do { \
+ VIR_MOCK_REAL_INIT_MAIN(name, #name); \
if (real_##name == NULL && \
!(real_##name = dlsym(RTLD_NEXT, \
#name))) { \
@@ -297,6 +309,7 @@
#define VIR_MOCK_REAL_INIT_ALIASED(name, alias) \
do { \
+ VIR_MOCK_REAL_INIT_MAIN(name, alias); \
if (real_##name == NULL && \
!(real_##name = dlsym(RTLD_NEXT, \
alias))) { \
diff --git a/tests/virpcimock.c b/tests/virpcimock.c
index 686f894e99..4aa96cae08 100644
--- a/tests/virpcimock.c
+++ b/tests/virpcimock.c
@@ -19,6 +19,7 @@
#include <config.h>
#if defined(__linux__) || defined(__FreeBSD__) || defined(__APPLE__)
+# define VIR_MOCK_LOOKUP_MAIN
# include "virmock.h"
# include <unistd.h>
# include <fcntl.h>
--
2.29.2
3:53 a.m.
New subject: [PATCH v2 2/6] tests: Fix mock chaining on macOS
On 11/23/20 11:10 PM, Roman Bolshakov wrote:
Some tests in qemuxml2argvtest need opendir() from virpcimock,
others
need opendir() from virfilewrapper.
But as of now, only opendir() from virpcimock has an effect.
real_opendir in virpcimock has a pointer to opendir$INODE64 in
libsystem_kernel.dylib instead of pointing to opendir$INODE64 in
qemuxml2argvtest (from virfilewrapper). And because the second one is
never used, tests that rely on prefixes added by virFileWrapperAddPrefix
fail.
That can be fixed if dlsym(3) is asked explicitly to search symbols in
main executable with RTLD_MAIN_ONLY before going to other dylibs.
Existing RTLD_NEXT handle results into libsystem_kernel.dylib being
searched before main executable.
Signed-off-by: Roman Bolshakov <r.bolshakov(a)yadro.com>
---
tests/virmock.h | 13 +++++++++++++
tests/virpcimock.c | 1 +
2 files changed, 14 insertions(+)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
4:10 p.m.
New subject: [PATCH v2 3/6] qemuxml2argvtest: Increase timeout on macOS
The test takes 40+ seconds on MBP 2012, MBA 2015. Cirrus completes the
test within default timeout, just above 29 seconds but the error margin
is narrow, under a second.
It'd be good to provide reasonable default timeout to avoid test suite
failure if "meson test" is invoked without arguments.
Closes https://gitlab.com/libvirt/libvirt/-/issues/58
Signed-off-by: Roman Bolshakov <r.bolshakov(a)yadro.com>
---
tests/meson.build | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/tests/meson.build b/tests/meson.build
index f88410ff33..cb06873ba5 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -574,7 +574,13 @@ foreach data : tests
],
export_dynamic: true,
)
- test(data['name'], test_bin, env: tests_env)
+ if data['name'] == 'qemuxml2argvtest' and host_machine.system() ==
'darwin'
+ timeout = 90
+ else
+ # default meson timeout
+ timeout = 30
+ endif
+ test(data['name'], test_bin, env: tests_env, timeout: timeout)
endforeach
--
2.29.2
3:53 a.m.
New subject: [PATCH v2 3/6] qemuxml2argvtest: Increase timeout on macOS
On 11/23/20 11:10 PM, Roman Bolshakov wrote:
The test takes 40+ seconds on MBP 2012, MBA 2015. Cirrus completes
the
test within default timeout, just above 29 seconds but the error margin
is narrow, under a second.
It'd be good to provide reasonable default timeout to avoid test suite
failure if "meson test" is invoked without arguments.
Closes https://gitlab.com/libvirt/libvirt/-/issues/58
Signed-off-by: Roman Bolshakov <r.bolshakov(a)yadro.com>
---
tests/meson.build | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/tests/meson.build b/tests/meson.build
index f88410ff33..cb06873ba5 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -574,7 +574,13 @@ foreach data : tests
],
export_dynamic: true,
)
- test(data['name'], test_bin, env: tests_env)
+ if data['name'] == 'qemuxml2argvtest' and host_machine.system() ==
'darwin'
If we are doing this, then I'd say do it host arch agnostic.
+ timeout = 90
+ else
+ # default meson timeout
+ timeout = 30
+ endif
+ test(data['name'], test_bin, env: tests_env, timeout: timeout)
endforeach
Michal
5:52 a.m.
New subject: [PATCH v2 3/6] qemuxml2argvtest: Increase timeout on macOS
On Fri, Nov 27, 2020 at 10:53:49AM +0100, Michal Prívozník wrote:
On 11/23/20 11:10 PM, Roman Bolshakov wrote:
> The test takes 40+ seconds on MBP 2012, MBA 2015. Cirrus completes the
> test within default timeout, just above 29 seconds but the error margin
> is narrow, under a second.
>
> It'd be good to provide reasonable default timeout to avoid test suite
> failure if "meson test" is invoked without arguments.
>
> Closes https://gitlab.com/libvirt/libvirt/-/issues/58
> Signed-off-by: Roman Bolshakov <r.bolshakov(a)yadro.com>
> ---
> tests/meson.build | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/tests/meson.build b/tests/meson.build
> index f88410ff33..cb06873ba5 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -574,7 +574,13 @@ foreach data : tests
> ],
> export_dynamic: true,
> )
> - test(data['name'], test_bin, env: tests_env)
> + if data['name'] == 'qemuxml2argvtest' and host_machine.system()
== 'darwin'
If we are doing this, then I'd say do it host arch agnostic.
I intended to do that but forgot about it. Thanks for catching that!
-Roman
> + timeout = 90
> + else
> + # default meson timeout
> + timeout = 30
> + endif
> + test(data['name'], test_bin, env: tests_env, timeout: timeout)
> endforeach
>
Michal
9:37 a.m.
New subject: [PATCH v2 3/6] qemuxml2argvtest: Increase timeout on macOS
On 11/27/20 12:52 PM, Roman Bolshakov wrote:
On Fri, Nov 27, 2020 at 10:53:49AM +0100, Michal Prívozník wrote:
> On 11/23/20 11:10 PM, Roman Bolshakov wrote:
>> The test takes 40+ seconds on MBP 2012, MBA 2015. Cirrus completes the
>> test within default timeout, just above 29 seconds but the error margin
>> is narrow, under a second.
>>
>> It'd be good to provide reasonable default timeout to avoid test suite
>> failure if "meson test" is invoked without arguments.
>>
>> Closes https://gitlab.com/libvirt/libvirt/-/issues/58
>> Signed-off-by: Roman Bolshakov <r.bolshakov(a)yadro.com>
>> ---
>> tests/meson.build | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/meson.build b/tests/meson.build
>> index f88410ff33..cb06873ba5 100644
>> --- a/tests/meson.build
>> +++ b/tests/meson.build
>> @@ -574,7 +574,13 @@ foreach data : tests
>> ],
>> export_dynamic: true,
>> )
>> - test(data['name'], test_bin, env: tests_env)
>> + if data['name'] == 'qemuxml2argvtest' and
host_machine.system() == 'darwin'
>
> If we are doing this, then I'd say do it host arch agnostic.
>
I intended to do that but forgot about it. Thanks for catching that!
Okay, I can fix that just before pushing.
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
and pushed.
Michal
10:42 a.m.
New subject: [PATCH v2 3/6] qemuxml2argvtest: Increase timeout on macOS
On Fri, Nov 27, 2020 at 04:37:53PM +0100, Michal Prívozník wrote:
On 11/27/20 12:52 PM, Roman Bolshakov wrote:
> On Fri, Nov 27, 2020 at 10:53:49AM +0100, Michal Prívozník wrote:
> > On 11/23/20 11:10 PM, Roman Bolshakov wrote:
> > > The test takes 40+ seconds on MBP 2012, MBA 2015. Cirrus completes the
> > > test within default timeout, just above 29 seconds but the error margin
> > > is narrow, under a second.
> > >
> > > It'd be good to provide reasonable default timeout to avoid test
suite
> > > failure if "meson test" is invoked without arguments.
> > >
> > > Closes https://gitlab.com/libvirt/libvirt/-/issues/58
> > > Signed-off-by: Roman Bolshakov <r.bolshakov(a)yadro.com>
> > > ---
> > > tests/meson.build | 8 +++++++-
> > > 1 file changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tests/meson.build b/tests/meson.build
> > > index f88410ff33..cb06873ba5 100644
> > > --- a/tests/meson.build
> > > +++ b/tests/meson.build
> > > @@ -574,7 +574,13 @@ foreach data : tests
> > > ],
> > > export_dynamic: true,
> > > )
> > > - test(data['name'], test_bin, env: tests_env)
> > > + if data['name'] == 'qemuxml2argvtest' and
host_machine.system() == 'darwin'
> >
> > If we are doing this, then I'd say do it host arch agnostic.
> >
>
> I intended to do that but forgot about it. Thanks for catching that!
Okay, I can fix that just before pushing.
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
and pushed.
Michal
I was about to resend v3 when received the email :)
Thanks for reviewing the series.
Best regards,
Roman
4:10 p.m.
New subject: [PATCH v2 4/6] ci: Run test suite on macOS
There's no need to have different CI process between macOS and FreeBSD
as test suite has been fixed on macOS.
Signed-off-by: Roman Bolshakov <r.bolshakov(a)yadro.com>
---
ci/cirrus/build.yml | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/ci/cirrus/build.yml b/ci/cirrus/build.yml
index 912284b906..dc1cccd135 100644
--- a/ci/cirrus/build.yml
+++ b/ci/cirrus/build.yml
@@ -20,5 +20,4 @@ build_task:
- git reset --hard "$CI_COMMIT_SHA"
build_script:
- meson build --prefix=$(pwd)/install-root
- - if test "$(uname)" = "FreeBSD"; then ninja -C build dist; fi
- - if test "$(uname)" = "Darwin"; then ninja -C build &&
ninja -C build install; fi
+ - ninja -C build dist
--
2.29.2
3:53 a.m.
New subject: [PATCH v2 4/6] ci: Run test suite on macOS
On 11/23/20 11:10 PM, Roman Bolshakov wrote:
There's no need to have different CI process between macOS and
FreeBSD
as test suite has been fixed on macOS.
Signed-off-by: Roman Bolshakov <r.bolshakov(a)yadro.com>
---
ci/cirrus/build.yml | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
4:10 p.m.
New subject: [PATCH v2 5/6] tests: Delay mock creation
There might be mocks that need to reference qemu test driver and link
with it. It's not possible now because qemu test driver is defined after
mocks.
While at it, add 'link_with' parameter to mock definition that allows to
specify a set of libraries the mock has to be linked with.
Signed-off-by: Roman Bolshakov <r.bolshakov(a)yadro.com>
---
tests/meson.build | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/tests/meson.build b/tests/meson.build
index cb06873ba5..ef8ded1ea4 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -115,24 +115,6 @@ if conf.has('WITH_SECDRIVER_SELINUX')
]
endif
-foreach mock : mock_libs
- shared_library(
- mock['name'],
- mock.get('sources', [ '@0@.c'.format(mock['name']) ]),
- override_options: [
- 'b_asneeded=false',
- 'b_lundef=false',
- ],
- dependencies: [
- tests_dep,
- mock.get('deps', []),
- ],
- link_with: [
- libvirt_lib,
- ],
- )
-endforeach
-
# build libraries used by tests
@@ -201,6 +183,24 @@ test_file_wrapper_lib = static_library(
dependencies: [ tests_dep ],
)
+foreach mock : mock_libs
+ shared_library(
+ mock['name'],
+ mock.get('sources', [ '@0@.c'.format(mock['name']) ]),
+ override_options: [
+ 'b_asneeded=false',
+ 'b_lundef=false',
+ ],
+ dependencies: [
+ tests_dep,
+ mock.get('deps', []),
+ ],
+ link_with: [
+ libvirt_lib,
+ mock.get('link_with', []),
+ ],
+ )
+endforeach
# build helpers used by tests
--
2.29.2
3:53 a.m.
New subject: [PATCH v2 5/6] tests: Delay mock creation
On 11/23/20 11:10 PM, Roman Bolshakov wrote:
There might be mocks that need to reference qemu test driver and
link
with it. It's not possible now because qemu test driver is defined after
mocks.
While at it, add 'link_with' parameter to mock definition that allows to
specify a set of libraries the mock has to be linked with.
Signed-off-by: Roman Bolshakov <r.bolshakov(a)yadro.com>
---
tests/meson.build | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Although I'd expect these foreach loops run only after the whole file
was processed. Just like 'make' does.
Michal
4:10 p.m.
New subject: [PATCH v2 6/6] qemucapsprobemock: Fix lookup of qemu functions
qemucapsprobemock can't find real versions of qemuMonitorSend() and
qemuMonitorJSONIOProcessLine() on macOS. That breaks qemucapsprobe.
The failure can be explained by documented behaviour of dlsym(3) on
macOS:
If dlsym() is called with the special handle RTLD_NEXT, then dyld
searches for the symbol in the dylibs the calling image linked against
when built.
[...] For flat linked images, the search starts in the load ordered
list of all images, in the image right after the caller's image.
That means qemucapsprobemock must be linked against qemu test driver to
find symbols there with RTLD_NEXT.
Signed-off-by: Roman Bolshakov <r.bolshakov(a)yadro.com>
---
tests/meson.build | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tests/meson.build b/tests/meson.build
index ef8ded1ea4..cc0657f3c1 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -101,7 +101,6 @@ endif
if conf.has('WITH_QEMU')
mock_libs += [
{ 'name': 'qemucaps2xmlmock' },
- { 'name': 'qemucapsprobemock' },
{ 'name': 'qemucpumock' },
{ 'name': 'qemuhotplugmock' },
{ 'name': 'qemuxml2argvmock' },
@@ -171,6 +170,10 @@ if conf.has('WITH_QEMU')
link_whole: [ qemu_driver_impl ],
link_with: [ libvirt_lib ],
)
+
+ mock_libs += [
+ { 'name': 'qemucapsprobemock', 'link_with': [
test_qemu_driver_lib ] },
+ ]
else
test_qemu_driver_lib = []
test_utils_qemu_lib = []
--
2.29.2
3:54 a.m.
New subject: [PATCH v2 6/6] qemucapsprobemock: Fix lookup of qemu functions
On 11/23/20 11:10 PM, Roman Bolshakov wrote:
qemucapsprobemock can't find real versions of qemuMonitorSend()
and
qemuMonitorJSONIOProcessLine() on macOS. That breaks qemucapsprobe.
The failure can be explained by documented behaviour of dlsym(3) on
macOS:
If dlsym() is called with the special handle RTLD_NEXT, then dyld
searches for the symbol in the dylibs the calling image linked against
when built.
[...] For flat linked images, the search starts in the load ordered
list of all images, in the image right after the caller's image.
That means qemucapsprobemock must be linked against qemu test driver to
find symbols there with RTLD_NEXT.
Signed-off-by: Roman Bolshakov <r.bolshakov(a)yadro.com>
---
tests/meson.build | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
1459
days inactive
1463
days old
14 comments
2 participants
participants (2)
-
Michal Prívozník -
Roman Bolshakov