[libvirt] [PATCH 1/3] LXC: avoid useless memory free in lxcContainerIdentifyCGroups

when lxcContainerIdentifyCGroups failed, lxcContainerSetupPivortRoot and lxcContainerSetupExtraMounts will free the memory that allocated in lxcContainerIdentifyCGroups. So we need not call lxcContainerCGroupFree when lxcContainerIdentifyCGroups failed. Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_container.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 4fbceb4..87de463 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1236,17 +1236,16 @@ static int lxcContainerIdentifyCGroups(struct lxcContainerCGroup **mountsret, } } - *mountsret = mounts; - *nmountsret = nmounts; ret = 0; cleanup: + *mountsret = mounts; + *nmountsret = nmounts; + closedir(dh); endmntent(procmnt); VIR_FREE(path); - if (ret < 0) - lxcContainerCGroupFree(mounts, nmounts); return ret; } -- 1.7.7.6

kill the "return 0;" code, it will cause memory leak. Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_container.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 87de463..a54c385 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1423,7 +1423,6 @@ static int lxcContainerSetupExtraMounts(virDomainDefPtr vmDef, goto cleanup; VIR_DEBUG("Mounting completed"); - return 0; ret = 0; -- 1.7.7.6

On 2012年06月15日 15:41, Gao feng wrote:
kill the "return 0;" code, it will cause memory leak.
Signed-off-by: Gao feng<gaofeng@cn.fujitsu.com> --- src/lxc/lxc_container.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 87de463..a54c385 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1423,7 +1423,6 @@ static int lxcContainerSetupExtraMounts(virDomainDefPtr vmDef, goto cleanup;
VIR_DEBUG("Mounting completed"); - return 0;
ret = 0;
ACK

print debug info "container support is enabled" when host support the user or net namespace. Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_container.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index a54c385..b2cbfb9 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1769,6 +1769,6 @@ int lxcContainerAvailable(int features) return -1; } - VIR_DEBUG("Mounted all filesystems"); + VIR_DEBUG("container support is enabled"); return 0; } -- 1.7.7.6

On 2012年06月15日 15:41, Gao feng wrote:
print debug info "container support is enabled" when host support the user or net namespace.
Signed-off-by: Gao feng<gaofeng@cn.fujitsu.com> --- src/lxc/lxc_container.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index a54c385..b2cbfb9 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1769,6 +1769,6 @@ int lxcContainerAvailable(int features) return -1; }
- VIR_DEBUG("Mounted all filesystems"); + VIR_DEBUG("container support is enabled"); return 0; }
ACK

On 06/18/2012 12:46 AM, Osier Yang wrote:
On 2012年06月15日 15:41, Gao feng wrote:
print debug info "container support is enabled" when host support the user or net namespace.
Signed-off-by: Gao feng<gaofeng@cn.fujitsu.com> --- src/lxc/lxc_container.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index a54c385..b2cbfb9 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1769,6 +1769,6 @@ int lxcContainerAvailable(int features) return -1; }
- VIR_DEBUG("Mounted all filesystems"); + VIR_DEBUG("container support is enabled"); return 0; }
ACK
I've pushed 2 and 3. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 2012年06月15日 15:41, Gao feng wrote:
when lxcContainerIdentifyCGroups failed, lxcContainerSetupPivortRoot and lxcContainerSetupExtraMounts will free the memory that allocated in lxcContainerIdentifyCGroups.
So we need not call lxcContainerCGroupFree when lxcContainerIdentifyCGroups failed.
Signed-off-by: Gao feng<gaofeng@cn.fujitsu.com> --- src/lxc/lxc_container.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 4fbceb4..87de463 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1236,17 +1236,16 @@ static int lxcContainerIdentifyCGroups(struct lxcContainerCGroup **mountsret, } }
- *mountsret = mounts; - *nmountsret = nmounts; ret = 0;
cleanup: + *mountsret = mounts; + *nmountsret = nmounts; + closedir(dh); endmntent(procmnt); VIR_FREE(path);
- if (ret< 0) - lxcContainerCGroupFree(mounts, nmounts); return ret; }
NACK. Personally I think removing the duplicate free() in the callers (only lxcContainerSetupPivortRoot and lxcContainerSetupExtraMounts now) is better. No reason to keep an allocated memory while the function itself fails. Regards, Osier

于 2012年06月18日 14:57, Osier Yang 写道:
On 2012年06月15日 15:41, Gao feng wrote:
when lxcContainerIdentifyCGroups failed, lxcContainerSetupPivortRoot and lxcContainerSetupExtraMounts will free the memory that allocated in lxcContainerIdentifyCGroups.
So we need not call lxcContainerCGroupFree when lxcContainerIdentifyCGroups failed.
Signed-off-by: Gao feng<gaofeng@cn.fujitsu.com> --- src/lxc/lxc_container.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 4fbceb4..87de463 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1236,17 +1236,16 @@ static int lxcContainerIdentifyCGroups(struct lxcContainerCGroup **mountsret, } }
- *mountsret = mounts; - *nmountsret = nmounts; ret = 0;
cleanup: + *mountsret = mounts; + *nmountsret = nmounts; + closedir(dh); endmntent(procmnt); VIR_FREE(path);
- if (ret< 0) - lxcContainerCGroupFree(mounts, nmounts); return ret; }
NACK. Personally I think removing the duplicate free() in the callers (only lxcContainerSetupPivortRoot and lxcContainerSetupExtraMounts now) is better. No reason to keep an allocated memory while the function itself fails.
Got it, I will resend this patch, thanks.
participants (3)
-
Eric Blake
-
Gao feng
-
Osier Yang