[PATCH] overlay: allow config override of metacopy/redirect defaults

[PATCH] overlay: allow config override of metacopy/redirect defaults

[Matt Coffin]

[Why]
Currently, if the redirect_dir option is set as a kernel or module
parameter, then even if metacopy is only enabled config, then both
metacopy and redirect_dir will be enabled when one creates a mount
point. This is not desirable because /sys/module/overlay/parameters will
still report that redirect_dir is not enabled, and there will be no
redirect_dir=on line in the mount options in /proc/mounts. The behavior
of setting redirect_dir globally for overlay is likely a common pattern
on docker workstations, as redirect_dir makes for slower building of
docker images.

[How]
This patch adds similar logic to that already in place for parsing mount
parameters. If the user explicitly sets redirect_dir via a kernel or
module parameter, then metacopy will become disabled, unless it was also
specified that way. Obviously, mount options still take precedence over
this process, so this logic only kicks in when neither redirect_dir or
metacopy were specified in the mount options.

[Related]
This undesirable logic was introduced by the commit that introduced the
mount option handling logic: d47748e5ae5af6572e520cc9767bbe70c22ea498
---
 fs/overlayfs/super.c | 48 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 39 insertions(+), 9 deletions(-)

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 0116735cc321..456d48f57700 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -30,10 +30,16 @@ struct ovl_dir_cache;
 #define OVL_MAX_STACK 500
 
 static bool ovl_redirect_dir_def = IS_ENABLED(CONFIG_OVERLAY_FS_REDIRECT_DIR);
-module_param_named(redirect_dir, ovl_redirect_dir_def, bool, 0644);
-MODULE_PARM_DESC(ovl_redirect_dir_def,
+static bool ovl_redirect_dir_param = IS_ENABLED(CONFIG_OVERLAY_FS_REDIRECT_DIR);
+module_param_named(redirect_dir, ovl_redirect_dir_param, bool, 0644);
+MODULE_PARM_DESC(ovl_redirect_dir_param,
 		 "Default to on or off for the redirect_dir feature");
 
+static inline bool ovl_redirect_dir_module_overrides_kernel(void)
+{
+	return (ovl_redirect_dir_def != ovl_redirect_dir_param);
+}
+
 static bool ovl_redirect_always_follow =
 	IS_ENABLED(CONFIG_OVERLAY_FS_REDIRECT_ALWAYS_FOLLOW);
 module_param_named(redirect_always_follow, ovl_redirect_always_follow,
@@ -65,10 +71,16 @@ static void ovl_entry_stack_free(struct ovl_entry *oe)
 }
 
 static bool ovl_metacopy_def = IS_ENABLED(CONFIG_OVERLAY_FS_METACOPY);
-module_param_named(metacopy, ovl_metacopy_def, bool, 0644);
-MODULE_PARM_DESC(ovl_metacopy_def,
+static bool ovl_metacopy_param = IS_ENABLED(CONFIG_OVERLAY_FS_METACOPY);
+module_param_named(metacopy, ovl_metacopy_param, bool, 0644);
+MODULE_PARM_DESC(ovl_metacopy_param,
 		 "Default to on or off for the metadata only copy up feature");
 
+static inline bool ovl_metacopy_module_overrides_kernel(void)
+{
+	return (ovl_metacopy_def != ovl_metacopy_param);
+}
+
 static void ovl_dentry_release(struct dentry *dentry)
 {
 	struct ovl_entry *oe = dentry->d_fsdata;
@@ -312,7 +324,7 @@ static bool ovl_force_readonly(struct ovl_fs *ofs)
 
 static const char *ovl_redirect_mode_def(void)
 {
-	return ovl_redirect_dir_def ? "on" : "off";
+	return ovl_redirect_dir_param ? "on" : "off";
 }
 
 enum {
@@ -359,7 +371,7 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
 						"on" : "off");
 	if (ofs->config.xino != ovl_xino_def())
 		seq_printf(m, ",xino=%s", ovl_xino_str[ofs->config.xino]);
-	if (ofs->config.metacopy != ovl_metacopy_def)
+	if (ofs->config.metacopy != ovl_metacopy_param)
 		seq_printf(m, ",metacopy=%s",
 			   ofs->config.metacopy ? "on" : "off");
 	return 0;
@@ -457,6 +469,7 @@ static int ovl_parse_redirect_mode(struct ovl_config *config, const char *mode)
 	} else if (strcmp(mode, "follow") == 0) {
 		config->redirect_follow = true;
 	} else if (strcmp(mode, "off") == 0) {
+		config->redirect_dir = false;
 		if (ovl_redirect_always_follow)
 			config->redirect_follow = true;
 	} else if (strcmp(mode, "nofollow") != 0) {
@@ -473,6 +486,8 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
 	char *p;
 	int err;
 	bool metacopy_opt = false, redirect_opt = false;
+	bool metacopy_override = ovl_metacopy_module_overrides_kernel();
+	bool redirect_override = ovl_redirect_dir_module_overrides_kernel();
 
 	config->redirect_mode = kstrdup(ovl_redirect_mode_def(), GFP_KERNEL);
 	if (!config->redirect_mode)
@@ -598,8 +613,23 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
 				config->redirect_mode);
 			config->metacopy = false;
 		} else {
-			/* Automatically enable redirect otherwise. */
-			config->redirect_follow = config->redirect_dir = true;
+			if (metacopy_override && redirect_override) {
+				pr_err("overlayfs: conflicting module params: metacopy=on,redirect_dir=%s\n",
+					config->redirect_mode);
+				return -EINVAL;
+			}
+			if (redirect_override) {
+				/*
+				 * There was an explicit redirect_dir=... that resulted
+				 * in this conflict (module param)
+				 */
+				pr_info("overlayfs: disabling metacopy due to module param redirect_dir=%s\n",
+					config->redirect_mode);
+				config->metacopy = false;
+			} else {
+				/* Automatically enable redirect otherwise. */
+				config->redirect_follow = config->redirect_dir = true;
+			}
 		}
 	}
 
@@ -1439,7 +1469,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 	ofs->config.index = ovl_index_def;
 	ofs->config.nfs_export = ovl_nfs_export_def;
 	ofs->config.xino = ovl_xino_def();
