Re: [PATCH v2 3/3] fuse: add COPY_FILE_RANGE_64 that allows large copies

[Joanne Koong <joannelkoong@gmail.com>]

On Thu, Aug 14, 2025 at 10:05 AM Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Wed, Aug 13, 2025 at 10:03:17AM -0700, Joanne Koong wrote:
> > On Wed, Aug 13, 2025 at 8:24 AM Miklos Szeredi <mszeredi@redhat.com> wrote:
> > >
> > > The FUSE protocol uses struct fuse_write_out to convey the return value of
> > > copy_file_range, which is restricted to uint32_t.  But the COPY_FILE_RANGE
> > > interface supports a 64-bit size copies and there's no reason why copies
> > > should be limited to 32-bit.
> > >
> > > Introduce a new op COPY_FILE_RANGE_64, which is identical, except the
> > > number of bytes copied is returned in a 64-bit value.
> > >
> > > If the fuse server does not support COPY_FILE_RANGE_64, fall back to
> > > COPY_FILE_RANGE.
> >
> > Is it unacceptable to add a union in struct fuse_write_out that
> > accepts a uint64_t bytes_copied?
> > struct fuse_write_out {
> >     union {
> >         struct {
> >             uint32_t size;
> >             uint32_t padding;
> >         };
> >         uint64_t bytes_copied;
> >     };
> > };
> >
> > Maybe a little ugly but that seems backwards-compatible to me and
> > would prevent needing a new FUSE_COPY_FILE_RANGE64.
>
> I wonder, does fuse_args::out_argvar==1 imply that you could create a
> new 64-bit fuse_write64_out:
>
> struct fuse_write64_out {
>         uint64_t size;
>         uint64_t padding;
> };
>
> and then fuse_copy_file_range declares a union:
>
> union fuse_cfr_out {
>         struct fuse_write_out out;
>         struct fuse_write64_out out64;
> };
>
> passes that into fuse_args:
>
>         union fuse_cfr_out outarg;
>
>         args.out_argvar = 1;
>         args.out_numargs = 1;
>         args.out_args[0].size = sizeof(outarg);
>         args.out_args[0].value = &outarg;
>
> and then we can switch on the results:
>
>         if (args.out_args[0].size == sizeof(fuse_write64_out))
>                 /* 64-bit return */
>         else if (args.out_args[0].size == sizeof(fuse_write_out))
>                 /* 32-bit return */
>         else
>                 /* error */
>
> I guess the problem is that userspace has to know that the kernel will
> accept a fuse_write64_out, because on an old kernel it'll get -EINVAL
> and ... then what?  I think an error return ends the request and the
> fuse server can't just try again with fuse_write_out.
>

I think this would also need the feature flag sent in the init call
which Miklos didn't like


> <shrug> Maybe I'm speculating stupi^Wwildly. ;)
>
> --D
>