Re: [PATCH v4] btrfs: send: add support for fs-verity

[Boris Burkov <boris@bur.io>]

On Thu, Aug 18, 2022 at 07:32:54PM +0200, David Sterba wrote:
> On Mon, Aug 15, 2022 at 01:54:28PM -0700, Boris Burkov wrote:
> > Preserve the fs-verity status of a btrfs file across send/recv.
> > 
> > There is no facility for installing the Merkle tree contents directly on
> > the receiving filesystem, so we package up the parameters used to enable
> > verity found in the verity descriptor. This gives the receive side
> > enough information to properly enable verity again. Note that this means
> > that receive will have to re-compute the whole Merkle tree, similar to
> > how compression worked before encoded_write.
> > 
> > Since the file becomes read-only after verity is enabled, it is
> > important that verity is added to the send stream after any file writes.
> > Therefore, when we process a verity item, merely note that it happened,
> > then actually create the command in the send stream during
> > 'finish_inode_if_needed'.
> > 
> > This also creates V3 of the send stream format, without any format
> > changes besides adding the new commands and attributes.
> > 
> > Signed-off-by: Boris Burkov <boris@bur.io>
> 
> As for the merge target, a realistic one seems to be 6.2, we have too
> many pending patches everywhere else. There's a todo list for v3 that
> I'd really like to get done.
> 
> To be able to test things incrementally until then we can add v3 support
> under debug config.

That all sounds good and reasonable to me. Would you like me to re-send
with gating V3 behind debug, or will you do it as part of taking it?

Also, this just popped in my head, but could we acheive what we want
with the "--proto" feature of the send CLI, and having a notion of a
provisional version that is not yet hardened and properly named/fixed
for future compatibility? For extra fanciness, we can do sub-versions
or hashes of the commands or something. Maybe proto=(u64)-1 means
experimental.

Anyway, I'm totally fine with putting it behind debug till it's done,
but figured I'd share that thought.

> 
> > --
> > Changes in v4:
> > - Use btrfs_get_verity_descriptor instead of verity ops get descriptor.
> >   Move that definition to ctree.h for conditional building. This cleaned
> >   up most of the conditional building issues, in my opinion.
> 
> Yes, that way it's ok.
> 
> > - Rename process_new_verity to process_verity.
> > - Use le-to-cpu conversion for all fsverity descriptor fields.
> > - Don't check NULL for kvfree of the send descriptor.
> 
> > --- a/fs/btrfs/ctree.h
> > +++ b/fs/btrfs/ctree.h
> > --- a/fs/btrfs/send.c
> > +++ b/fs/btrfs/send.c
> > @@ -3,6 +3,7 @@
> >   * Copyright (C) 2012 Alexander Block.  All rights reserved.
> >   */
> >  
> > +#include "linux/compiler_attributes.h"
> 
> As Eric pointed out, this is not necessary, I'll delete the line, no
> need to resend just for that.