[lxc-devel] [PATCH 2/2] daemon: remove pidfile when daemon container is stopped

[Qiang Huang]

On 2014/1/21 11:38, Stéphane Graber wrote:
> On Tue, Jan 21, 2014 at 11:33:42AM +0800, Qiang Huang wrote:
>> On 2014/1/21 11:20, Stéphane Graber wrote:
>>> On Tue, Jan 21, 2014 at 10:54:50AM +0800, Qiang Huang wrote:
>>>> Hi Stéphane,
>>>>
>>>> I'm not sure I understand this.
>>>>
>>>> In my understanding, anything using the API to control an already
>>>> running backgrounded container will use lxc_container_new to create
>>>> a new lxc_container, it only share the original one's name and config
>>>> and so on. After dealing, will call lxc_container_put to free this
>>>> lxc_container, this *new* lxc_container doesn't contain the PID file
>>>> information, so it's freeing won't remove PID file.
>>>
>>> Hmm, yeah, seems like you're right, I should have rechecked the patch
>>> rather than speak from the vague memory of what I saw when you initialy
>>> posted it.
>>>
>>>
>>> So, re-reading more carefully, I don't completely disagree with the
>>> approach, though I don't like having to extend lxc_container just for
>>> that...
>>
>> Without 1/2 patch, this patch won't work, I'll send new patches.
>> So would it be better if I put this into lxc_conf?
>>
>>>
>>> There's however one rather big problem, the PID file will contain the
>>> wrong PID. The PID being written is getpid() from the parent lxc-start,
>>> not the running, forked lxc-start process and that child PID is only
>>> known by the start() function in lxccontainer.c, so you can't create the
>>> PID file from lxc_start.c
>>
>> Yeah, I noticed that before, I thought that's exactly what we want,
>> because we can get the init pid of container from lxc_info command.
>>
>> If it's not, we can fix it incrementally. :)
> 
> No, what I meant is that the PID we are writing is that of a
> non-existing process, which makes the pid file entirely useless.
> 
> When you start a container, you have at least 3 processes:
>  1) The command the user start (lxc-start -d)
>  2) The backgrounded fork of that command after start() is done
>  3) The container init process
> 
> lxc-info and the API init_pid() function gives us (3), what we want to
> see in a pidfile is (2) and what we are currently writing is (1).
> 
> As (1) exits as soon as the container is started, it's completely
> useless as you can't use that to check whether LXC still has control
> over the container (i.e. is lxc-start still running in the background).
> 

Oh, that's right, in the daemon model the PID file stores wrong pid,
I'll think about it and fix it in the new patch.

Thanks Stéphane.