Just one question. If each r/o layer does not require a workdir, why
would a stack of r/o layers require one - and hence the requirement that
the top layer must be r/w?
Does it has to do with why workdir was introduced in the first place?
Sorry but I couldn't find information about why workdir was introduced. I
suppose it was to support some functions that older versions can't.
Alex
On Thu, January 15, 2015 12:20 am, Seunghun Lee wrote:
> On January 15, 2015 2:20:57 PM GMT+09:00, hujianyang
> <hujianyang@xxxxxxxxxx> wrote:
>>After importing multi-lower layer support, users could mount a r/o
>>partition as the left most lowerdir instead of using it as upperdir.
>>And a r/o upperdir may cause an error like
>>
>> overlayfs: failed to create directory ./workdir/work
>>
>>during mount.
>>
>>This patch check the *s_flags* of upper fs and return an error if
>>it is a r/o partition. The checking of *upper_mnt->mnt_sb->s_flags*
>>can be removed now.
>>
>>This patch also remove
>>
>> /* FIXME: workdir is not needed for a R/O mount */
>>
>>from ovl_fill_super() because:
>>
>>1) for upper fs r/o case
>>Setting a r/o partition as upper is prevented, no need to care about
>>workdir in this case.
>>
>>2) for "mount overlay -o ro" with a r/w upper fs case
>>Users could remount overlayfs to r/w in this case, so workdir should
>>not be omitted.
>>
>>Signed-off-by: hujianyang <hujianyang@xxxxxxxxxx>
>>---
>> fs/overlayfs/super.c | 24 +++++++++++++++++++-----
>> 1 files changed, 19 insertions(+), 5 deletions(-)
>>
>>diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
>>index edbb3eb..0e7a477 100644
>>--- a/fs/overlayfs/super.c
>>+++ b/fs/overlayfs/super.c
>>@@ -529,8 +529,7 @@ static int ovl_remount(struct super_block *sb, int
>>*flags, char *data)
>> {
>> struct ovl_fs *ufs = sb->s_fs_info;
>>
>>- if (!(*flags & MS_RDONLY) &&
>>- (!ufs->upper_mnt || (ufs->upper_mnt->mnt_sb->s_flags &
>>MS_RDONLY)))
>>+ if (!(*flags & MS_RDONLY) && (!ufs->upper_mnt))
>> return -EROFS;
>>
>> return 0;
>>@@ -619,6 +618,15 @@ static int ovl_parse_opt(char *opt, struct
>>ovl_config *config)
>> return -EINVAL;
>> }
>> }
>>+
>>+ /* Workdir is useless in non-upper mount */
>>+ if (!config->upperdir && config->workdir) {
>>+ pr_info("overlayfs: option \"workdir=%s\" is useless in a non-upper
>>mount, ignore\n",
>>+ config->workdir);
>>+ kfree(config->workdir);
>>+ config->workdir = NULL;
>>+ }
>>+
>> return 0;
>> }
>>
>>@@ -838,7 +846,6 @@ static int ovl_fill_super(struct super_block *sb,
>>void *data, int silent)
>>
>> sb->s_stack_depth = 0;
>> if (ufs->config.upperdir) {
>>- /* FIXME: workdir is not needed for a R/O mount */
>> if (!ufs->config.workdir) {
>> pr_err("overlayfs: missing 'workdir'\n");
>> goto out_free_config;
>>@@ -848,6 +855,13 @@ static int ovl_fill_super(struct super_block *sb,
>>void *data, int silent)
>> if (err)
>> goto out_free_config;
>>
>>+ /* Upper fs should not be r/o */
>>+ if (upperpath.mnt->mnt_sb->s_flags & MS_RDONLY) {
>>+ pr_err("overlayfs: upper fs is r/o, try multi-lower layers
>>mount\n");
>>+ err = -EINVAL;
>>+ goto out_put_upperpath;
>>+ }
>>+
>> err = ovl_mount_dir(ufs->config.workdir, &workpath);
>> if (err)
>> goto out_put_upperpath;
>>@@ -939,8 +953,8 @@ static int ovl_fill_super(struct super_block *sb,
>>void *data, int silent)
>> ufs->numlower++;
>> }
>>
>>- /* If the upper fs is r/o or nonexistent, we mark overlayfs r/o too
>>*/
>>- if (!ufs->upper_mnt || (ufs->upper_mnt->mnt_sb->s_flags & MS_RDONLY))
>>+ /* If the upper fs is nonexistent, we mark overlayfs r/o too */
>>+ if (!ufs->upper_mnt)
>> sb->s_flags |= MS_RDONLY;
>>
>> sb->s_d_op = &ovl_dentry_operations;
>
> It works fine to me.
> And I think it is better than my implementation : )
>
> Thanks.
>
> --
> 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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
