Quoting Qiang Huang (h.huangqi...@huawei.com):
> On 2014/1/24 1:17, Serge Hallyn wrote:
> > Quoting Qiang Huang (h.huangqi...@huawei.com):
> >> I already reasoned in the added comment, we need this because if not,
> >> lxc_container won't be freed when daemon exits, and PID file won't
> >> be unlinked either.
> > 
> > I see, but
> > 
> >> What about this:
> >>
> >> From 80f3862f9c4dbc8a05e79e50c50e79e30ffebc25 Mon Sep 17 00:00:00 2001
> >> From: Qiang Huang <h.huangqi...@huawei.com>
> >> Date: Thu, 23 Jan 2014 14:25:31 +0800
> >> Subject: [PATCH] daemon: add lxc_container_put to free container when 
> >> daemon exits
> >>
> >> PID file in lxc_container is unlinked when lxc_container_free,
> >> if we leak the container, the PID file also won't be removed
> >> after container down.
> >>
> >> Signed-off-by: Qiang Huang <h.huangqi...@huawei.com>
> > 
> > I was about to ack this, but then it occurred to me that
> > this seems to show that the pidfile free shouldn't be done
> > through lxc_container_free().  The pidfile always gets
> > written right before the reboot: label, and should simply be
> > explicitly removed in the place where you are adding a
> > lxc_container_put().  That way it is symmetric;  by having
> > it in lxc_container_free(), it is not.
> > 
> 
> You are right, please review this:
> 
> ---
> 
> From 4ed573856d9f0ec56b8e522efd40f986a0409aa1 Mon Sep 17 00:00:00 2001
> From: Qiang Huang <h.huangqi...@huawei.com>
> Date: Fri, 24 Jan 2014 11:41:27 +0800
> Subject: [PATCH] lxccontainer: remove PID file after lxc_start return
> 
> Make the way symmetric. This also fix the file leak in
> daemon model.
> 
> Signed-off-by: Qiang Huang <h.huangqi...@huawei.com>

Thanks, perfect!

Acked-by: Serge E. Hallyn <serge.hal...@ubuntu.com>

> ---
>  src/lxc/lxccontainer.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index 1520cd3..9ebb27e 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -233,11 +233,6 @@ static void lxc_container_free(struct lxc_container *c)
>               free(c->config_path);
>               c->config_path = NULL;
>       }
> -     if (c->pidfile) {
> -             unlink(c->pidfile);
> -             free(c->pidfile);
> -             c->pidfile = NULL;
> -     }
> 
>       free(c);
>  }
> @@ -665,6 +660,12 @@ reboot:
>               goto reboot;
>       }
> 
> +     if (c->pidfile) {
> +             unlink(c->pidfile);
> +             free(c->pidfile);
> +             c->pidfile = NULL;
> +     }
> +
>       if (daemonize)
>               exit (ret == 0 ? true : false);
>       else
> -- 
> 1.8.3
> 
> 
> 
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

Reply via email to