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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxx>
> ---
>  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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]