Change in vdsm[master]: init: add --pidfile option in vdsmd.service

Hunt Xu has uploaded a new change for review.

Change subject: init: add --pidfile option in vdsmd.service
......................................................................

init: add --pidfile option in vdsmd.service

The pid file is not automatically created when using systemd. However
the file is still needed by vdsm/storage/protect/spmprotect.sh.

Change-Id: I2d8c5efd84d3838f54116d493ad7eac36ced68b4
Signed-off-by: huntxu 
---
M init/systemd/vdsmd.service.in
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/22/21222/1

diff --git a/init/systemd/vdsmd.service.in b/init/systemd/vdsmd.service.in
index 94be97a..ed7f905 100644
--- a/init/systemd/vdsmd.service.in
+++ b/init/systemd/vdsmd.service.in
@@ -9,7 +9,7 @@
 LimitCORE=infinity
 EnvironmentFile=-/etc/sysconfig/vdsm
 ExecStartPre=@LIBEXECDIR@/vdsmd_init_common.sh --pre-start
-ExecStart=@VDSMDIR@/daemonAdapter -0 /dev/null -1 /dev/null -2 /dev/null "@VDSMDIR@/vdsm"
+ExecStart=@VDSMDIR@/daemonAdapter -0 /dev/null -1 /dev/null -2 /dev/null "@VDSMDIR@/vdsm" --pidfile=@VDSMRUNDIR@/vdsmd.pid
 ExecStopPost=@LIBEXECDIR@/vdsmd_init_common.sh --post-stop
 Restart=always
 Nice=-20

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: init: add --pidfile option in vdsmd.service
......................................................................

Patch Set 1:

No Builds Executed

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4603/ : To avoid overloading the infrastructure, a whitelist for running gerrit triggered jobs has been set in place, if you feel like you should be in it, please contact infra at ovirt dot org.

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5403/ : To avoid overloading the infrastructure, a whitelist for running gerrit triggered jobs has been set in place, if you feel like you should be in it, please contact infra at ovirt dot org.

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5482/ : To avoid overloading the infrastructure, a whitelist for running gerrit triggered jobs has been set in place, if you feel like you should be in it, please contact infra at ovirt dot org.

Itamar Heim has posted comments on this change.

Change subject: init: add --pidfile option in vdsmd.service
......................................................................

Patch Set 1:

ping?

Yaniv Bronhaim has posted comments on this change.

Change subject: init: add --pidfile option in vdsmd.service
......................................................................

Patch Set 1: Code-Review-1

we don't have --pidfile option as part of daemonAdapter, it needs to follow a patch that adds it or abandoned till full solution

Yaniv Bronhaim has posted comments on this change.

Change subject: init: add --pidfile option in vdsmd.service
......................................................................

Patch Set 1:

(1 comment)

....................................................
File init/systemd/vdsmd.service.in
Line 8: Type=simple
Line 9: LimitCORE=infinity
Line 10: EnvironmentFile=-/etc/sysconfig/vdsm
Line 11: ExecStartPre=@LIBEXECDIR@/vdsmd_init_common.sh --pre-start
Line 12: ExecStart=@VDSMDIR@/daemonAdapter -0 /dev/null -1 /dev/null -2 /dev/null "@VDSMDIR@/vdsm" --pidfile=@VDSMRUNDIR@/vdsmd.pid
why not to use PIDFile option:

http://www.freedesktop.org/software/systemd/man/systemd.service.html
Line 13: ExecStopPost=@LIBEXECDIR@/vdsmd_init_common.sh --post-stop
Line 14: Restart=always
Line 15: Nice=-20
Line 16: User=@VDSMUSER@

Hunt Xu has posted comments on this change.

Change subject: init: add --pidfile option in vdsmd.service
......................................................................

Patch Set 1:

(1 comment)

The --pidfile option would be parsed as targetOptions in the daemonAdapter script and it's already part of the vdsm script. We use the same way to call daemonAdapter in sysvinit scripts.

....................................................
File init/systemd/vdsmd.service.in
Line 8: Type=simple
Line 9: LimitCORE=infinity
Line 10: EnvironmentFile=-/etc/sysconfig/vdsm
Line 11: ExecStartPre=@LIBEXECDIR@/vdsmd_init_common.sh --pre-start
Line 12: ExecStart=@VDSMDIR@/daemonAdapter -0 /dev/null -1 /dev/null -2 /dev/null "@VDSMDIR@/vdsm" --pidfile=@VDSMRUNDIR@/vdsmd.pid
Did you try that? I doubt whether it works. Because using the option is not telling systemd to create the pidfile automatically thus it won't solve the problem.
Line 13: ExecStopPost=@LIBEXECDIR@/vdsmd_init_common.sh --post-stop
Line 14: Restart=always
Line 15: Nice=-20
Line 16: User=@VDSMUSER@

Yaniv Bronhaim has posted comments on this change.

Change subject: init: add --pidfile option in vdsmd.service
......................................................................

Patch Set 1:

but isn't it better practice to have PIDFile option for systemd ? if you prefer to use that can you verify that the pidfile is actually being created and add it also to the upstart script?

Yaniv Bronhaim has posted comments on this change.

Change subject: init: add --pidfile option in vdsmd.service
......................................................................

Patch Set 1:

and im not sure you're right. the --pidfile is passed to target which means to respawn. in systemd we don't use respawn script so this will be ignored.

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: init: add --pidfile option in vdsmd.service
......................................................................

Patch Set 1:

Please open an ovirt bug for this the pid file is not always created but needed, and add a Bug-Url to the commit message.

Itamar Heim has posted comments on this change.

Change subject: init: add --pidfile option in vdsmd.service
......................................................................

Patch Set 1:

so what's the verdict on this one?

Hunt Xu has posted comments on this change.

Change subject: init: add --pidfile option in vdsmd.service
......................................................................

Patch Set 1:

Sorry for not following this cuz I was occupied by other things.

I will leave this one as abandoned and later come back to it after I reproduce the problem and give a bug report.

Hunt Xu has abandoned this change.

Change subject: init: add --pidfile option in vdsmd.service
......................................................................

Abandoned