Re: [RFC PATCH 03/24] erofs: add Errno in Rust

[Gao Xiang <hsiangkao@linux.alibaba.com>]

Hi Gary,

On 2024/9/17 04:01, Gary Guo wrote:
> On Mon, 16 Sep 2024 21:56:13 +0800
> Yiyang Wu <toolmanp@tlmp.cc> wrote:
> 
>> Introduce Errno to Rust side code. Note that in current Rust For Linux,
>> Errnos are implemented as core::ffi::c_uint unit structs.
>> However, EUCLEAN, a.k.a EFSCORRUPTED is missing from error crate.
>>
>> Since the errno_base hasn't changed for over 13 years,
>> This patch merely serves as a temporary workaround for the missing
>> errno in the Rust For Linux.
>>
>> Signed-off-by: Yiyang Wu <toolmanp@tlmp.cc>
> 
> As Greg said, please add missing errno that you need to kernel crate
> instead.

I've answered Greg about this in another email.

> 
> Also, it seems that you're building abstractions into EROFS directly
> without building a generic abstraction. We have been avoiding that. If
> there's an abstraction that you need and missing, please add that
> abstraction. In fact, there're a bunch of people trying to add FS

No, I'd like to try to replace some EROFS C logic first to Rust (by
using EROFS C API interfaces) and try if Rust is really useful for
a real in-tree filesystem.  If Rust can improve EROFS security or
performance (although I'm sceptical on performance), As an EROFS
maintainer, I'm totally fine to accept EROFS Rust logic landed to
help the whole filesystem better.

For Rust VFS abstraction, that is a different and indepenent story,
Yiyang don't have any bandwidth on this due to his limited time.
And I _also_ don't think an incomplete ROFS VFS Rust abstraction
is useful to Linux community (because IMO for generic interface
design, we need a global vision for all filesystems instead of
just ROFSes.  No existing user is not an excuse for an incomplete
abstraction.)

If a reasonble Rust VFS abstraction landed, I think we will switch
to use that, but as I said, they are completely two stories.

> support, please coordinate instead of rolling your own.
> 
> You also have been referencing `kernel::bindings::` directly in various
> places in the patch series. The module is marked as `#[doc(hidden)]`
> for a reason -- it's not supposed to referenced directly. It's only
> exposed so that macros can reference them. In fact, we have a policy
> that direct reference to raw bindings are not allowed from drivers.

This patch can be avoided if EUCLEAN is added to errno.

Thanks,
Gao Xiang