All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] ovl: print error message for invalid mount options
@ 2015-01-15  5:17 ` hujianyang
  0 siblings, 0 replies; 13+ messages in thread
From: hujianyang @ 2015-01-15  5:17 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs, linux-fsdevel, Seunghun Lee, Fabian Sturm

Overlayfs should print an error message if an incorrect mount option
is caught like other filesystems.

After this patch, improper option input could be clearly known.

Reported-by: Fabian Sturm <fabian.sturm@aduu.de>
Signed-off-by: hujianyang <hujianyang@huawei.com>
---
 fs/overlayfs/super.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index b90952f..ab3c8cb 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -615,6 +615,7 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
 			break;

 		default:
+			pr_err("overlayfs: unrecognized mount option \"%s\" or missing value\n", p);
 			return -EINVAL;
 		}
 	}
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 1/3] ovl: print error message for invalid mount options
@ 2015-01-15  5:17 ` hujianyang
  0 siblings, 0 replies; 13+ messages in thread
From: hujianyang @ 2015-01-15  5:17 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs, linux-fsdevel, Seunghun Lee, Fabian Sturm

Overlayfs should print an error message if an incorrect mount option
is caught like other filesystems.

After this patch, improper option input could be clearly known.

Reported-by: Fabian Sturm <fabian.sturm@aduu.de>
Signed-off-by: hujianyang <hujianyang@huawei.com>
---
 fs/overlayfs/super.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index b90952f..ab3c8cb 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -615,6 +615,7 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
 			break;

 		default:
+			pr_err("overlayfs: unrecognized mount option \"%s\" or missing value\n", p);
 			return -EINVAL;
 		}
 	}
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/3] ovl: check lowerdir amount for non-upper mount
  2015-01-15  5:17 ` hujianyang
@ 2015-01-15  5:19   ` hujianyang
  -1 siblings, 0 replies; 13+ messages in thread
From: hujianyang @ 2015-01-15  5:19 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs, linux-fsdevel, Seunghun Lee, Fabian Sturm

Recently multi-lower layer mount support allow upperdir and workdir
to be omitted, then cause overlayfs can be mount with only one
lowerdir directory. This action make no sense and have potential risk.

This patch check the total number of lower directories to prevent
mounting overlayfs with only one directory.

Also, an error message is added to indicate lower directories exceed
*OVL_MAX_STACK* limit.

Signed-off-by: hujianyang <hujianyang@huawei.com>
---
 fs/overlayfs/super.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index ab3c8cb..edbb3eb 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -870,8 +870,14 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)

 	err = -EINVAL;
 	stacklen = ovl_split_lowerdirs(lowertmp);
-	if (stacklen > OVL_MAX_STACK)
+	if (stacklen > OVL_MAX_STACK) {
+		pr_err("overlayfs: too many lower directries, limit is %d\n",
+		       OVL_MAX_STACK);
 		goto out_free_lowertmp;
+	} else if (!ufs->config.upperdir && stacklen == 1) {
+		pr_err("overlayfs: at least 2 lowerdir are needed while upperdir nonexistent\n");
+		goto out_free_lowertmp;
+	}

 	stack = kcalloc(stacklen, sizeof(struct path), GFP_KERNEL);
 	if (!stack)
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/3] ovl: check lowerdir amount for non-upper mount
@ 2015-01-15  5:19   ` hujianyang
  0 siblings, 0 replies; 13+ messages in thread
From: hujianyang @ 2015-01-15  5:19 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs, linux-fsdevel, Seunghun Lee, Fabian Sturm

Recently multi-lower layer mount support allow upperdir and workdir
to be omitted, then cause overlayfs can be mount with only one
lowerdir directory. This action make no sense and have potential risk.

This patch check the total number of lower directories to prevent
mounting overlayfs with only one directory.

Also, an error message is added to indicate lower directories exceed
*OVL_MAX_STACK* limit.

Signed-off-by: hujianyang <hujianyang@huawei.com>
---
 fs/overlayfs/super.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index ab3c8cb..edbb3eb 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -870,8 +870,14 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)

 	err = -EINVAL;
 	stacklen = ovl_split_lowerdirs(lowertmp);
-	if (stacklen > OVL_MAX_STACK)
+	if (stacklen > OVL_MAX_STACK) {
+		pr_err("overlayfs: too many lower directries, limit is %d\n",
+		       OVL_MAX_STACK);
 		goto out_free_lowertmp;
+	} else if (!ufs->config.upperdir && stacklen == 1) {
+		pr_err("overlayfs: at least 2 lowerdir are needed while upperdir nonexistent\n");
+		goto out_free_lowertmp;
+	}

 	stack = kcalloc(stacklen, sizeof(struct path), GFP_KERNEL);
 	if (!stack)
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/3] ovl: upper fs should not be R/O
  2015-01-15  5:17 ` hujianyang
