Add constructor of `std::fs::Metadata` from `struct statx`

[Darksonn]

Proposal

Problem statement

The only way to construct std::fs::Metadata is to use the std::fs::metadata() method, and possibly other similar stdlib methods. It's impossible to invoke the statx syscall manually and create a stdlib std::fs::Metadata struct from the result.

Motivating examples or use cases

Tokio currently implements tokio::fs::metadata() by calling std::fs::metadata() from a thread-pool. However, we would like to provide an optional implementation that looks up the metadata via io_uring instead. Unfortunately, Tokio has made the design mistake of using std::fs::Metadata as the return value of tokio::fs::metadata(), instead of using our own custom Metadata struct.

However, since there is no way to convert the output of the io_uring statx operation into a std::fs::Metadata value, it's currently impossible for Tokio to provide an io-uring backed tokio::fs::metadata() method without a breaking change.

Solution sketch

Add a method to the Linux MetadataExt trait that takes a *const c_void to a struct populated by statx syscall. This avoids defining the statx struct in the standard library.

There are many variations of this. I want to avoid spending a lot of time on the precise API design until we've discussed whether the stdlib is willing in principle to add such a constructor, assuming a good design can be found.

I acknowledge that you may not want to make stdlib changes to fix what is essentially just a design mistake in a third-party library.

Downsides

If a statx2 syscall or similar is added in the future with additional fields, then Metadata cannot be evolved to unconditionally provide those fields (even on Linux only via MetadataExt), as it may be constructed from a struct statx where that information is not present. On the other hand, we probably need to do that anyway since older Linux versions will not have this new syscall available, so stdlib will have to use the syscall of today on some Linux systems.

Alternatives

Live with the design mistake in Tokio forever and never implement tokio::fs::metadata() in terms of io_uring.

Links and related work

tokio-rs/tokio#7616

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

• We think this problem seems worth solving, and the standard library might be the right place to solve it.
• We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

• We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
• We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.

[max-heller]

One alternative that feels bad but I'll mention regardless would be to add a Default impl (or analogous dummy constructor) and set_*() functions to the MetadataExt traits. This would be a lot of surface area and probably wouldn't produce very efficient code, but it avoids the "type who shall not be named" *const c_void weirdness.

[joshtriplett]

We talked about this in today's @rust-lang/libs-api meeting.

We were generally supportive of adding a Linux-specific conversion method in MetadataExt. However, we'd like to discuss this in this week's Rust for Linux meeting, to check some assumptions about possible evolutions of the statx ABI. In particular, we'd like to know if the statx structure can grow in the future, and if so how, and how we could handle that if we wanted some of the new data if available.

[Darksonn]

Here is one possible API design we discussed in the RfL meeting that should be quite forwards compatible:

/// Opaque statx struct.
pub struct statx {
    inner: libc::statx,
}

impl statx {
    /// Create a statx struct ready to be passed to the statx syscall.
    pub fn new() -> Self { ... }

    /// Returns the `mask` that should be passed to the statx syscall with
    /// this struct.
    pub fn mask(&self) -> c_int { ... }

    /// Converted a filled-in struct to a [`Metadata`].
    ///
    /// This should only be called after the statx syscall has been invoked with
    /// a pointer to this struct, using the mask provided by `mask`. Otherwise,
    /// if this struct contains invalid or incomplete data, this call may panic.
    pub fn into_metadata(self) -> Metadata { ... }
}

This way, the stdlib gets to decide all of:

1. How large should the struct passed to the syscall be?
2. What contents should be pre-filled in the statx syscall?
3. What mask should be passed to the syscall?

We can declare in the API that calling into_metadata() may panic if the provided struct was not constructed by invoking the relevant syscall. This way, if the stdlib wishes, stdlib can detect the kernel version and expect that the kernel provides certain fields always, even if older kernels don't provide that field, and panic if a library manually constructs statx in a way that the current kernel version would not construct it.

We also discussed contacting the fs maintainers and asking them about the API evolution possibilities.

[joshtriplett]

We discussed this in today's @rust-lang/libs-api meeting.

We're happy to see a solution added for this.

We don't want to add the opaque struct statx, because people will want to get at the internal structure (e.g. setting additional mask bits, or reading fields out before converting it). People would end up wanting to transmute, and if we create the struct then we have to manage the size and the memory. (Which might also prevent future clever APIs that use kernel-managed buffers or arrays of statx structures or similar, for instance.)

Instead, we'd like to add the original proposed API: an unsafe MetadataExt method from_statx taking a void pointer. We'd also be interested in adding a constant for "the statx flags std normally requests", so that code wanting to request everything std knows about can use those.

from_statx should be defensive in its future evolution, and not read any future field from the struct if the corresponding future flag isn't set.

We're prepared to rely on the stability of the Linux statx structure. We can bring it up with upstream if it breaks compatibility. In theory, we might one day need a future from_statx2, and that's okay too.

[tgross35]

Live with the design mistake in Tokio forever and never implement tokio::fs::metadata() in terms of io_uring.

To possibly clarify the obvious: couldn't Tokio add a fs::metadata2 function? I'm wondering a bit about the utility of this API beyond Tokio's uses (which are of course very valid).

[Darksonn]

To possibly clarify the obvious: couldn't Tokio add a fs::metadata2 function?

We could, but I don't want to.