> -----Original Message----- > From: Gao feng [mailto:gaofeng@xxxxxxxxxxxxxx] > Sent: Friday, October 04, 2013 12:55 PM > To: Purcareata Bogdan-B43198 > Cc: libvir-list@xxxxxxxxxx > Subject: Re: [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@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. 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list