vfs: Generate IN_MODIFY on open(O_TRUNC) for existing files#12854
vfs: Generate IN_MODIFY on open(O_TRUNC) for existing files#12854tanyifeng wants to merge 1 commit intogoogle:masterfrom
Conversation
Linux's handle_truncate() emits IN_MODIFY before IN_OPEN when an existing file is opened with O_TRUNC. gVisor only generated IN_OPEN, breaking programs like syncthing that rely on inotify to detect file truncation via open(). Add a created flag (analogous to FMODE_CREATED in linux) to FileDescription so OpenAt can distinguish new files from existing ones, and emit IN_MODIFY only for existing files. Reproducer: open an existing file with O_WRONLY|O_TRUNC while watching with inotify — gVisor missed the IN_MODIFY event. Signed-off-by: Tan Yifeng <yiftan@tencent.com>
| // FMODE_CREATED and is used by VirtualFilesystem.OpenAt() to avoid | ||
| // generating a spurious IN_MODIFY inotify event for O_TRUNC on newly | ||
| // created (empty) files. | ||
| created bool |
There was a problem hiding this comment.
Can this be moved next to the readable/writeable bool fields above?
I'm not 100% sure but I'd imagine this would allow for tighter bitpacking for this struct.
There was a problem hiding this comment.
I think you should rename this to fmodeFlags (which is a mask in Linux containing FMODE_CREATED and other FMODE_* flags) and move it below statusFlags above. This also needs synchronization. You can have flagsMu protect it. Subsequently update SetCreated() and IsCreated() to take the lock and add/remove FMODE_CREATED from this mask. This will make it easier to add support for more fmode flags in the future as well.
|
Do we need to update other openat functions as well? For example:https://github.com/google/gvisor/blob/master/pkg/sentry/fsimpl/erofs/filesystem.go#L219 |
| parent.inode.Watches().Notify(ctx, pc, linux.IN_CREATE, 0, vfs.PathEvent, false /* unlinked */) | ||
| fd, err := child.inode.Open(ctx, rp, &child, opts) | ||
| child.DecRef(ctx) | ||
| if fd != nil { | ||
| fd.SetCreated() | ||
| } |
There was a problem hiding this comment.
Move the IN_CREATE notification below child.inode.Open() and only do it if FD open succeeds. This is what we do in other filesystems as well. Seems like kernfs has a bug here:
fd, err := child.inode.Open(ctx, rp, &child, opts)
child.DecRef(ctx)
if err != nil {
return nil, err
}
parent.inode.Watches().Notify(ctx, pc, linux.IN_CREATE, 0, vfs.PathEvent, false /* unlinked */)
fd.SetCreated()
return fd, nil
@aaltinaydev EROFS is a read-only filesystem and does not support writable operations like O_TRUNC. |
Linux's handle_truncate() emits IN_MODIFY before IN_OPEN when an existing file is opened with O_TRUNC. gVisor only generated IN_OPEN, breaking programs like syncthing that rely on inotify to detect file truncation via open().
Add a created flag (analogous to FMODE_CREATED in linux) to FileDescription so OpenAt can distinguish new files from existing ones, and emit IN_MODIFY only for existing files.
Reproducer: open an existing file with O_WRONLY|O_TRUNC while watching with inotify — gVisor missed the IN_MODIFY event.