-----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@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@freescale.com> --- 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.
Right. I just noticed the search for "proc" fails, since /proc/filesystem requires procfs to be mounted. (Un)fortunately, my handling of lxcCheckFSSupport() bypassed this error, and mounted procfs anyways. I will update the code with a proper handle for the error code. I just don't see how I can handle all filesystem entries in an uniform manner, since each one is so special.
+ 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.
Thanks, I will update.
+ 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.
So I thought it would be like this: - -1 - error encountered - 0 - no error, no kernel support for the filesystem - 1 - no error, kernel support present
+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).
My specific problem was that mounting security failed even before reaching the actual mount syscall. It failed when doing virFileMakePath("/sys/kernel/securityfs"), because /sys is previously mounted read only (I realized this just now). root@p4080ds:/sys/kernel# mkdir securityfs mkdir: cannot create directory 'securityfs': No such file or directory
#if WITH_SELINUX if (STREQ(mnt->src, SELINUX_MOUNT) && (!is_selinux_enabled() || userns_enabled))
Thanks