On 08/09/2012 09:20 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> ---
> 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@.
> +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@xxxxxxxxxx +1-919-301-3266
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