7:17 a.m.
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(a)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'
8:38 a.m.
Tue, 25 Jan 2022 14:17:23 +0100 Olaf Hering <olaf(a)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
11:16 a.m.
On Tue, Jan 25, 2022 at 03:38:33PM +0100, Olaf Hering wrote:
Tue, 25 Jan 2022 14:17:23 +0100 Olaf Hering <olaf(a)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
11:30 a.m.
Tue, 25 Jan 2022 09:16:33 -0800 Andrea Bolognani <abologna(a)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
12:11 p.m.
On Tue, Jan 25, 2022 at 06:30:43PM +0100, Olaf Hering wrote:
Tue, 25 Jan 2022 09:16:33 -0800 Andrea Bolognani
<abologna(a)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
7:31 a.m.
Tue, 25 Jan 2022 10:11:01 -0800 Andrea Bolognani <abologna(a)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
9:14 a.m.
On Wed, Jan 26, 2022 at 02:31:01PM +0100, Olaf Hering wrote:
Tue, 25 Jan 2022 10:11:01 -0800 Andrea Bolognani
<abologna(a)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
9:59 a.m.
Wed, 26 Jan 2022 07:14:34 -0800 Andrea Bolognani <abologna(a)redhat.com>:
Looks good at a fist glance, thanks!
Olaf
11:09 a.m.
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(a)aepfle.de>
---
meson.build | 1 +
1 file changed, 1 insertion(+)
Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
--
Andrea Bolognani / Red Hat / Virtualization
1032
days inactive
1033
days old
9 comments
2 participants
participants (2)
-
Andrea Bolognani -
Olaf Hering