-	ofs->config.metacopy = ovl_metacopy_def;
+	ofs->config.metacopy = ovl_metacopy_param;
 	err = ovl_parse_opt((char *) data, &ofs->config);
 	if (err)
 		goto out_err;
-- 
2.21.0

[PATCH v2] overlay: allow config override of metacopy/redirect defaults

[Matt Coffin]

[Why]
Currently, if the redirect_dir option is set as a kernel or module
parameter, then even if metacopy is only enabled config, then both
metacopy and redirect_dir will be enabled when one creates a mount
point. This is not desirable because /sys/module/overlay/parameters will
still report that redirect_dir is not enabled, and there will be no
redirect_dir=on line in the mount options in /proc/mounts. The behavior
of setting redirect_dir globally for overlay is likely a common pattern
on docker workstations, as redirect_dir makes for slower building of
docker images.

[How]
This patch adds similar logic to that already in place for parsing mount
parameters. If the user explicitly sets redirect_dir via a kernel or
module parameter, then metacopy will become disabled, unless it was also
specified that way. Obviously, mount options still take precedence over
this process, so this logic only kicks in when neither redirect_dir or
metacopy were specified in the mount options.

[Related]
This undesirable logic was introduced by the commit that introduced the
mount option handling logic: d47748e5ae5af6572e520cc9767bbe70c22ea498

[v2]
Clean up some formatting on lines that were too long
---
 fs/overlayfs/super.c | 49 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 40 insertions(+), 9 deletions(-)

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 5ec4fc2f5d7e..bd1a1329aa70 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -30,10 +30,16 @@ struct ovl_dir_cache;
 #define OVL_MAX_STACK 500
 
 static bool ovl_redirect_dir_def = IS_ENABLED(CONFIG_OVERLAY_FS_REDIRECT_DIR);
-module_param_named(redirect_dir, ovl_redirect_dir_def, bool, 0644);
-MODULE_PARM_DESC(ovl_redirect_dir_def,
+static bool ovl_redirect_dir_param = IS_ENABLED(CONFIG_OVERLAY_FS_REDIRECT_DIR);
+module_param_named(redirect_dir, ovl_redirect_dir_param, bool, 0644);
+MODULE_PARM_DESC(ovl_redirect_dir_param,
 		 "Default to on or off for the redirect_dir feature");
 
+static inline bool ovl_redirect_dir_module_overrides_kernel(void)
+{
+	return (ovl_redirect_dir_def != ovl_redirect_dir_param);
+}
+
 static bool ovl_redirect_always_follow =
 	IS_ENABLED(CONFIG_OVERLAY_FS_REDIRECT_ALWAYS_FOLLOW);
 module_param_named(redirect_always_follow, ovl_redirect_always_follow,
@@ -65,10 +71,16 @@ static void ovl_entry_stack_free(struct ovl_entry *oe)
 }
 
 static bool ovl_metacopy_def = IS_ENABLED(CONFIG_OVERLAY_FS_METACOPY);
-module_param_named(metacopy, ovl_metacopy_def, bool, 0644);
-MODULE_PARM_DESC(ovl_metacopy_def,
+static bool ovl_metacopy_param = IS_ENABLED(CONFIG_OVERLAY_FS_METACOPY);
+module_param_named(metacopy, ovl_metacopy_param, bool, 0644);
+MODULE_PARM_DESC(ovl_metacopy_param,
 		 "Default to on or off for the metadata only copy up feature");
 
+static inline bool ovl_metacopy_module_overrides_kernel(void)
+{
+	return (ovl_metacopy_def != ovl_metacopy_param);
+}
+
 static void ovl_dentry_release(struct dentry *dentry)
 {
 	struct ovl_entry *oe = dentry->d_fsdata;
@@ -310,7 +322,7 @@ static bool ovl_force_readonly(struct ovl_fs *ofs)
 
 static const char *ovl_redirect_mode_def(void)
 {
-	return ovl_redirect_dir_def ? "on" : "off";
+	return ovl_redirect_dir_param ? "on" : "off";
 }
 
 enum {
@@ -357,7 +369,7 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
 						"on" : "off");
 	if (ofs->config.xino != ovl_xino_def())
 		seq_printf(m, ",xino=%s", ovl_xino_str[ofs->config.xino]);
-	if (ofs->config.metacopy != ovl_metacopy_def)
+	if (ofs->config.metacopy != ovl_metacopy_param)
 		seq_printf(m, ",metacopy=%s",
 			   ofs->config.metacopy ? "on" : "off");
 	return 0;
@@ -456,6 +468,7 @@ static int ovl_parse_redirect_mode(struct ovl_config *config, const char *mode)
 	} else if (strcmp(mode, "follow") == 0) {
 		config->redirect_follow = true;
 	} else if (strcmp(mode, "off") == 0) {
+		config->redirect_dir = false;
 		if (ovl_redirect_always_follow)
 			config->redirect_follow = true;
 	} else if (strcmp(mode, "nofollow") != 0) {
@@ -472,6 +485,8 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
 	char *p;
 	int err;
 	bool metacopy_opt = false, redirect_opt = false;
+	bool metacopy_override = ovl_metacopy_module_overrides_kernel();
+	bool redirect_override = ovl_redirect_dir_module_overrides_kernel();
 
 	config->redirect_mode = kstrdup(ovl_redirect_mode_def(), GFP_KERNEL);
 	if (!config->redirect_mode)
@@ -597,8 +612,24 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
 				config->redirect_mode);
 			config->metacopy = false;
 		} else {
-			/* Automatically enable redirect otherwise. */
-			config->redirect_follow = config->redirect_dir = true;
+			if (metacopy_override && redirect_override) {
+				pr_err("overlayfs: conflicting module params: metacopy=on,redirect_dir=%s\n",
+					config->redirect_mode);
+				return -EINVAL;
+			}
+			if (redirect_override) {
+				/*
+				 * There was an explicit redirect_dir=...
+				 * that resulted in this conflict (module param)
+				 */
+				pr_info("overlayfs: disabling metacopy due to module param redirect_dir=%s\n",
+					config->redirect_mode);
+				config->metacopy = false;
+			} else {
+				/* Automatically enable redirect otherwise. */
+				config->redirect_follow =
+					config->redirect_dir = true;
+			}
 		}
 	}
 
@@ -1438,7 +1469,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 	ofs->config.index = ovl_index_def;
 	ofs->config.nfs_export = ovl_nfs_export_def;
 	ofs->config.xino = ovl_xino_def();
-	ofs->config.metacopy = ovl_metacopy_def;
+	ofs->config.metacopy = ovl_metacopy_param;
 	err = ovl_parse_opt((char *) data, &ofs->config);
 	if (err)
 		goto out_err;
-- 
2.21.0

Re: [PATCH v2] overlay: allow config override of metacopy/redirect defaults

[Amir Goldstein]

Hi Matt,

Thank you for trying to address this, but I see problems both in Why and
How you did it.

On Fri, Jun 7, 2019 at 11:51 PM Matt Coffin <mcoffin13@gmail.com> wrote:
>
> [Why]
> Currently, if the redirect_dir option is set as a kernel or module
> parameter, then even if metacopy is only enabled config, then both
> metacopy and redirect_dir will be enabled when one creates a mount
> point. This is not desirable because /sys/module/overlay/parameters will
> still report that redirect_dir is not enabled

/sys/module/overlay/parameters reports that redirect_dir is not enabled
*by default* not per mount.

> and there will be no redirect_dir=on line in the mount options in /proc/mounts.

That is a bug. This code:
/* Automatically enable redirect otherwise. */
config->redirect_follow = config->redirect_dir = true;

Needs to update of config->redirect_mode.

You are very welcome to send a fix patch.

> The behavior
> of setting redirect_dir globally for overlay is likely a common pattern
> on docker workstations, as redirect_dir makes for slower building of
> docker images.

I haven't been following the progress in docker w.r.t redirect_dir,
but IMO the right way for docker is to always mandate overlayfs
mount option parameters based on user meaningful storage driver
config options, see:
https://github.com/moby/moby/pull/34342#issuecomment-320669900

Instead of depending on admin to set /sys/module/overlay/parameters
globally, docker should always pass explicit redirect_dir,metacopy
values in mount options.
Docker should check for existence of /sys/module/overlay/parameters
feature file to know if kernel supports the mount option.

BTW, docker should be treating metacopy exactly the same way as
redirect_dir because the native diff driver does not know about metacopy,
so docket should (IMO) disable metacopy in mount option unless user
explicitly opted-in for in per image and fallback to naiive diff driver if
user opted-in to enable metacopy.

IMO, docker overlay2 storage driver should have a well documented
user meaningful per container option such as:
"optimize for efficient runtime" (redirect_dir=on,metacopy=on)
vs.
"optimize for efficient image export" (redirect_dir=off,metacopy=off)

Or even simpler:
"Exportable = true/false"
Because users know if they run a container that they don't intend
to export. In that case, it makes no sense to deny user of the benefits
of redirect_dir=on,metacopy=on.

>
> [How]
> This patch adds similar logic to that already in place for parsing mount
> parameters. If the user explicitly sets redirect_dir via a kernel or
> module parameter, then metacopy will become disabled, unless it was also
> specified that way. Obviously, mount options still take precedence over
> this process, so this logic only kicks in when neither redirect_dir or
> metacopy were specified in the mount options.
>

If even we did pass the "Why", the "How" is unacceptable IMO.
It extends something that is already a local special case hack
with redirect_opt/metacopy_opt.

If we ever try to improve parameter/config/option parsing
and dependency checks we need to do it in a much more generic
way and there are much more things to consider.
I am sorry that I cannot provide you guidelines for how to
do this. I took a stab at this once, then Miklos said he will try to
redo it and came back with:
"...this is more complicated than I thought."
https://marc.info/?l=linux-unionfs&m=154110487305120&w=2

If you do insist to try and follow this path, my only suggestion
is - start with a patch to Documentation/filesystems/overlayfs.txt.
If you cannot shortly and properly describe the behavior of
your change in documentation it is a no go.

Thanks,
Amir.

Re: [PATCH v2] overlay: allow config override of metacopy/redirect defaults

[Matt Coffin]

Thanks for the comments, Amir. It's my first kernel patch so it's a
learning process.

I'll take a stab at fleshing out what the documentation would look like
for a different config system, and submit a patch for the redirect_dir
not showing in procfs bug. Unless there's opposition, I'll also add a
log line (similar to the one that warns about disabling metacopy) for
when redirect_dir becomes enabled due to an explicitly configured
metacopy for the mount point. A line like that in dmesg would have saved
me a log of trouble trying to figure out WHY this was happening in the
first place.

As far as docker goes, I think you're absolutely correct there. There's
no reason that the overlay2 backend shouldn't be passing these
parameters explicitly to the mount point. That said, even with the
visibility improvements above, it does seem odd to me that passing
"redirect_dir=0" explicitly to the module wouldn't actually disable it,
just because the "metacopy" parameter is defaulting it to being on. Even
if we display the overridden options in /proc/mounts, and log in dmesg,
it seems odd to me that a default on another setting would override an
explicitly passed other setting.

Hopefully, there won't be many people actually doing that globally once
the overlay2 backend behaves properly, but it still seems like odd
behavior to me; but, if that's intended and desired then it's not the
end of the world.

Thanks again for the comments and working with me. Sorry if the
[How]/[Why] formatting isn't the standard here, I just pulled it from
the amd-gfx mailing list since I've never submitted a patch before.

So, if one were to take a stab at implementing a more generic
configuration approach, what would the ideal desired behavior for the
options look like?

1. Should kernel config not be override-able? The current behavior is to
allow overriding kernel config with mount options/params. It might be
confusing to change that, so likely the desire would be to keep the
existing behavior?
2. Should mount options take precedence over module params? The current
behavior is that mount options take precedence, and likely I think
that's desired as well.

Thanks in advance for the help,
Matt Coffin

