On Mon, Dec 18, 2017 at 11:01 AM, Chengguang Xu <cgxu519@xxxxxxxxxx> wrote:
> If upper fs supports tmpfile, check link ability between
> upperdir and workdir and print a warning message if check
> fails.
>
> Signed-off-by: Chengguang Xu <cgxu519@xxxxxxxxxx>
Code looks good, but missing some commentary.
Please mention in commit message as well as in comment above code
that the motivation for the check is different project id.
> ---
> 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.
Not that I care too much, but did you choose not to fallback to read-only?
>
> fs/overlayfs/super.c | 23 +++++++++++++++++------
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index 76440fe..829aea9 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);
> @@ -954,11 +954,22 @@ static int ovl_make_workdir(struct ovl_fs *ofs, struct path *workpath)
> 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
> + 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);
> + if (!IS_ERR(worktemp)) {
> + inode_lock_nested(ofs->workdir->d_inode, I_MUTEX_PARENT);
> + err = ovl_do_link(uppertemp, ofs->workdir->d_inode, worktemp, true);
> + inode_unlock(ofs->workdir->d_inode);
> + if (err)
> + pr_warn("overlayfs: can not move files between upperdir and workdir, it may cause write error.\n");
For the record, the way copy up works with index=on for lower hardlinks
is that an index file in work dir is linked (and not moved) to upper dir.
The the warning may as well be "cannot link files..." or "cannot
link/move files..."
at least it will be a statement that is closer to what we actually tested.
> + else
> + ovl_cleanup(ofs->workdir->d_inode, worktemp);
> + dput(worktemp);
> + }
> + dput(uppertemp);
> + } else
> pr_warn("overlayfs: upper fs does not support tmpfile.\n");
>
> /*
> --
> 1.8.3.1
>
--
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
