[libvirt] [PATCH 20/23] Enable systemd socket activation with virtlockd

[Eric Blake]

On 08/09/2012 09:20 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at redhat.com>
> 
> This enhancement virtlockd so that it can receive a pre-opened

s/enhancement/enhances/

> UNIX domain socket from systemd at launch time, and adds the
> systemd service/socket unit files
> 
> * daemon/libvirtd.service.in: Require virtlockd to be running
> * libvirt.spec.in: Add virtlockd systemd files
> * src/Makefile.am: Install systemd files
> * src/locking/lock_daemon.c: Support socket activation
> * src/locking/virtlockd.service.in, src/locking/virtlockd.socket.in:
>   systemd unit files
> * src/rpc/virnetserverservice.c, src/rpc/virnetserverservice.h:
>   Add virNetServerServiceNewFD() method
> * src/rpc/virnetsocket.c, src/rpc/virnetsocket.h: Add virNetSocketNewListenFD
>   method
> 

I didn't closely read the systemd documentation to see if you integrated
correctly, so I have to assume that you tested this and that it worked.
 That said, I'll review the C code for accuracy.

> Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> ---
>  libvirt.spec.in                  |  6 ++++
>  src/Makefile.am                  | 51 ++++++++++++++++++++++++++--
>  src/locking/lock_daemon.c        | 73 ++++++++++++++++++++++++++++++++++++++--
>  src/locking/virtlockd.service.in | 13 +++++++
>  src/locking/virtlockd.socket.in  |  8 +++++
>  5 files changed, 147 insertions(+), 4 deletions(-)
>  create mode 100644 src/locking/virtlockd.service.in
>  create mode 100644 src/locking/virtlockd.socket.in
> 
> +++ b/src/Makefile.am
> @@ -1552,6 +1552,53 @@ virtlockd.init: locking/virtlockd.init.in $(top_builddir)/config.status
>  
>  
>  
> +EXTRA_DIST += locking/virtlockd.service.in locking/virtlockd.socket.in
> +
> +if WITH_LIBVIRTD
> +if LIBVIRT_INIT_SCRIPT_SYSTEMD
> +
> +SYSTEMD_UNIT_DIR = /lib/systemd/system
> +
> +BUILT_SOURCES += virtlockd.service virtlockd.socket
> +
> +install-systemd: virtlockd.init install-sysconfig
> +	mkdir -p $(DESTDIR)$(SYSTEMD_UNIT_DIR)

$(MKDIR_P)

> +	$(INSTALL_SCRIPT) virtlockd.service \
> +	  $(DESTDIR)$(SYSTEMD_UNIT_DIR)/
> +	$(INSTALL_SCRIPT) virtlockd.socket \
> +	  $(DESTDIR)$(SYSTEMD_UNIT_DIR)/

s/INSTALL_SCRIPT/INSTALL_DATA/2 - systemd files should not be marked
executable (cf. install-init-systemd in daemon/Makefile.am).

> 
> +
> +virtlockd.service: locking/virtlockd.service.in $(top_builddir)/config.status
> +	$(AM_V_GEN)sed					\
> +	    -e "s!::localstatedir::!$(localstatedir)!g"	\
> +	    -e "s!::sbindir::!$(sbindir)!g"		\
> +	    -e "s!::sysconfdir::!$(sysconfdir)!g"	\
> +	    < $< > $@-t &&				\
> +	    chmod a+x $@-t &&				\
> +	    mv $@-t $@
> +

Problem.  Here, you look for '::sbindir::'...

> +++ b/src/locking/lock_daemon.c
> @@ -454,8 +454,69 @@ virLockDaemonSetupSignals(virNetServerPtr srv)
>      return 0;
>  }
>  
> +
>  static int
> -virLockDaemonSetupNetworking(virNetServerPtr srv, const char *sock_path)
> +virLockDaemonSetupNetworkingSystemD(virNetServerPtr srv)
> +{

> +    unsetenv("LISTEN_PID");
> +    unsetenv("LISTEN_FDS");

I was scratching my head on this one, until I realized you want to
re-exec, and the re-exec uses our mechanisms rather than systemd
mechanisms to reuse the fd.

> +++ b/src/locking/virtlockd.service.in
> @@ -0,0 +1,13 @@
> +[Unit]
> +Description=Virtual machine lock manager
> +Requires=virtlockd.socket
> +After=syslog.target
> +
> +[Service]
> +EnvironmentFile=-/etc/sysconfig/virtlockd
> +ExecStart=@sbindir@/virtlockd

...but here, you used @sbindir at .

> +ExecReload=/bin/kill -HUP $MAINPID
> +# Loosing the locks is a really bad thing that will

s/Loosing/Losing/

> +# cause the machine to be fenced (rebooted), so make
> +# sure we discourage OOM killer
> +OOMScoreAdjust=-900

Losing locks is bad, but does virtlockd really cause fencing on failure,
or are you over-exaggerating the effects due to copying from something
stricter?

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120817/46b2548d/attachment-0001.sig>