Re: [libvirt] [PATCH] Add support for systemd init service

On 11/11/2011 07:32 AM, Daniel P. Berrange wrote: > From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; > > This patch adds support for a systemd init service for libvirtd > and libvirt-guests. The libvirtd.service is *not* written to use > socket activation, since we want libvirtd to start on boot so it > can do guest auto-start. > > +++ b/configure.ac > @@ -329,16 +329,30 @@ dnl init script flavor > dnl > AC_MSG_CHECKING([for init script flavor]) > AC_ARG_WITH([init-script], > - [AC_HELP_STRING([--with-init-script=@<:@redhat|auto|none@:>@], > + [AC_HELP_STRING([--with-init-script=@<:@redhat|systemd|systemd+redhat|upstart|auto|none@:>@], > [Style of init script to install @<:@default=auto@:>@])]) That's a bit long. Perhaps it would be better as: AC_HELP_STRING([--with-init-script@<:@=STYLE@:>@], [Style of init script to install: redhat, systemd, systemd+redhat, upstart, auto, none @<:@default=auto@:>@]) so that ./configure --help can take advantage of better word wrap

> -AM_CONDITIONAL([LIBVIRT_INIT_SCRIPT_RED_HAT], test x$with_init_script = xredhat) > +init_redhat=no > +init_systemd=no > +case "$with_init_script" in > + systemd+redhat) > + init_redhat=yes > + init_systemd=yes > + ;; > + systemd) > + init_systemd=yes > + ;; > + redhat) > + init_redhat=yes > + ;; > + *) > + if test "$cross_compiling" != yes && test -f /etc/redhat-release; then > + init_redhat=yes > + with_init_script=redhat Shouldn't this line be 'init_redhat=yes'?

> @@ -111,6 +112,11 @@ > %define with_hyperv 0 > %endif > > +# Although earlier Fedora has systemd, libvirt still used sysvinit > +%if 0%{?fedora} >= 17 > +%define with_systemd 1 > +%endif But if we use the configure option, then what's to stop the systemd script from working in older Fedora? That is, why not make this >= 16, not 17, so that people using the virt-preview repo to get 0.9.8 on F16 will benefit from systemd?

> +++ b/po/POTFILES.in > @@ -151,5 +151,5 @@ src/xenapi/xenapi_utils.c > src/xenxs/xen_sxpr.c > src/xenxs/xen_xm.c > tools/console.c > -tools/libvirt-guests.init.sh > +tools/libvirt-guests.init.in ... > diff --git a/tools/libvirt-guests.init.sh b/tools/libvirt-guests.init.in > similarity index 100% > rename from tools/libvirt-guests.init.sh > rename to tools/libvirt-guests.init.in I'm a bit worried on whether this will do the correct things with translations embedded in the file. My recollection was that we _had_ to name it .sh instead of .in in order to get xgettext to properly parse out the strings marked for translations into the .pot file. I haven't yet double-checked the resulting .pot file pre- and post-patch, but think you may have to revert this particular change.