Hi Amir! On Tue 11-04-23 15:40:37, Amir Goldstein wrote: > If kernel supports FAN_REPORT_ANY_FID, use this flag to allow testing > also filesystems that do not support fsid or NFS file handles (e.g. fuse). > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> > --- > > Jan, > > I wanted to run an idea by you. > > My motivation is to close functional gaps between fanotify and inotify. > > One of the largest gaps right now is that FAN_REPORT_FID is limited > to a subset of local filesystems. > > The idea is to report fid's that are "good enough" and that there > is no need to require that fid's can be used by open_by_handle_at() > because that is a non-requirement for most use cases, unpriv listener > in particular. OK. I'd note that if you report only inode number, you are prone to the problem that some inode gets freed (file deleted) and then reallocated (new file created) and the resulting identifier is the same. It can be problematic for a listener to detect these cases and deal with them. Inotify does not have this problem at least for some cases because 'wd' uniquely identifies the marked inode. For other cases (like watching dirs) inotify has similar sort of problems. I'm muttering over this because in theory filesystems not having i_generation counter on disk could approach the problem in a similar way as FAT and then we could just use FILEID_INO64_GEN for the file handle. Also I have noticed your workaround with using st_dev for fsid. As I've checked, there are actually very few filesystems that don't set fsid these days. So maybe we could just get away with still refusing to report on filesystems without fsid and possibly fixup filesystems which don't set fsid yet and are used enough so that users complain? > I chose a rather generic name for the flag to opt-in for "good enough" > fid's. At first, I was going to make those fid's self describing the > fact that they are not NFS file handles, but in the name of simplicity > to the API, I decided that this is not needed. I'd like to discuss a bit about the meaning of the flag. On the first look it is a bit strange to have a flag that says "give me a fh, if you don't have it, give me ino". It would seem cleaner to have "give me fh" kind of interface (FAN_REPORT_FID) and "give me ino" kind of interface (new FAN_REPORT_* flag). I suspect you've chosen the more complex meaning because you want to allow a usecase where watches of filesystems which don't support filehandles are mixed with watches of filesystems which do support filehandles in one notification group and getting filehandles is actually prefered over getting just inode numbers? Do you see real benefit in getting file handles when userspace has to implement fallback for getting just inode numbers anyway? > The patch below is from the LTP test [1] that verifies reported fid's. > I am posting it because I think that the function fanotify_get_fid() > demonstrates well, how a would-be fanotify library could be used to get > a canonical fid. > > That would-be routine can easily return the source of the fid values > for a given filesystem and that information is constant for all objects > on a given filesystem instance. > > The choise to encode an actual file_handle of type FILEID_INO64 may > seem controversial at first, but it simplifies things so much, that I > grew very fond of it. FILEID_INO64 is a bit of a hack in particular because it's difficult to pretend FILEID_INO64 can be used for NFS. But I agree it is very convenient :). If we were to do this cleanly we'd have to introduce a new info structure with ino instead of handle and three new FAN_EVENT_INFO_TYPE_* types. As I wrote above, we might be able to actually fill-in FILEID_INO64_GEN which would be less controversial then I suppose. > The LTP patch also demonstrated how terribly trivial it would be to > adapt any existing fanotify programs to support any fs. > > Kernel patches [2] are pretty simple IMO and > man page patch [3] demonstrates that the API changes are minimal. > > Thoughts? Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
