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

[Stéphane Graber]

On Tue, Jan 21, 2014 at 10:54:50AM +0800, Qiang Huang wrote:
> Hi Stéphane,
> 
> On 2014/1/21 0:07, Stéphane Graber wrote:
> > On Mon, Jan 20, 2014 at 03:22:31PM +0800, Qiang Huang wrote:
> >> On 2014/1/19 8:57, Stéphane Graber wrote:
> >>> On Sat, Jan 18, 2014 at 03:00:00PM +0800, Qiang Huang wrote:
> >>>> This is for bug: https://github.com/lxc/lxc/issues/89
> >>>>
> >>>> When start container with daemon model, the daemon process's
> >>>> father will return back to main in start time, and pidfile
> >>>> is removed then, that's not right.
> >>>> So we store pidfile to lxc_container, and remove it when
> >>>> lxc_container_free.
> >>>
> >>> That one looks wrong to me, removing the pid file on lxc_container_free
> >>> is wrong. We want the pidfile removed when the background lxc-start
> >>> exits, not whenever a random API client flushes the container from
> >>> memory.
> >>>
> >>> With your patch, doing something like:
> >>>  - lxc-start -n p1 -d -p /tmp/pid
> >>>  - python3
> >>>      import lxc
> >>>      p1 = lxc.Container("p1")
> >>>      p1 = None
> >>
> >> I'm sorry I'm not family with python, can you explain how this would
> >> happen in real world? Thanks.
> > 
> > In the real world, anything using the API to control an already running
> > backgrounded container with a PID file and that does lxc_container_put
> > once it's done dealing with the container object will cause the PID file
> > to be removed.
> 
> 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...

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

> 
> Anyway, my patch is based on one theory:
> A running container will have an lxc_container to hold it's information,
> which is created from start, the struct lxc_container lasts for all of
> container's life cycle.
> 
> If it's wrong, my question is:
> How could it be reasonable for a container running without an lxc_container?
> 
> > 
> > Actually, even calling lxc-list should cause the PID file to be removed
> > as lxc-list calls list_containers which calls list_defined_containers
> > which in turns iterate through all containers, get their struct and then
> > lxc_container_put them.
> > 
> >>
> >>>
> >>> Or the equivalent with any binding, or directly in C, will destroy the
> >>> pid file even though the container is clearly still running.
> >>
> >> I thought when the backgroud lxc-start exits, it's time for container
> >> to free, because there are no other place to do lxc_contaier_get to
> >> hold the container from freeing.
> >>
> >> I must missed something :( , so waiting for your more details.
> >>
> >>
> > 
> 
> 

-- 
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20140120/17f9600a/attachment.pgp>