Re: [PATCH v4] ovl: Check link ability between upperdir and workdir

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

 



> 
> 在 2018年1月20日,上午12:25,Miklos Szeredi <miklos@xxxxxxxxxx> 写道:
> 
> On Wed, Dec 20, 2017 at 3:09 AM, Chengguang Xu <cgxu519@xxxxxxxxxx> wrote:
>> Inspired by encountering unexpected write error with
>> error code -EXDEV when upperdir and workdir having
>> different project quotas. The reason of the error is
>> project quota asks files in it strictly inherit
>> project id of it’s own, so rename/link operations
>> between those directories will fail and it may cause
>> failure during copy-up/index-link operations in overlayfs.
>> 
>> So if upper filesystem supports O_TMPFILE, try to check
>> link ability between upperdir and workdir and if check
>> fails, print a proper warning message to indicate potential
>> possibility of write failure.
>> 
>> The failure of the check does not directly lead to
>> read-only mounting because in some use cases may only
>> write to upperdir and do not modify anything in lowerdirs.
>> 
>> Signed-off-by: Chengguang Xu <cgxu519@xxxxxxxxxx>
>> ---
>> Changes since v3:
>> - Add more detail comment about motivation to changelog.
>> 
>> Changes since v2:
>> - Add comment about motivation.
>> - Modify warning message.
>> 
>> Changes since v1:
>> - Check link ablity between upperdir and workdir instead of checking
>> project ids of upperdir and workdir.
>> - Print a warning message instead of falling to readonly mode.
>> 
>> fs/overlayfs/super.c | 30 +++++++++++++++++++++++-------
>> 1 file changed, 23 insertions(+), 7 deletions(-)
>> 
>> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
>> index 76440fe..7d6bd82 100644
>> --- a/fs/overlayfs/super.c
>> +++ b/fs/overlayfs/super.c
>> @@ -929,7 +929,7 @@ static int ovl_get_upper(struct ovl_fs *ofs, struct path *upperpath)
>> 
>> static int ovl_make_workdir(struct ovl_fs *ofs, struct path *workpath)
>> {
>> -       struct dentry *temp;
>> +       struct dentry *uppertemp, *worktemp;
>>        int err;
>> 
>>        ofs->workdir = ovl_workdir_create(ofs, OVL_WORKDIR_NAME, false);
>> @@ -953,12 +953,28 @@ static int ovl_make_workdir(struct ovl_fs *ofs, struct path *workpath)
>>        if (!err)
>>                pr_warn("overlayfs: upper fs needs to support d_type.\n");
>> 
>> -       /* Check if upper/work fs supports O_TMPFILE */
>> -       temp = ovl_do_tmpfile(ofs->workdir, S_IFREG | 0);
>> -       ofs->tmpfile = !IS_ERR(temp);
>> -       if (ofs->tmpfile)
>> -               dput(temp);
>> -       else
>> +       /*
>> +        * Check if upper/work fs supports O_TMPFILE and if support, check
>> +        * link ability between upperdir and workdir, this is inspired by
>> +        * encountering unexpected write error when upperdir and workdir
>> +        * having different project ids.
>> +        */
>> +       uppertemp = ovl_do_tmpfile(ofs->upper_mnt->mnt_root, S_IFREG | 0);
>> +       ofs->tmpfile = !IS_ERR(uppertemp);
>> +       if (ofs->tmpfile) {
>> +               worktemp = ovl_lookup_temp(ofs->workdir);
> 
> Have you tested this?  Because there's a warning at the top of
> lookup_one_len() that triggers because of no locking of parent.
> 
> I can add the locking, it's trivial, but I'd still like you to test
> this on your setup, if it actually does what you promise it does.


Hi Miklos,

Sorry that I could not reproduce it on my setup for the patch because
the test vm has already reclaimed, but I can see the warning in my new
test vm, so I guess maybe ‘DEBUG_BUGVERBOSE’ configuration was not 
properly set in the previous test setup.


Thanks,
Chengguang.


--
To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Filesystems Devel]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux