Re: [PATCH] Add missing files for OpenRC

[Michal Prívozník <mprivozn@xxxxxxxxxx>]

On 2/22/20 6:01 AM, Ryan Moeller wrote:
> Signed-off-by: Ryan Moeller <ryan@xxxxxxxxxxxxx>
> ---
>  src/locking/Makefile.inc.am   |  7 +++++++
>  src/locking/virtlockd.init.in | 14 ++++++++++++++
>  src/logging/Makefile.inc.am   | 10 ++++++++++
>  src/logging/virtlogd.init.in  | 14 ++++++++++++++
>  4 files changed, 45 insertions(+)
>  create mode 100644 src/locking/virtlockd.init.in
>  create mode 100644 src/logging/virtlogd.init.in
> 
> diff --git a/src/locking/Makefile.inc.am b/src/locking/Makefile.inc.am
> index e663d7146b..243e3ae767 100644
> --- a/src/locking/Makefile.inc.am
> +++ b/src/locking/Makefile.inc.am
> @@ -79,6 +79,13 @@ VIRTLOCKD_UNIT_FILES_IN = \
>  SYSTEMD_UNIT_FILES += $(notdir $(VIRTLOCKD_UNIT_FILES_IN:%.in=%))
>  SYSTEMD_UNIT_FILES_IN += $(VIRTLOCKD_UNIT_FILES_IN)
>  
> +OPENRC_INIT_FILES += \
> +	virtlockd.init \
> +	$(NULL)
> +OPENRC_INIT_FILES_IN += \
> +	virtlockd.init.in \
> +	$(NULL)
> +
>  noinst_LTLIBRARIES += libvirt_driver_lock.la
>  
>  libvirt_la_BUILT_LIBADD += libvirt_driver_lock.la

Ooops, no rule to generate virtlockd.init.

> diff --git a/src/locking/virtlockd.init.in b/src/locking/virtlockd.init.in
> new file mode 100644
> index 0000000000..45eaed7971
> --- /dev/null
> +++ b/src/locking/virtlockd.init.in
> @@ -0,0 +1,14 @@
> +#!/sbin/openrc-run
> +
> +name=virtlogd

This is not necessary since the filename matches the daemon name. But,
I'd rather see some description.

> +
> +command=@sbindir@/virtlockd
> +pidfile="@runstatedir@/virtlockd.pid"
> +command_args="--daemon --pid-file=${pidfile}"
> +PATH="${PATH}:@sbindir@:@bindir@"
> +supervisor=supervise-daemon

I don't think this is right. The way supervise-daemon work is that it
runs the daemon in foreground, therefore we don't want to pass any
--daemon nor --pid-file arguments.

> +
> +depend() {
> +    provide virtlockd
> +    keyword -shutdown
> +}

I think we want reload() too, so that we don't lose opened FDs.


However, I have done fixes locally. How do you feel about me squashing
this in and then pushing?


diff --git i/src/locking/Makefile.inc.am w/src/locking/Makefile.inc.am
index 243e3ae767..d0e36705b2 100644
--- i/src/locking/Makefile.inc.am
+++ w/src/locking/Makefile.inc.am
@@ -282,6 +282,8 @@ locking/lock_daemon_dispatch_stubs.h: $(LOCK_PROTOCOL) \
 	    virLockSpaceProtocol VIR_LOCK_SPACE_PROTOCOL \
 	    $(LOCK_PROTOCOL) > locking/lock_daemon_dispatch_stubs.h

+virtlockd.init: locking/virtlockd.init.in $(top_builddir)/config.status
+	$(AM_V_GEN)$(SED) $(COMMON_UNIT_VARS) $< > $@-t && mv $@-t $@

 virtlockd.service: locking/virtlockd.service.in
$(top_builddir)/config.status
 	$(AM_V_GEN)sed $(COMMON_UNIT_VARS) $< > $@-t && mv $@-t $@
diff --git i/src/locking/virtlockd.init.in w/src/locking/virtlockd.init.in
index 45eaed7971..e5264fd211 100644
--- i/src/locking/virtlockd.init.in
+++ w/src/locking/virtlockd.init.in
@@ -1,14 +1,24 @@
 #!/sbin/openrc-run

-name=virtlogd
+description="Virtualization locking daemon"

-command=@sbindir@/virtlockd
-pidfile="@runstatedir@/virtlockd.pid"
-command_args="--daemon --pid-file=${pidfile}"
-PATH="${PATH}:@sbindir@:@bindir@"
-supervisor=supervise-daemon
+VIRTLOCKD_OPTS=${VIRTLOCKD_OPTS:-"${VIRTLOCKD_OPTS}"}
+
+command="@sbindir@/virtlockd"
+command_args="${VIRTLOCKD_OPTS}"
+supervisor="supervise-daemon"
+
+extra_started_commands="reload"
+description_reload="re-exec the daemon to enforce configuration reload"

 depend() {
-    provide virtlockd
     keyword -shutdown
+    after nfs nfsmount
+}
+
+reload() {
+    ebegin "re-exec() virtlockd"
+
+    ${supervisor} ${RC_SVCNAME} --signal HUP
+    eend $?
 }
diff --git i/src/logging/virtlogd.init.in w/src/logging/virtlogd.init.in
index 61e41f7689..4f804f42d7 100644
--- i/src/logging/virtlogd.init.in
+++ w/src/logging/virtlogd.init.in
@@ -1,14 +1,24 @@
 #!/sbin/openrc-run

-name=virtlogd
+description="Virtualization logging daemon"

-command=@sbindir@/virtlogd
-pidfile="@runstatedir@/virtlogd.pid"
-command_args="--daemon --pid-file=${pidfile}"
-PATH="${PATH}:@sbindir@:@bindir@"
-supervisor=supervise-daemon
+VIRTLOGD_OPTS=${VIRTLOGD_OPTS:-"${VIRTLOGD_OPTS}"}
+
+command="@sbindir@/virtlogd"
+command_args="${VIRTLOGD_OPTS}"
+supervisor="supervise-daemon"
+
+extra_started_commands="reload"
+description_reload="re-exec the daemon to enforce configuration reload"

 depend() {
-    provide virtlogd
     keyword -shutdown
+    after nfs nfsmount
+}
+
+reload() {
+    ebegin "re-exec() virtlogd"
+
+    ${supervisor} ${RC_SVCNAME} --signal HUP
+    eend $?
 }


Michal