[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] xenconsoled: Remove unexpected daemonize behavior
On Mon, Nov 02, 2015 at 04:45:37PM +0000, Ross Lagerwall wrote: > On 11/02/2015 04:37 PM, Wei Liu wrote: > >On Mon, Nov 02, 2015 at 11:17:38AM +0000, Ross Lagerwall wrote: > >>Previously, xenconsoled's daemonize function would do nothing if its > >>parent process is init (as it is under systemd but not sysv init). > >>This is confusing. Instead, always daemonize when asked to, but use the > >>"interactive" switch when running from the systemd service. > >> > >>Because a pidfile is only written when daemonizing, drop the pidfile > >>parameters from the service file (systemd keeps track of the pids > >>anyway). > >> > >>Signed-off-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx> > >>--- > >> tools/console/daemon/utils.c | 4 ---- > >> tools/hotplug/Linux/systemd/xenconsoled.service.in | 3 +-- > >> 2 files changed, 1 insertion(+), 6 deletions(-) > >> > >>diff --git a/tools/console/daemon/utils.c b/tools/console/daemon/utils.c > >>index dbb3b12..644f6af 100644 > >>--- a/tools/console/daemon/utils.c > >>+++ b/tools/console/daemon/utils.c > >>@@ -52,10 +52,6 @@ void daemonize(const char *pidfile) > >> int i; > >> char buf[100]; > >> > >>- if (getppid() == 1) { > >>- return; > >>- } > >>- > > > >Er, I never noticed this before. And code archeology doesn't tell me why > >it was written like this either. > > The fact that the service type was set to simple rather than forking was a > sign... > > > > >> if ((pid = fork()) > 0) { > >> exit(0); > >> } else if (pid == -1) { > >>diff --git a/tools/hotplug/Linux/systemd/xenconsoled.service.in > >>b/tools/hotplug/Linux/systemd/xenconsoled.service.in > >>index cd282bf..8e333b1 100644 > >>--- a/tools/hotplug/Linux/systemd/xenconsoled.service.in > >>+++ b/tools/hotplug/Linux/systemd/xenconsoled.service.in > >>@@ -10,10 +10,9 @@ Environment=XENCONSOLED_ARGS= > >> Environment=XENCONSOLED_TRACE=none > >> Environment=XENCONSOLED_LOG_DIR=@XEN_LOG_DIR@/console > >> EnvironmentFile=@CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons > >>-PIDFile=@XEN_RUN_DIR@/xenconsoled.pid > >> ExecStartPre=/bin/grep -q control_d /proc/xen/capabilities > >> ExecStartPre=/bin/mkdir -p ${XENCONSOLED_LOG_DIR} > >>-ExecStart=@sbindir@/xenconsoled --pid-file @XEN_RUN_DIR@/xenconsoled.pid > >>--log=${XENCONSOLED_TRACE} --log-dir=${XENCONSOLED_LOG_DIR} > >>$XENCONSOLED_ARGS > >>+ExecStart=@sbindir@/xenconsoled -i --log=${XENCONSOLED_TRACE} > >>--log-dir=${XENCONSOLED_LOG_DIR} $XENCONSOLED_ARGS > >> > > > >To the best of my knowledge this seems to conform with man 7 daemon in > >Linux, "New-Style Daemons" session: > > > >"For developing a new-style daemon, none of the initialization steps > >recommended for SysV daemons need to be implemented. New-style init > >systems such as systemd make all of them redundant. Moreover, since some > >of these steps interfere with process monitoring, file descriptor > >passing and other functionality of the init system, it is recommended > >not to execute them when run as new-style service." > > > >So the use of "-i" seems justified. > > > > If a service needs to listen on a port or something and other services need > to depend on it, then the preferred method would be using something like > sd_notify. A less satisfactory approach would be to use forking, and then > only writing the pidfile after the port is opened. > > In this case, I don't think xenconsoled has any of these requirements so > using Type=simple and keeping it in the foreground is the correct thing to > do. > I think I'm convinced. Acked-by: Wei Liu <wei.liu2@xxxxxxxxxx> > -- > Ross Lagerwall _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |