Skip to contentSkip to conversation

Allow non-workdir mount in overlayfs?

PATCH hujianyang 2 participants 3 messages Jan 14, 2015 Overlay filesystem

lore
Share & tools
lore

hujianyang

author
#0 lore raw

Hi Miklos,

I was considering the "FIXME: workdir is not needed for a R/O mount" you left in ovl_fill_super() these days.

Actually Seunghun (Seunghun Lee <[email protected]>) has sent a patch about these problem. But I have some different ideas. I think there are two ways to fix this problem. One, just remove this *FIXME*. Another, Allow non-workdir mount.

1) Remove *FIXME*; Further, use workdir in R/W case only

As multi-layer mount are allowed in ovl, users could initialize a R/O mount by setting multi lower directories. Workdir should only be used in R/W case with upperdir exist. So this *FIXME* can be removed.

We should add some information in the code and document about this.

2) Allow non-workdir mount

This logic is simple, just mark upper_mnt as R/O if workdir is not exist in the option line.

I've written a patch about it below. But I'm not sure which way is better, or there are other acceptable solutions. I need your advisement.

Here is a problem, if upper partition is R/O mount, ovl can't create *work* directory in workdir and return an error:

overlayfs: failed to create directory ./workdir/work

I guess this problem can be fixed if we decide how to deal with R/O mount.

Thanks,
Hu

Non-workdir mount patch:

fs/overlayfs/super.c +33 -27
index b90952f..c7141ff 100644
@@ -520,7 +520,8 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
 	seq_printf(m, ",lowerdir=%s", ufs->config.lowerdir);
 	if (ufs->config.upperdir) {
 		seq_printf(m, ",upperdir=%s", ufs->config.upperdir);
-		seq_printf(m, ",workdir=%s", ufs->config.workdir);
+		if (ufs->config.workdir)
+			seq_printf(m, ",workdir=%s", ufs->config.workdir);
 	}
 	return 0;
 }

@@ -530,7 +531,8 @@ 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)))
+	    (!ufs->upper_mnt || !ufs->workdir ||
+	    (ufs->upper_mnt->mnt_sb->s_flags & MS_RDONLY)))
 		return -EROFS;
 
 	return 0;

@@ -837,28 +839,24 @@ 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;
-		}
-
 		err = ovl_mount_dir(ufs->config.upperdir, &upperpath);
 		if (err)
 			goto out_free_config;
 
-		err = ovl_mount_dir(ufs->config.workdir, &workpath);
-		if (err)
-			goto out_put_upperpath;
+		if (ufs->config.workdir) {
+			err = ovl_mount_dir(ufs->config.workdir, &workpath);
+			if (err)
+				goto out_put_upperpath;
 
-		err = -EINVAL;
-		if (upperpath.mnt != workpath.mnt) {
-			pr_err("overlayfs: workdir and upperdir must reside under the same mount\n");
-			goto out_put_workpath;
-		}
-		if (!ovl_workdir_ok(workpath.dentry, upperpath.dentry)) {
-			pr_err("overlayfs: workdir and upperdir must be separate subtrees\n");
-			goto out_put_workpath;
+			err = -EINVAL;
+			if (upperpath.mnt != workpath.mnt) {
+				pr_err("overlayfs: workdir and upperdir must reside under the same mount\n");
+				goto out_put_workpath;
+			}
+			if (!ovl_workdir_ok(workpath.dentry, upperpath.dentry)) {
+				pr_err("overlayfs: workdir and upperdir must be separate subtrees\n");
+				goto out_put_workpath;
+			}
 		}
 		sb->s_stack_depth = upperpath.mnt->mnt_sb->s_stack_depth;
 	}

@@ -901,12 +899,16 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 			goto out_put_lowerpath;
 		}
 
-		ufs->workdir = ovl_workdir_create(ufs->upper_mnt, workpath.dentry);
-		err = PTR_ERR(ufs->workdir);
-		if (IS_ERR(ufs->workdir)) {
-			pr_err("overlayfs: failed to create directory %s/%s\n",
-			       ufs->config.workdir, OVL_WORKDIR_NAME);
-			goto out_put_upper_mnt;
+		if (ufs->config.workdir) {
+			ufs->workdir = ovl_workdir_create(ufs->upper_mnt, workpath.dentry);
+			err = PTR_ERR(ufs->workdir);
+			if (IS_ERR(ufs->workdir)) {
+				pr_err("overlayfs: failed to create directory %s/%s\n",
+				       ufs->config.workdir, OVL_WORKDIR_NAME);
+				goto out_put_upper_mnt;
+			}
+		} else {
+			ufs->upper_mnt->mnt_flags |= MNT_READONLY;
 		}
 	}
 

@@ -932,8 +934,12 @@ 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 r/o or nonexistent, or workdir is absent,
+	 * we mark overlayfs r/o too
+	 */
+	if (!ufs->upper_mnt || !ufs->workdir ||
+	    (ufs->upper_mnt->mnt_sb->s_flags & MS_RDONLY))
 		sb->s_flags |= MS_RDONLY;
 
 	sb->s_d_op = &ovl_dentry_operations;
maintainer
#1 lore raw
Quoted 10 lines · hujianyang

Hi Miklos,

I was considering the "FIXME: workdir is not needed for a R/O mount" you left in ovl_fill_super() these days.

Actually Seunghun (Seunghun Lee <[email protected]>) has sent a patch about these problem. But I have some different ideas. I think there are two ways to fix this problem. One, just remove this *FIXME*. Another, Allow non-workdir mount.

Quoted 7 lines

1) Remove *FIXME*; Further, use workdir in R/W case only

As multi-layer mount are allowed in ovl, users could initialize a R/O mount by setting multi lower directories. Workdir should only be used in R/W case with upperdir exist. So this *FIXME* can be removed.

We should add some information in the code and document about this.

I'd vote for this.

If there's no workdir, just use multiple lowedirs.

If there is a workdir, then specify it (and overlay can still be mounted R/O, possibly remounted R/W)

Thanks,
Miklos

author
#2 lore raw
Quoted 17 lines · Miklos Szeredi
Quoted 16 lines · hujianyang

Hi Miklos,

I was considering the "FIXME: workdir is not needed for a R/O mount" you left in ovl_fill_super() these days.

Actually Seunghun (Seunghun Lee <[email protected]>) has sent a patch about these problem. But I have some different ideas. I think there are two ways to fix this problem. One, just remove this *FIXME*. Another, Allow non-workdir mount.

1) Remove *FIXME*; Further, use workdir in R/W case only

As multi-layer mount are allowed in ovl, users could initialize a R/O mount by setting multi lower directories. Workdir should only be used in R/W case with upperdir exist. So this *FIXME* can be removed.

Quoted 9 lines
Quoted 1 line

We should add some information in the code and document about this.

I'd vote for this.

If there's no workdir, just use multiple lowedirs.

If there is a workdir, then specify it (and overlay can still be mounted R/O, possibly remounted R/W)

Got it~!

I'd like to send a patch about it tomorrow.

By the way, I think we should add a error message in parse_opt, see the mail send by Fabian, do you agree with it?

Thanks,
Hu