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.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list