On Mon, Jun 19, 2017 at 2:44 PM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote: > On Mon, Jun 19, 2017 at 1:19 PM, zhangyi (F) <yi.zhang@xxxxxxxxxx> wrote: >> On 2017/6/19 16:28, Amir Goldstein wrote: >>> On Mon, Jun 19, 2017 at 10:59 AM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote: >>>> On Mon, Jun 19, 2017 at 6:00 AM, zhangyi (F) <yi.zhang@xxxxxxxxxx> wrote: >>>> >>>> I still don't like adding the impure flag because of a whiteout. >>>> Impure dir means it contains a copied up object. In fact a valid (but >>>> probably not worthwhile) optimizition would be to remove the impure >>>> flag on removal of the last copied up object. >>>> >>>> What's wrong with testing for (origin || merge) to see if we need to >>>> check for whiteouts? >>>> >>> >>> Fine by me. >>> >>> Yi, >>> >>> Please note that you cannot use the test OVL_TYPE_ORIGIN(type) >>> in ovl_dir_is_real(), because that flag is only set when the lower >>> dentry is found by lookup. Instead, you should explicitly check for >>> existence of OVL_XATTR_ORIGIN. >>> Note that even ovl_get_origin_fh() is too strict, because it discards >>> empty (unkown) origin, but for your use case, unknown origin is also >>> candidate to contain whiteouts. >>> >> >> I realize that impure flag is not quite suitable. If we use (origin || merge) >> to see whether we need to check whiteouts or not, I think we still have a >> problem. >> >> Thought: In my current test case, testdir was copyuped when we delete >> the test file, so it have OVL_XATTR_ORIGIN (it's OK), but if the testdir >> is a merged dir originally, it will not be copied up and the OVL_XATTR_ORIGIN >> will not be set. >> >> So, if we want to use (origin || merge), we have to set OVL_XATTR_ORIGIN for >> the merged parent dir when it create whiteout, any side effects? > > It's done for the root of the overlay by Amir's patchset, but that > probably should be done generically for all merged directories not > already having an origin marking. It could be done at lookup time, > or at whiteout time. Not sure which is better. > Checking merge dir origin is something I planned to do anyway, because it is needed for NFS export (only verified lower can be indexed). In fact, I already did it, but yanked it out of the current series because it is not needed for indexing non-dirs, see: https://github.com/amir73il/linux/commit/9af404799ba4ba4d08e147c7d54f6bcef0527bc9#diff-643262c1d5b2cba0bc9500e531831c12R481 On top of my current ovl-hardlinks branch, the following change in lookup would get the required origin mark on merge dirs and code will be inline with upcoming NFS export changes: @@ -416,6 +478,17 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry, if (err) goto out_put; + /* Verify that uppermost lower matches the copy up origin fh */ + if (this && upperdentry && !ctr && + ovl_indexdir(dentry->d_sb))) { + err = ovl_verify_set_origin(upperdentry, lowerpath.mnt, + this, "merge", true); + if (err && err != -ENODATA) { + dput(this); + break; + } + } + if (!this) continue; 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