@ 2015-01-15  5:20   ` hujianyang
  -1 siblings, 0 replies; 13+ messages in thread
From: hujianyang @ 2015-01-15  5:20 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs, linux-fsdevel, Seunghun Lee, Fabian Sturm

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@huawei.com>
---
 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;
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/3] ovl: upper fs should not be R/O
@ 2015-01-15  5:20   ` hujianyang
  0 siblings, 0 replies; 13+ messages in thread
From: hujianyang @ 2015-01-15  5:20 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs, linux-fsdevel, Seunghun Lee, Fabian Sturm

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@huawei.com>
---
 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;
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] ovl: print error message for invalid mount options
  2015-01-15  5:17 ` hujianyang
@ 2015-01-15  5:52   ` hujianyang
  -1 siblings, 0 replies; 13+ messages in thread
From: hujianyang @ 2015-01-15  5:52 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs, linux-fsdevel, Seunghun Lee, Fabian Sturm

On 2015/1/15 13:17, hujianyang wrote:
> Overlayfs should print an error message if an incorrect mount option
> is caught like other filesystems.
> 
> After this patch, improper option input could be clearly known.
> 
> Reported-by: Fabian Sturm <fabian.sturm@aduu.de>
> Signed-off-by: hujianyang <hujianyang@huawei.com>
> ---
>  fs/overlayfs/super.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index b90952f..ab3c8cb 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -615,6 +615,7 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
>  			break;
> 
>  		default:
> +			pr_err("overlayfs: unrecognized mount option \"%s\" or missing value\n", p);

Hi Miklos,

I'm sorry. The parameter "p" should begin with a new line.

I'd like to resend a new patch after you review these patches.

Thanks,
Hu

>  			return -EINVAL;
>  		}
>  	}
> 



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] ovl: print error message for invalid mount options
@ 2015-01-15  5:52   ` hujianyang
  0 siblings, 0 replies; 13+ messages in thread
From: hujianyang @ 2015-01-15  5:52 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-unionfs, linux-fsdevel, Seunghun Lee, Fabian Sturm

On 2015/1/15 13:17, hujianyang wrote:
> Overlayfs should print an error message if an incorrect mount option
> is caught like other filesystems.
> 
> After this patch, improper option input could be clearly known.
> 
> Reported-by: Fabian Sturm <fabian.sturm@aduu.de>
> Signed-off-by: hujianyang <hujianyang@huawei.com>
> ---
>  fs/overlayfs/super.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index b90952f..ab3c8cb 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -615,6 +615,7 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
>  			break;
> 
>  		default:
> +			pr_err("overlayfs: unrecognized mount option \"%s\" or missing value\n", p);

Hi Miklos,

I'm sorry. The parameter "p" should begin with a new line.

I'd like to resend a new patch after you review these patches.

Thanks,
Hu

>  			return -EINVAL;
>  		}
>  	}
> 



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/3] ovl: upper fs should not be R/O
  2015-01-15  5:20   ` hujianyang
  (?)
