On Fri, Jun 16, 2017 at 10:39 AM, zhangyi (F) <yi.zhang@xxxxxxxxxx> wrote: > On 2017/6/16 14:02, Amir Goldstein Wrote: >> On Fri, Jun 16, 2017 at 6:55 AM, zhangyi (F) <yi.zhang@xxxxxxxxxx> wrote: >>> Hi All: >>> >>> I found another problem about impure upper dir which have left >>> whiteout files. If an "impure && not merged" upper dir have >>> left whiteout files(last mount created), ovl cannot clear this >>> dir when we removing it, which may lead to rmdir fail or temp >>> file left in workdir. >>> >>> Reproducer: >>> mkdir lower upper work merge >>> mkdir -p lower/dir >>> touch lower/dir/a >>> mount -t overlay overlay -olowerdir=lower,upperdir=upper,\ >>> workdir=work merge >>> rm merge/dir/a >>> umount merge >>> rm -rf lower/* >>> touch lower/dir (*) >>> mount -t overlay overlay -olowerdir=lower,upperdir=upper,\ >>> workdir=work merge >>> rm -rf merge/dir >> >> Can you please post an xfstest with that reproducer. >> A good starting point is test overlay/012 >> > > Okay，I will write a new xfstest case with this reproducer, thanks! > >>> --- >>> fs/overlayfs/dir.c | 13 ++++++++++--- >>> 1 file changed, 10 insertions(+), 3 deletions(-) >>> >>> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c >>> index a63a716..9535e2d 100644 >>> --- a/fs/overlayfs/dir.c >>> +++ b/fs/overlayfs/dir.c >>> @@ -336,7 +336,8 @@ static struct dentry *ovl_check_empty_and_clear(struct dentry *dentry) >>> * Doesn't matter, since copy-up can't create a non-empty directory >>> * from an empty one. >>> */ >>> - if (OVL_TYPE_UPPER(type) && OVL_TYPE_MERGE(type)) >>> + if (OVL_TYPE_UPPER(type) && (OVL_TYPE_MERGE(type) || >>> + ovl_dentry_is_impure(dentry))) >>> ret = ovl_clear_empty(dentry, &list); >>> >>> out_free: >>> @@ -633,6 +634,11 @@ static int ovl_remove_and_whiteout(struct dentry *dentry, bool is_dir) >>> goto out; >>> } >>> >>> + /* Mark parent "impure" because it may now contain non-pure upper */ >>> + err = ovl_set_impure(dentry->d_parent, upperdir); >>> + if (err) >>> + goto out_dput; >>> + >> >> This should be done before ovl_whiteout(). >> You rather have false positive impure than the other way around. >> Also please fix comment to "... may now contain whiteouts". >> > > Thanks for the review comments. I will fix it. > > The first suggestion I don't quite understand. As you said, ovl_set_impure() > should done before ovl_whiteout()，I understand that have false impure is > doesn't matter if subsequent code return error. My patch call ovl_set_impure() > before ovl_lock_rename_workdir() is also before ovl_whiteout(). > > Do I misunderstand or do you mean just before ovl_whiteout() ? I misread. Your patch is correct > -- 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
