On Wed, Feb 19, 2025 at 12:06 AM Pali Rohár <pali@xxxxxxxxxx> wrote: > > On Wednesday 19 February 2025 09:56:28 Dave Chinner wrote: > > On Tue, Feb 18, 2025 at 08:27:01PM +0100, Pali Rohár wrote: > > > On Tuesday 18 February 2025 10:13:46 Amir Goldstein wrote: > > > > > and there is no need for whacky field > > > > > masks or anything like that. All it needs is a single bit to > > > > > indicate if the windows attributes are supported, and they are all > > > > > implemented as normal FS_XFLAG fields in the fsx_xflags field. > > > > > > > > > > > If MS adds 3 new attributes then we cannot add them to fsx_xflags > > > because all bits of fsx_xflags would be exhausted. > > > > And then we can discuss how to extend the fsxattr structure to > > implement more flags. > > > > In this scenario we'd also need another flag bit to indicate that > > there is a second set of windows attributes that are supported... > > > > i.e. this isn't a problem we need to solve right now. > > Ok, that is possible solution for now. > > > > Just having only one FS_XFLAGS_HAS_WIN_ATTRS flag for determining windows > > > attribute support is not enough, as it would not say anything useful for > > > userspace. > > > > IDGI. > > > > That flag is only needed to tell userspace "this filesystem supports > > windows attributes". Then GET will return the ones that are set, > > and SET will return -EINVAL for those that it can't set (e.g. > > compress, encrypt). What more does userspace actually need? Let me state my opinion clearly. I think this API smells. I do not object to it, but I think we can do better. I do however object to treating different flags in fsx_xflags differently - this is unacceptable IMO. The difference I am referring to is a nuance, but IMO an important one - It's fine for GET to raise a flag "this filesystem does not accept SET of any unknown flags". It's not fine IMO for GET to raise a flag "this filesystem does not accept SET of unknown flags except for a constant subset of flags that filesystem may ignore". It's not fine IMO, because it makes userspace life harder for no good reason. This former still allows filesystems to opt-in one by one to the extended API, but it does not assume anything about the subset of windows attributes and legacy Linux attributes that need to be supported. For example, adding support for set/get hidden/system/archive/readonly fo fs/fat, does not require supporting all FS_XFLAG_COMMON by fs/fat and an attempt to set unsupported FS_XFLAG_COMMON flags would result in -EINVAL just as an attempt to set an unsupported win flag. But if we agree on setting one special flag in GET to say "new SET semantics", I do not understand the objection to fsx_xflags_mask. Dave, if you actually object to fsx_xflags_mask please explain why. "What more does userspace actually need?" is not a good enough reason IMO. Userspace could make use of fsx_xflags_mask. > > Userspace backup utility would like to backup as many attributes as > possible by what is supported by the target filesystem. What would such > utility would do if the target filesystem supports only HIDDEN > attribute, and source file has all windows attributes set? It would be > needed to issue 2*N syscalls in the worst case to set attributes. > It would be combination of GET+SET for every one windows attribute > because userspace does not know what is supported and what not. > > IMHO this is suboptimal. If filesystem would provide API to get list of > supported attributes then this can be done by 2-3 syscalls. I agree that getting the "attributes supported by filesystem" is important and even getting the "gettable" subset and "settable" subset and I also agree with Dave that this could be done once and no need to do it for every file (although different file types may support a different subsets). Let's stop for a moment to talk about statx. I think you should include a statx support path in your series - not later. If anything, statx support for exporting those flags should be done before adding the GET/SET API. Why? because there is nothing controversial about it. - add a bunch of new STATX_ATTR_ flags - filesystems that support them will publish that in stx_attributes_mask - COMPR/ENCRYPT are already exported in statx With that, backup/sync programs are able to query filesystem support even without fsx_xflags_mask. I think this is a hacky way around a proper GET/SET API, but possible. Thanks, Amir.
