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

[Daniel P. Berrange]

On Fri, Nov 11, 2011 at 12:18:32PM -0700, Eric Blake wrote:
> On 11/11/2011 07:32 AM, Daniel P. Berrange wrote:
> > From: "Daniel P. Berrange" <berrange at redhat.com>
> > 
> > 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

Yep, done that.

> > -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'?

No, we already set that on the line above. This line is setting the
CLI arg variable, so a later AC_MSG_RESULT shows 'redhat'.

> > @@ -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?

I didn't want todo that, because switching between init systems is a bit
complex, and if someone installs this and then tries to downgrade to the
original distro packages the results will not be pleasant.

> 
> > +++ 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.

Ok, I reverted this rename

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|