On 6/8/19 3:04 AM, Amir Goldstein wrote:
> Hi Matt,
> 
> Thank you for trying to address this, but I see problems both in Why and
> How you did it.
> 
> On Fri, Jun 7, 2019 at 11:51 PM Matt Coffin <mcoffin13@gmail.com> wrote:
>>
>> [Why]
>> Currently, if the redirect_dir option is set as a kernel or module
>> parameter, then even if metacopy is only enabled config, then both
>> metacopy and redirect_dir will be enabled when one creates a mount
>> point. This is not desirable because /sys/module/overlay/parameters will
>> still report that redirect_dir is not enabled
> 
> /sys/module/overlay/parameters reports that redirect_dir is not enabled
> *by default* not per mount.
> 
>> and there will be no redirect_dir=on line in the mount options in /proc/mounts.
> 
> That is a bug. This code:
> /* Automatically enable redirect otherwise. */
> config->redirect_follow = config->redirect_dir = true;
> 
> Needs to update of config->redirect_mode.
> 
> You are very welcome to send a fix patch.
> 
>> The behavior
>> of setting redirect_dir globally for overlay is likely a common pattern
>> on docker workstations, as redirect_dir makes for slower building of
>> docker images.
> 
> I haven't been following the progress in docker w.r.t redirect_dir,
> but IMO the right way for docker is to always mandate overlayfs
> mount option parameters based on user meaningful storage driver
> config options, see:
> https://github.com/moby/moby/pull/34342#issuecomment-320669900
> 
> Instead of depending on admin to set /sys/module/overlay/parameters
> globally, docker should always pass explicit redirect_dir,metacopy
> values in mount options.
> Docker should check for existence of /sys/module/overlay/parameters
> feature file to know if kernel supports the mount option.
> 
> BTW, docker should be treating metacopy exactly the same way as
> redirect_dir because the native diff driver does not know about metacopy,
> so docket should (IMO) disable metacopy in mount option unless user
> explicitly opted-in for in per image and fallback to naiive diff driver if
> user opted-in to enable metacopy.
> 
> IMO, docker overlay2 storage driver should have a well documented
> user meaningful per container option such as:
> "optimize for efficient runtime" (redirect_dir=on,metacopy=on)
> vs.
> "optimize for efficient image export" (redirect_dir=off,metacopy=off)
> 
> Or even simpler:
> "Exportable = true/false"
> Because users know if they run a container that they don't intend
> to export. In that case, it makes no sense to deny user of the benefits
> of redirect_dir=on,metacopy=on.
> 
>>
>> [How]
>> This patch adds similar logic to that already in place for parsing mount
>> parameters. If the user explicitly sets redirect_dir via a kernel or
>> module parameter, then metacopy will become disabled, unless it was also
>> specified that way. Obviously, mount options still take precedence over
>> this process, so this logic only kicks in when neither redirect_dir or
>> metacopy were specified in the mount options.
>>
> 
> If even we did pass the "Why", the "How" is unacceptable IMO.
> It extends something that is already a local special case hack
> with redirect_opt/metacopy_opt.
> 
> If we ever try to improve parameter/config/option parsing
> and dependency checks we need to do it in a much more generic
> way and there are much more things to consider.
> I am sorry that I cannot provide you guidelines for how to
> do this. I took a stab at this once, then Miklos said he will try to
> redo it and came back with:
> "...this is more complicated than I thought."
> https://marc.info/?l=linux-unionfs&m=154110487305120&w=2
> 
> If you do insist to try and follow this path, my only suggestion
> is - start with a patch to Documentation/filesystems/overlayfs.txt.
> If you cannot shortly and properly describe the behavior of
> your change in documentation it is a no go.
> 
> Thanks,
> Amir.
> 

Re: [PATCH v2] overlay: allow config override of metacopy/redirect defaults

[Amir Goldstein]

On Sat, Jun 8, 2019 at 8:28 PM Matt Coffin <mcoffin13@gmail.com> wrote:
>
> Thanks for the comments, Amir. It's my first kernel patch so it's a
> learning process.
>
> I'll take a stab at fleshing out what the documentation would look like
> for a different config system, and submit a patch for the redirect_dir
> not showing in procfs bug. Unless there's opposition, I'll also add a
> log line (similar to the one that warns about disabling metacopy) for
> when redirect_dir becomes enabled due to an explicitly configured
> metacopy for the mount point. A line like that in dmesg would have saved
> me a log of trouble trying to figure out WHY this was happening in the
> first place.

No objection. You *are* the user so you are the proof that a message
is useful.

>
> As far as docker goes, I think you're absolutely correct there. There's
> no reason that the overlay2 backend shouldn't be passing these
> parameters explicitly to the mount point. That said, even with the
> visibility improvements above, it does seem odd to me that passing
> "redirect_dir=0" explicitly to the module wouldn't actually disable it,
> just because the "metacopy" parameter is defaulting it to being on. Even
> if we display the overridden options in /proc/mounts, and log in dmesg,
> it seems odd to me that a default on another setting would override an
> explicitly passed other setting.
>
> Hopefully, there won't be many people actually doing that globally once
> the overlay2 backend behaves properly, but it still seems like odd
> behavior to me; but, if that's intended and desired then it's not the
> end of the world.
>

I think nobody disagrees that current  behavior is sub-optimal,
but the general vibe is that to fix a corner case of configuration
would take a lot of effort and a lot of added code complexity, so
the trade off may not be worth it (as long as we duly report what's
happened to user). OTOH, the options parsing code is a bit kludgy
already. If you are able to make it better and in the process improve
the behavior, that would be worth while IMO.

> Thanks again for the comments and working with me. Sorry if the
> [How]/[Why] formatting isn't the standard here, I just pulled it from
> the amd-gfx mailing list since I've never submitted a patch before.
>

It may not be the standard format, but it providing the Why and the How
should definitely be the standard.

> So, if one were to take a stab at implementing a more generic
> configuration approach, what would the ideal desired behavior for the
> options look like?
>
> 1. Should kernel config not be override-able? The current behavior is to
> allow overriding kernel config with mount options/params. It might be
> confusing to change that, so likely the desire would be to keep the
> existing behavior?
> 2. Should mount options take precedence over module params? The current
> behavior is that mount options take precedence, and likely I think
> that's desired as well.

There is a clear hierarchy for the config options.
module parameter overrides build config.
mount option overrides both.

The problem is that in the general case, once a mount option has been
parsed and config value modified, we loose the origin of that config value.
metacopy_opt/redirect_opt were added as a band aid to address a specific
issue. That's a special case code not generic code.

One possible way to generalize this code would be to keep the entire
information, i.e. the builds defaults, the module param defaults and
the mount options that can override them in a data structure that could
be processed in a generic way and also a data structure that can represent
the inter-dependencies between the options.

And then every time that a feature needs to be turned off for some reason
that also needs to be taken into account.
IOW, I advise against diving into this mess. You have been warned ;-)

