> > > HOWEVER! look at the way we implemented reporting of FAN_RENAME
> > > (i.e. match_mask). We report_new location only if watching sb or watching
> > > new dir. We did that for a reason because watcher may not have permissions
> > > to read new dir. We could revisit this decision for a privileged group, but will
> > > need to go back reading all the discussions we had about this point to see
> > > if there were other reasons(?).
> >
> > Yeah, this is a good point. We are able to safely report the new parent
> > only if the watching process is able to prove it is able to watch it.
> > Adding even more special cases there would be ugly and error prone I'm
> > afraid. We could certainly make this available only to priviledged
> > notification groups but still it is one more odd corner case and the
> > usecase does not seem to be that big.
>
> Sorry, I'm confused about the conclusion we've drawn here. Are we hard
> up against not extending FAN_RENAME for the sole reason that the
> implementation might be ugly and error prone?
>
> Can we not expose this case exclusively to privileged notification
> groups/watchers? This case seems far simpler than what has already
> been implemented in the FAN_RENAME series, that is as you mentioned,
> trying to safely report the new parent only if the watching process is
> able to prove it is able to watch it. If anything, I would've expected
> the privileged case to be implemented prior to attempting to cover
> whether the super block or target directory is being watched.
To be fair, that is what the "added complexity" for the privileged use
case looks like:
/* Report both old and new parent+name if sb watching */
report_old = report_new =
+ !FAN_GROUP_FLAG(group, FANOTIFY_UNPRIV) ||
match_mask & (1U << FSNOTIFY_ITER_TYPE_SB);
report_old |=
match_mask & (1U << FSNOTIFY_ITER_TYPE_INODE);
There is a bit more complexity to replace FSNOTIFY_ITER_TYPE_INODE2
with FSNOTIFY_ITER_TYPE_DIR1 and FSNOTIFY_ITER_TYPE_DIR1.
But I understand why Jan is hesitant about increasing the cases for
already highly
specialized code.
My only argument in favor of this case is that had we though about it before
merging FAN_RENAME we would have probably included it.(?)
>
> > So maybe watching on sb for FAN_RENAME and just quickly filtering
> > based on child FID would be better solution than fiddling with new
> > event for files?
>
> Ah, I really wanted to stay away from watching the super block for all
> FAN_RENAME events. I feel like userspace wearing the pain for such
> cases is suboptimal, as this is something that can effectively be done
> in-kernel.
>
I'm not opposing. Letting Jan make the call.
Thanks,
Amir.
