Re: [libvirt] [PATCH v2] LXC: Detect fs support. Mount only supported filesystems

-----Original Message----- From: Gao feng [mailto:gaofeng@cn.fujitsu.com] Sent: Friday, October 04, 2013 12:55 PM To: Purcareata Bogdan-B43198 Cc: libvir-list(a)redhat.com Subject: Re: [libvirt] [PATCH v2] LXC: Detect fs support. Mount only supported filesystems On 10/02/2013 10:05 PM, Bogdan Purcareata wrote: > Kept ((access(dstpath, R_OK) < 0) || (!lxcCheckFSSupport(mnt->type))) > when determining support for the mount. Even if the filesystem type is > supported, there is still a chance to fail when building the dstpath > (virFileMakePath). If that call fails, starting the container will fail. > Specifically encountered this problem for securityfs, as I was unable > to mkdir /sys/kernel/security. > > Signed-off-by: Bogdan Purcareata <bogdan.purcareata(a)freescale.com&gt; > --- > src/lxc/lxc_container.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 67 insertions(+) > > diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c > index 989e920..496443d 100644 > --- a/src/lxc/lxc_container.c > +++ b/src/lxc/lxc_container.c > @@ -509,6 +509,67 @@ static int lxcContainerChildMountSort(const void *a, const void *b) > # define MS_SLAVE (1<<19) > #endif > > +/* > + * This function attempts to detect kernel support > + * for a specific filesystem type. This is done by > + * inspecting /proc/filesystems. > + */ > +static int lxcCheckFSSupport(const char *fs_type) > +{ > + FILE *fp = NULL; > + int ret = -1; > + const char *fslist = "/proc/filesystems"; > + char *line = NULL; > + char *type; > + size_t n; > + > + /* there should be no problem mounting an entry > + * with NULL fs type, hence NULL fs types are > + * supported */ > + if (!fs_type) { > + ret = 1; > + goto out; > + } > + > + VIR_DEBUG("Checking kernel support for %s in %s", fs_type, fslist); > + > + if (!(fp = fopen(fslist, "r"))) { I don't know if we can open /proc/filesystems successfully here if container shares root directory with host, since the /proc filesystem has been unmounted in lxcContainerUnmountForSharedRoot.

> + virReportSystemError(errno, > + _("Unable to read %s"), > + fslist); > + goto out; > + } > + > + while(getline(&line, &n, fp) > 0) { > + type = strstr(line, fs_type); > + > + if (!type) > + continue; > + > + if (!strncmp(type, fs_type, strlen(type))) { The strncmp() function compares the only first (at most) n bytes of s1 and s2. please use STREQ here.

> + ret = 1; > + goto cleanup; > + } > + } > + > + if (ferror(fp)) { > + virReportSystemError(errno, > + _("Error reading line from %s"), > + fslist); > + goto cleanup; > + } > + > + VIR_DEBUG("No kernel support for %s", fs_type); > + > + ret = 0; > + You set ret to 0 here, so the return value 0 means this filesystem is unsupported by kernel, right? what the meaning of return value -1? you return -1 when ferror(fp) is true.

> +cleanup: > + VIR_FREE(line); > + VIR_FORCE_FCLOSE(fp); > +out: > + return ret; > +} > + > static int lxcContainerGetSubtree(const char *prefix, > char ***mountsret, > size_t *nmountsret) > @@ -789,17 +850,23 @@ static int lxcContainerMountBasicFS(bool userns_enabled) > for (i = 0; i < ARRAY_CARDINALITY(lxcBasicMounts); i++) { > virLXCBasicMountInfo const *mnt = &lxcBasicMounts[i]; > const char *srcpath = NULL; > + const char *dstpath = NULL; > > VIR_DEBUG("Processing %s -> %s", > mnt->src, mnt->dst); > > srcpath = mnt->src; > + dstpath = mnt->dst; > > /* Skip if mount doesn't exist in source */ > if ((srcpath[0] == '/') && > (access(srcpath, R_OK) < 0)) > continue; > > + if ((access(dstpath, R_OK) < 0) || /* mount is not present on host */ > + (!lxcCheckFSSupport(mnt->type))) /* no fs support in kernel */ > + continue; > + The access is in the incorrect place, it should be called after we create mnt- >dst. so Move this check after virFileMakePath(mnt->dst).

> #if WITH_SELINUX > if (STREQ(mnt->src, SELINUX_MOUNT) && > (!is_selinux_enabled() || userns_enabled)) > Thanks