Re: [libvirt] [PATCH] Unmount existing filesystems under user specified mounts in LXC
On 04/08/2013 09:12 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
If the user requests a mount for /run, this may hide any existing
mounts that are lower down in /run. The result is that the
container still sees the mounts in /proc/mounts, but cannot
access them
sh-4.2# df
df: '/run/user/501/gvfs': No such file or directory
df: '/run/media/berrange/LIVE': No such file or directory
df: '/run/media/berrange/SecureDiskA1': No such file or directory
df: '/run/libvirt/lxc/sandbox': No such file or directory
Filesystem 1K-blocks Used Available Use% Mounted on
/dev/mapper/vg_t500wlan-lv_root 151476396 135390200 8384900 95% /
tmpfs 1970888 3204 1967684 1% /run
/dev/sda1 194241 155940 28061 85% /boot
devfs 64 0 64 0% /dev
tmpfs 64 0 64 0% /sys/fs/cgroup
tmpfs 1970888 1200 1969688 1%
/etc/libvirt-sandbox/scratch
Before mounting any filesystem at a particular location, we
must recursively unmount anything at or below the target mount
point
Makes sense.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
src/lxc/lxc_container.c | 218 ++++++++++++++++++++++++------------------------
1 file changed, 111 insertions(+), 107 deletions(-)
+static int lxcContainerGetSubtree(const char *prefix,
+ char ***mountsret,
+ size_t *nmountsret)
We aren't very consistent on the style:
static int
lxcContainerGetSubtree(...
+{
+ FILE *procmnt;
+ struct mntent mntent;
+ char mntbuf[1024];
fixed-width buffer...
+ int ret = -1;
+ char **mounts = NULL;
+ size_t nmounts = 0;
+
+ VIR_DEBUG("prefix=%s", prefix);
+
+ *mountsret = NULL;
+ *nmountsret = 0;
+
+ if (!(procmnt = setmntent("/proc/mounts", "r"))) {
+ virReportSystemError(errno, "%s",
+ _("Failed to read /proc/mounts"));
+ return -1;
+ }
+
+ while (getmntent_r(procmnt, &mntent, mntbuf, sizeof(mntbuf)) != NULL) {
...and getmntent_r can return ERANGE if it is too small (where it is
definitely feasible that the user could supply a mount point larger than
1024) [well, I assume an ERANGE failure, even though the man page is
buggy for failing to mention errno values on failure, based on
similarity to other functions that fail with ERANGE when given a
too-small buffer]...
+ VIR_DEBUG("Got %s", mntent.mnt_dir);
+ if (!STRPREFIX(mntent.mnt_dir, prefix))
+ continue;
+
+ if (VIR_REALLOC_N(mounts, nmounts+1) < 0) {
Is VIR_EXPAND or VIR_RESIZE better than VIR_REALLOC_N?
+ virReportOOMError();
Another place that will conflict with OOM overhaul :)
> + goto cleanup;
> + }
> + if (!(mounts[nmounts] = strdup(mntent.mnt_dir))) {
+ virReportOOMError();
> + goto
cleanup;
> + }
> + nmounts++;
> + VIR_DEBUG("Grabbed %s", mntent.mnt_dir);
> + }
...but you aren't checking errno or allowing for the possibility of
needing to enlarge the buffer.
+
+ if (mounts)
+ qsort(mounts, nmounts, sizeof(mounts[0]),
+ lxcContainerChildMountSort);
+
+ ret = 0;
+cleanup:
+ *mountsret = mounts;
+ *nmountsret = nmounts;
+ endmntent(procmnt);
+ return ret;
+}
+
-static int lxcContainerUnmountSubtree(const char *prefix,
- bool isOldRootFS)
-{
Looks like this patch is a bit of code shuffling coupled with some
tweaks; would it be better if it were split into two parts so that the
code shuffling is done with no semantic change, making it easier for the
second patch to show only what changed?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 621 bytes)