@ 2015-01-15  8:20   ` Seunghun Lee
  2015-01-15 17:09     ` A. Wan
  -1 siblings, 1 reply; 13+ messages in thread
From: Seunghun Lee @ 2015-01-15  8:20 UTC (permalink / raw)
  To: hujianyang, Miklos Szeredi; +Cc: linux-unionfs, linux-fsdevel, Fabian Sturm

On January 15, 2015 2:20:57 PM GMT+09:00, hujianyang <hujianyang@huawei.com> 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@huawei.com>
>---
> 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.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/3] ovl: upper fs should not be R/O
  2015-01-15  8:20   ` Seunghun Lee
@ 2015-01-15 17:09     ` A. Wan
  2015-01-16  2:39         ` hujianyang
  0 siblings, 1 reply; 13+ messages in thread
From: A. Wan @ 2015-01-15 17:09 UTC (permalink / raw)
  To: linux-unionfs, linux-fsdevel

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@huawei.com> 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@huawei.com>
>>---
>> 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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>





^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/3] ovl: upper fs should not be R/O
  2015-01-15 17:09     ` A. Wan
@ 2015-01-16  2:39         ` hujianyang
  0 siblings, 0 replies; 13+ messages in thread
From: hujianyang @ 2015-01-16  2:39 UTC (permalink / raw)
  To: jm; +Cc: linux-unionfs, linux-fsdevel, Miklos Szeredi

Hi Alex,

I think your question is about why workdir was introduced. I'd like
to share my thought with you. I should say Miklos is the best candidate
to explain it, because he is the author of this filesystem.

On 2015/1/16 1:09, A. Wan wrote:
> 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?
> 

As far as I know, "workdir" is not belong to any r/o or r/w layer in
overlayfs. Actually it belong to a overlayfs mount itself. "Workdir" is
not mandatory after recently multi-lower layer feature. It is used only
in r/w case, and only valuable in r/w case. My patch wants to make sure
the specified "workdir" in option line is significative.

If you just mount a stack of r/o layers, you don't need any "workdir",
but the mounted overlayfs partition will be marked as r/o. If you want a
r/w mount, you must specify a upperdir, the r/w layer on the top and
specify a workdir in the same mount with upperdir.

Overlayfs is a union filesystem. It gives ability to combine directories
in different mounts together. For r/w mount case, overlayfs allow users
change any file in its mount. But the implement of overlayfs not directly
write to each lower fs. All the write is reflected on the upper layer,
the top r/w layer.

See this link about unionfs:

http://lwn.net/Articles/324291/

"Workdir" works as the temp directory of a overlayfs mount. The file
changing is first done in it and then use rename() move to upper directory.
That's why "workdir" must in the same mount with upperdir.

For example,

if users delete a file which belong to a lower a/o layer in overlayfs
partition, the deletion is not perform on lower fs. Instead, a whiteout
file is created in "workdir" and then move to upperdir.

Other operations, like "copy_up", "rename" also use "workdir" as the
temporary directory. You can read this code in fs/overlayfs yourself.

I think "workdir" is used to keep the consistency of overlayfs and avoid
corrupt data damaging the filesystem. I'd like Miklos could explain more
for us.

> Does it has to do with why workdir was introduced in the first place?

It's not the business of my patch, "workdir" is not introduced by it.
But after rescanning the document in kernel, I think more explanation
about "workdir" is worth to be done.

> 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.
> 

No, "workdir" is introduced since overlayfs first merged into mainline
and is still needed in r/w mount. But I wonder if there are other
implements instead. Oh, Miklos may be unhappy with my unorthodox notion.

I'm sorry to say I'm not good at English expression, but I'm happy to
clarify any points that are still unclear.

Thanks,
Hu

> 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@huawei.com> 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.



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/3] ovl: upper fs should not be R/O
@ 2015-01-16  2:39         ` hujianyang
  0 siblings, 0 replies; 13+ messages in thread
