Re: [PATCH v15 3/7] btrfs: add send stream v2 definitions

[David Sterba <dsterba@suse.cz>]

On Wed, May 18, 2022 at 03:25:34PM -0700, Omar Sandoval wrote:
> On Wed, May 18, 2022 at 11:00:03PM +0200, David Sterba wrote:
> > On Mon, Apr 04, 2022 at 10:29:05AM -0700, Omar Sandoval wrote:
> > > @@ -80,16 +84,20 @@ enum btrfs_send_cmd {
> > >  	BTRFS_SEND_C_MAX_V1 = 22,
> > >  
> > >  	/* Version 2 */
> > > -	BTRFS_SEND_C_MAX_V2 = 22,
> > > +	BTRFS_SEND_C_FALLOCATE = 23,
> > > +	BTRFS_SEND_C_SETFLAGS = 24,
> > 
> > Do you have patches that implement the fallocate modes and setflags? I
> > don't see it in this patchset.
> 
> Nope, as discussed before, in order to keep the patch series managable,
> this series adds the definitions and receive support for fallocate and
> setflags, but leaves the send side to be implemented at a later time.
> 
> I implemented fallocate for send back in 2019:
> https://github.com/osandov/linux/commits/btrfs-send-v2. It passed some
> basic testing back then, but it'd need a big rebase and more testing.

The patches in the branch are partially cleanups and preparatory work,
so at least avoiding sending the holes would be nice to have for v2 as
it was one of the first bugs reported. The falllocate modes seem to be
easy. The rest is about the versioning infrastructure that we already
have merged.

> > The setflags should be switched to
> > something closer to the recent refactoring that unifies all the
> > flags/attrs to fileattr. I have a prototype patch for that, comparing
> > the inode flags in the same way as file mode, the tricky part is on the
> > receive side how to apply them correctly. On the sending side it's
> > simple though.
> 
> The way this series documents (and implements in receive)
> BTRFS_SEND_C_SETFLAGS is that it's a simple call to FS_IOC_SETFLAGS with
> given flags. I don't think this is affected by the change to fileattr,
> unless I'm misunderstanding.

The SETFLAGS ioctls are obsolete and I don't want to make them part of
the protocol defition because the bit namespace contains flags we don't
have implemented or are not releated to anything in btrfs.

https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/fs.h#L220

It's basically just naming and specifying what exactly is the value so
we should pick the most recent interface name that superseded SETFLAGS
and the XFLAGS.

> This is in line with the other commands being straightforward system
> calls, but it does mean that the sending side has to deal with the
> complexities of an immutable or append-only file being modified between
> incremental sends (by temporarily clearing the flag), and of inherited
> flags (e.g., a COW file inside of a NOCOW directory).

Yeah the receiving side needs to understand the constraints of the
flags, it has only the information about the final state and not the
order in which the flags get applied.

> I suppose it'd
> also be possible to have SETFLAGS define the final flags and leave it up
> to receive to make that happen by temporarily setting/clearing flags as
> necessary, but that is a bit inconsistent with how we've handled other
> commands.

I'm not sure we can always stick to 1:1 mapping to syscalls or ioctls,
of course it's the best option, but the protocol can transfer eg.
more complete information and it's up to the receiving side to apply it
(like if a file has NODATASUM flag set).

From the other side there are multiple actions for something that could
be just one, like creating file first as an orphan and then renaming it.
So I'd like to look at it from the protocol perspective and not
necessarily blindly copy the OS interfaces.