All of lore.kernel.org
 help / color / mirror / Atom feed
* EIO for removed redirected files?
@ 2020-08-12 13:55 Kevin Locke
  2020-08-12 15:21 ` Amir Goldstein
  0 siblings, 1 reply; 7+ messages in thread
From: Kevin Locke @ 2020-08-12 13:55 UTC (permalink / raw)
  To: linux-unionfs

Hi All,

I recently encountered files on an overlayfs which returned EIO
(Input/output error) for open, stat, and unlink (and presumably other)
syscalls.  I eventually determined that the files had been redirected
and the target removed from the lower level.  The behavior can be
reproduced as follows:

# Create overlay with foo.txt on lower level
mkdir -p lower upper work merged
touch lower/foo.txt
mount -t overlay -o lowerdir=lower,upperdir=upper,workdir=work,metacopy=on none merged

# Redirect bar.txt on upper to foo.txt on lower
mv merged/foo.txt merged/bar.txt

# Remove foo.txt on lower while unmounted
umount merged
rm lower/foo.txt

# open, stat, and unlink on bar.txt now fail with EIO:
mount -t overlay -o lowerdir=lower,upperdir=upper,workdir=work,metacopy=on none merged
cat merged/bar.txt
stat merged/bar.txt
rm merged/bar.txt

At this point, the only way to recover appears to be unmounting the
overlay and removing the file from upper (or updating the
overlay.redirect xattr to a valid location).  Is that correct?

Is this the intended behavior?  I didn't see any tests covering it in
unionmount-testsuite.  If so, perhaps the behavior could be noted in
"Changes to underlying filesystems" in
Documentation/filesystems/overlayfs.rst?  I'd be willing to write a
first draft.  (I doubt I understand it well enough to get everything
right on the first try.)

Also, if there is any way this could be made easier to debug, it might
be helpful for users, particularly newbies like myself.  Perhaps logging
bad redirects at KERN_ERR?  If that would be too noisy, perhaps only
behind a debug module option?  Again, if this is acceptable I'd be
willing to draft a patch.  (Same caveat as above.)

Thanks for considering,
Kevin

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

* Re: EIO for removed redirected files?
  2020-08-12 13:55 EIO for removed redirected files? Kevin Locke
@ 2020-08-12 15:21 ` Amir Goldstein
  2020-08-12 16:05   ` Kevin Locke
  0 siblings, 1 reply; 7+ messages in thread
From: Amir Goldstein @ 2020-08-12 15:21 UTC (permalink / raw)
  To: Kevin Locke, overlayfs

On Wed, Aug 12, 2020 at 5:06 PM Kevin Locke <kevin@kevinlocke.name> wrote:
>
> Hi All,
>
> I recently encountered files on an overlayfs which returned EIO
> (Input/output error) for open, stat, and unlink (and presumably other)
> syscalls.  I eventually determined that the files had been redirected

It's *empty* redirected files that cause the alleged problem.
If files were not empty, the EIO would have been expected, because...

> and the target removed from the lower level.  The behavior can be
> reproduced as follows:
>
> # Create overlay with foo.txt on lower level
> mkdir -p lower upper work merged
> touch lower/foo.txt

Suppose this was:
echo 123 >  lower/foo.txt

> mount -t overlay -o lowerdir=lower,upperdir=upper,workdir=work,metacopy=on none merged
>
> # Redirect bar.txt on upper to foo.txt on lower
> mv merged/foo.txt merged/bar.txt
>

...this mv does not copy up "123" to foo.txt before renaming it to bar.txt...

> # Remove foo.txt on lower while unmounted
> umount merged
> rm lower/foo.txt
>
> # open, stat, and unlink on bar.txt now fail with EIO:
> mount -t overlay -o lowerdir=lower,upperdir=upper,workdir=work,metacopy=on none merged
> cat merged/bar.txt
> stat merged/bar.txt
> rm merged/bar.txt
>
> At this point, the only way to recover appears to be unmounting the
> overlay and removing the file from upper (or updating the
> overlay.redirect xattr to a valid location).  Is that correct?
>
> Is this the intended behavior?

Yes.
What would you expect to happen when data of metacopy file has been removed?

> I didn't see any tests covering it in

We have some tests in xfstests to prove that modifying underlying
layers does not
result in a crash, but otherwise the behavior is undefined, so it is
hard to write tests.
There is some code and tests in place for better behavior on
underlying file changes
like not exposing whiteouts in upper dir when lower dir was removed.

> unionmount-testsuite.  If so, perhaps the behavior could be noted in
> "Changes to underlying filesystems" in
> Documentation/filesystems/overlayfs.rst?  I'd be willing to write a
> first draft.  (I doubt I understand it well enough to get everything
> right on the first try.)
>

I guess the only thing we could document is that changes to underlying
layers with metacopy and redirects have undefined results.
Vivek was a proponent of making the statements about outcome of
changes to underlying layers sound more harsh.

> Also, if there is any way this could be made easier to debug, it might
> be helpful for users, particularly newbies like myself.  Perhaps logging
> bad redirects at KERN_ERR?  If that would be too noisy, perhaps only
> behind a debug module option?  Again, if this is acceptable I'd be
> willing to draft a patch.  (Same caveat as above.)
>

There are a handful of places in overlayfs where returning EIO is
combined with informative pr_warn_ratelimited().
You can see some examples in ovl_lookup(), which is where the reported
EIO is coming from:
        if (d.metacopy || (uppermetacopy && !ctr)) {
                err = -EIO;


Having said all that, metacopy and redirects on lower empty files seems
like an unintentional outcome.

If you care about this use case particularly, you may try this untested patch:

Thanks,
Amir.

---
diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index d07fb92b7253..178067cb6583 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -764,7 +764,7 @@ static int ovl_do_copy_up(struct ovl_copy_up_ctx *c)
        return err;
 }

-static bool ovl_need_meta_copy_up(struct dentry *dentry, umode_t mode,
+static bool ovl_need_meta_copy_up(struct dentry *dentry, struct kstat *stat,
                                  int flags)
 {
        struct ovl_fs *ofs = dentry->d_sb->s_fs_info;
@@ -772,7 +772,7 @@ static bool ovl_need_meta_copy_up(struct dentry
*dentry, umode_t mode,
        if (!ofs->config.metacopy)
                return false;

-       if (!S_ISREG(mode))
+       if (!S_ISREG(stat->mode) || !stat->size)
                return false;

        if (flags && ((OPEN_FMODE(flags) & FMODE_WRITE) || (flags & O_TRUNC)))
@@ -852,8 +852,6 @@ static int ovl_copy_up_one(struct dentry *parent,
struct dentry *dentry,
        if (err)
                return err;

-       ctx.metacopy = ovl_need_meta_copy_up(dentry, ctx.stat.mode, flags);
-
        if (parent) {
                ovl_path_upper(parent, &parentpath);
                ctx.destdir = parentpath.dentry;
@@ -870,6 +868,8 @@ static int ovl_copy_up_one(struct dentry *parent,
struct dentry *dentry,
        if (flags & O_TRUNC)
                ctx.stat.size = 0;

+       ctx.metacopy = ovl_need_meta_copy_up(dentry, &ctx.stat, flags);
+
        if (S_ISLNK(ctx.stat.mode)) {
                ctx.link = vfs_get_link(ctx.lowerpath.dentry, &done);
                if (IS_ERR(ctx.link))

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

* Re: EIO for removed redirected files?
  2020-08-12 15:21 ` Amir Goldstein
