[libvirt] [PATCH 3/4] Trim /proc & /sys subtrees before mounting new instances

[Daniel P. Berrange]

On Fri, May 11, 2012 at 10:04:24PM -0400, Stefan Berger wrote:
> On 05/11/2012 12:48 PM, Daniel P. Berrange wrote:
> >From: "Daniel P. Berrange"<berrange at redhat.com>
> >
> >Both /proc and /sys may have sub-mounts in them from the host
> >OS. We must explicitly unmount them all before mounting the
> >new instance over that location. If we don't then /proc/mounts
> >will show the sub-mounts as existing, even though nothing will
> >be able to access them, due to the over-mount.
> >
> >Signed-off-by: Daniel P. Berrange<berrange at redhat.com>
> >---
> >  src/lxc/lxc_container.c |   61 ++++++++++++++++++++++++++++++++++++-----------
> >  1 file changed, 47 insertions(+), 14 deletions(-)
> >
> >@@ -1054,13 +1057,36 @@ static int lxcContainerUnmountOldFS(void)
> >          qsort(mounts, nmounts, sizeof(mounts[0]),
> >                lxcContainerChildMountSort);
> >
> >+    *mountsret = mounts;
> >+    *nmountsret = nmounts;
> >+    ret = 0;
> >+
> >+cleanup:
> >+    endmntent(procmnt);
> >+    return ret;
> >+}
> >+
> >+static int lxcContainerUnmountSubtree(const char *prefix,
> >+                                      bool isOldRootFS)
> >+{
> >+    char **mounts = NULL;
> >+    size_t nmounts = 0;
> >+    size_t i;
> >+    int saveErrno;
> >+    const char *failedUmount = NULL;
> >+    int ret = -1;
> >+
> >+    VIR_DEBUG("Unmount subtreee from %s", prefix);
> >+
> >+    if (lxcContainerGetSubtree(prefix,&mounts,&nmounts)<  0)
> >+        return -1;
> >      for (i = 0 ; i<  nmounts ; i++) {
> >          VIR_DEBUG("Umount %s", mounts[i]);
> >          if (umount(mounts[i])<  0) {
> >              char ebuf[1024];
> >              failedUmount = mounts[i];
> >              saveErrno = errno;
> >-            VIR_WARN("Failed to unmount '%s', trying to detach root '%s': %s",
> >+            VIR_WARN("Failed to unmount '%s', trying to detach subtree '%s': %s",
> >                       failedUmount, mounts[nmounts-1],
> >                       virStrerror(errno, ebuf, sizeof(ebuf)));
> >              break;
> 
> 
> This may be an existing issue - should the code not try to continue
> unmounting rather than break'ing follwing error above? Would leaving
> the loop here leave stale mounts behind?

In this scenario, if we fail to unmount any path, then code later will
check 'failedUmount' and unmount2(MNT_DETACH) the entire root filesystem.
So any mounts are lazily cleaned up by the kernel & inaccessible to the
container.

That said, we probably could continue trying to unmount other dirs,
just to avoid wasting kernel memory from the detached mount tree.


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