[libvirt] [PATCH] libvirt: lxc: fix incorrect parameter of lxcContainerMountProcFuse

when we have no host's src mapped to container's root. there is not .oldroot dir,we should pass "/" to lxcContainerMountProcFuse in function lxcContainerSetupExtraMounts. Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_container.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index d234426..09a4365 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -2059,7 +2059,7 @@ static int lxcContainerSetupExtraMounts(virDomainDefPtr vmDef, goto cleanup; /* Mounts /proc/meminfo etc sysinfo */ - if (lxcContainerMountProcFuse(vmDef, "/.oldroot") < 0) + if (lxcContainerMountProcFuse(vmDef, "/") < 0) goto cleanup; /* Now we can re-mount the cgroups controllers in the -- 1.7.11.7

On Wed, Jan 09, 2013 at 04:05:58PM +0800, Gao feng wrote:
when we have no host's src mapped to container's root. there is not .oldroot dir,we should pass "/" to lxcContainerMountProcFuse in function lxcContainerSetupExtraMounts.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_container.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index d234426..09a4365 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -2059,7 +2059,7 @@ static int lxcContainerSetupExtraMounts(virDomainDefPtr vmDef, goto cleanup;
/* Mounts /proc/meminfo etc sysinfo */ - if (lxcContainerMountProcFuse(vmDef, "/.oldroot") < 0) + if (lxcContainerMountProcFuse(vmDef, "/") < 0) goto cleanup;
/* Now we can re-mount the cgroups controllers in the
No, we should be passing NULL in that case, and make the method handle NULL correctly. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 2013/01/09 18:33, Daniel P. Berrange wrote:
On Wed, Jan 09, 2013 at 04:05:58PM +0800, Gao feng wrote:
when we have no host's src mapped to container's root. there is not .oldroot dir,we should pass "/" to lxcContainerMountProcFuse in function lxcContainerSetupExtraMounts.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_container.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index d234426..09a4365 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -2059,7 +2059,7 @@ static int lxcContainerSetupExtraMounts(virDomainDefPtr vmDef, goto cleanup;
/* Mounts /proc/meminfo etc sysinfo */ - if (lxcContainerMountProcFuse(vmDef, "/.oldroot") < 0) + if (lxcContainerMountProcFuse(vmDef, "/") < 0) goto cleanup;
/* Now we can re-mount the cgroups controllers in the
No, we should be passing NULL in that case, and make the method handle NULL correctly.
You mean this? diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index d234426..9f22923 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -605,7 +605,7 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def, if ((ret = virAsprintf(&meminfo_path, "%s/%s/%s/meminfo", - srcprefix, LXC_STATE_DIR, + srcprefix ? srcprefix : "", LXC_STATE_DIR, def->name)) < 0) return ret; @@ -2059,7 +2059,7 @@ static int lxcContainerSetupExtraMounts(virDomainDefPtr vm Def, goto cleanup; /* Mounts /proc/meminfo etc sysinfo */ - if (lxcContainerMountProcFuse(vmDef, "/.oldroot") < 0) + if (lxcContainerMountProcFuse(vmDef, NULL) < 0) goto cleanup; /* Now we can re-mount the cgroups controllers in the Thanks!

On Wed, Jan 09, 2013 at 06:45:48PM +0800, Gao feng wrote:
On 2013/01/09 18:33, Daniel P. Berrange wrote:
On Wed, Jan 09, 2013 at 04:05:58PM +0800, Gao feng wrote:
when we have no host's src mapped to container's root. there is not .oldroot dir,we should pass "/" to lxcContainerMountProcFuse in function lxcContainerSetupExtraMounts.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_container.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index d234426..09a4365 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -2059,7 +2059,7 @@ static int lxcContainerSetupExtraMounts(virDomainDefPtr vmDef, goto cleanup;
/* Mounts /proc/meminfo etc sysinfo */ - if (lxcContainerMountProcFuse(vmDef, "/.oldroot") < 0) + if (lxcContainerMountProcFuse(vmDef, "/") < 0) goto cleanup;
/* Now we can re-mount the cgroups controllers in the
No, we should be passing NULL in that case, and make the method handle NULL correctly.
You mean this?
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index d234426..9f22923 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -605,7 +605,7 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def,
if ((ret = virAsprintf(&meminfo_path, "%s/%s/%s/meminfo", - srcprefix, LXC_STATE_DIR, + srcprefix ? srcprefix : "", LXC_STATE_DIR, def->name)) < 0) return ret;
@@ -2059,7 +2059,7 @@ static int lxcContainerSetupExtraMounts(virDomainDefPtr vm Def, goto cleanup;
/* Mounts /proc/meminfo etc sysinfo */ - if (lxcContainerMountProcFuse(vmDef, "/.oldroot") < 0) + if (lxcContainerMountProcFuse(vmDef, NULL) < 0) goto cleanup;
/* Now we can re-mount the cgroups controllers in the
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

when we has no host's src mapped to container. there is no .oldroot dir,so libvirt lxc will fail to start when mouting meminfo. in this case,the parameter srcprefix of function lxcContainerMountProcFuse should be NULL.and make this method handle NULL correctly. Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_container.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index d234426..9f22923 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -605,7 +605,7 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def, if ((ret = virAsprintf(&meminfo_path, "%s/%s/%s/meminfo", - srcprefix, LXC_STATE_DIR, + srcprefix ? srcprefix : "", LXC_STATE_DIR, def->name)) < 0) return ret; @@ -2059,7 +2059,7 @@ static int lxcContainerSetupExtraMounts(virDomainDefPtr vmDef, goto cleanup; /* Mounts /proc/meminfo etc sysinfo */ - if (lxcContainerMountProcFuse(vmDef, "/.oldroot") < 0) + if (lxcContainerMountProcFuse(vmDef, NULL) < 0) goto cleanup; /* Now we can re-mount the cgroups controllers in the -- 1.7.11.7

On 09.01.2013 12:01, Gao feng wrote:
when we has no host's src mapped to container. there is no .oldroot dir,so libvirt lxc will fail to start when mouting meminfo.
in this case,the parameter srcprefix of function lxcContainerMountProcFuse should be NULL.and make this method handle NULL correctly.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_container.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index d234426..9f22923 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -605,7 +605,7 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def,
if ((ret = virAsprintf(&meminfo_path, "%s/%s/%s/meminfo", - srcprefix, LXC_STATE_DIR, + srcprefix ? srcprefix : "", LXC_STATE_DIR, def->name)) < 0) return ret;
@@ -2059,7 +2059,7 @@ static int lxcContainerSetupExtraMounts(virDomainDefPtr vmDef, goto cleanup;
/* Mounts /proc/meminfo etc sysinfo */ - if (lxcContainerMountProcFuse(vmDef, "/.oldroot") < 0) + if (lxcContainerMountProcFuse(vmDef, NULL) < 0) goto cleanup;
/* Now we can re-mount the cgroups controllers in the
Now pushed. Thanks. Michal

(2013/01/09 23:16), Michal Privoznik wrote:
On 09.01.2013 12:01, Gao feng wrote:
when we has no host's src mapped to container. there is no .oldroot dir,so libvirt lxc will fail to start when mouting meminfo.
in this case,the parameter srcprefix of function lxcContainerMountProcFuse should be NULL.and make this method handle NULL correctly.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_container.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index d234426..9f22923 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -605,7 +605,7 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def,
if ((ret = virAsprintf(&meminfo_path, "%s/%s/%s/meminfo", - srcprefix, LXC_STATE_DIR, + srcprefix ? srcprefix : "", LXC_STATE_DIR, def->name)) < 0) return ret;
@@ -2059,7 +2059,7 @@ static int lxcContainerSetupExtraMounts(virDomainDefPtr vmDef, goto cleanup;
/* Mounts /proc/meminfo etc sysinfo */ - if (lxcContainerMountProcFuse(vmDef, "/.oldroot") < 0) + if (lxcContainerMountProcFuse(vmDef, NULL) < 0) goto cleanup;
/* Now we can re-mount the cgroups controllers in the
Now pushed. Thanks.
Isn't it better to add following implicitly ? <filesystem type='mount' accessmode='passthrough'> <source dir='/'/> <target dir='/'/> </filesystem> Then, non-chroot lxc container will run the same code path with chroot container. No ? Thanks, -Kame

On 2013/01/11 07:36, Kamezawa Hiroyuki wrote:
(2013/01/09 23:16), Michal Privoznik wrote:
On 09.01.2013 12:01, Gao feng wrote:
when we has no host's src mapped to container. there is no .oldroot dir,so libvirt lxc will fail to start when mouting meminfo.
in this case,the parameter srcprefix of function lxcContainerMountProcFuse should be NULL.and make this method handle NULL correctly.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_container.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index d234426..9f22923 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -605,7 +605,7 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def,
if ((ret = virAsprintf(&meminfo_path, "%s/%s/%s/meminfo", - srcprefix, LXC_STATE_DIR, + srcprefix ? srcprefix : "", LXC_STATE_DIR, def->name)) < 0) return ret;
@@ -2059,7 +2059,7 @@ static int lxcContainerSetupExtraMounts(virDomainDefPtr vmDef, goto cleanup;
/* Mounts /proc/meminfo etc sysinfo */ - if (lxcContainerMountProcFuse(vmDef, "/.oldroot") < 0) + if (lxcContainerMountProcFuse(vmDef, NULL) < 0) goto cleanup;
/* Now we can re-mount the cgroups controllers in the
Now pushed. Thanks.
Isn't it better to add following implicitly ?
<filesystem type='mount' accessmode='passthrough'> <source dir='/'/> <target dir='/'/> </filesystem>
Then, non-chroot lxc container will run the same code path with chroot container. No ?
I'm not quite understand what's the difference between passing host's root to container and nothing being passed to container. It seems one difference is if the /dev directory is shared between host and container? Thanks.

On Fri, Jan 11, 2013 at 08:36:46AM +0900, Kamezawa Hiroyuki wrote:
(2013/01/09 23:16), Michal Privoznik wrote:
On 09.01.2013 12:01, Gao feng wrote:
when we has no host's src mapped to container. there is no .oldroot dir,so libvirt lxc will fail to start when mouting meminfo.
in this case,the parameter srcprefix of function lxcContainerMountProcFuse should be NULL.and make this method handle NULL correctly.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> --- src/lxc/lxc_container.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index d234426..9f22923 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -605,7 +605,7 @@ static int lxcContainerMountProcFuse(virDomainDefPtr def,
if ((ret = virAsprintf(&meminfo_path, "%s/%s/%s/meminfo", - srcprefix, LXC_STATE_DIR, + srcprefix ? srcprefix : "", LXC_STATE_DIR, def->name)) < 0) return ret;
@@ -2059,7 +2059,7 @@ static int lxcContainerSetupExtraMounts(virDomainDefPtr vmDef, goto cleanup;
/* Mounts /proc/meminfo etc sysinfo */ - if (lxcContainerMountProcFuse(vmDef, "/.oldroot") < 0) + if (lxcContainerMountProcFuse(vmDef, NULL) < 0) goto cleanup;
/* Now we can re-mount the cgroups controllers in the
Now pushed. Thanks.
Isn't it better to add following implicitly ?
<filesystem type='mount' accessmode='passthrough'> <source dir='/'/> <target dir='/'/> </filesystem>
Then, non-chroot lxc container will run the same code path with chroot container.
There'd still be one minor difference - /dev is shared in the non-chroot case, while the chroot case has completely private /dev. I agree that it'd be nice to unify the code paths though. There's another issue which is going to require us to refactor the code anyway. Some users want to be able to have mounts from the host inherited into the container. This requires us to remove the pivot_root code and use a different approach to setting up the private root. When doing this change I think it'll be possible unify the 2 codepaths. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (4)
-
Daniel P. Berrange
-
Gao feng
-
Kamezawa Hiroyuki
-
Michal Privoznik