[libvirt] [PATCH v2 0/3] Improve init script choice

Changes from [v1]: * look for systemctl(1) to detect systemd instead of relying on whether or not the /etc/systemd directory is present, as suggested by Martin * use 'systemd+redhat' instead of plain 'systemd' on RHEL * move everything to a macro tucked away in a separate file Tested on Debian sid, Fedora rawhide, FreeBSD 10.3 and CentOS 6. [v1] https://www.redhat.com/archives/libvir-list/2016-April/msg01930.html Andrea Bolognani (3): configure: Improve --with-init-script=check configure: Add systemd detection to --with-init-script=check configure: Introduce LIBVIRT_{CHECK,RESULT}_INIT_SCRIPT configure.ac | 48 ++--------------------------- m4/virt-init-script.m4 | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 46 deletions(-) create mode 100644 m4/virt-init-script.m4 -- 2.5.5

If we didn't find a match, either because we're cross compiling or because we're not building on RHEL, we won't install any init script. Make sure this is reported correctly in the configure summary. --- configure.ac | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/configure.ac b/configure.ac index 4380724..4149e20 100644 --- a/configure.ac +++ b/configure.ac @@ -630,6 +630,19 @@ AC_ARG_WITH([init-script], init_redhat=no init_systemd=no init_upstart=no +if test "$with_init_script" = check; then + if test "$cross_compiling" = yes; then + with_init_script=none + fi + if test -f /etc/redhat-release; then + if test "$with_init_script" = check; then + with_init_script=redhat + fi + fi + if test "$with_init_script" = check; then + with_init_script=none + fi +fi case "$with_init_script" in systemd+redhat) init_redhat=yes @@ -646,12 +659,6 @@ case "$with_init_script" in ;; none) ;; - check) - if test "$cross_compiling" != yes && test -f /etc/redhat-release; then - init_redhat=yes - with_init_script=redhat - fi - ;; *) AC_MSG_ERROR([Unknown initscript flavour $with_init_script]) ;; -- 2.5.5

Most distributions, including RHEL, have switched to systemd, so we should detect it and act accordingly. This also means that 'systemd+redhat' should be preferred to legacy 'redhat'. Our witness for the check is the availability of the systemctl command on the host. --- configure.ac | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/configure.ac b/configure.ac index 4149e20..f5cb1c3 100644 --- a/configure.ac +++ b/configure.ac @@ -634,7 +634,15 @@ if test "$with_init_script" = check; then if test "$cross_compiling" = yes; then with_init_script=none fi + if type systemctl >/dev/null 2>&1; then + if test "$with_init_script" = check; then + with_init_script=systemd + fi + fi if test -f /etc/redhat-release; then + if test "$with_init_script" = systemd; then + with_init_script=systemd+redhat + fi if test "$with_init_script" = check; then with_init_script=redhat fi -- 2.5.5

On Mon, May 02, 2016 at 01:51:26PM +0200, Andrea Bolognani wrote:
Most distributions, including RHEL, have switched to systemd, so we should detect it and act accordingly. This also means that 'systemd+redhat' should be preferred to legacy 'redhat'.
Our witness for the check is the availability of the systemctl command on the host. --- configure.ac | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/configure.ac b/configure.ac index 4149e20..f5cb1c3 100644 --- a/configure.ac +++ b/configure.ac @@ -634,7 +634,15 @@ if test "$with_init_script" = check; then if test "$cross_compiling" = yes; then with_init_script=none fi + if type systemctl >/dev/null 2>&1; then + if test "$with_init_script" = check; then + with_init_script=systemd + fi
I liked the /etc/systemd check a bit more, but this also works. ACK Jan

On Mon, 2016-05-02 at 17:01 +0200, Ján Tomko wrote:
On Mon, May 02, 2016 at 01:51:26PM +0200, Andrea Bolognani wrote:
Most distributions, including RHEL, have switched to systemd, so we should detect it and act accordingly. This also means that 'systemd+redhat' should be preferred to legacy 'redhat'.
Our witness for the check is the availability of the systemctl command on the host. --- configure.ac | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/configure.ac b/configure.ac index 4149e20..f5cb1c3 100644 --- a/configure.ac +++ b/configure.ac @@ -634,7 +634,15 @@ if test "$with_init_script" = check; then if test "$cross_compiling" = yes; then with_init_script=none fi + if type systemctl >/dev/null 2>&1; then + if test "$with_init_script" = check; then + with_init_script=systemd + fi
I liked the /etc/systemd check a bit more, but this also works.
This check should be more accurate, though, eg. won't install the systemd unit files on your OpenRC-powered Gentoo box, whereas the previous code would have. -- Andrea Bolognani Software Engineer - Virtualization Team

