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>
Attachment:
signature.asc
Description: PGP signature
