NAME="SLES"
VERSION="15-SP3"
VERSION_ID="15.3"
PRETTY_NAME="SUSE Linux Enterprise Server 15 SP3"
ID="sles"
ID_LIKE="suse"
ANSI_COLOR="0;32"
CPE_NAME="cpe:/o:suse:sles:15:sp3"
DOCUMENTATION_URL="https://documentation.suse.com/"
Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
meson.build | 1 +
1 file changed, 1 insertion(+)
Commit 4c69d64efa3731d074d198f871fd42e74c4a39f6 revealed the bug, /etc/os-release must exist during build.
diff --git a/meson.build b/meson.build
index b6d1286f3f..b2d9e8ed65 100644
--- a/meson.build
+++ b/meson.build
@@ -1673,6 +1673,7 @@ if not get_option('driver_qemu').disabled()
os_release.contains('fedora') or
os_release.contains('gentoo') or
os_release.contains('rhel') or
+ os_release.contains('sles') or
os_release.contains('suse'))
default_qemu_user = 'qemu'
default_qemu_group = 'qemu'
Tue, 25 Jan 2022 14:17:23 +0100 Olaf Hering <olaf@aepfle.de>: > Commit 4c69d64efa3731d074d198f871fd42e74c4a39f6 revealed the bug, /etc/os-release must exist during build. Now that a missing ^ID= became fatal, was it actually a good idea to be explicit? I think poking around for clues in ^ID_LIKE= will be more robust, appending 'sles' would be not required. Olaf
On Tue, Jan 25, 2022 at 03:38:33PM +0100, Olaf Hering wrote: > Tue, 25 Jan 2022 14:17:23 +0100 Olaf Hering <olaf@aepfle.de>: > > > Commit 4c69d64efa3731d074d198f871fd42e74c4a39f6 revealed the bug, /etc/os-release must exist during build. > > Now that a missing ^ID= became fatal, was it actually a good idea to be explicit? > I think poking around for clues in ^ID_LIKE= will be more robust, appending 'sles' would be not required. You're right, /etc/os-release being absent should not result in a build failure. And instead of matching on ID specifically, we could probably use something like grep -E '^ID(_LIKE)*=' /etc/os-release instead and check only a subset of the strings we currently do. RHEL and CentOS for example are both ID_LIKE=fedora. We'd have to be careful about the order in which the Debian and Ubuntu branches appear though. -- Andrea Bolognani / Red Hat / Virtualization
Tue, 25 Jan 2022 09:16:33 -0800 Andrea Bolognani <abologna@redhat.com>: > /etc/os-release being absent should not result in a build failure. Yes. This should be the first thing to fix. Commit 29cd1877acd91883df32bf71ec07fe908e96db32 does not say why ID= was used. This grep should be relaxed to ID_LIKE=. Furthermore, if both qemu_user and qemu_group are known, no grep is required. Olaf
On Tue, Jan 25, 2022 at 06:30:43PM +0100, Olaf Hering wrote: > Tue, 25 Jan 2022 09:16:33 -0800 Andrea Bolognani <abologna@redhat.com>: > > /etc/os-release being absent should not result in a build failure. > > Yes. This should be the first thing to fix. > > Commit 29cd1877acd91883df32bf71ec07fe908e96db32 does not say why ID= > was used. This grep should be relaxed to ID_LIKE=. We have to consider *both*, because only looking at ID_LIKE would not work on some distros: $ grep -E '^ID(_LIKE)*=' /etc/os-release ID=fedora $ > Furthermore, if both qemu_user and qemu_group are known, no grep is required. Do you mean that if the user has passed -Dqemu_user=... -Dqemu_group=... to meson we could skip the detection logic? That'd be nice, but I'm afraid it might make the code less readable for very little gain. Are you going to take a stab at these patches? -- Andrea Bolognani / Red Hat / Virtualization
Tue, 25 Jan 2022 10:11:01 -0800 Andrea Bolognani <abologna@redhat.com>: > Are you going to take a stab at these patches? No, I will just add sles-release to BuildRequires. Olaf
Tue, 25 Jan 2022 10:11:01 -0800 Andrea Bolognani <abologna@redhat.com>:
> Do you mean that if the user has passed
> -Dqemu_user=... -Dqemu_group=...
> to meson we could skip the detection logic? That'd be nice, but I'm
> afraid it might make the code less readable for very little gain.
The whole thing will be just a whitespace change because it goes into its own block:
qemu_user = get_option('qemu_user')
qemu_group = get_option('qemu_group')
if qemu_user == '' or qemu_group == ''
if host_machine.system() in [ 'freebsd', 'darwin' ]
default_qemu_user = 'root'
default_qemu_group = 'wheel'
else
os_release = run_command('grep', '^ID=', '/etc/os-release', check: true).stdout()
if os_release.contains('arch')
default_qemu_user = 'nobody'
default_qemu_group = 'nobody'
elif (os_release.contains('centos') or
os_release.contains('fedora') or
os_release.contains('gentoo') or
os_release.contains('rhel') or
os_release.contains('sles') or
os_release.contains('suse'))
default_qemu_user = 'qemu'
default_qemu_group = 'qemu'
elif os_release.contains('debian')
default_qemu_user = 'libvirt-qemu'
default_qemu_group = 'libvirt-qemu'
elif os_release.contains('ubuntu')
default_qemu_user = 'libvirt-qemu'
default_qemu_group = 'kvm'
else
default_qemu_user = 'root'
default_qemu_group = 'root'
endif
# If the expected user and group don't exist, or we haven't hit any
# of the cases above bacuse we're running on an unknown OS, the only
# sensible fallback is root:root
if (run_command('getent', 'passwd', default_qemu_user, check: false).returncode() != 0 and
run_command('getent', 'group', default_qemu_group, check: false).returncode() != 0)
default_qemu_user = 'root'
default_qemu_group = 'root'
endif
endif
qemu_group = default_qemu_group
qemu_user = default_qemu_user
endif
conf.set_quoted('QEMU_USER', qemu_user)
conf.set_quoted('QEMU_GROUP', qemu_group)
This looks like an opportunity to get rid of default_qemu_user/group.
The getent part is bogus because it forces the user/group to be present on the system
which is building libvirt. This build system is usually not the system which runs libvirt.
Please fix bug this as well.
Olaf
On Wed, Jan 26, 2022 at 02:31:01PM +0100, Olaf Hering wrote: > Tue, 25 Jan 2022 10:11:01 -0800 Andrea Bolognani <abologna@redhat.com>: > > Do you mean that if the user has passed > > -Dqemu_user=... -Dqemu_group=... > > to meson we could skip the detection logic? That'd be nice, but I'm > > afraid it might make the code less readable for very little gain. > > The whole thing will be just a whitespace change because it goes into its own block: > This looks like an opportunity to get rid of default_qemu_user/group. > > The getent part is bogus because it forces the user/group to be present on the system > which is building libvirt. This build system is usually not the system which runs libvirt. > Please fix bug this as well. Patches here: https://listman.redhat.com/archives/libvir-list/2022-January/msg01189.html -- Andrea Bolognani / Red Hat / Virtualization
Wed, 26 Jan 2022 07:14:34 -0800 Andrea Bolognani <abologna@redhat.com>: > Patches here: > https://listman.redhat.com/archives/libvir-list/2022-January/msg01189.html Looks good at a fist glance, thanks! Olaf
On Tue, Jan 25, 2022 at 02:17:23PM +0100, Olaf Hering wrote: > NAME="SLES" > VERSION="15-SP3" > VERSION_ID="15.3" > PRETTY_NAME="SUSE Linux Enterprise Server 15 SP3" > ID="sles" > ID_LIKE="suse" > ANSI_COLOR="0;32" > CPE_NAME="cpe:/o:suse:sles:15:sp3" > DOCUMENTATION_URL="https://documentation.suse.com/" > > > Signed-off-by: Olaf Hering <olaf@aepfle.de> > --- > meson.build | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization
© 2016 - 2024 Red Hat, Inc.