On Tue, Mar 30, 2021 at 05:56:25PM +0300, Amir Goldstein wrote: > > > > My example probably would be something like: > > > > > > > > mount -t ext4 /dev/sdb /A > > > > > > > > 1. FAN_MARK_MOUNT(/A) > > > > > > > > mount --bind /A /B > > > > > > > > 2. FAN_MARK_MOUNT(/B) > > > > > > > > mount -t ecryptfs /B /C > > > > > > > > 3. FAN_MARK_MOUNT(/C) > > > > > > > > let's say I now do > > > > > > > > touch /C/bla > > > > > > > > I may be way off here but intuitively it seems both 1. and 2. should get > > > > a creation event but not 3., right? > > > > > > > > > > Why not 3? > > > You explicitly set a mark on /C requesting to be notified when > > > objects are created via /C. > > > > Sorry, that was a typo. I meant to write, both 2. and 3. should get a > > creation event but not 1. > > > > > > > > > But with your proposal would both 1. and 2. still get a creation event? > > > > > > > > Same obvious typo. The correct question would be: with your proposal do > > 2. and 3. both get an event? > > > > Because it feels like they both should since /C is mounted on top of /B > > and ecryptfs acts as a shim. Both FAN_MARK_MOUNT(/B) and > > FAN_MARK_MOUNT(/C) should get a creation event after all both will have > > mnt->mnt_fsnotify_marks set. > > > > Right. > > There are two ways to address this inconsistency: > 1. Change internal callers of vfs_ helpers to use a private mount, > as you yourself suggested for ecryptfs and cachefiles I feel like this is he correct thing to do independently of the fanotify considerations. I think I'll send an RFC for this today or later this week. > 2. Add fsnotify_path_ hooks at caller site - that would be the > correct thing to do for nfsd IMO I do not have an informed opinion yet on nfsd so I simply need to trust you here. :) > > > > > > > They would not get an event, because fsnotify() looks for CREATE event > > > subscribers on inode->i_fsnotify_marks and inode->i_sb_s_fsnotify_marks > > > and does not find any. > > > > Well yes, but my example has FAN_MARK_MOUNT(/B) set. So fanotify > > _should_ look at > > (!mnt || !mnt->mnt_fsnotify_marks) && > > and see that there are subscribers and should notify the subscribers in > > /B even if the file is created through /C. > > > > My point is with your solution this can't be handled and I want to make > > sure that this is ok. Because right now you'd not be notified about a > > new file having been created in /B even though mnt->mnt_fsnotify_marks > > is set and the creation went through /B via /C. > > > > If you are referring to the ecryptfs use case specifically, then I think it is > ok. After all, whether ecryptfs uses a private mount clone or not is not > something the user can know. > > > _Unless_ we switch to an argument like overlayfs and say "This is a > > private mount which is opaque and so we don't need to generate events.". > > Overlayfs handles this cleanly due to clone_private_mount() which will > > shed all mnt->mnt_fsnotify_marks and ecryptfs should too if that is the > > argument we follow, no? > > > > There is simply no way that the user can infer from the documentation > of FAN_MARK_MOUNT that the event on /B is expected when /B is > underlying layer of ecryptfs or overlayfs. > It requires deep internal knowledge of the stacked fs implementation. > In best case, the user can infer that she MAY get an event on /B. > Some users MAY also expect to get an event on /A because they do not > understand the concept of bind mounts... > Clone a mount ns and you will get more lost users... I disagree to some extent. For example, a user might remount /B read-only at which point /C is effectively read-only too which makes it plain obvious to the user that /C piggy-backs on /B. But leaving that aside my questioning is more concerned with whether the implementation we're aiming for is consistent and intuitive and that stacking example came to my mind pretty quickly. > > > > > > > The vfs_create() -> fsnotify_create() hook passes data_type inode to > > > fsnotify() so there is no fsnotify_data_path() to extract mnt event > > > subscribers from. > > > > Right, that was my point. You don't have the mnt context for the > > underlying fs at a time when e.g. call vfs_link() which ultimately calls > > fsnotify_create/link() which I'm saying might be a problem. > > > > It's a problem. If it wasn't a problem I wouldn't need to work around it ;-) > > It would be a problem if people think that the FAN_MOUNT_MARK > is a subtree mark, which it certainly is not. And I have no doubt that I don't think subtree marks figure into the example above. But we digress. > as Jan said, people really do want a subtree mark. > > My question to you with this RFC is: Does the ability to subscribe to > CREATE/DELETE/MOVE events on a mount help any of your use > cases? With or without the property that mount marks are allowed Since I explicitly pointed on in a prior mail that it would be great to have the same events as for a regular fanotify watch I think I already answered that question. :) > inside userns for idmapped mounts. But if it helps then I'll do it once: yes, both would indeed be very useful. > > Note that if we think the semantics of this are useful for container > managers, but too complex for most mortals, we may decide to > restrict the ability to subscribe to those events to idmapped mounts(?). I don't think it's too complex for most users. But supervisors and container managers are prime users of a feature like this. Christian
