[libvirt] [PATCH v2] LXC: Detect fs support. Mount only supported filesystems
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"))) { + 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))) { + 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; + +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; + #if WITH_SELINUX if (STREQ(mnt->src, SELINUX_MOUNT) && (!is_selinux_enabled() || userns_enabled)) -- 1.7.11.7
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.
+ 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
-----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
On 10/04/2013 07:33 PM, Purcareata Bogdan-B43198 wrote:
+/* + * 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.
So save the supported filesystem list before we unmount the proc filesystem, and in lxcCheckFSSupport use this list to check if the filesystem is supported by kernel. btw it's better to return the error of fopen to user.
+ 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
I don't know how this occurred, since the directory securityfs is created when you mount sysfs. Actually virFileMakePath will not create securityfs directory since it already exists. Thanks
On 10/07/2013 11:44 AM, Gao feng wrote:
On 10/04/2013 07:33 PM, Purcareata Bogdan-B43198 wrote:
+/* + * 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.
So save the supported filesystem list before we unmount the proc filesystem, and in lxcCheckFSSupport use this list to check if the filesystem is supported by kernel.
btw it's better to return the error of fopen to user.
+ 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
I don't know how this occurred, since the directory securityfs is created when you mount sysfs. Actually virFileMakePath will not create securityfs directory since it already exists.
So I think you can remove the check of access(dstpath, R_OK). if securityfs is supported, virFileMakePath must be successful. the reason fail to create directory securityfs is creating files is unsupported by sysfs.
participants (3)
-
Bogdan Purcareata -
Gao feng -
Purcareata Bogdan-B43198