overlayfs: Return error if metadata only copy-up can't be enabled
PATCH 1/1 overlayfs: Return error if metadata only copy-up can't be enabled
Assume a user wanted to enable metadata only copy-up and passes metacopy=on. If this feature can't be enabled, we disable metacopy=off and just leave a warning in logs. metacopy=on requires redirect_dir=on (for upper dir) or redirect_dir=follow (for non-upper mount).
As user does not see mount failure, he/she assumes metadata only copy-up has been enabled but that's not the case.
So instead of disabling metacopy, return an error to user and leave a message in logs. That will allow user to fix mount options and retry.
--- fs/overlayfs/super.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
fs/overlayfs/super.c +4 -4
@@ -574,11 +574,11 @@ static int ovl_parse_opt(char *opt, stru/* metacopy feature with upper requires redirect_dir=on */ if (config->upperdir && config->metacopy && !config->redirect_dir) { -pr_warn ("overlayfs: metadata only copy up requires \"redirect_dir=on\", falling back to metacopy=off.\n" ); - config->metacopy = false; +pr_err ("overlayfs: metadata only copy up requires \"redirect_dir=on\".\n" ); +return -EINVAL ; }else if (config->metacopy && !config->redirect_follow) { -pr_warn ("overlayfs: metadata only copy up requires \"redirect_dir=follow\" on non-upper mount, falling back to metacopy=off.\n" ); - config->metacopy = false; +pr_err ("overlayfs: metadata only copy up requires either \"redirect_dir=follow\" or \"redirect_dir=on\" on non-upper mount.\n" ); +return -EINVAL ; }return 0 ;
Miklos Szeredi
maintainerQuoted 4 lines · Vivek Goyal
Assume a user wanted to enable metadata only copy-up and passes metacopy=on. If this feature can't be enabled, we disable metacopy=off and just leave a warning in logs. metacopy=on requires redirect_dir=on (for upper dir) or redirect_dir=follow (for non-upper mount).
What's the reason we can't enable these for metacopy=on automatically?
I mean, the simplest and least surprising behavior would be if metacopy would simply imply redirect_dir, no?
Thanks,
Miklos
Quoted 10 lines · Miklos Szeredi
Quoted 4 lines · Vivek Goyal
Assume a user wanted to enable metadata only copy-up and passes metacopy=on. If this feature can't be enabled, we disable metacopy=off and just leave a warning in logs. metacopy=on requires redirect_dir=on (for upper dir) or redirect_dir=follow (for non-upper mount).
What's the reason we can't enable these for metacopy=on automatically?
I mean, the simplest and least surprising behavior would be if metacopy would simply imply
redirect_dir, no?
redirect_dir=on is a significant change. And it might be better if users passes it in (instead of enabling it automatically).
For example, when redirect_dir is enabled, container runtime software (docker, podman), their logic to generate image does not work and they have to fall back to *slower* method of diff.
https://github.com/moby/moby/pull/34342
IOW, enabling redirect_dir automatically has performance implications for container image building process. Given that, it might be better to fail and let user conciously make that choise and in the process understand that there can be downsides to enabling metacopy.
But I am not very particular about it. Automatically enabling redirect_dir sounds better than disabling metacopy automatically.
Thanks
Vivek
Quoted 12 lines · Vivek Goyal
Quoted 10 lines · Miklos Szeredi
Quoted 4 lines · Vivek Goyal
Assume a user wanted to enable metadata only copy-up and passes metacopy=on. If this feature can't be enabled, we disable metacopy=off and just leave a warning in logs. metacopy=on requires redirect_dir=on (for upper dir) or redirect_dir=follow (for non-upper mount).
What's the reason we can't enable these for metacopy=on automatically?
I mean, the simplest and least surprising behavior would be if metacopy would simply imply
redirect_dir, no?
Quoted 8 lines
redirect_dir=on is a significant change. And it might be better if users passes it in (instead of enabling it automatically).
For example, when
redirect_diris enabled, container runtime software (docker, podman), their logic to generate image does not work and they have to fall back to *slower* method of diff.
Having said that, metacopy=on will have same issue and user need to fall back to slower naivediff interface for image generation.
So enabling redirect_dir automatically probably is good. We could just drop a message in logs that it has been enabled automatically.
IOW, I can go either way depending on how others feel about it.
Thanks
Vivek
Quoted 22 lines · Vivek Goyal
Quoted 10 lines · Vivek Goyal
Quoted 9 lines · Miklos Szeredi
Quoted 4 lines · Vivek Goyal
Assume a user wanted to enable metadata only copy-up and passes metacopy=on. If this feature can't be enabled, we disable metacopy=off and just leave a warning in logs. metacopy=on requires redirect_dir=on (for upper dir) or redirect_dir=follow (for non-upper mount).
What's the reason we can't enable these for metacopy=on automatically?
I mean, the simplest and least surprising behavior would be if metacopy would simply imply
redirect_dir, no?Quoted 8 lines
redirect_dir=on is a significant change. And it might be better if users passes it in (instead of enabling it automatically).
For example, when
redirect_diris enabled, container runtime software (docker, podman), their logic to generate image does not work and they have to fall back to *slower* method of diff.Having said that, metacopy=on will have same issue and user need to fall back to slower naivediff interface for image generation.
Quoted 7 lines
So enabling
redirect_dirautomatically probably is good. We could just drop a message in logs that it has been enabled automatically.IOW, I can go either way depending on how others feel about it.
Thanks
Vivek
I think turning them both on and DOCUMENTING the behavior would be good.