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-unionfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html