On 11/11/2011 07:32 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> 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.
Overall, though, I think the patch is good.
--
Eric Blake eblake@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list