Thanks,
Amir.

Re: [PATCH v2] overlay: allow config override of metacopy/redirect defaults

[Miklos Szeredi]

On Sat, Jun 8, 2019 at 8:47 PM Amir Goldstein <amir73il@gmail.com> wrote:

> And then every time that a feature needs to be turned off for some reason
> that also needs to be taken into account.
> IOW, I advise against diving into this mess. You have been warned ;-)

Also a much more productive direction would be to optimize building
the docker image based on the specific format used by overlayfs for
readirect_dir/metacopy.

To me it seems like a no-brainer, but I don't know much about docker, so...

Thanks,
Miklos

Re: [PATCH v2] overlay: allow config override of metacopy/redirect defaults

[Vivek Goyal]

On Sat, Jun 08, 2019 at 12:04:54PM +0300, Amir Goldstein wrote:
> Hi Matt,
> 
> Thank you for trying to address this, but I see problems both in Why and
> How you did it.
> 
> On Fri, Jun 7, 2019 at 11:51 PM Matt Coffin <mcoffin13@gmail.com> wrote:
> >
> > [Why]
> > Currently, if the redirect_dir option is set as a kernel or module
> > parameter, then even if metacopy is only enabled config, then both
> > metacopy and redirect_dir will be enabled when one creates a mount
> > point. This is not desirable because /sys/module/overlay/parameters will
> > still report that redirect_dir is not enabled
> 
> /sys/module/overlay/parameters reports that redirect_dir is not enabled
> *by default* not per mount.
> 
> > and there will be no redirect_dir=on line in the mount options in /proc/mounts.
> 
> That is a bug. This code:
> /* Automatically enable redirect otherwise. */
> config->redirect_follow = config->redirect_dir = true;
> 
> Needs to update of config->redirect_mode.

Hi Amir,

It took me a while to understand what's the problem. So IIUC, issue is
that kernel has enabled redirect_dir by default. But it was disabled
using module parameter. But it was enabled again as a side affect because
metacopy=on was passed as mount option. And /proc/self/mountinfo does
not show redirect_dir=on and hence the confusion.

IIRC, once you mentioned that we only show those options which needs
to be specified if same mount has to be reproduced on different machine
with same kernel/module options. If yes, then setting
config->redirect_dir=on is not needed because passing metacopy=on will
ensure that.

To me problem is that sometimes I want to know what options are enabled
on a particular mount. And in this case user can't figure that out by
parsing /proc/self/mountinfo.

And that's the reason apps create a overlay mount point, do bunch of
file operations and analyze xattrs to figure out what options are enabled.

It will be nice if there was an interface to query all that. And if there
is one, then this should go in there.

Otherwise given the current interface, it does not sound like a bug to
me. Current interface does not tell you what all features are enabled.
It only tells you what you need to provide as mount option to create
similar mount point.

Thanks
Vivek

Re: [PATCH v2] overlay: allow config override of metacopy/redirect defaults

[Vivek Goyal]

On Sun, Jun 09, 2019 at 09:14:38PM +0200, Miklos Szeredi wrote:
> On Sat, Jun 8, 2019 at 8:47 PM Amir Goldstein <amir73il@gmail.com> wrote:
> 
> > And then every time that a feature needs to be turned off for some reason
> > that also needs to be taken into account.
> > IOW, I advise against diving into this mess. You have been warned ;-)
> 
> Also a much more productive direction would be to optimize building
> the docker image based on the specific format used by overlayfs for
> readirect_dir/metacopy.
> 
> To me it seems like a no-brainer, but I don't know much about docker, so...

[ cc Daniel Walsh]

Hi Miklos,

Can you elaborate a bit more on what docker/container-storoage and do
here to expedite image generation with redirect_dir/metacopy enabled.

They can't pack these xattrs in image because image will not be portable.
It will be overlayfs specific and can't be made to work on target without
overlayfs.

Thanks
Vivek

Re: [PATCH v2] overlay: allow config override of metacopy/redirect defaults

[Vivek Goyal]

On Mon, Jun 10, 2019 at 02:40:43PM -0400, Vivek Goyal wrote:
> On Sun, Jun 09, 2019 at 09:14:38PM +0200, Miklos Szeredi wrote:
> > On Sat, Jun 8, 2019 at 8:47 PM Amir Goldstein <amir73il@gmail.com> wrote:
> > 
> > > And then every time that a feature needs to be turned off for some reason
> > > that also needs to be taken into account.
> > > IOW, I advise against diving into this mess. You have been warned ;-)
> > 
> > Also a much more productive direction would be to optimize building
> > the docker image based on the specific format used by overlayfs for
> > readirect_dir/metacopy.
> > 
> > To me it seems like a no-brainer, but I don't know much about docker, so...
> 
> [ cc Daniel Walsh]
> 
> Hi Miklos,
> 
> Can you elaborate a bit more on what docker/container-storoage and do
> here to expedite image generation with redirect_dir/metacopy enabled.
> 
> They can't pack these xattrs in image because image will not be portable.
> It will be overlayfs specific and can't be made to work on target without
> overlayfs.

Are you referring to apps being able to traverse lower layers and do
the redirect_dir and metacopy resoltion as kernel does. To me that process
is not trivial. Having a library might help with adoption though.

Vivek

Re: [PATCH v2] overlay: allow config override of metacopy/redirect defaults

[Amir Goldstein]

On Mon, Jun 10, 2019 at 9:31 PM Vivek Goyal <vgoyal@redhat.com> wrote:
>
> On Sat, Jun 08, 2019 at 12:04:54PM +0300, Amir Goldstein wrote:
> > Hi Matt,
> >
> > Thank you for trying to address this, but I see problems both in Why and
> > How you did it.
> >
> > On Fri, Jun 7, 2019 at 11:51 PM Matt Coffin <mcoffin13@gmail.com> wrote:
> > >
> > > [Why]
> > > Currently, if the redirect_dir option is set as a kernel or module
> > > parameter, then even if metacopy is only enabled config, then both
> > > metacopy and redirect_dir will be enabled when one creates a mount
> > > point. This is not desirable because /sys/module/overlay/parameters will
> > > still report that redirect_dir is not enabled
> >
> > /sys/module/overlay/parameters reports that redirect_dir is not enabled
> > *by default* not per mount.
> >
> > > and there will be no redirect_dir=on line in the mount options in /proc/mounts.
> >
> > That is a bug. This code:
> > /* Automatically enable redirect otherwise. */
> > config->redirect_follow = config->redirect_dir = true;
> >
> > Needs to update of config->redirect_mode.
>
> Hi Amir,
>
> It took me a while to understand what's the problem. So IIUC, issue is
> that kernel has enabled redirect_dir by default. But it was disabled
> using module parameter. But it was enabled again as a side affect because
> metacopy=on was passed as mount option. And /proc/self/mountinfo does
> not show redirect_dir=on and hence the confusion.
>
> IIRC, once you mentioned that we only show those options which needs
> to be specified if same mount has to be reproduced on different machine
> with same kernel/module options. If yes, then setting
> config->redirect_dir=on is not needed because passing metacopy=on will
> ensure that.

