Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3;
MIME-Version: 1.0
References: <20231026100157.735d7dee@canb.auug.org.au>
In-Reply-To: <20231026100157.735d7dee@canb.auug.org.au>
From:   Amir Goldstein <amir73il@gmail.com>
Date:   Thu, 26 Oct 2023 08:16:14 +0300
Message-ID: <CAOQ4uxjmRena4AB3yMQhBJ58c6DRtkDJJrnTgFe=gWsadSdbQw@mail.gmail.com>
Subject: Re: linux-next: manual merge of the vfs-brauner tree with the
 bcachefs tree
To:     Stephen Rothwell <sfr@canb.auug.org.au>
Cc:     Christian Brauner <brauner@kernel.org>,
        Kent Overstreet <kent.overstreet@linux.dev>,
        Kent Overstreet <kent.overstreet@gmail.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Linux Next Mailing List <linux-next@vger.kernel.org>,
        Jeff Layton <jlayton@kernel.org>,
        Chuck Lever <chuck.lever@oracle.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Oct 26, 2023 at 2:02=E2=80=AFAM Stephen Rothwell <sfr@canb.auug.org=
.au> wrote:
>
> Hi all,
>
> Today's linux-next merge of the vfs-brauner tree got a conflict in:
>
>   include/linux/exportfs.h
>
> between commit:
>
>   85e95ca7cc48 ("bcachefs: Update export_operations for snapshots")
>
> from the bcachefs tree and commit:
>
>   2560fa66d2ac ("exportfs: define FILEID_INO64_GEN* file handle types")
>
> from the vfs-brauner tree.
>
> I fixed it up (see below) and can carry the fix as necessary. This
> is now fixed as far as linux-next is concerned, but any non trivial
> conflicts should be mentioned to your upstream maintainer when your tree
> is submitted for merging.  You may also want to consider cooperating
> with the maintainer of the conflicting tree to minimise any particularly
> complex conflicts.
>
> --
> Cheers,
> Stephen Rothwell

[adding exportfs maintainers]

>
> diff --cc include/linux/exportfs.h
> index be9900cc8786,21bae8bfeef1..000000000000
> --- a/include/linux/exportfs.h
> +++ b/include/linux/exportfs.h
> @@@ -98,12 -98,17 +98,23 @@@ enum fid_type
>          */
>         FILEID_FAT_WITH_PARENT =3D 0x72,
>
>  +      /*
>  +       * 64 bit inode number, 32 bit subvolume, 32 bit generation numbe=
r:
>  +       */
>  +      FILEID_BCACHEFS_WITHOUT_PARENT =3D 0x80,
>  +      FILEID_BCACHEFS_WITH_PARENT =3D 0x81,
>  +
> +       /*
> +        * 64 bit inode number, 32 bit generation number.
> +        */
>  -      FILEID_INO64_GEN =3D 0x81,
> ++      FILEID_INO64_GEN =3D 0x82,
> +
> +       /*
> +        * 64 bit inode number, 32 bit generation number,
> +        * 64 bit parent inode number, 32 bit parent generation.
> +        */
>  -      FILEID_INO64_GEN_PARENT =3D 0x82,
> ++      FILEID_INO64_GEN_PARENT =3D 0x83,
> +

This is wrong.
Those are filesystem defined constants.
Please don't change them.

0x81/0x82 have been used by xfs and fuse for years,
even though neither defined a constant in this enum so far.

Conflicting with FILEID_BCACHEFS_WITH_PARENT is not
a serious issue, but I encourage Kent to pick different constants
for bcachefs or keep the bcachefs constants out of this enum.

It is a slight inconvenience for users that have bcachefs exported
to NFS clients and upgrade their server, but maybe that is acceptable.
In overlayfs, we encoded type OVL_FILEID_V0 and switched to encoding
type OVL_FILEID_V1, but we still accept decoding of both types, neither
of which are listed in this enum BTW.

Adding fid types to this enum is not required.
This enum is a place to standardize and for different fs to share the same
fid type/encoding as is the case with  FILEID_INO{32,64}_GEN*.
IMO, the bcachefs constant do not follow the convention in this
enum and their format is unlikely to be used by other fs, so
they should not be added to this enum at all.

Thanks,
Amir.