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
signature.asc
Description: Digital signature
_______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel
