On Mon, 3 Jun 2024 at 17:28, Stefan Hajnoczi <stefanha@xxxxxxxxxx> wrote: > > On Mon, Jun 03, 2024 at 04:56:14PM +0200, Miklos Szeredi wrote: > > On Mon, Jun 3, 2024 at 3:44 PM Stefan Hajnoczi <stefanha@xxxxxxxxxx> wrote: > > > > > > On Mon, Jun 03, 2024 at 11:06:19AM +0200, Miklos Szeredi wrote: > > > > On Mon, 3 Jun 2024 at 10:53, Peter-Jan Gootzen <pgootzen@xxxxxxxxxx> wrote: > > > > > > > > > We also considered this idea, it would kind of be like locking FUSE into > > > > > being x86. However I think this is not backwards compatible. Currently > > > > > an ARM64 client and ARM64 server work just fine. But making such a > > > > > change would break if the client has the new driver version and the > > > > > server is not updated to know that it should interpret x86 specifically. > > > > > > > > This would need to be negotiated, of course. > > > > > > > > But it's certainly simpler to just indicate the client arch in the > > > > INIT request. Let's go with that for now. > > > > > > In the long term it would be cleanest to choose a single canonical > > > format instead of requiring drivers and devices to implement many > > > arch-specific formats. I liked the single canonical format idea you > > > suggested. > > > > > > My only concern is whether there are more commands/fields in FUSE that > > > operate in an arch-specific way (e.g. ioctl)? If there really are parts > > > that need to be arch-specific, then it might be necessary to negotiate > > > an architecture after all. > > > > How about something like this: > > > > - by default fall back to no translation for backward compatibility > > - server may request matching by sending its own arch identifier in > > fuse_init_in > > - client sends back its arch identifier in fuse_init_out > > - client also sends back a flag indicating whether it will transform > > to canonical or not > > > > This means the client does not have to implement translation for every > > architecture, only ones which are frequently used as guest. The > > server may opt to implement its own translation if it's lacking in the > > client, or it can just fail. > > From the client perspective: > > 1. Do not negotiate arch in fuse_init_out - hopefully client and server > know what they are doing :). This is the current behavior. > 2. Reply to fuse_init_in with server's arch in fuse_init_out - client > translates according to server's arch. > 3. Reply to fuse_init_in with canonical flag set in fuse_init_out - > client and server use canonical format. > > From the server perspective: > > 1. Client does not negotiate arch - the current behavior (good luck!). > 2. Arch received in fuse_init_out from client - must be equal to > server's arch since there is no way for the server to reject the > arch. > 3. Canonical flag received in fuse_init_out from client - client and > server use canonical format. Yeah, something like that (I got the direction of the negotiation wrong). See below patch. I'm thinking that fuse_init_out need not even have the server arch, since the client will only be translating to the canonical arch. The client sends its own arch in fuse_init_in.arch_id and advertises with FUSE_CANON_ARCH set in fuse_init_in.flags whether it supports transforming to canonical. If the server wants canonicalization, then it responds with FUSE_CANON_ARCH set in fuse_init_out.flags. This works for legacy server that doesn't interpret the new flag and field, and also for legacy client that doesn't set either (zero arch_id means: unknown architecture). arch_id could be a hash of the arch name, so that the fuse header file doesn't need to be updated whenever a new architecture is added. Thanks, Miklos diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h index d08b99d60f6f..c63d8bab2d37 100644 --- a/include/uapi/linux/fuse.h +++ b/include/uapi/linux/fuse.h @@ -421,6 +421,7 @@ struct fuse_file_lock { * FUSE_NO_EXPORT_SUPPORT: explicitly disable export support * FUSE_HAS_RESEND: kernel supports resending pending requests, and the high bit * FUSE_NO_EXPORT_SUPPORT: explicitly disable export support * FUSE_HAS_RESEND: kernel supports resending pending requests, and the high bit * of the request ID indicates resend requests + * FUSE_CANON_ARCH: translate arch specific constants to canonical */ #define FUSE_ASYNC_READ (1 << 0) #define FUSE_POSIX_LOCKS (1 << 1) @@ -463,6 +464,7 @@ struct fuse_file_lock { #define FUSE_PASSTHROUGH (1ULL << 37) #define FUSE_NO_EXPORT_SUPPORT (1ULL << 38) #define FUSE_HAS_RESEND (1ULL << 39) +#define FUSE_CANON_ARCH (1ULL << 40) /* Obsolete alias for FUSE_DIRECT_IO_ALLOW_MMAP */ #define FUSE_DIRECT_IO_RELAX FUSE_DIRECT_IO_ALLOW_MMAP @@ -874,7 +876,8 @@ struct fuse_init_in { uint32_t max_readahead; uint32_t flags; uint32_t flags2; - uint32_t unused[11]; + uint32_t arch_id; + uint32_t unused[10]; }; #define FUSE_COMPAT_INIT_OUT_SIZE 8