On Fri, Jul 17, 2020 at 05:16:00PM +0100, Daniel P. Berrangé wrote:
On Fri, Jul 17, 2020 at 03:51:21PM +0100, Daniel P. Berrangé wrote:
On Thu, Jul 16, 2020 at 11:53:56AM +0200, Pavel Hrdina wrote:
So I was finally able to produce the patches to port libvirt to Meson. Obviously, it is a lot of changes. It might look that some of the patches could be squashed together but I would rather have it as separated as possible to make the review not that difficult.
Once we are done with review I suggest to squash all patches to single patch as it doesn't make sense to keep them separated as it will not be possible to build complete libvirt code by any of the build systems. Trying to achieve that would be even more challenging and the review would me more difficult.
The reasoning behind taking this approach is to have 1:1 conversion from autotools to Meson where each patch removes that part from autotools. It serves as a check that nothing is skipped and to make sure that the conversion is complete.
As probably most of us know Meson is completely different build system and one of the most challenging things was to deal with the fact that meson doesn't allow user functions and that everything has to be defined before it is used.
Patches are available in my Gitlab repo as well:
git clone -b meson https://gitlab.com/phrdina/libvirt.git
I compared the contents of config.h and meson-config.h for the before and after state, looking at wha "#define" are present. I couple of problems appear
HAVE_DECL_SEEK_HOLE, HAVE_LIBATTR, and HAVE_LIBUTIL, WITH_PM_UTILS were not set in meson-config.h
I have also compared symbols in the libvirt.so file
Run:
nm -a libvirt.so.0.6006.0 | cut -c18- | sort > a
and then diff the autotools vs meson build, and we get
1a2,4
a admin_protocol.c a admin_server.c a admin_server_dispatch.c
Dunno why these file names are listed as symbols now
Related to the admin symbols, I incorrectly added libvirt_admin_driver.a static library into libvirt.so
738a742,743
d adminNProcs d adminProcs 1436a1442 d virLogSelf
Also dunno why we have a couple of new data symbols
All of them related to the admin issue.
6715a6763,6794
t adminClientClose t adminClientGetInfo t adminClientGetInfo.cold t adminConnectListServers t adminConnectLookupServer t adminDispatchClientCloseHelper t adminDispatchClientGetInfoHelper t adminDispatchConnectCloseHelper t adminDispatchConnectGetLibVersionHelper t adminDispatchConnectGetLoggingFiltersHelper t adminDispatchConnectGetLoggingOutputsHelper t adminDispatchConnectListServersHelper t adminDispatchConnectLookupServerHelper t adminDispatchConnectOpenHelper t adminDispatchConnectSetLoggingFiltersHelper t adminDispatchConnectSetLoggingOutputsHelper t adminDispatchServerGetClientLimitsHelper t adminDispatchServerGetThreadpoolParametersHelper t adminDispatchServerListClientsHelper t adminDispatchServerLookupClientHelper t adminDispatchServerSetClientLimitsHelper t adminDispatchServerSetThreadpoolParametersHelper t adminDispatchServerUpdateTlsFilesHelper t adminServerGetClientLimits t adminServerGetClientLimits.cold t adminServerGetThreadPoolParameters t adminServerGetThreadPoolParameters.cold t adminServerListClients t adminServerLookupClient t adminServerSetClientLimits t adminServerSetThreadPoolParameters t adminServerUpdateTlsFiles 8392a8472 t make_nonnull_client 8525a8606,8609 t remoteAdmClientFree t remoteAdmClientNew t remoteAdmClientNewPostExecRestart t remoteAdmClientPreExecRestart
These are strange, as the admin stuff should be in libvirt-admin.so instead. Did some files get built into the wrong binary ?
As explained above, incorrect static library was linked into libvirt.so. I'll fix it and push into gitlab.
12218a12303
t virFileActivateDirOverrideForProg.cold
I guess this is new ?
No, my guess is that this is related to the rewrite by patch: meson: src/util/virfile: rewrite virFileActivateDirOverrideForProg
14125,14126d14209 < t virNodeSuspendSupportsTarget < t virNodeSuspendSupportsTarget.cold
I think this is might be because meson failed to detect pm-utils which I already reported.
Already solved in different thread. [...]
17039a17155
U g_getenv
Seems due to a code change
Correct, patch changed the code of virFileActivateDirOverrideForProg: meson: src/util/virfile: rewrite virFileActivateDirOverrideForProg
16961c17077 < U fcntl@@GLIBC_2.2.5 ---
U fcntl64@@GLIBC_2.28 16971c17087 < U fopen@@GLIBC_2.2.5
U fopen64@@GLIBC_2.2.5 16980,16981c17096,17097 < U ftruncate@@GLIBC_2.2.5 < U __fxstat@@GLIBC_2.2.5
U ftruncate64@@GLIBC_2.2.5 U __fxstat64@@GLIBC_2.2.5 17030c17146 < U getrlimit@@GLIBC_2.2.5
U getrlimit64@@GLIBC_2.2.5 17035d17150 < U getxattr@@GLIBC_2.3 17236,17237c17352,17353 < U lseek@@GLIBC_2.2.5 < U __lxstat@@GLIBC_2.2.5
U lseek64@@GLIBC_2.2.5 U __lxstat64@@GLIBC_2.2.5 17292c17408,17409 < U __open_2@@GLIBC_2.7
U __open64_2@@GLIBC_2.7 U open64@@GLIBC_2.2.5 17294d17410 < U open@@GLIBC_2.2.5 17300c17416 < U posix_fallocate@@GLIBC_2.2.5
U posix_fallocate64@@GLIBC_2.2.5 17302,17303c17418,17419 < U pread@@GLIBC_2.2.5 < U prlimit@@GLIBC_2.13
U pread64@@GLIBC_2.2.5 U prlimit64@@GLIBC_2.13 17337c17453 < U readdir@@GLIBC_2.2.5
U readdir64@@GLIBC_2.2.5 17341d17456 < U removexattr@@GLIBC_2.3 17385c17500 < U setrlimit@@GLIBC_2.2.5
U setrlimit64@@GLIBC_2.2.5 17389d17503 < U setxattr@@GLIBC_2.3 17443c17557 < U statfs@@GLIBC_2.2.5
U statfs64@@GLIBC_2.2.5 17589c17703 < U __xstat@@GLIBC_2.2.5
U __xstat64@@GLIBC_2.2.5
These ones are pretty interesting.
It appears we're setting -D_FILE_OFFSET_BITS=64 in meson, even though we're on a 64-bit platform already.
IIUC, in autoconf we only set this in 32-bit platforms.
I think this is probably harmless, as on 64-bit the "64" suffixed symbols should be identical to the non-"64" suffixed symbols. Just mention it in case it is a sign of a bug somewhere.
Meson adds the -D_FILE_OFFSET_BITS=64 regardless of 32-bit or 64-bit platform [1]. So it's not a bug but intentional decision. Thanks for the review. Pavel [1] <https://github.com/mesonbuild/meson/commit/853634a48da025c59eef70161dba0d150833f60d>