I agree it is not a must fix bug, more of a nice to have.
In all other similar cases (actual differs from module param value) we would
have presented the option in /proc/mounts. It is only because redirect_dir is
not consistent with redirect_mode that we do not.
This seems inconsistent and hence I called it a bug.

Thanks,
Amir.

Re: [PATCH v2] overlay: allow config override of metacopy/redirect defaults

[Miklos Szeredi]

On Mon, Jun 10, 2019 at 8:45 PM Vivek Goyal <vgoyal@redhat.com> wrote:
>
> On Mon, Jun 10, 2019 at 02:40:43PM -0400, Vivek Goyal wrote:
> > On Sun, Jun 09, 2019 at 09:14:38PM +0200, Miklos Szeredi wrote:
> > > On Sat, Jun 8, 2019 at 8:47 PM Amir Goldstein <amir73il@gmail.com> wrote:
> > >
> > > > And then every time that a feature needs to be turned off for some reason
> > > > that also needs to be taken into account.
> > > > IOW, I advise against diving into this mess. You have been warned ;-)
> > >
> > > Also a much more productive direction would be to optimize building
> > > the docker image based on the specific format used by overlayfs for
> > > readirect_dir/metacopy.
> > >
> > > To me it seems like a no-brainer, but I don't know much about docker, so...
> >
> > [ cc Daniel Walsh]
> >
> > Hi Miklos,
> >
> > Can you elaborate a bit more on what docker/container-storoage and do
> > here to expedite image generation with redirect_dir/metacopy enabled.
> >
> > They can't pack these xattrs in image because image will not be portable.
> > It will be overlayfs specific and can't be made to work on target without
> > overlayfs.
>
> Are you referring to apps being able to traverse lower layers and do
> the redirect_dir and metacopy resoltion as kernel does. To me that process
> is not trivial. Having a library might help with adoption though.

AFAICS what happens when generating a layer is to start with a clean
upper layer, do some operations, then save the contents of the upper
layer.  If redirect or metacopy is enabled, then the contents of the
upper layer won't be portable.  So need to do something like this:

traverse(overlay_dir, upper_dir, target_dir)
{
    iterate name for entries in $upper_dir {
        if ($name is non-directory) {
            copy($overlay_dir/$name, $target_dir/$name)
        } else if ($name is redirect) {
            copy-recursive($overlay_dir/$name, $target_dir/$name)
        } else {
            copy($overlay_dir/$name, $target_dir/$name)
            traverse($overlay_dir/$name, $upper_dir/$name, $target_dir/$name)
        }
    }
}

Basically: traverse the *upper layer* but copy files and directories
from the *overlay*.  Does that make sense?

Thanks,
Miklos

Re: [PATCH v2] overlay: allow config override of metacopy/redirect defaults

[Vivek Goyal]

On Tue, Jun 11, 2019 at 02:37:34PM +0200, Miklos Szeredi wrote:
> On Mon, Jun 10, 2019 at 8:45 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> >
> > On Mon, Jun 10, 2019 at 02:40:43PM -0400, Vivek Goyal wrote:
> > > On Sun, Jun 09, 2019 at 09:14:38PM +0200, Miklos Szeredi wrote:
> > > > On Sat, Jun 8, 2019 at 8:47 PM Amir Goldstein <amir73il@gmail.com> wrote:
> > > >
> > > > > And then every time that a feature needs to be turned off for some reason
> > > > > that also needs to be taken into account.
> > > > > IOW, I advise against diving into this mess. You have been warned ;-)
> > > >
> > > > Also a much more productive direction would be to optimize building
> > > > the docker image based on the specific format used by overlayfs for
> > > > readirect_dir/metacopy.
> > > >
> > > > To me it seems like a no-brainer, but I don't know much about docker, so...
> > >
> > > [ cc Daniel Walsh]
> > >
> > > Hi Miklos,
> > >
> > > Can you elaborate a bit more on what docker/container-storoage and do
> > > here to expedite image generation with redirect_dir/metacopy enabled.
> > >
> > > They can't pack these xattrs in image because image will not be portable.
> > > It will be overlayfs specific and can't be made to work on target without
> > > overlayfs.
> >
> > Are you referring to apps being able to traverse lower layers and do
> > the redirect_dir and metacopy resoltion as kernel does. To me that process
> > is not trivial. Having a library might help with adoption though.
> 
> AFAICS what happens when generating a layer is to start with a clean
> upper layer, do some operations, then save the contents of the upper
> layer.  If redirect or metacopy is enabled, then the contents of the
> upper layer won't be portable.  So need to do something like this:
> 
> traverse(overlay_dir, upper_dir, target_dir)
> {
>     iterate name for entries in $upper_dir {
>         if ($name is non-directory) {
>             copy($overlay_dir/$name, $target_dir/$name)
>         } else if ($name is redirect) {
>             copy-recursive($overlay_dir/$name, $target_dir/$name)
>         } else {
>             copy($overlay_dir/$name, $target_dir/$name)
>             traverse($overlay_dir/$name, $upper_dir/$name, $target_dir/$name)
>         }
>     }
> }
> 
> Basically: traverse the *upper layer* but copy files and directories
> from the *overlay*.  Does that make sense?

[ cc nalin ]

Aha... This makes sense to me. This does away with need of separate
library or user space tool and hopefully its faster than naivediff
interface. 

Dan, Nalin, what do you think about above idea.

Thanks
Vivek

