On Fri, Aug 21, 2020 at 04:46:44PM +0200, Miklos Szeredi wrote: > On Fri, Jul 24, 2020 at 8:38 PM Vivek Goyal <vgoyal@xxxxxxxxxx> wrote: > > > If you are concerned about regression w.r.t clear of caps, then we > > can think of enabling SB_NOSEC conditionally. Say user chooses it > > as mount option. But given caps is just an outlier and currently > > we clear suid/sgid based on cache (and not based on state on server), > > I feel it might not be a huge issue. > > > > What do you think? > > I think enabling xattr caching should be a separate feature, and yes, > SB_NOSEC would effectively enable xattr caching. > > We could add the FUSE_CACHE_XATTR feature flag without actually adding > real caching, just SB_NOSEC... > > Does that sound sane? Hi Miklos, I found one the old threads on this topic here. https://patchwork.kernel.org/patch/9306393/ In the end you have suggested few solutions to the problem and frankly speaking I think I like following the best one. "Perhaps add a "local fs" mode where we can assume proper consistency between cache and backing." Distributed filesystems are complicated and we need a proper protocol so that file server can tell fuse its a distributed filesystem and also come up with a way to invalidate various cached attributes (depending on cache coherency model). For example, shouldn't file server notify fuse that certain attr got invalidated. (If it detects that another client modified it). Even that will be racy because some other operation might already be making use of stale attribute while we are invalidating it. That's where a better method like delegation or something else will be needed, probably But in case of local fs (ex. non-shared mode of virtiofs), all the cached data should be valid as all changes should go through single fuse instance. If fuse knows that, then probably lot of code can be simplified for this important use case. Including setting SB_NOSEC. To me caching xattr will bring another set of complex considrations about how and when xattrs are invalidated and a lot will depend on what guarantees said distributed filesystem is providing. So I am little vary of going in that direction and make SB_NOSEC conditional on FUSE_CACHE_XATTR. I am afraid that I will just define this flag today without defining rest of the behavior of xattr caching and that will probably break things later. This probably should be done when we are actually implementing xattr caching and keeping distributed filesystems in mind. So how about, we instead implement a flag which tells fuse that file server is implementing a local filesystem and it does not expect anything to changed outside fuse. This will make sure distributed filesystems like gluster don't regress because of this change and a class of local filesystems can gain from this. Once we support sharing mode in virtiofs, then we will need to revisit it again and do it right for distributed filesystems (depending on their invalidation mechanism). Thanks Vivek
