1:53 p.m.
Recently introduced as part of [1].
We can skip the meson part for this one, and deal with everything
in the spec file only.
Test pipeline: https://gitlab.com/abologna/libvirt/-/pipelines/1176890275
[1]
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/DK...
Andrea Bolognani (3):
rpm: Always enable sysctl_config
rpm: Always enable userfaultfd_sysctl
meson: Drop userfaultfd_sysctl option
libvirt.spec.in | 13 +++++--------
meson.build | 7 -------
meson_options.txt | 1 -
src/qemu/meson.build | 2 +-
4 files changed, 6 insertions(+), 17 deletions(-)
--
2.43.0
1:53 p.m.
New subject: [PATCH 1/3] rpm: Always enable sysctl_config
We currently rely on it being enabled by default.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
libvirt.spec.in | 1 +
1 file changed, 1 insertion(+)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 49ce717e1b..af2ba20c02 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1368,6 +1368,7 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y'
%{_specdir}/libvirt.spec)
-Dqemu_moddir=%{qemu_moddir} \
-Dqemu_datadir=%{qemu_datadir} \
-Dtls_priority=%{tls_priority} \
+ -Dsysctl_config=enabled \
%{?arg_userfaultfd_sysctl} \
%{?enable_werror} \
-Dexpensive_tests=enabled \
--
2.43.0
2:12 a.m.
New subject: [PATCH 1/3] rpm: Always enable sysctl_config
On Wed, Feb 14, 2024 at 20:53:22 +0100, Andrea Bolognani wrote:
We currently rely on it being enabled by default.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
libvirt.spec.in | 1 +
1 file changed, 1 insertion(+)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 49ce717e1b..af2ba20c02 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1368,6 +1368,7 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y'
%{_specdir}/libvirt.spec)
-Dqemu_moddir=%{qemu_moddir} \
-Dqemu_datadir=%{qemu_datadir} \
-Dtls_priority=%{tls_priority} \
+ -Dsysctl_config=enabled \
%{?arg_userfaultfd_sysctl} \
%{?enable_werror} \
-Dexpensive_tests=enabled \
Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com>
1:53 p.m.
New subject: [PATCH 2/3] rpm: Always enable userfaultfd_sysctl
Instead of enabling the feature conditionally, always enable
it and then remove the installed file when building on a
platform that doesn't need it.
We already perform the complementary check to decide whether
the file should be included in the package.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
libvirt.spec.in | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index af2ba20c02..70d2d37502 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1283,12 +1283,6 @@ exit 1
%define arg_remote_mode -Dremote_default_mode=legacy
%endif
-%if %{with_userfaultfd_sysctl}
- %define arg_userfaultfd_sysctl -Duserfaultfd_sysctl=enabled
-%else
- %define arg_userfaultfd_sysctl -Duserfaultfd_sysctl=disabled
-%endif
-
%define when %(date +"%%F-%%T")
%define where %(hostname)
%define who %{?packager}%{!?packager:Unknown}
@@ -1369,7 +1363,7 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y'
%{_specdir}/libvirt.spec)
-Dqemu_datadir=%{qemu_datadir} \
-Dtls_priority=%{tls_priority} \
-Dsysctl_config=enabled \
- %{?arg_userfaultfd_sysctl} \
+ -Duserfaultfd_sysctl=enabled \
%{?enable_werror} \
-Dexpensive_tests=enabled \
-Dinit_script=systemd \
@@ -1518,6 +1512,10 @@ rm -f $RPM_BUILD_ROOT%{_datadir}/augeas/lenses/libvirtd_libxl.aug
rm -f $RPM_BUILD_ROOT%{_datadir}/augeas/lenses/tests/test_libvirtd_libxl.aug
%endif
+ %if ! %{with_userfaultfd_sysctl}
+rm -f $RPM_BUILD_ROOT%{_prefix}/lib/sysctl.d/60-qemu-postcopy-migration.conf
+ %endif
+
# Copied into libvirt-docs subpackage eventually
mv $RPM_BUILD_ROOT%{_datadir}/doc/libvirt libvirt-docs
--
2.43.0
1:53 p.m.
New subject: [PATCH 3/3] meson: Drop userfaultfd_sysctl option
It was introduced to make installation of the corresponding
file conditional based on the build platform, but we've
already changed the spec file so that all the decisions on
the matter happen there, which makes having yet another meson
option for the purpose unnecessary.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
libvirt.spec.in | 2 --
meson.build | 7 -------
meson_options.txt | 1 -
src/qemu/meson.build | 2 +-
4 files changed, 1 insertion(+), 11 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 70d2d37502..b3208eb21e 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1363,7 +1363,6 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y'
%{_specdir}/libvirt.spec)
-Dqemu_datadir=%{qemu_datadir} \
-Dtls_priority=%{tls_priority} \
-Dsysctl_config=enabled \
- -Duserfaultfd_sysctl=enabled \
%{?enable_werror} \
-Dexpensive_tests=enabled \
-Dinit_script=systemd \
@@ -1447,7 +1446,6 @@ export SOURCE_DATE_EPOCH=$(stat --printf='%Y'
%{_specdir}/libvirt.spec)
-Dstorage_vstorage=disabled \
-Dstorage_zfs=disabled \
-Dsysctl_config=disabled \
- -Duserfaultfd_sysctl=disabled \
-Dtests=disabled \
-Dudev=disabled \
-Dwireshark_dissector=disabled \
diff --git a/meson.build b/meson.build
index dbd9cad6df..63983d2a10 100644
--- a/meson.build
+++ b/meson.build
@@ -2019,12 +2019,6 @@ elif get_option('sysctl_config').enabled()
error('sysctl configuration is supported only on linux')
endif
-if not get_option('userfaultfd_sysctl').disabled() and
conf.has('WITH_SYSCTL')
- conf.set('WITH_USERFAULTFD_SYSCTL', 1)
-elif get_option('userfaultfd_sysctl').enabled()
- error('userfaultfd_sysctl option requires sysctl_config to be enabled')
-endif
-
conf.set_quoted('TLS_PRIORITY', get_option('tls_priority'))
@@ -2325,7 +2319,6 @@ misc_summary = {
'virt-host-validate': conf.has('WITH_HOST_VALIDATE'),
'TLS priority': conf.get_unquoted('TLS_PRIORITY'),
'sysctl config': conf.has('WITH_SYSCTL'),
- 'userfaultfd sysctl': conf.has('WITH_USERFAULTFD_SYSCTL'),
}
summary(misc_summary, section: 'Miscellaneous', bool_yn: true, list_sep: '
')
diff --git a/meson_options.txt b/meson_options.txt
index ed91d97abf..182e28b3d1 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -108,5 +108,4 @@ option('nbdkit', type: 'feature', value:
'auto', description: 'Build nbdkit stor
option('nbdkit_config_default', type: 'feature', value: 'auto',
description: 'Whether to use nbdkit storage backend for network disks by default
(configurable)')
option('pm_utils', type: 'feature', value: 'auto', description:
'use pm-utils for power management')
option('sysctl_config', type: 'feature', value: 'auto',
description: 'Whether to install sysctl configs')
-option('userfaultfd_sysctl', type: 'feature', value: 'auto',
description: 'Whether to install sysctl config for enabling unprivileged
userfaultfd')
option('tls_priority', type: 'string', value: 'NORMAL',
description: 'set the default TLS session priority string')
diff --git a/src/qemu/meson.build b/src/qemu/meson.build
index 907893d431..7a0e908a66 100644
--- a/src/qemu/meson.build
+++ b/src/qemu/meson.build
@@ -209,7 +209,7 @@ if conf.has('WITH_QEMU')
'in_file': files('virtqemud.init.in'),
}
- if conf.has('WITH_USERFAULTFD_SYSCTL')
+ if conf.has('WITH_SYSCTL')
install_data(
'postcopy-migration.sysctl',
install_dir: prefix / 'lib' / 'sysctl.d',
--
2.43.0
2:21 a.m.
On Wed, Feb 14, 2024 at 20:53:21 +0100, Andrea Bolognani wrote:
Recently introduced as part of [1].
We can skip the meson part for this one, and deal with everything
in the spec file only.
Test pipeline: https://gitlab.com/abologna/libvirt/-/pipelines/1176890275
[1]
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/DK...
Except for 1/3 (which I acked) I think this series is a little bit too
spec-centric. We can do a lot of things just in the spec file, but
having meson options still makes sense for people not building rpms.
Whether we use that option in our spec file or handle it differently is
another question, but I would vote against removing the meson option.
Jirka
3:20 a.m.
On Thu, Feb 15, 2024 at 09:21:15AM +0100, Jiri Denemark wrote:
On Wed, Feb 14, 2024 at 20:53:21 +0100, Andrea Bolognani wrote:
> Recently introduced as part of [1].
>
> We can skip the meson part for this one, and deal with everything
> in the spec file only.
>
> Test pipeline: https://gitlab.com/abologna/libvirt/-/pipelines/1176890275
>
> [1]
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/DK...
Except for 1/3 (which I acked) I think this series is a little bit too
spec-centric. We can do a lot of things just in the spec file, but
having meson options still makes sense for people not building rpms.
Whether we use that option in our spec file or handle it differently is
another question, but I would vote against removing the meson option.
It's not always clear-cut.
A meson option is definitely necessary when it affects how the code
is built, or when we want something to be disabled by default while
giving user a convenient way to enable it. Having some non-trivial
logic deciding whether or not it should be enabled is also a hint
that something should be a meson option.
In this case, it's enabled by default and the way to undo its effects
after the file is simply to delete a single file. Doesn't quite
justify introducing yet another meson option just for it IMO.
Anyway, I just put this out there to see how people reacted to it. If
the option remains, so be it. I just think we could do without :)
--
Andrea Bolognani / Red Hat / Virtualization
316
days inactive
317
days old
6 comments
2 participants
participants (2)
-
Andrea Bolognani -
Jiri Denemark