On Mon, Aug 31, 2020 at 09:45:15AM +0200, Katerina Koukiou wrote:

Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- data/system/libvirt-dbus.service.in | 21 +++++++++++++++ data/system/meson.build | 30 +++++++++++++++++----- data/system/org.libvirt-systemd.service.in | 5 ++++ libvirt-dbus.spec.in | 4 ++- meson.build | 12 +++++++++ meson_options.txt | 1 + 6 files changed, 66 insertions(+), 7 deletions(-) create mode 100644 data/system/libvirt-dbus.service.in create mode 100644 data/system/org.libvirt-systemd.service.in

diff --git data/system/libvirt-dbus.service.in data/system/libvirt-dbus.service.in new file mode 100644 index 0000000..862a366 --- /dev/null +++ data/system/libvirt-dbus.service.in @@ -0,0 +1,21 @@ +# SPDX-License-Identifier: LGPL-2.1+ +# +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version.

This doesn't look correct as you are contributing to libvirt-dbus, not systemd.

+ +[Unit] +Description=Libvirt DBus Service + +[Service] +BusName=org.libvirt +DynamicUser=yes +User=@SYSTEM_USER@ +Group=@SYSTEM_USER@ +ExecStart=@sbindir@/libvirt-dbus --system + +[Install] +Alias=org.libvirt.service diff --git data/system/meson.build data/system/meson.build index 74f1949..67657d4 100644 --- data/system/meson.build +++ data/system/meson.build @@ -1,9 +1,18 @@ -configure_file( - configuration: conf, - input: 'org.libvirt.service.in', - output: 'org.libvirt.service', - install_dir: dbus_system_services_dir, -) +if init_script == 'systemd' + configure_file( + configuration: conf, + input: 'org.libvirt-systemd.service.in', + output: 'org.libvirt.service', + install_dir: dbus_system_services_dir, + ) +else + configure_file( + configuration: conf, + input: 'org.libvirt.service.in', + output: 'org.libvirt.service', + install_dir: dbus_system_services_dir, + ) +endif

Here you can use this to reduce code duplication: if init_script == 'systemd' dbus_service_in = 'org.libvirt-systemd.service.in' else dbus_service_in = 'org.libvirt.service.in' endif configure_file( configuration: conf, input: dbus_service_in, output: 'org.libvirt.service', install_dir: dbus_system_services_dir, )

configure_file( configuration: conf, input: 'org.libvirt.conf.in', @@ -16,3 +25,12 @@ configure_file( output: 'libvirt-dbus.rules', install_dir: polkit_rules_dir, ) +if init_script == 'systemd' + systemd_system_unit_dir = systemd_dep.get_pkgconfig_variable('systemdsystemunitdir') + configure_file( + configuration: conf, + input: 'libvirt-dbus.service.in', + output: 'libvirt-dbus.service', + install_dir: systemd_system_unit_dir, + ) +endif diff --git data/system/org.libvirt-systemd.service.in data/system/org.libvirt-systemd.service.in new file mode 100644 index 0000000..ba260b2 --- /dev/null +++ data/system/org.libvirt-systemd.service.in @@ -0,0 +1,5 @@ +[D-BUS Service] +Name=org.libvirt +Exec=/bin/false +User=@SYSTEM_USER@ +SystemdService=libvirt-dbus.service diff --git libvirt-dbus.spec.in libvirt-dbus.spec.in index 4e6ff85..8286609 100644 --- libvirt-dbus.spec.in +++ libvirt-dbus.spec.in @@ -40,7 +40,8 @@ This package provides D-Bus API for libvirt %autosetup

%build -%meson +%meson \ + -Dinit_script=systemd

I would say 4 spaces are good enough as indentation.

%meson_build

%install @@ -57,6 +58,7 @@ exit 0 %doc AUTHORS.rst NEWS.rst %license COPYING %{_sbindir}/libvirt-dbus +%{_unitdir}/libvirt-dbus.service %{_datadir}/dbus-1/services/org.libvirt.service %{_datadir}/dbus-1/system-services/org.libvirt.service %{_datadir}/dbus-1/system.d/org.libvirt.conf diff --git meson.build meson.build index e765ed6..c34c07d 100644 --- meson.build +++ meson.build @@ -12,6 +12,18 @@ project( prefix = get_option('prefix') datadir = prefix / get_option('datadir') sbindir = prefix / get_option('sbindir') +if get_option('init_script') == 'check' + if find_program('systemctl', required: false).found() + init_script = 'systemd' + else + init_script = 'other' + endif +else + init_script = get_option('init_script') +endif

Inconsistent indentation, in libvirt-dbus we use 4 spaces.

+if init_script == 'systemd' + systemd_dep = dependency('systemd') +endif

opt_dirs = [ 'dbus_interfaces', diff --git meson_options.txt meson_options.txt index 36e8065..41d348f 100644 --- meson_options.txt +++ meson_options.txt @@ -4,3 +4,4 @@ option('dbus_system_policies', type: 'string', value: 'dbus-1/system.d', descrip option('dbus_interfaces', type: 'string', value: 'dbus-1/interfaces', description: 'D-Bus interfaces directory') option('polkit_rules', type: 'string', value: 'polkit-1/rules.d', description: 'polkit rules directory') option('system_user', type: 'string', value: 'libvirtdbus', description: 'username to run system instance as') +option('init_script', type: 'combo', choices: ['systemd', 'other', 'check'], value: 'check', description: 'Style of init script to install') -- 2.26.2

Overall looks good. One note though, libvirt-dbus switched to gitlab and uses merge request workflow. So would you please create a merge request here: https://gitlab.com/libvirt/libvirt-dbus Pavel

On Mon, Aug 31, 2020 at 09:45:16AM +0200, Katerina Koukiou wrote:

See https://bugs.launchpad.net/ubuntu/+source/libvirt-dbus/+bug/1892756

Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- data/system/libvirt-dbus.service.in | 1 + 1 file changed, 1 insertion(+)

diff --git data/system/libvirt-dbus.service.in data/system/libvirt-dbus.service.in index 862a366..aa46217 100644 --- data/system/libvirt-dbus.service.in +++ data/system/libvirt-dbus.service.in @@ -15,6 +15,7 @@ BusName=org.libvirt DynamicUser=yes User=@SYSTEM_USER@ Group=@SYSTEM_USER@ +SupplementaryGroups=libvirt

This is not a standard a group that exists on other distros. So either this needs to be configurable, or it needs to be made as a downstream change only. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|