@ 2020-08-12 16:05   ` Kevin Locke
  2020-08-12 17:06     ` Amir Goldstein
  0 siblings, 1 reply; 7+ messages in thread
From: Kevin Locke @ 2020-08-12 16:05 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: overlayfs

Thanks for the quick response Amir!

On Wed, 2020-08-12 at 18:21 +0300, Amir Goldstein wrote:
> On Wed, Aug 12, 2020 at 5:06 PM Kevin Locke <kevin@kevinlocke.name> wrote:
>> I recently encountered files on an overlayfs which returned EIO
>> (Input/output error) for open, stat, and unlink (and presumably other)
>> syscalls.  I eventually determined that the files had been redirected
> 
> It's *empty* redirected files that cause the alleged problem.

When I replace `touch foo.txt` with `echo 123 > foo.txt` I observe the
same behavior.  If I understand you correctly, you are saying that EIO
is correct for non-empty files, but potentially incorrect for empty
files (which could be copied rather than redirected, since there is no
space saving)?

>> At this point, the only way to recover appears to be unmounting the
>> overlay and removing the file from upper (or updating the
>> overlay.redirect xattr to a valid location).  Is that correct?
>>
>> Is this the intended behavior?
> 
> Yes.
> What would you expect to happen when data of metacopy file has been removed?

After reflection, EIO probably makes the most sense for open/stat.  It
might be nice to be able to unlink the file to allow recovery (in the
sense of being able to reuse the name) without unmounting the overlay,
but the documentation updates may be sufficient to keep users from
getting into this state.

>> unionmount-testsuite.  If so, perhaps the behavior could be noted in
>> "Changes to underlying filesystems" in
>> Documentation/filesystems/overlayfs.rst?  I'd be willing to write a
>> first draft.  (I doubt I understand it well enough to get everything
>> right on the first try.)
> 
> I guess the only thing we could document is that changes to underlying
> layers with metacopy and redirects have undefined results.
> Vivek was a proponent of making the statements about outcome of
> changes to underlying layers sound more harsh.

That sounds good to me.  My current use case involves offline changes to
the lower layer on a routine basis, and I interpreted the current
wording "Offline changes, when the overlay is not mounted, are allowed
to either the upper or the lower trees." to mean that such offline
modifications would not break things in unexpected ways.

In retrospect, I should have expected this behavior, but as someone
previously unfamiliar with overlayfs, I hadn't considered that metacopy
results in file redirects and that if the underlying file were removed
without removing any redirects pointing to it that it would manifest in
this way and be so difficult to clean up.

If metacopy and dir_redirect are disabled, are offline modifications to
the lower layer permitted, or could any such modification result in
undefined behavior?

>> Also, if there is any way this could be made easier to debug, it might
>> be helpful for users, particularly newbies like myself.  Perhaps logging
>> bad redirects at KERN_ERR?  If that would be too noisy, perhaps only
>> behind a debug module option?  Again, if this is acceptable I'd be
>> willing to draft a patch.  (Same caveat as above.)
> 
> There are a handful of places in overlayfs where returning EIO is
> combined with informative pr_warn_ratelimited().

Ah, indeed.  Would doing so for missing/invalid metacopy/redirect make
sense?

> You can see some examples in ovl_lookup(), which is where the reported
> EIO is coming from:
>         if (d.metacopy || (uppermetacopy && !ctr)) {
>                 err = -EIO;
> 
> Having said all that, metacopy and redirects on lower empty files seems
> like an unintentional outcome.
> 
> If you care about this use case particularly, you may try this untested patch:

Thanks for the patch!  (Especially so quickly.)  I'll try it out soon.

Thanks again,
Kevin

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

* Re: EIO for removed redirected files?
  2020-08-12 16:05   ` Kevin Locke
@ 2020-08-12 17:06     ` Amir Goldstein
  2020-08-13 17:22       ` Kevin Locke
  2020-08-17 13:56       ` Vivek Goyal
  0 siblings, 2 replies; 7+ messages in thread
From: Amir Goldstein @ 2020-08-12 17:06 UTC (permalink / raw)
  To: Kevin Locke, Amir Goldstein; +Cc: overlayfs, Vivek Goyal

On Wed, Aug 12, 2020 at 7:05 PM Kevin Locke <kevin@kevinlocke.name> wrote:
>
> Thanks for the quick response Amir!
>
> On Wed, 2020-08-12 at 18:21 +0300, Amir Goldstein wrote:
> > On Wed, Aug 12, 2020 at 5:06 PM Kevin Locke <kevin@kevinlocke.name> wrote:
> >> I recently encountered files on an overlayfs which returned EIO
> >> (Input/output error) for open, stat, and unlink (and presumably other)
> >> syscalls.  I eventually determined that the files had been redirected
> >
> > It's *empty* redirected files that cause the alleged problem.
>
> When I replace `touch foo.txt` with `echo 123 > foo.txt` I observe the
> same behavior.  If I understand you correctly, you are saying that EIO
> is correct for non-empty files, but potentially incorrect for empty
> files (which could be copied rather than redirected, since there is no
> space saving)?
>

I wouldn't call it "incorrect" more like "unnecessary".

> >> At this point, the only way to recover appears to be unmounting the
> >> overlay and removing the file from upper (or updating the
> >> overlay.redirect xattr to a valid location).  Is that correct?
> >>
> >> Is this the intended behavior?
> >
> > Yes.
> > What would you expect to happen when data of metacopy file has been removed?
>
> After reflection, EIO probably makes the most sense for open/stat.  It
> might be nice to be able to unlink the file to allow recovery (in the
> sense of being able to reuse the name) without unmounting the overlay,

It would be nice, but somebody needs to care enough to implement it
and it is not going to be trivial, because error on lookup is much easier
then selective error on a "broken" dentry depending on the operation...

> but the documentation updates may be sufficient to keep users from
> getting into this state.
>
> >> unionmount-testsuite.  If so, perhaps the behavior could be noted in
> >> "Changes to underlying filesystems" in
> >> Documentation/filesystems/overlayfs.rst?  I'd be willing to write a
> >> first draft.  (I doubt I understand it well enough to get everything
> >> right on the first try.)
> >
> > I guess the only thing we could document is that changes to underlying
> > layers with metacopy and redirects have undefined results.
> > Vivek was a proponent of making the statements about outcome of
> > changes to underlying layers sound more harsh.
>
> That sounds good to me.  My current use case involves offline changes to
> the lower layer on a routine basis, and I interpreted the current

You are not the only one, I hear of many users that do that, but nobody ever
bothered to sit down and document the requirements - what exactly is the
use case and what is the expected outcome.

> wording "Offline changes, when the overlay is not mounted, are allowed
> to either the upper or the lower trees." to mean that such offline
> modifications would not break things in unexpected ways.
>

The truth is that this documentation is old, before all the new features
were added. See here [1], Vivek suggested:
"Modifying/recreating lower layer only works when
 metacopy/index/nfs_export are not enabled at any point of time. This
 also will change inode number reporting behavior."

> In retrospect, I should have expected this behavior, but as someone
> previously unfamiliar with overlayfs, I hadn't considered that metacopy
> results in file redirects and that if the underlying file were removed
> without removing any redirects pointing to it that it would manifest in
> this way and be so difficult to clean up.
>
> If metacopy and dir_redirect are disabled, are offline modifications to
> the lower layer permitted, or could any such modification result in
> undefined behavior?
>

With metacopy/index/nfs_export/redirect_dir disabled code behaves mostly
like it did at the time that this documentation was written, so I guess you may
say that changes are permitted and result in "defined" behavior.

> >> Also, if there is any way this could be made easier to debug, it might
> >> be helpful for users, particularly newbies like myself.  Perhaps logging
> >> bad redirects at KERN_ERR?  If that would be too noisy, perhaps only
> >> behind a debug module option?  Again, if this is acceptable I'd be
> >> willing to draft a patch.  (Same caveat as above.)
> >
> > There are a handful of places in overlayfs where returning EIO is
> > combined with informative pr_warn_ratelimited().
>
> Ah, indeed.  Would doing so for missing/invalid metacopy/redirect make
> sense?
>

It seems fine to me.

Thanks,
Amir.

[1] https://lore.kernel.org/linux-unionfs/20200708173601.GD103536@redhat.com/T/#m75db6db2fc8d9739ce6fed9dfebe81bb1dd53bf9

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

* Re: EIO for removed redirected files?
  2020-08-12 17:06     ` Amir Goldstein
@ 2020-08-13 17:22       ` Kevin Locke
  2020-08-14  6:20         ` Amir Goldstein
  2020-08-17 13:56       ` Vivek Goyal
  1 sibling, 1 reply; 7+ messages in thread
From: Kevin Locke @ 2020-08-13 17:22 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: overlayfs, Vivek Goyal

Thanks again Amir!  I'll work on patches for the docs and adding
pr_warn_ratelimited() for invalid metacopy/redirect as soon as I get a
chance.

On Wed, 2020-08-12 at 20:06 +0300, Amir Goldstein wrote:
> On Wed, Aug 12, 2020 at 7:05 PM Kevin Locke <kevin@kevinlocke.name> wrote:
>> On Wed, 2020-08-12 at 18:21 +0300, Amir Goldstein wrote:
>>> I guess the only thing we could document is that changes to underlying
>>> layers with metacopy and redirects have undefined results.
>>> Vivek was a proponent of making the statements about outcome of
>>> changes to underlying layers sound more harsh.
>>
>> That sounds good to me.  My current use case involves offline changes to
>> the lower layer on a routine basis, and I interpreted the current
> 
> You are not the only one, I hear of many users that do that, but nobody ever
> bothered to sit down and document the requirements - what exactly is the
> use case and what is the expected outcome.

I can elaborate a bit.  Keep in mind that it's a personal use case which
is flexible, so it's probably not worth supporting specifically, but may
be useful to discuss/consider:

A few machines that I manage are dual-boot between Windows and Linux,
with software that runs on both OSes (Steam).  This software installs a
lot (>100GB) of semi-static data which is mostly (>90%) the same between
OSes, but not partitioned by folder or designed to be shared between
them.  The software includes mechanisms for validating the data files
and automatically updating/repairing any files which do not match
expectations.

I currently mount an overlayfs of the Windows data directory on the
Linux data directory to avoid storing multiple copies of common data.
After any data changes in Windows, I re-run the data file validation in
Linux to ensure the data is consistent.  I also occasionally run a
deduplication script[1] to remove files which may have been updated on
Linux and later updated to the same contents on Windows.

To support this use, I'm looking for a way to configure overlayfs such
that offline changes to the lower dir do not break things in a way that
can't be recovered by naive file content validation.  Beyond that, any
performance-enhancing and space-saving features are great.

metacopy and redirection would be nice to have, but are not essential as
the program does not frequently move data files or modify their
metadata.  If accessing an invalid metacopy behaved like a 0-length
file, it would be ideal for my use case (since it would be deleted and
re-created by file validation) but I can understand why this would be
undesirable for other cases and problematic to implement.  (I'm
experimenting with seccomp to prevent/ignore metadata changes, since the
program should run on filesystems which do not support them.  An option
to ignore/reject metadata changes would be handy, but may not be
justified.)

Does that explain?  Does it seem reasonable?  Is disabling metacopy and
redirect_dir likely to be sufficient?

Best,
Kevin

[1]: Do you know of any overlayfs-aware deduplication programs?  If not,
I may consider cleaning up and publishing mine at some point.

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

* Re: EIO for removed redirected files?
  2020-08-13 17:22       ` Kevin Locke
@ 2020-08-14  6:20         ` Amir Goldstein
  0 siblings, 0 replies; 7+ messages in thread
From: Amir Goldstein @ 2020-08-14  6:20 UTC (permalink / raw)
  To: Kevin Locke, Amir Goldstein, overlayfs, Vivek Goyal; +Cc: kmxz

On Thu, Aug 13, 2020 at 8:22 PM Kevin Locke <kevin@kevinlocke.name> wrote:
>
> Thanks again Amir!  I'll work on patches for the docs and adding
> pr_warn_ratelimited() for invalid metacopy/redirect as soon as I get a
> chance.
>
> On Wed, 2020-08-12 at 20:06 +0300, Amir Goldstein wrote:
> > On Wed, Aug 12, 2020 at 7:05 PM Kevin Locke <kevin@kevinlocke.name> wrote:
> >> On Wed, 2020-08-12 at 18:21 +0300, Amir Goldstein wrote:
> >>> I guess the only thing we could document is that changes to underlying
> >>> layers with metacopy and redirects have undefined results.
> >>> Vivek was a proponent of making the statements about outcome of
> >>> changes to underlying layers sound more harsh.
> >>
> >> That sounds good to me.  My current use case involves offline changes to
> >> the lower layer on a routine basis, and I interpreted the current
> >
> > You are not the only one, I hear of many users that do that, but nobody ever
> > bothered to sit down and document the requirements - what exactly is the
> > use case and what is the expected outcome.
>
> I can elaborate a bit.  Keep in mind that it's a personal use case which
> is flexible, so it's probably not worth supporting specifically, but may
> be useful to discuss/consider:
>
> A few machines that I manage are dual-boot between Windows and Linux,
> with software that runs on both OSes (Steam).  This software installs a
> lot (>100GB) of semi-static data which is mostly (>90%) the same between
> OSes, but not partitioned by folder or designed to be shared between
> them.  The software includes mechanisms for validating the data files
> and automatically updating/repairing any files which do not match
> expectations.
>
> I currently mount an overlayfs of the Windows data directory on the
> Linux data directory to avoid storing multiple copies of common data.
> After any data changes in Windows, I re-run the data file validation in
> Linux to ensure the data is consistent.  I also occasionally run a
> deduplication script[1] to remove files which may have been updated on
> Linux and later updated to the same contents on Windows.
>

Nice use case.
It may be a niche use case the way to describe it, but the general concept
of "updatable software" at the lower layer is not unique to your use case.
See this [1] recent example that spawned the thread about updating the
documentation w.r.t changing underlying layers.

[1] https://lore.kernel.org/linux-unionfs/32532923.JtPX5UtSzP@fgdesktop/

> To support this use, I'm looking for a way to configure overlayfs such
> that offline changes to the lower dir do not break things in a way that
> can't be recovered by naive file content validation.  Beyond that, any
> performance-enhancing and space-saving features are great.
>
> metacopy and redirection would be nice to have, but are not essential as
> the program does not frequently move data files or modify their
> metadata.

That's what I figured.

> If accessing an invalid metacopy behaved like a 0-length
> file, it would be ideal for my use case (since it would be deleted and
> re-created by file validation) but I can understand why this would be
> undesirable for other cases and problematic to implement.  (I'm

I wouldn't say it is "problematic" to implement. It is simple to convert the
EIO to warning (with opt-in option). What would be a challenge to implement
is the behavior, where metadata access is allowed for broken metacopy,
but data access results in EIO.

> experimenting with seccomp to prevent/ignore metadata changes, since the
> program should run on filesystems which do not support them.  An option
> to ignore/reject metadata changes would be handy, but may not be
> justified.)
>
> Does that explain?  Does it seem reasonable?  Is disabling metacopy and
> redirect_dir likely to be sufficient?

Yes, disabling metacopy and redirect_dir sounds like the right thing to do,
because I don't think they gain you too much anyway.

>
> Best,
> Kevin
>
> [1]: Do you know of any overlayfs-aware deduplication programs?  If not,
> I may consider cleaning up and publishing mine at some point.

I know about overlayfs-tools's "merge" command.
I do not know if anyone is using this tool besides perhaps it's author (?).
Incidentally, I recently implemented the "deref" command for overlayfs-tools [2]
which unfolds metacopy and redirect_dir and creates an upper layer without
them. The resulting layer can then be deduped with lower layer using the
"merge" command.

[2] https://github.com/kmxz/overlayfs-tools/pull/11

I also implemented (in the same pull request) awareness of overlayfs-tools
to metacopy and redirect_dir with existing commands. "merge" command
simply aborts when they are encountered, but "vacuum" and "diff" commands
work correctly. I also added the "overlay diff -b" variant, which
creates an output
equivalent to that of the standard diff tool (diffutils) just by
analyzing the layers.

Thanks,
Amir.

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

* Re: EIO for removed redirected files?
  2020-08-12 17:06     ` Amir Goldstein
  2020-08-13 17:22       ` Kevin Locke
@ 2020-08-17 13:56       ` Vivek Goyal
  1 sibling, 0 replies; 7+ messages in thread
From: Vivek Goyal @ 2020-08-17 13:56 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: Kevin Locke, overlayfs

On Wed, Aug 12, 2020 at 08:06:34PM +0300, Amir Goldstein wrote:
> On Wed, Aug 12, 2020 at 7:05 PM Kevin Locke <kevin@kevinlocke.name> wrote:
> >
> > Thanks for the quick response Amir!
> >
> > On Wed, 2020-08-12 at 18:21 +0300, Amir Goldstein wrote:
> > > On Wed, Aug 12, 2020 at 5:06 PM Kevin Locke <kevin@kevinlocke.name> wrote:
> > >> I recently encountered files on an overlayfs which returned EIO
> > >> (Input/output error) for open, stat, and unlink (and presumably other)
> > >> syscalls.  I eventually determined that the files had been redirected
> > >
> > > It's *empty* redirected files that cause the alleged problem.
> >
> > When I replace `touch foo.txt` with `echo 123 > foo.txt` I observe the
> > same behavior.  If I understand you correctly, you are saying that EIO
> > is correct for non-empty files, but potentially incorrect for empty
> > files (which could be copied rather than redirected, since there is no
> > space saving)?
> >
> 
> I wouldn't call it "incorrect" more like "unnecessary".
> 
> > >> At this point, the only way to recover appears to be unmounting the
> > >> overlay and removing the file from upper (or updating the
> > >> overlay.redirect xattr to a valid location).  Is that correct?
> > >>
> > >> Is this the intended behavior?
> > >
> > > Yes.
> > > What would you expect to happen when data of metacopy file has been removed?
> >
> > After reflection, EIO probably makes the most sense for open/stat.  It
> > might be nice to be able to unlink the file to allow recovery (in the
> > sense of being able to reuse the name) without unmounting the overlay,
> 
> It would be nice, but somebody needs to care enough to implement it
> and it is not going to be trivial, because error on lookup is much easier
> then selective error on a "broken" dentry depending on the operation...
> 
> > but the documentation updates may be sufficient to keep users from
> > getting into this state.
> >
> > >> unionmount-testsuite.  If so, perhaps the behavior could be noted in
> > >> "Changes to underlying filesystems" in
> > >> Documentation/filesystems/overlayfs.rst?  I'd be willing to write a
> > >> first draft.  (I doubt I understand it well enough to get everything
> > >> right on the first try.)
> > >
> > > I guess the only thing we could document is that changes to underlying
> > > layers with metacopy and redirects have undefined results.
> > > Vivek was a proponent of making the statements about outcome of
> > > changes to underlying layers sound more harsh.
> >
> > That sounds good to me.  My current use case involves offline changes to
> > the lower layer on a routine basis, and I interpreted the current
> 
> You are not the only one, I hear of many users that do that, but nobody ever
> bothered to sit down and document the requirements - what exactly is the
> use case and what is the expected outcome.
> 
> > wording "Offline changes, when the overlay is not mounted, are allowed
> > to either the upper or the lower trees." to mean that such offline
> > modifications would not break things in unexpected ways.
> >
> 
> The truth is that this documentation is old, before all the new features
> were added. See here [1], Vivek suggested:
> "Modifying/recreating lower layer only works when
>  metacopy/index/nfs_export are not enabled at any point of time. This
>  also will change inode number reporting behavior."
> 
> > In retrospect, I should have expected this behavior, but as someone
> > previously unfamiliar with overlayfs, I hadn't considered that metacopy
> > results in file redirects and that if the underlying file were removed
> > without removing any redirects pointing to it that it would manifest in
> > this way and be so difficult to clean up.
> >
> > If metacopy and dir_redirect are disabled, are offline modifications to
> > the lower layer permitted, or could any such modification result in
> > undefined behavior?
> >
> 
> With metacopy/index/nfs_export/redirect_dir disabled code behaves mostly
> like it did at the time that this documentation was written, so I guess you may
> say that changes are permitted and result in "defined" behavior.

I still think that we should make it clear in documentation that
modifying and recreating lower layers is not allowed when advanced
features like metacopy/index/nfs_export/redirect_dir are enabled. If
one does so, expect the unexpected.

https://lore.kernel.org/linux-unionfs/20200709153616.GE150543@redhat.com/T/#t

To me, if we allow any random modification/recreation of lower filesystem,
then definiting the behavior in all the corner cases becomes hard and
it also makes design of overlayfs more complicated because anytime you
are implementing something, now you have to worry about modifications to
lower layer as well.

And I am also not aware of any good use cases which justify supporting
lower layer modification with new features. So remain of the opinion
that don't support it.

Thanks
Vivek


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

end of thread, other threads:[~2020-08-17 13:57 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-12 13:55 EIO for removed redirected files? Kevin Locke
2020-08-12 15:21 ` Amir Goldstein
2020-08-12 16:05   ` Kevin Locke
2020-08-12 17:06     ` Amir Goldstein
2020-08-13 17:22       ` Kevin Locke
2020-08-14  6:20         ` Amir Goldstein
2020-08-17 13:56       ` Vivek Goyal

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.