Re: [PATCH v2] overlay: allow config override of metacopy/redirect defaults

[Daniel Walsh]

On 6/11/19 9:09 AM, Vivek Goyal wrote:
> On Tue, Jun 11, 2019 at 02:37:34PM +0200, Miklos Szeredi wrote:
>> On Mon, Jun 10, 2019 at 8:45 PM Vivek Goyal <vgoyal@redhat.com> wrote:
>>> On Mon, Jun 10, 2019 at 02:40:43PM -0400, Vivek Goyal wrote:
>>>> On Sun, Jun 09, 2019 at 09:14:38PM +0200, Miklos Szeredi wrote:
>>>>> On Sat, Jun 8, 2019 at 8:47 PM Amir Goldstein <amir73il@gmail.com> wrote:
>>>>>
>>>>>> And then every time that a feature needs to be turned off for some reason
>>>>>> that also needs to be taken into account.
>>>>>> IOW, I advise against diving into this mess. You have been warned ;-)
>>>>> Also a much more productive direction would be to optimize building
>>>>> the docker image based on the specific format used by overlayfs for
>>>>> readirect_dir/metacopy.
>>>>>
>>>>> To me it seems like a no-brainer, but I don't know much about docker, so...
>>>> [ cc Daniel Walsh]
>>>>
>>>> Hi Miklos,
>>>>
>>>> Can you elaborate a bit more on what docker/container-storoage and do
>>>> here to expedite image generation with redirect_dir/metacopy enabled.
>>>>
>>>> They can't pack these xattrs in image because image will not be portable.
>>>> It will be overlayfs specific and can't be made to work on target without
>>>> overlayfs.
>>> Are you referring to apps being able to traverse lower layers and do
>>> the redirect_dir and metacopy resoltion as kernel does. To me that process
>>> is not trivial. Having a library might help with adoption though.
>> AFAICS what happens when generating a layer is to start with a clean
>> upper layer, do some operations, then save the contents of the upper
>> layer.  If redirect or metacopy is enabled, then the contents of the
>> upper layer won't be portable.  So need to do something like this:
>>
>> traverse(overlay_dir, upper_dir, target_dir)
>> {
>>     iterate name for entries in $upper_dir {
>>         if ($name is non-directory) {
>>             copy($overlay_dir/$name, $target_dir/$name)
>>         } else if ($name is redirect) {
>>             copy-recursive($overlay_dir/$name, $target_dir/$name)
>>         } else {
>>             copy($overlay_dir/$name, $target_dir/$name)
>>             traverse($overlay_dir/$name, $upper_dir/$name, $target_dir/$name)
>>         }
>>     }
>> }
>>
>> Basically: traverse the *upper layer* but copy files and directories
>> from the *overlay*.  Does that make sense?
> [ cc nalin ]
>
> Aha... This makes sense to me. This does away with need of separate
> library or user space tool and hopefully its faster than naivediff
> interface. 
>
> Dan, Nalin, what do you think about above idea.
>
> Thanks
> Vivek

This is something we would add to containers/storage?

Re: [PATCH v2] overlay: allow config override of metacopy/redirect defaults

[Vivek Goyal]

On Tue, Jun 11, 2019 at 05:44:33PM -0400, Daniel Walsh wrote:
> On 6/11/19 9:09 AM, Vivek Goyal wrote:
> > On Tue, Jun 11, 2019 at 02:37:34PM +0200, Miklos Szeredi wrote:
> >> On Mon, Jun 10, 2019 at 8:45 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> >>> On Mon, Jun 10, 2019 at 02:40:43PM -0400, Vivek Goyal wrote:
> >>>> On Sun, Jun 09, 2019 at 09:14:38PM +0200, Miklos Szeredi wrote:
> >>>>> On Sat, Jun 8, 2019 at 8:47 PM Amir Goldstein <amir73il@gmail.com> wrote:
> >>>>>
> >>>>>> And then every time that a feature needs to be turned off for some reason
> >>>>>> that also needs to be taken into account.
> >>>>>> IOW, I advise against diving into this mess. You have been warned ;-)
> >>>>> Also a much more productive direction would be to optimize building
> >>>>> the docker image based on the specific format used by overlayfs for
> >>>>> readirect_dir/metacopy.
> >>>>>
> >>>>> To me it seems like a no-brainer, but I don't know much about docker, so...
> >>>> [ cc Daniel Walsh]
> >>>>
> >>>> Hi Miklos,
> >>>>
> >>>> Can you elaborate a bit more on what docker/container-storoage and do
> >>>> here to expedite image generation with redirect_dir/metacopy enabled.
> >>>>
> >>>> They can't pack these xattrs in image because image will not be portable.
> >>>> It will be overlayfs specific and can't be made to work on target without
> >>>> overlayfs.
> >>> Are you referring to apps being able to traverse lower layers and do
> >>> the redirect_dir and metacopy resoltion as kernel does. To me that process
> >>> is not trivial. Having a library might help with adoption though.
> >> AFAICS what happens when generating a layer is to start with a clean
> >> upper layer, do some operations, then save the contents of the upper
> >> layer.  If redirect or metacopy is enabled, then the contents of the
> >> upper layer won't be portable.  So need to do something like this:
> >>
> >> traverse(overlay_dir, upper_dir, target_dir)
> >> {
> >>     iterate name for entries in $upper_dir {
> >>         if ($name is non-directory) {
> >>             copy($overlay_dir/$name, $target_dir/$name)
> >>         } else if ($name is redirect) {
> >>             copy-recursive($overlay_dir/$name, $target_dir/$name)
> >>         } else {
> >>             copy($overlay_dir/$name, $target_dir/$name)
> >>             traverse($overlay_dir/$name, $upper_dir/$name, $target_dir/$name)
> >>         }
> >>     }
> >> }
> >>
> >> Basically: traverse the *upper layer* but copy files and directories
> >> from the *overlay*.  Does that make sense?
> > [ cc nalin ]
> >
> > Aha... This makes sense to me. This does away with need of separate
> > library or user space tool and hopefully its faster than naivediff
> > interface. 
> >
> > Dan, Nalin, what do you think about above idea.
> >
> > Thanks
> > Vivek
> 
> This is something we would add to containers/storage?

I think yes. But I don't know this code and its dependencies on other
packages and libraries, so hard to say.

Vivek

