[PATCH v3 0/3] Add missing flags in the kernel.

[PATCH v3 0/3] Add missing flags in the kernel.

[Anand Jain]

v2->v3:
Modify BTRFS_SUPER_FLAG_SUPP so that it would fail instead of warn.

v1->v2:
Fail to mount if BTRFS_SUPER_FLAG_CHANGING_FSID is set.

Following bits are already used in user land, so bring it to the kernel
as well.

#define BTRFS_SUPER_FLAG_METADUMP_V2   (1ULL << 34)
#define BTRFS_SUPER_FLAG_CHANGING_FSID (1ULL << 35)

Anand Jain (3):
  btrfs: fail mount when sb flag is not in BTRFS_SUPER_FLAG_SUPP
  btrfs: define SUPER_FLAG_METADUMP_V2
  btrfs: add support for SUPER_FLAG_CHANGING_FSID in btrfs.ko

 fs/btrfs/disk-io.c              | 6 ++++--
 include/uapi/linux/btrfs_tree.h | 2 ++
 2 files changed, 6 insertions(+), 2 deletions(-)

-- 
2.15.0

[PATCH 1/3] btrfs: fail mount when sb flag is not in BTRFS_SUPER_FLAG_SUPP

[Anand Jain]

It appear from the original commit [1] that there isn't any design
specific reason not to fail the mount instead of just warning. This
patch will change it to fail.

[1]
 commit 319e4d0661e5323c9f9945f0f8fb5905e5fe74c3
    btrfs: Enhance super validation check

Signed-off-by: Anand Jain <anand.jain@oracle.com>
  cc: wqu@suse.com
---
 fs/btrfs/disk-io.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index a69e5944dc08..b04da740a9a2 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3906,9 +3906,11 @@ static int btrfs_check_super_valid(struct btrfs_fs_info *fs_info)
 		btrfs_err(fs_info, "no valid FS found");
 		ret = -EINVAL;
 	}
