[CRIU] [PATCH]v2 crtools: write pidfile, when service/page server is run as daemon and "--pidfile" is set

[Cyrill Gorcunov]

On Mon, Sep 16, 2013 at 01:39:09AM +0400, Ruslan Kuprieiev wrote:
> On 09/16/2013 01:12 AM, Cyrill Gorcunov wrote:
> >On Mon, Sep 16, 2013 at 12:58:49AM +0400, Ruslan Kuprieiev wrote:
> >>@@ -971,8 +956,11 @@ static inline int fork_with_pid(struct pstree_item *item)
> >>  	if (ca.clone_flags & CLONE_NEWPID)
> >>  		item->pid.real = ret;
> >>-	if (opts.pidfile && root_item == item)
> >>-		write_pidfile(opts.pidfile, ret);
> >>+	if (opts.pidfile && root_item == item) {
> >>+		ret = write_pidfile(opts.pidfile, ret);
> >>+		if (ret < 0)
> >>+			pr_perror("Can't write pidfile");
> >>+	}
> >I must admit I have no clue what you're guys doing so it
> >might be by purpose, but don't you need to exit with error
> >here if write_pidfile failed or it's safe?
>
> I've done like it is in previous "if". Monkey see, monkey do=). Also
> there are "err_unlock" and "err" labels, that are right below.

Ah, this comes from commit

commit 827c633b26197fd464551f0d5b22bfcc3653136f
Author: Pavel Emelyanov <xemul at parallels.com>
Date:   Tue Nov 27 21:59:58 2012 +0300

    rst: Write pidfile in separate fn
    
    Just for better readability.
    
    Signed-off-by: Pavel Emelyanov <xemul at parallels.com>

and write_pidfile is

static void write_pidfile(char *pfname, int pid)
{
	int fd;

	fd = open(pfname, O_WRONLY | O_TRUNC | O_CREAT, 0600);
	if (fd == -1) {
		pr_perror("Can't open %s", pfname);
		kill(pid, SIGKILL);
		return;
	}

	dprintf(fd, "%d", pid);
	close(fd);
}

Pavel, why kill() here, don't you need to pass exeution to err_unlock:
and such?