On Tue, Jul 25, 2017 at 6:16 PM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote: > On Tue, Jul 25, 2017 at 4:30 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote: >> On Tue, Jul 25, 2017 at 2:33 PM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote: >>> On Mon, Jul 24, 2017 at 2:14 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote: >>>> On Mon, Jul 24, 2017 at 11:48 AM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote: >>>>> On Fri, Jul 14, 2017 at 12:58 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote: >>>>> >>>>>> If you don't want to bundle index=on with NFS export, then no rush to merge >>>>>> these patches now. The added benefit on patch 9 (retroactive marking merge >>>>>> dir with origin for not exposing whiteouts) is relevant to a patch that is not >>>>>> going to land in v4.13 anyway. right? >>>>> >>>>> Right. >>>>> >>>>> Can we maybe postpone the verification until fh decode time? >>>> >>>> Not sure I follow. >>>> Failure to verify lower dir can result in merge dir not being merged or being >>>> merged with a different lower dir (followed by fh). >>>> >>>> The question is which inode we choose to encode in case lower dir cannot be >>>> verified: the unverified lower inode, the stored origin inode or the >>>> upper inode. >>>> >>>> Are you suggesting that "normal" lookup will merge the unverified lower dir >>>> and then a pair of encode/decode will result in another dentry? >>>> or are you suggesting to return ESTALE in that case? >>>> If latter, then that error we be permanent? encode/decode will alway end up >>>> with ESTALE?? >>> >>> Putting a bit more thought into this... >>> >>> I think we have several different cases >>> >>> a) offline move of lower dir, no active NFS export >>> >>> b) online move of lower dir, no active NFS export >>> >>> c) offline move of lower dir, active NFS export >>> >>> d) online move of lower dir, active NFS export >>> >>> e) offline move of lower dir, snapshotfs >>> >>> f) online move of lower dir, snapshotfs >>> >>> The behavior of (a) is well defined: we do not follow origin of >>> directories for merging. It might make sense to introduce a variant >>> that does follow origin, but I'm not sure I see the usefulness. >>> >>> The behavior of (b) is undefined, it might make sense to add an >>> ovl_d_revalidate() and make that case behave like (a) >>> >>> For first version we can say that (c) and (d) are both undefined. I >>> think the logical behavior would be to return ESTALE when we detect >>> such an inconsistency (not sure how expensive that would be) and then >>> the lookup would end up with the updated lower. >>> >> >> I understand. This sounds reasonable. The hard part about making >> behavior of (d) defined is that we cannot tell if a lower dir that was >> encoded has not been copied up or was copied up and then lower has >> moved. > > Wait, copy-up creates index, no? That's what I thought at first, but then realized we only *need* to index renamed and unlinked upper objects for NFS export. My concern w.r.t. indexing on copy up was that performance of index create and lookup can degrade for large index dir, so better index only what we need in order to be able to decode. But if copy up creates index then behavior of (d) can be well defined. B.t.w my current NFS export WIP does index on copy up. > >> Bottom line is that splitting follow origin to a new mount flag for >> snapshot is fine by me. Do you intend to preserve the behavior change >> of NOT following unverified lower dir? Without this change ESTALE >> error could become persistent. > > I don't get it. Right. That involves some more lower renames. Imagine 2 merged dirs A and B both decoded by origin. Then the 2 lower dirs swapped. Now decode of A yields an inconsistent index so return ESTALE. But lookup of A will follow to unverified lower B and encode of that dentry will be same as encode of B before the swap, so we are bound to always get ESTALE after encode/decode. I suppose there are other ways out of this loop hole besides not following unverified lower. I just don't see them right now. Amir. -- To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
