On Tue, Apr 18, 2023 at 4:33 PM Christian Brauner <brauner@xxxxxxxxxx> wrote:
>
> On Fri, Apr 14, 2023 at 09:29:01PM +0300, Amir Goldstein wrote:
> > Jan,
> >
> > Followup on my quest to close the gap with inotify functionality,
> > here is a proposal for FAN_UNMOUNT event.
> >
> > I have had many design questions about this:
>
> I'm going to humbly express what I feel makes sense to me when looking
> at this from a user perspective:
>
> > 1) Should we also report FAN_UNMOUNT for marked inodes and sb
> > on sb shutdown (same as IN_UNMOUNT)?
>
> My preference would be if this would be a separate event type.
> FAN_SB_SHUTDOWN or something.
If we implement an event for this at all, I would suggest FAN_IGNORED
or FAN_EVICTED, which has the same meaning as IN_IGNORED.
When you get an event that the watch went away, it could be because of:
1. watch removed by user
2. watch removed because inode was evicted (with FAN_MARK_EVICTABLE)
3. inode deleted
4. sb shutdown
IN_IGNORED is generated in all of the above except for inode evict
that is not possible with inotify.
User can figure out on his own if the inode was deleted or if fs was unmounted,
so there is not really a need for FAN_SB_SHUTDOWN IMO.
Actually, I think that FAN_IGNORED would be quite useful for the
FAN_MARK_EVICTABLE case, but it is a bit less trivial to implement
than FAN_UNMOUNT was.
>
> > 2) Should we also report FAN_UNMOUNT on sb mark for any unmounts
> > of that sb?
>
> I don't think so. It feels to me that if you watch an sb you don't
> necessarily want to watch bind mounts of that sb.
>
> > 3) Should we report also the fid of the mount root? and if we do...
> > 4) Should we report/consider FAN_ONDIR filter?
> >
> > All of the questions above I answered "not unless somebody requests"
> > in this first RFC.
>
> Fwiw, I agree.
>
> >
> > Specifically, I did get a request for an unmount event for containers
> > use case.
> >
> > I have also had doubts regarding the info records.
> > I decided that reporting fsid and mntid is minimum, but couldn't
> > decide if they were better of in a single MNTID record or seprate
> > records.
> >
> > I went with separate records, because:
> > a) FAN_FS_ERROR has set a precendent of separate fid record with
> > fsid and empty fid, so I followed this precendent
> > b) MNTID record we may want to add later with FAN_REPORT_MNTID
> > to all the path events, so better that it is independent
>
Just thought of another reason:
c) FAN_UNMOUNT does not need to require FAN_REPORT_FID
so it does not depend on filesystem having a valid f_fsid nor
exports_ops. In case of "pseudo" fs, FAN_UNMOUNT can report
only MNTID record (I will amend the patch with this minor change).
> I agree.
>
> >
> > There is test for the proposed API extention [1].
> >
> > Thoughts?
>
> I think this is a rather useful extension!
Thanks for the API review!
Amir.