-	if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP)
-		btrfs_warn(fs_info, "unrecognized super flag: %llu",
+	if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP) {
+		btrfs_err(fs_info, "unrecognized or unsupported super flag: %llu",
 				btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP);
+		ret = -EINVAL;
+	}
 	if (btrfs_super_root_level(sb) >= BTRFS_MAX_LEVEL) {
 		btrfs_err(fs_info, "tree_root level too big: %d >= %d",
 				btrfs_super_root_level(sb), BTRFS_MAX_LEVEL);
-- 
2.15.0

[PATCH 2/3] btrfs: define SUPER_FLAG_METADUMP_V2

[Anand Jain]

btrfs-progs uses super flag bit BTRFS_SUPER_FLAG_METADUMP_V2 (1ULL << 34).
So just define that in kernel so that we know its been used. This patch
does not add it to BTRFS_SUPER_FLAG_SUPP yet.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 include/uapi/linux/btrfs_tree.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/btrfs_tree.h b/include/uapi/linux/btrfs_tree.h
index 6d6e5da51527..38ab0e06259a 100644
--- a/include/uapi/linux/btrfs_tree.h
+++ b/include/uapi/linux/btrfs_tree.h
@@ -456,6 +456,7 @@ struct btrfs_free_space_header {
 
 #define BTRFS_SUPER_FLAG_SEEDING	(1ULL << 32)
 #define BTRFS_SUPER_FLAG_METADUMP	(1ULL << 33)
+#define BTRFS_SUPER_FLAG_METADUMP_V2	(1ULL << 34)
 
 
 /*
-- 
2.15.0

[PATCH 3/3] btrfs: add support for SUPER_FLAG_CHANGING_FSID in btrfs.ko

[Anand Jain]

Userland sets SUPER_FLAG_CHANGING_FSID and resets it only when changing
fsid is complete. Its not a good idea to mount the device anything in
between, so this patch fails the mount if SB SUPER_FLAG_CHANGING_FSID
is set. As we don't add SUPER_FLAG_CHANGING_FSID into
BTRFS_SUPER_FLAG_SUPP list, so mount will fail if SUPER_FLAG_CHANGING_FSID
is set in the user land.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
cc: wqu@suse.com
---
 include/uapi/linux/btrfs_tree.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/btrfs_tree.h b/include/uapi/linux/btrfs_tree.h
index 38ab0e06259a..aff1356c2bb8 100644
--- a/include/uapi/linux/btrfs_tree.h
+++ b/include/uapi/linux/btrfs_tree.h
@@ -457,6 +457,7 @@ struct btrfs_free_space_header {
 #define BTRFS_SUPER_FLAG_SEEDING	(1ULL << 32)
 #define BTRFS_SUPER_FLAG_METADUMP	(1ULL << 33)
 #define BTRFS_SUPER_FLAG_METADUMP_V2	(1ULL << 34)
+#define BTRFS_SUPER_FLAG_CHANGING_FSID	(1ULL << 35)
 
 
 /*
-- 
2.15.0

Re: [PATCH 1/3] btrfs: fail mount when sb flag is not in BTRFS_SUPER_FLAG_SUPP

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 1538 bytes --]



On 2018年01月08日 16:04, Anand Jain wrote:
> It appear from the original commit [1] that there isn't any design
> specific reason not to fail the mount instead of just warning. This
> patch will change it to fail.
> 
> [1]
>  commit 319e4d0661e5323c9f9945f0f8fb5905e5fe74c3
>     btrfs: Enhance super validation check
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>   cc: wqu@suse.com

The patch itself is good.

But it should be the last patch, to avoid breaking bisect.

With that fixed, feel free to add my tag:

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  fs/btrfs/disk-io.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index a69e5944dc08..b04da740a9a2 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -3906,9 +3906,11 @@ static int btrfs_check_super_valid(struct btrfs_fs_info *fs_info)
>  		btrfs_err(fs_info, "no valid FS found");
>  		ret = -EINVAL;
>  	}
> -	if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP)
> -		btrfs_warn(fs_info, "unrecognized super flag: %llu",
> +	if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP) {
> +		btrfs_err(fs_info, "unrecognized or unsupported super flag: %llu",
>  				btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP);
> +		ret = -EINVAL;
> +	}
>  	if (btrfs_super_root_level(sb) >= BTRFS_MAX_LEVEL) {
>  		btrfs_err(fs_info, "tree_root level too big: %d >= %d",
>  				btrfs_super_root_level(sb), BTRFS_MAX_LEVEL);
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]

Re: [PATCH 2/3] btrfs: define SUPER_FLAG_METADUMP_V2

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 1108 bytes --]



On 2018年01月08日 16:04, Anand Jain wrote:
> btrfs-progs uses super flag bit BTRFS_SUPER_FLAG_METADUMP_V2 (1ULL << 34).
> So just define that in kernel so that we know its been used. This patch
> does not add it to BTRFS_SUPER_FLAG_SUPP yet.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  include/uapi/linux/btrfs_tree.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/uapi/linux/btrfs_tree.h b/include/uapi/linux/btrfs_tree.h
> index 6d6e5da51527..38ab0e06259a 100644
> --- a/include/uapi/linux/btrfs_tree.h
> +++ b/include/uapi/linux/btrfs_tree.h
> @@ -456,6 +456,7 @@ struct btrfs_free_space_header {
>  
>  #define BTRFS_SUPER_FLAG_SEEDING	(1ULL << 32)
>  #define BTRFS_SUPER_FLAG_METADUMP	(1ULL << 33)
> +#define BTRFS_SUPER_FLAG_METADUMP_V2	(1ULL << 34)

Please also include this to BTRFS_SUPER_FLAG_SUPP.

It's quite common developers (at least myself) may try to mount a
recovered btrfs image.

(That's also the reason why the patch should be put before the
BTRFS_SUPER_FLAG_SUPP check patch)

Thanks,
Qu
>  
>  
>  /*
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]

Re: [PATCH 3/3] btrfs: add support for SUPER_FLAG_CHANGING_FSID in btrfs.ko

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 1155 bytes --]



On 2018年01月08日 16:04, Anand Jain wrote:
> Userland sets SUPER_FLAG_CHANGING_FSID and resets it only when changing
> fsid is complete. Its not a good idea to mount the device anything in
> between, so this patch fails the mount if SB SUPER_FLAG_CHANGING_FSID
> is set. As we don't add SUPER_FLAG_CHANGING_FSID into
> BTRFS_SUPER_FLAG_SUPP list, so mount will fail if SUPER_FLAG_CHANGING_FSID
> is set in the user land.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> cc: wqu@suse.com

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  include/uapi/linux/btrfs_tree.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/uapi/linux/btrfs_tree.h b/include/uapi/linux/btrfs_tree.h
> index 38ab0e06259a..aff1356c2bb8 100644
> --- a/include/uapi/linux/btrfs_tree.h
> +++ b/include/uapi/linux/btrfs_tree.h
> @@ -457,6 +457,7 @@ struct btrfs_free_space_header {
>  #define BTRFS_SUPER_FLAG_SEEDING	(1ULL << 32)
>  #define BTRFS_SUPER_FLAG_METADUMP	(1ULL << 33)
>  #define BTRFS_SUPER_FLAG_METADUMP_V2	(1ULL << 34)
> +#define BTRFS_SUPER_FLAG_CHANGING_FSID	(1ULL << 35)
>  
>  
>  /*
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]

Re: [PATCH 2/3] btrfs: define SUPER_FLAG_METADUMP_V2

[David Sterba]

On Mon, Jan 08, 2018 at 04:20:58PM +0800, Qu Wenruo wrote:
> 
> 
> On 2018年01月08日 16:04, Anand Jain wrote:
> > btrfs-progs uses super flag bit BTRFS_SUPER_FLAG_METADUMP_V2 (1ULL << 34).
> > So just define that in kernel so that we know its been used. This patch
> > does not add it to BTRFS_SUPER_FLAG_SUPP yet.
> > 
> > Signed-off-by: Anand Jain <anand.jain@oracle.com>
> > ---
> >  include/uapi/linux/btrfs_tree.h | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/include/uapi/linux/btrfs_tree.h b/include/uapi/linux/btrfs_tree.h
> > index 6d6e5da51527..38ab0e06259a 100644
> > --- a/include/uapi/linux/btrfs_tree.h
> > +++ b/include/uapi/linux/btrfs_tree.h
> > @@ -456,6 +456,7 @@ struct btrfs_free_space_header {
> >  
> >  #define BTRFS_SUPER_FLAG_SEEDING	(1ULL << 32)
> >  #define BTRFS_SUPER_FLAG_METADUMP	(1ULL << 33)
> > +#define BTRFS_SUPER_FLAG_METADUMP_V2	(1ULL << 34)
> 
> Please also include this to BTRFS_SUPER_FLAG_SUPP.
> 
> It's quite common developers (at least myself) may try to mount a
> recovered btrfs image.
> 
> (That's also the reason why the patch should be put before the
> BTRFS_SUPER_FLAG_SUPP check patch)

Agreed.