From: hujianyang @ 2015-01-16  2:39 UTC (permalink / raw)
  To: jm; +Cc: linux-unionfs, linux-fsdevel, Miklos Szeredi

Hi Alex,

I think your question is about why workdir was introduced. I'd like
to share my thought with you. I should say Miklos is the best candidate
to explain it, because he is the author of this filesystem.

On 2015/1/16 1:09, A. Wan wrote:
> 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?
> 

As far as I know, "workdir" is not belong to any r/o or r/w layer in
overlayfs. Actually it belong to a overlayfs mount itself. "Workdir" is
not mandatory after recently multi-lower layer feature. It is used only
in r/w case, and only valuable in r/w case. My patch wants to make sure
the specified "workdir" in option line is significative.

If you just mount a stack of r/o layers, you don't need any "workdir",
but the mounted overlayfs partition will be marked as r/o. If you want a
r/w mount, you must specify a upperdir, the r/w layer on the top and
specify a workdir in the same mount with upperdir.

Overlayfs is a union filesystem. It gives ability to combine directories
in different mounts together. For r/w mount case, overlayfs allow users
change any file in its mount. But the implement of overlayfs not directly
write to each lower fs. All the write is reflected on the upper layer,
the top r/w layer.

See this link about unionfs:

http://lwn.net/Articles/324291/

"Workdir" works as the temp directory of a overlayfs mount. The file
changing is first done in it and then use rename() move to upper directory.
That's why "workdir" must in the same mount with upperdir.

For example,

if users delete a file which belong to a lower a/o layer in overlayfs
partition, the deletion is not perform on lower fs. Instead, a whiteout
file is created in "workdir" and then move to upperdir.

Other operations, like "copy_up", "rename" also use "workdir" as the
temporary directory. You can read this code in fs/overlayfs yourself.

I think "workdir" is used to keep the consistency of overlayfs and avoid
corrupt data damaging the filesystem. I'd like Miklos could explain more
for us.

> Does it has to do with why workdir was introduced in the first place?

It's not the business of my patch, "workdir" is not introduced by it.
But after rescanning the document in kernel, I think more explanation
about "workdir" is worth to be done.

> 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.
> 

No, "workdir" is introduced since overlayfs first merged into mainline
and is still needed in r/w mount. But I wonder if there are other
implements instead. Oh, Miklos may be unhappy with my unorthodox notion.

I'm sorry to say I'm not good at English expression, but I'm happy to
clarify any points that are still unclear.

Thanks,
Hu

> 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@huawei.com> 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.



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/3] ovl: upper fs should not be R/O
  2015-01-15  5:20   ` hujianyang
  (?)
  (?)
@ 2015-03-18  9:35   ` Miklos Szeredi
  -1 siblings, 0 replies; 13+ messages in thread
From: Miklos Szeredi @ 2015-03-18  9:35 UTC (permalink / raw)
  To: hujianyang; +Cc: linux-unionfs, linux-fsdevel, Seunghun Lee, Fabian Sturm

The three patches are queued at

  git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git overlayfs-next

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2015-03-18  9:35 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-15  5:17 [PATCH 1/3] ovl: print error message for invalid mount options hujianyang
2015-01-15  5:17 ` hujianyang
2015-01-15  5:19 ` [PATCH 2/3] ovl: check lowerdir amount for non-upper mount hujianyang
2015-01-15  5:19   ` hujianyang
2015-01-15  5:20 ` [PATCH 3/3] ovl: upper fs should not be R/O hujianyang
2015-01-15  5:20   ` hujianyang
2015-01-15  8:20   ` Seunghun Lee
2015-01-15 17:09     ` A. Wan
2015-01-16  2:39       ` hujianyang
2015-01-16  2:39         ` hujianyang
2015-03-18  9:35   ` Miklos Szeredi
2015-01-15  5:52 ` [PATCH 1/3] ovl: print error message for invalid mount options hujianyang
2015-01-15  5:52   ` hujianyang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.