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>
---
 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