Move the code dealing with init scripts to a separate file so configure.ac itself can be a little bit smaller. --- First time writing an autoconf macro, so I might very possibly have gotten something wrong. It seems to be working, though configure.ac | 63 ++----------------------------------- m4/virt-init-script.m4 | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 61 deletions(-) create mode 100644 m4/virt-init-script.m4 diff --git a/configure.ac b/configure.ac index f5cb1c3..88e2e20 100644 --- a/configure.ac +++ b/configure.ac @@ -616,66 +616,7 @@ if test x"$enable_debug" = x"yes"; then AC_DEFINE([ENABLE_DEBUG], [], [whether debugging is enabled]) fi - - -dnl -dnl init script flavor -dnl -AC_MSG_CHECKING([for init script flavor]) -AC_ARG_WITH([init-script], - [AS_HELP_STRING([--with-init-script@<:@=STYLE@:>@], - [Style of init script to install: redhat, systemd, systemd+redhat, - upstart, check, none @<:@default=check@:>@])], - [],[with_init_script=check]) -init_redhat=no -init_systemd=no -init_upstart=no -if test "$with_init_script" = check; then - if test "$cross_compiling" = yes; then - with_init_script=none - fi - if type systemctl >/dev/null 2>&1; then - if test "$with_init_script" = check; then - with_init_script=systemd - fi - fi - if test -f /etc/redhat-release; then - if test "$with_init_script" = systemd; then - with_init_script=systemd+redhat - fi - if test "$with_init_script" = check; then - with_init_script=redhat - fi - fi - if test "$with_init_script" = check; then - with_init_script=none - fi -fi -case "$with_init_script" in - systemd+redhat) - init_redhat=yes - init_systemd=yes - ;; - systemd) - init_systemd=yes - ;; - upstart) - init_upstart=yes - ;; - redhat) - init_redhat=yes - ;; - none) - ;; - *) - AC_MSG_ERROR([Unknown initscript flavour $with_init_script]) - ;; -esac -AM_CONDITIONAL([LIBVIRT_INIT_SCRIPT_RED_HAT], test "$init_redhat" = "yes") -AM_CONDITIONAL([LIBVIRT_INIT_SCRIPT_UPSTART], test "$init_upstart" = "yes") -AM_CONDITIONAL([LIBVIRT_INIT_SCRIPT_SYSTEMD], test "$init_systemd" = "yes") -AC_MSG_RESULT($with_init_script) - +LIBVIRT_CHECK_INIT_SCRIPT AC_MSG_CHECKING([for whether to install sysctl config]) AC_ARG_WITH([sysctl], @@ -2959,7 +2900,7 @@ AC_MSG_NOTICE([ Warning Flags: $WARN_CFLAGS]) AC_MSG_NOTICE([ DTrace: $with_dtrace]) AC_MSG_NOTICE([ numad: $with_numad]) AC_MSG_NOTICE([ XML Catalog: $XML_CATALOG_FILE]) -AC_MSG_NOTICE([ Init script: $with_init_script]) +LIBVIRT_RESULT_INIT_SCRIPT AC_MSG_NOTICE([ Char device locks: $with_chrdev_lock_files]) AC_MSG_NOTICE([ Default Editor: $DEFAULT_EDITOR]) AC_MSG_NOTICE([ Loader/NVRAM: $with_loader_nvram]) diff --git a/m4/virt-init-script.m4 b/m4/virt-init-script.m4 new file mode 100644 index 0000000..9d0d5c7 --- /dev/null +++ b/m4/virt-init-script.m4 @@ -0,0 +1,84 @@ +dnl Init script type +dnl +dnl Copyright (C) 2005-2016 Red Hat, Inc. +dnl +dnl This library is free software; you can redistribute it and/or +dnl modify it under the terms of the GNU Lesser General Public +dnl License as published by the Free Software Foundation; either +dnl version 2.1 of the License, or (at your option) any later version. +dnl +dnl This library is distributed in the hope that it will be useful, +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +dnl Lesser General Public License for more details. +dnl +dnl You should have received a copy of the GNU Lesser General Public +dnl License along with this library. If not, see +dnl <http://www.gnu.org/licenses/>. +dnl + +AC_DEFUN([LIBVIRT_CHECK_INIT_SCRIPT],[ + AC_ARG_WITH([init-script], + [AS_HELP_STRING([--with-init-script@<:@=STYLE@:>@], + [Style of init script to install: redhat, systemd, systemd+redhat, + upstart, check, none @<:@default=check@:>@])], + [],[with_init_script=check]) + + AC_MSG_CHECKING([for init script type]) + + init_redhat=no + init_systemd=no + init_upstart=no + + if test "$with_init_script" = check; then + if test "$cross_compiling" = yes; then + with_init_script=none + fi + fi + if type systemctl >/dev/null 2>&1; then + if test "$with_init_script" = check; then + with_init_script=systemd + fi + fi + if test -f /etc/redhat-release; then + if test "$with_init_script" = systemd; then + with_init_script=systemd+redhat + fi + if test "$with_init_script" = check; then + with_init_script=redhat + fi + fi + if test "$with_init_script" = check; then + with_init_script=none + fi + + AS_CASE([$with_init_script], + [systemd+redhat],[ + init_redhat=yes + init_systemd=yes + ], + [systemd],[ + init_systemd=yes + ], + [upstart],[ + init_upstart=yes + ], + [redhat],[ + init_redhat=yes + ], + [none],[], + [*],[ + AC_MSG_ERROR([Unknown initscript flavour $with_init_script]) + ] + ) + + AM_CONDITIONAL([LIBVIRT_INIT_SCRIPT_RED_HAT], test "$init_redhat" = "yes") + AM_CONDITIONAL([LIBVIRT_INIT_SCRIPT_UPSTART], test "$init_upstart" = "yes") + AM_CONDITIONAL([LIBVIRT_INIT_SCRIPT_SYSTEMD], test "$init_systemd" = "yes") + + AC_MSG_RESULT($with_init_script) +]) + +AC_DEFUN([LIBVIRT_RESULT_INIT_SCRIPT],[ + AC_MSG_NOTICE([ Init script: $with_init_script]) +]) -- 2.5.5

On Mon, May 02, 2016 at 01:51:24PM +0200, Andrea Bolognani wrote:
Changes from [v1]:
* look for systemctl(1) to detect systemd instead of relying on whether or not the /etc/systemd directory is present, as suggested by Martin
* use 'systemd+redhat' instead of plain 'systemd' on RHEL
* move everything to a macro tucked away in a separate file
Tested on Debian sid, Fedora rawhide, FreeBSD 10.3 and CentOS 6.
[v1] https://www.redhat.com/archives/libvir-list/2016-April/msg01930.html
Andrea Bolognani (3): configure: Improve --with-init-script=check configure: Add systemd detection to --with-init-script=check configure: Introduce LIBVIRT_{CHECK,RESULT}_INIT_SCRIPT
configure.ac | 48 ++--------------------------- m4/virt-init-script.m4 | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 46 deletions(-) create mode 100644 m4/virt-init-script.m4
ACK series Jan

On Mon, 2016-05-02 at 17:01 +0200, Ján Tomko wrote:
On Mon, May 02, 2016 at 01:51:24PM +0200, Andrea Bolognani wrote:
Changes from [v1]:
* look for systemctl(1) to detect systemd instead of relying on whether or not the /etc/systemd directory is present, as suggested by Martin
* use 'systemd+redhat' instead of plain 'systemd' on RHEL
* move everything to a macro tucked away in a separate file
Tested on Debian sid, Fedora rawhide, FreeBSD 10.3 and CentOS 6.
[v1] https://www.redhat.com/archives/libvir-list/2016-April/msg01930.html
Andrea Bolognani (3): configure: Improve --with-init-script=check configure: Add systemd detection to --with-init-script=check configure: Introduce LIBVIRT_{CHECK,RESULT}_INIT_SCRIPT
configure.ac | 48 ++--------------------------- m4/virt-init-script.m4 | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 46 deletions(-) create mode 100644 m4/virt-init-script.m4
ACK series
Pushed, thanks for your review(s) :) -- Andrea Bolognani Software Engineer - Virtualization Team
participants (2)
-
Andrea Bolognani
-
Ján Tomko