Re: [PATCH v2] overlay: allow config override of metacopy/redirect defaults

[Matt Coffin]

This could just be because I don't understand the implications here, but
wouldn't it be easier, at least for now, to just mount with

redirect_dir=0,metacopy=0

in the mount parameters when building images, but allow the user's
default settings to still take over when just executing a container?

On 6/11/19 3:49 PM, Vivek Goyal wrote:
> On Tue, Jun 11, 2019 at 05:44:33PM -0400, Daniel Walsh wrote:
>> On 6/11/19 9:09 AM, Vivek Goyal wrote:
>>> On Tue, Jun 11, 2019 at 02:37:34PM +0200, Miklos Szeredi wrote:
>>>> On Mon, Jun 10, 2019 at 8:45 PM Vivek Goyal <vgoyal@redhat.com> wrote:
>>>> AFAICS what happens when generating a layer is to start with a clean
>>>> upper layer, do some operations, then save the contents of the upper
>>>> layer.  If redirect or metacopy is enabled, then the contents of the
>>>> upper layer won't be portable.  So need to do something like this:
>>>>
>>>> traverse(overlay_dir, upper_dir, target_dir)
>>>> {
>>>>     iterate name for entries in $upper_dir {
>>>>         if ($name is non-directory) {
>>>>             copy($overlay_dir/$name, $target_dir/$name)
>>>>         } else if ($name is redirect) {
>>>>             copy-recursive($overlay_dir/$name, $target_dir/$name)
>>>>         } else {
>>>>             copy($overlay_dir/$name, $target_dir/$name)
>>>>             traverse($overlay_dir/$name, $upper_dir/$name, $target_dir/$name)
>>>>         }
>>>>     }
>>>> }
>>>>
>>>> Basically: traverse the *upper layer* but copy files and directories
>>>> from the *overlay*.  Does that make sense?

Re: [PATCH v2] overlay: allow config override of metacopy/redirect defaults

[Daniel Walsh]

On 6/11/19 5:57 PM, Matt Coffin wrote:
> This could just be because I don't understand the implications here, but
> wouldn't it be easier, at least for now, to just mount with
>
> redirect_dir=0,metacopy=0
We want the advantages of metacopy for use with User Namespace.
> in the mount parameters when building images, but allow the user's
> default settings to still take over when just executing a container?
>
> On 6/11/19 3:49 PM, Vivek Goyal wrote:
>> On Tue, Jun 11, 2019 at 05:44:33PM -0400, Daniel Walsh wrote:
>>> On 6/11/19 9:09 AM, Vivek Goyal wrote:
>>>> On Tue, Jun 11, 2019 at 02:37:34PM +0200, Miklos Szeredi wrote:
>>>>> On Mon, Jun 10, 2019 at 8:45 PM Vivek Goyal <vgoyal@redhat.com> wrote:
>>>>> AFAICS what happens when generating a layer is to start with a clean
>>>>> upper layer, do some operations, then save the contents of the upper
>>>>> layer.  If redirect or metacopy is enabled, then the contents of the
>>>>> upper layer won't be portable.  So need to do something like this:
>>>>>
>>>>> traverse(overlay_dir, upper_dir, target_dir)
>>>>> {
>>>>>     iterate name for entries in $upper_dir {
>>>>>         if ($name is non-directory) {
>>>>>             copy($overlay_dir/$name, $target_dir/$name)
>>>>>         } else if ($name is redirect) {
>>>>>             copy-recursive($overlay_dir/$name, $target_dir/$name)
>>>>>         } else {
>>>>>             copy($overlay_dir/$name, $target_dir/$name)
>>>>>             traverse($overlay_dir/$name, $upper_dir/$name, $target_dir/$name)
>>>>>         }
>>>>>     }
>>>>> }
>>>>>
>>>>> Basically: traverse the *upper layer* but copy files and directories
>>>>> from the *overlay*.  Does that make sense?

Re: [PATCH v2] overlay: allow config override of metacopy/redirect defaults

[Vivek Goyal]

On Tue, Jun 11, 2019 at 03:57:03PM -0600, Matt Coffin wrote:
> This could just be because I don't understand the implications here, but
> wouldn't it be easier, at least for now, to just mount with
> 
> redirect_dir=0,metacopy=0
> 
> in the mount parameters when building images, but allow the user's
> default settings to still take over when just executing a container?

I think that's what docker does by default, isn't it? That is redirect_dir
and metacopy features are disabled by default (until and unless user
decides to enable it).

Dan walsh recently changed podman to enable metacopy feature by default
(which in-turn will enable redirect as well). He wants to make use of
user namespaces with containers and chown the images with metacopy
enabled. We don't have shiftfs upstream yet.

BTW, how slow is image building with naivediff interface.

Vivek

> 
> On 6/11/19 3:49 PM, Vivek Goyal wrote:
> > On Tue, Jun 11, 2019 at 05:44:33PM -0400, Daniel Walsh wrote:
> >> On 6/11/19 9:09 AM, Vivek Goyal wrote:
> >>> On Tue, Jun 11, 2019 at 02:37:34PM +0200, Miklos Szeredi wrote:
> >>>> On Mon, Jun 10, 2019 at 8:45 PM Vivek Goyal <vgoyal@redhat.com> wrote:
> >>>> AFAICS what happens when generating a layer is to start with a clean
> >>>> upper layer, do some operations, then save the contents of the upper
> >>>> layer.  If redirect or metacopy is enabled, then the contents of the
> >>>> upper layer won't be portable.  So need to do something like this:
> >>>>
> >>>> traverse(overlay_dir, upper_dir, target_dir)
> >>>> {
> >>>>     iterate name for entries in $upper_dir {
> >>>>         if ($name is non-directory) {
> >>>>             copy($overlay_dir/$name, $target_dir/$name)
> >>>>         } else if ($name is redirect) {
> >>>>             copy-recursive($overlay_dir/$name, $target_dir/$name)
> >>>>         } else {
> >>>>             copy($overlay_dir/$name, $target_dir/$name)
> >>>>             traverse($overlay_dir/$name, $upper_dir/$name, $target_dir/$name)
> >>>>         }
> >>>>     }
> >>>> }
> >>>>
> >>>> Basically: traverse the *upper layer* but copy files and directories
> >>>> from the *overlay*.  Does that make sense?