Ferenc Wagner wrote:
Ferenc Wagner <wf...@niif.hu> writes:

Daniel Lezcano <dlezc...@fr.ibm.com> writes:

Ferenc Wagner wrote:

Daniel Lezcano <daniel.lezc...@free.fr> writes:

Ferenc Wagner wrote:

While playing with lxc-start, I noticed that /tmp is infested by
empty lxc-r* directories: [...] Ok, this name comes from lxc-rootfs
in conf.c:setup_rootfs.  After setup_rootfs_pivot_root returns, the
original /tmp is not available anymore, so rmdir(tmpname) at the
bottom of setup_rootfs can't achieve much.  Why is this temporary
name needed anyway?  Is pivoting impossible without it?
That was put in place with chroot, before pivot_root, so the distro's
scripts can remount their '/' without failing.

Now we have pivot_root, I suppose we can change that to something cleaner...
Like simply nuking it?  Shall I send a patch?
Sure, if we can kill it, I will be glad to take your patch :)
I can't see any reason why lxc-start couldn't do without that temporary
recursive bind mount of the original root.  If neither do you, I'll
patch it out and see if it still flies.

For my purposes the patch below works fine.  I only run applications,
though, not full systems, so wider testing is definitely needed.

Thanks,
Feri.

>From 98b24c13f809f18ab8969fb4d84defe6f812b25c Mon Sep 17 00:00:00 2001
From: Ferenc Wagner <wf...@niif.hu>
Date: Thu, 6 May 2010 14:47:39 +0200
Subject: [PATCH] no need to use a temporary directory for pivoting

That was put in place before lxc-start started using pivot_root, so
the distro scripts can remount / without problems.

Signed-off-by: Ferenc Wagner <wf...@niif.hu>
---
 src/lxc/conf.c |   28 +++-------------------------
 1 files changed, 3 insertions(+), 25 deletions(-)

diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index b27a11d..4379a32 100644
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -588,37 +588,15 @@ static int setup_rootfs_pivot_root(const char *rootfs, 
const char *pivotdir)
static int setup_rootfs(const char *rootfs, const char *pivotdir)
 {
-       char *tmpname;
-       int ret = -1;
-
        if (!rootfs)
                return 0;
- tmpname = tempnam("/tmp", "lxc-rootfs");
-       if (!tmpname) {
-               SYSERROR("failed to generate temporary name");
-               return -1;
-       }
-
-       if (mkdir(tmpname, 0700)) {
-               SYSERROR("failed to create temporary directory '%s'", tmpname);
-               return -1;
-       }
-
-       if (mount(rootfs, tmpname, "none", MS_BIND|MS_REC, NULL)) {
-               SYSERROR("failed to mount '%s'->'%s'", rootfs, tmpname);
-               goto out;
-       }
-
-       if (setup_rootfs_pivot_root(tmpname, pivotdir)) {
+       if (setup_rootfs_pivot_root(rootfs, pivotdir)) {
                ERROR("failed to pivot_root to '%s'", rootfs);
-               goto out;
+               return -1;
        }
- ret = 0;
-out:
-       rmdir(tmpname);
-       return ret;
+       return 0;
 }
static int setup_pts(int pts)

We can't simply remove it because of the pivot_root which returns EBUSY.

I suppose it's coming from:
"new_root and put_old must not be on the same file system as the current root."

But as we will pivot_root right after, we won't reuse the real rootfs, so we can safely use the host /tmp.

I tried the following patch and it worked.

Comments ?



Subject: no need to use a temporary directory for pivoting

From: Ferenc Wagner <wf...@niif.hu>

Ferenc Wagner <wf...@niif.hu> writes:

> Daniel Lezcano <dlezc...@fr.ibm.com> writes:
>
>> Ferenc Wagner wrote:
>>
>>> Daniel Lezcano <daniel.lezc...@free.fr> writes:
>>>
>>>> Ferenc Wagner wrote:
>>>>
>>>>> While playing with lxc-start, I noticed that /tmp is infested by
>>>>> empty lxc-r* directories: [...] Ok, this name comes from lxc-rootfs
>>>>> in conf.c:setup_rootfs.  After setup_rootfs_pivot_root returns, the
>>>>> original /tmp is not available anymore, so rmdir(tmpname) at the
>>>>> bottom of setup_rootfs can't achieve much.  Why is this temporary
>>>>> name needed anyway?  Is pivoting impossible without it?
>>>>
>>>> That was put in place with chroot, before pivot_root, so the distro's
>>>> scripts can remount their '/' without failing.
>>>>
>>>> Now we have pivot_root, I suppose we can change that to something cleaner...
>>>
>>> Like simply nuking it?  Shall I send a patch?
>>
>> Sure, if we can kill it, I will be glad to take your patch :)
>
> I can't see any reason why lxc-start couldn't do without that temporary
> recursive bind mount of the original root.  If neither do you, I'll
> patch it out and see if it still flies.

For my purposes the patch below works fine.  I only run applications,
though, not full systems, so wider testing is definitely needed.

Thanks,
Feri.

>From 98b24c13f809f18ab8969fb4d84defe6f812b25c Mon Sep 17 00:00:00 2001
Date: Thu, 6 May 2010 14:47:39 +0200

That was put in place before lxc-start started using pivot_root, so
the distro scripts can remount / without problems.

Signed-off-by: Ferenc Wagner <wf...@niif.hu>
---
 src/lxc/conf.c |   27 +++++++--------------------
 1 file changed, 7 insertions(+), 20 deletions(-)

Index: lxc/src/lxc/conf.c
===================================================================
--- lxc.orig/src/lxc/conf.c
+++ lxc/src/lxc/conf.c
@@ -581,37 +581,24 @@ static int setup_rootfs_pivot_root(const
 
 static int setup_rootfs(const char *rootfs, const char *pivotdir)
 {
-	char *tmpname;
-	int ret = -1;
+	const char *tmpfs = "/tmp";
 
 	if (!rootfs)
 		return 0;
 
-	tmpname = tempnam("/tmp", "lxc-rootfs");
-	if (!tmpname) {
-		SYSERROR("failed to generate temporary name");
+	if (mount(rootfs, tmpfs, "none", MS_BIND|MS_REC, NULL)) {
+		SYSERROR("failed to mount '%s'->'%s'", rootfs, "/tmp");
 		return -1;
 	}
 
-	if (mkdir(tmpname, 0700)) {
-		SYSERROR("failed to create temporary directory '%s'", tmpname);
-		return -1;
-	}
-
-	if (mount(rootfs, tmpname, "none", MS_BIND|MS_REC, NULL)) {
-		SYSERROR("failed to mount '%s'->'%s'", rootfs, tmpname);
-		goto out;
-	}
+	DEBUG("mounted '%s' on '%s'", rootfs, tmpfs);
 
-	if (setup_rootfs_pivot_root(tmpname, pivotdir)) {
+	if (setup_rootfs_pivot_root(tmpfs, pivotdir)) {
 		ERROR("failed to pivot_root to '%s'", rootfs);
-		goto out;
+		return -1;
 	}
 
-	ret = 0;
-out:
-	rmdir(tmpname);
-	return ret;
+	return 0;
 }
 
 static int setup_pts(int pts)
------------------------------------------------------------------------------

_______________________________________________
Lxc-users mailing list
Lxc-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lxc-users

Reply via email to