* [PATCH 1/2] btrfs: add missing BTRFS_SUPER_FLAG define
@ 2018-01-08 3:05 Anand Jain
2018-01-08 3:05 ` [PATCH 2/2] btrfs: add support for SUPER_FLAG_CHANGING_FSID in btrfs.ko Anand Jain
2018-01-08 4:52 ` [PATCH 1/2] btrfs: add missing BTRFS_SUPER_FLAG define Qu Wenruo
0 siblings, 2 replies; 12+ messages in thread
From: Anand Jain @ 2018-01-08 3:05 UTC (permalink / raw)
To: linux-btrfs; +Cc: wqu
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, and will continue to warn
about unsupported flag.
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] btrfs: add support for SUPER_FLAG_CHANGING_FSID in btrfs.ko
2018-01-08 3:05 [PATCH 1/2] btrfs: add missing BTRFS_SUPER_FLAG define Anand Jain
@ 2018-01-08 3:05 ` Anand Jain
2018-01-08 4:55 ` Qu Wenruo
2018-01-08 5:04 ` [PATCH v2 1/2] " Anand Jain
2018-01-08 4:52 ` [PATCH 1/2] btrfs: add missing BTRFS_SUPER_FLAG define Qu Wenruo
1 sibling, 2 replies; 12+ messages in thread
From: Anand Jain @ 2018-01-08 3:05 UTC (permalink / raw)
To: linux-btrfs; +Cc: wqu
Userland uses this flag and resets it only when changing the 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.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
cc: wqu@suse.com
---
fs/btrfs/disk-io.c | 7 ++++++-
include/uapi/linux/btrfs_tree.h | 1 +
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index a69e5944dc08..8b76faa77235 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -61,7 +61,8 @@
BTRFS_HEADER_FLAG_RELOC |\
BTRFS_SUPER_FLAG_ERROR |\
BTRFS_SUPER_FLAG_SEEDING |\
- BTRFS_SUPER_FLAG_METADUMP)
+ BTRFS_SUPER_FLAG_METADUMP |\
+ BTRFS_SUPER_FLAG_CHANGING_FSID)
static const struct extent_io_ops btree_extent_io_ops;
static void end_workqueue_fn(struct btrfs_work *work);
@@ -3906,6 +3907,10 @@ 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_CHANGING_FSID) {
+ btrfs_err(fs_info, "SUPER_FLAG_CHANGING_FSID is set");
+ ret = -EINVAL;
+ }
if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP)
btrfs_warn(fs_info, "unrecognized super flag: %llu",
btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP);
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] btrfs: add missing BTRFS_SUPER_FLAG define
2018-01-08 3:05 [PATCH 1/2] btrfs: add missing BTRFS_SUPER_FLAG define Anand Jain
2018-01-08 3:05 ` [PATCH 2/2] btrfs: add support for SUPER_FLAG_CHANGING_FSID in btrfs.ko Anand Jain
@ 2018-01-08 4:52 ` Qu Wenruo
1 sibling, 0 replies; 12+ messages in thread
From: Qu Wenruo @ 2018-01-08 4:52 UTC (permalink / raw)
To: Anand Jain, linux-btrfs; +Cc: wqu
[-- Attachment #1.1: Type: text/plain, Size: 955 bytes --]
On 2018年01月08日 11:05, 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, and will continue to warn
> about unsupported flag.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.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 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)
>
>
> /*
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] btrfs: add support for SUPER_FLAG_CHANGING_FSID in btrfs.ko
2018-01-08 3:05 ` [PATCH 2/2] btrfs: add support for SUPER_FLAG_CHANGING_FSID in btrfs.ko Anand Jain
@ 2018-01-08 4:55 ` Qu Wenruo
2018-01-08 5:05 ` Anand Jain
2018-01-08 5:10 ` Anand Jain
2018-01-08 5:04 ` [PATCH v2 1/2] " Anand Jain
1 sibling, 2 replies; 12+ messages in thread
From: Qu Wenruo @ 2018-01-08 4:55 UTC (permalink / raw)
To: Anand Jain, linux-btrfs; +Cc: wqu
[-- Attachment #1.1: Type: text/plain, Size: 2271 bytes --]
On 2018年01月08日 11:05, Anand Jain wrote:
> Userland uses this flag and resets it only when changing the 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.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> cc: wqu@suse.com
> ---
> fs/btrfs/disk-io.c | 7 ++++++-
> include/uapi/linux/btrfs_tree.h | 1 +
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index a69e5944dc08..8b76faa77235 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -61,7 +61,8 @@
> BTRFS_HEADER_FLAG_RELOC |\
> BTRFS_SUPER_FLAG_ERROR |\
> BTRFS_SUPER_FLAG_SEEDING |\
> - BTRFS_SUPER_FLAG_METADUMP)
> + BTRFS_SUPER_FLAG_METADUMP |\
> + BTRFS_SUPER_FLAG_CHANGING_FSID)
If we don't want the half changed fs to be mounted, it's better not to
include FLAG_CHANGING_FSID into BTRFS_SUPER_FLAG_SUPP.
>
> static const struct extent_io_ops btree_extent_io_ops;
> static void end_workqueue_fn(struct btrfs_work *work);
> @@ -3906,6 +3907,10 @@ 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_CHANGING_FSID) {
This doesn't seems right.
Shouldn't it be "btrfs_super_flags(sb) & BTRFS_SUPER_FLAG_CHANING_FSID"?
Thanks,
Qu
> + btrfs_err(fs_info, "SUPER_FLAG_CHANGING_FSID is set");
> + ret = -EINVAL;
> + }
> if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP)
> btrfs_warn(fs_info, "unrecognized super flag: %llu",
> btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP);
> 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 --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/2] btrfs: add support for SUPER_FLAG_CHANGING_FSID in btrfs.ko
2018-01-08 3:05 ` [PATCH 2/2] btrfs: add support for SUPER_FLAG_CHANGING_FSID in btrfs.ko Anand Jain
2018-01-08 4:55 ` Qu Wenruo
@ 2018-01-08 5:04 ` Anand Jain
2018-01-08 5:08 ` Qu Wenruo
1 sibling, 1 reply; 12+ messages in thread
From: Anand Jain @ 2018-01-08 5:04 UTC (permalink / raw)
To: linux-btrfs; +Cc: wqu
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.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
cc: wqu@suse.com
---
v1->v2: Oops. Fix cut and paste error, remove ~. My bad.
fs/btrfs/disk-io.c | 7 ++++++-
include/uapi/linux/btrfs_tree.h | 1 +
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index a69e5944dc08..0dd215258ff9 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -61,7 +61,8 @@
BTRFS_HEADER_FLAG_RELOC |\
BTRFS_SUPER_FLAG_ERROR |\
BTRFS_SUPER_FLAG_SEEDING |\
- BTRFS_SUPER_FLAG_METADUMP)
+ BTRFS_SUPER_FLAG_METADUMP |\
+ BTRFS_SUPER_FLAG_CHANGING_FSID)
static const struct extent_io_ops btree_extent_io_ops;
static void end_workqueue_fn(struct btrfs_work *work);
@@ -3906,6 +3907,10 @@ 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_CHANGING_FSID) {
+ btrfs_err(fs_info, "SUPER_FLAG_CHANGING_FSID is set");
+ ret = -EINVAL;
+ }
if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP)
btrfs_warn(fs_info, "unrecognized super flag: %llu",
btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP);
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] btrfs: add support for SUPER_FLAG_CHANGING_FSID in btrfs.ko
2018-01-08 4:55 ` Qu Wenruo
@ 2018-01-08 5:05 ` Anand Jain
2018-01-08 5:10 ` Anand Jain
1 sibling, 0 replies; 12+ messages in thread
From: Anand Jain @ 2018-01-08 5:05 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs; +Cc: wqu
On 01/08/2018 12:55 PM, Qu Wenruo wrote:
>
>
> On 2018年01月08日 11:05, Anand Jain wrote:
>> Userland uses this flag and resets it only when changing the 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.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> cc: wqu@suse.com
>> ---
>> fs/btrfs/disk-io.c | 7 ++++++-
>> include/uapi/linux/btrfs_tree.h | 1 +
>> 2 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index a69e5944dc08..8b76faa77235 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -61,7 +61,8 @@
>> BTRFS_HEADER_FLAG_RELOC |\
>> BTRFS_SUPER_FLAG_ERROR |\
>> BTRFS_SUPER_FLAG_SEEDING |\
>> - BTRFS_SUPER_FLAG_METADUMP)
>> + BTRFS_SUPER_FLAG_METADUMP |\
>> + BTRFS_SUPER_FLAG_CHANGING_FSID)
>
> If we don't want the half changed fs to be mounted, it's better not to
> include FLAG_CHANGING_FSID into BTRFS_SUPER_FLAG_SUPP.
>
>>
>> static const struct extent_io_ops btree_extent_io_ops;
>> static void end_workqueue_fn(struct btrfs_work *work);
>> @@ -3906,6 +3907,10 @@ 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_CHANGING_FSID) {
>
> This doesn't seems right.
>
> Shouldn't it be "btrfs_super_flags(sb) & BTRFS_SUPER_FLAG_CHANING_FSID"?
Yep. I just noticed too. Made a mistake cut and paste. Posted v2.
Thanks.
Anand
> Thanks,
> Qu
>
>> + btrfs_err(fs_info, "SUPER_FLAG_CHANGING_FSID is set");
>> + ret = -EINVAL;
>> + }
>> if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP)
>> btrfs_warn(fs_info, "unrecognized super flag: %llu",
>> btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP);
>> 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)
>>
>>
>> /*
>>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] btrfs: add support for SUPER_FLAG_CHANGING_FSID in btrfs.ko
2018-01-08 5:04 ` [PATCH v2 1/2] " Anand Jain
@ 2018-01-08 5:08 ` Qu Wenruo
2018-01-08 5:13 ` Anand Jain
0 siblings, 1 reply; 12+ messages in thread
From: Qu Wenruo @ 2018-01-08 5:08 UTC (permalink / raw)
To: Anand Jain, linux-btrfs; +Cc: wqu
[-- Attachment #1.1: Type: text/plain, Size: 2333 bytes --]
On 2018年01月08日 13: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.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> cc: wqu@suse.com
> ---
> v1->v2: Oops. Fix cut and paste error, remove ~. My bad.
>
> fs/btrfs/disk-io.c | 7 ++++++-
> include/uapi/linux/btrfs_tree.h | 1 +
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index a69e5944dc08..0dd215258ff9 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -61,7 +61,8 @@
> BTRFS_HEADER_FLAG_RELOC |\
> BTRFS_SUPER_FLAG_ERROR |\
> BTRFS_SUPER_FLAG_SEEDING |\
> - BTRFS_SUPER_FLAG_METADUMP)
> + BTRFS_SUPER_FLAG_METADUMP |\
> + BTRFS_SUPER_FLAG_CHANGING_FSID)
Still the same problem here.
Since that super flag is excluded in btrfs_check_super_valid(), I didn't
see any meaning including it into BTRFS_SUPER_FLAG_SUPP, and it will
confuse later BTRFS_SUPER_FLAG_SUPP users.
Thanks,
Qu
>
> static const struct extent_io_ops btree_extent_io_ops;
> static void end_workqueue_fn(struct btrfs_work *work);
> @@ -3906,6 +3907,10 @@ 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_CHANGING_FSID) {
> + btrfs_err(fs_info, "SUPER_FLAG_CHANGING_FSID is set");
> + ret = -EINVAL;
> + }
> if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP)
> btrfs_warn(fs_info, "unrecognized super flag: %llu",
> btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP);
> 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 --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] btrfs: add support for SUPER_FLAG_CHANGING_FSID in btrfs.ko
2018-01-08 4:55 ` Qu Wenruo
2018-01-08 5:05 ` Anand Jain
@ 2018-01-08 5:10 ` Anand Jain
1 sibling, 0 replies; 12+ messages in thread
From: Anand Jain @ 2018-01-08 5:10 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs; +Cc: wqu
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index a69e5944dc08..8b76faa77235 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -61,7 +61,8 @@
>> BTRFS_HEADER_FLAG_RELOC |\
>> BTRFS_SUPER_FLAG_ERROR |\
>> BTRFS_SUPER_FLAG_SEEDING |\
>> - BTRFS_SUPER_FLAG_METADUMP)
>> + BTRFS_SUPER_FLAG_METADUMP |\
>> + BTRFS_SUPER_FLAG_CHANGING_FSID)
>
> If we don't want the half changed fs to be mounted, it's better not to
> include FLAG_CHANGING_FSID into BTRFS_SUPER_FLAG_SUPP.
Anyway BTRFS_SUPER_FLAG_SUPP as such does not stop to mount, it
just warns. So when there is a new flag which is not supported
we get this warning. As in the case with BTRFS_SUPER_FLAG_METADUMP_V2.
As in 1/2 BTRFS_SUPER_FLAG_METADUMP_V2 in kernel is defined but not
supported. However since we are supporting
BTRFS_SUPER_FLAG_CHANGING_FSID, so adding to BTRFS_SUPER_FLAG_SUPP make
sense to me. ?
Thanks, Anand
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] btrfs: add support for SUPER_FLAG_CHANGING_FSID in btrfs.ko
2018-01-08 5:08 ` Qu Wenruo
@ 2018-01-08 5:13 ` Anand Jain
2018-01-08 5:25 ` Qu Wenruo
0 siblings, 1 reply; 12+ messages in thread
From: Anand Jain @ 2018-01-08 5:13 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs; +Cc: wqu
On 01/08/2018 01:08 PM, Qu Wenruo wrote:
>
>
> On 2018年01月08日 13: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.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> cc: wqu@suse.com
>> ---
>> v1->v2: Oops. Fix cut and paste error, remove ~. My bad.
>>
>> fs/btrfs/disk-io.c | 7 ++++++-
>> include/uapi/linux/btrfs_tree.h | 1 +
>> 2 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index a69e5944dc08..0dd215258ff9 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -61,7 +61,8 @@
>> BTRFS_HEADER_FLAG_RELOC |\
>> BTRFS_SUPER_FLAG_ERROR |\
>> BTRFS_SUPER_FLAG_SEEDING |\
>> - BTRFS_SUPER_FLAG_METADUMP)
>> + BTRFS_SUPER_FLAG_METADUMP |\
>> + BTRFS_SUPER_FLAG_CHANGING_FSID)
>
> Still the same problem here.
>
> Since that super flag is excluded in btrfs_check_super_valid(), I didn't
> see any meaning including it into BTRFS_SUPER_FLAG_SUPP, and it will
> confuse later BTRFS_SUPER_FLAG_SUPP users.
Anyway BTRFS_SUPER_FLAG_SUPP as such does not stop to mount, it
just warns. So when there is a new flag which is not supported
we get this warning. As in the case with BTRFS_SUPER_FLAG_METADUMP_V2.
BTRFS_SUPER_FLAG_METADUMP_V2 in kernel is defined but not supported.
However since we are supporting BTRFS_SUPER_FLAG_CHANGING_FSID, so
adding to BTRFS_SUPER_FLAG_SUPP make sense to me. ?
Thanks, Anand
> Thanks,
> Qu
>
>>
>> static const struct extent_io_ops btree_extent_io_ops;
>> static void end_workqueue_fn(struct btrfs_work *work);
>> @@ -3906,6 +3907,10 @@ 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_CHANGING_FSID) {
>> + btrfs_err(fs_info, "SUPER_FLAG_CHANGING_FSID is set");
>> + ret = -EINVAL;
>> + }
>> if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP)
>> btrfs_warn(fs_info, "unrecognized super flag: %llu",
>> btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP);
>> 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)
>>
>>
>> /*
>>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] btrfs: add support for SUPER_FLAG_CHANGING_FSID in btrfs.ko
2018-01-08 5:13 ` Anand Jain
@ 2018-01-08 5:25 ` Qu Wenruo
2018-01-08 5:58 ` Anand Jain
0 siblings, 1 reply; 12+ messages in thread
From: Qu Wenruo @ 2018-01-08 5:25 UTC (permalink / raw)
To: Anand Jain, linux-btrfs; +Cc: wqu
[-- Attachment #1.1: Type: text/plain, Size: 3927 bytes --]
On 2018年01月08日 13:13, Anand Jain wrote:
>
>
> On 01/08/2018 01:08 PM, Qu Wenruo wrote:
>>
>>
>> On 2018年01月08日 13: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.
>>>
>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>> cc: wqu@suse.com
>>> ---
>>> v1->v2: Oops. Fix cut and paste error, remove ~. My bad.
>>>
>>> fs/btrfs/disk-io.c | 7 ++++++-
>>> include/uapi/linux/btrfs_tree.h | 1 +
>>> 2 files changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>>> index a69e5944dc08..0dd215258ff9 100644
>>> --- a/fs/btrfs/disk-io.c
>>> +++ b/fs/btrfs/disk-io.c
>>> @@ -61,7 +61,8 @@
>>> BTRFS_HEADER_FLAG_RELOC |\
>>> BTRFS_SUPER_FLAG_ERROR |\
>>> BTRFS_SUPER_FLAG_SEEDING |\
>>> - BTRFS_SUPER_FLAG_METADUMP)
>>> + BTRFS_SUPER_FLAG_METADUMP |\
>>> + BTRFS_SUPER_FLAG_CHANGING_FSID)
>>
>> Still the same problem here.
>>
>> Since that super flag is excluded in btrfs_check_super_valid(), I didn't
>> see any meaning including it into BTRFS_SUPER_FLAG_SUPP, and it will
>> confuse later BTRFS_SUPER_FLAG_SUPP users.
>
>
> Anyway BTRFS_SUPER_FLAG_SUPP as such does not stop to mount, it
> just warns. So when there is a new flag which is not supported
> we get this warning. As in the case with BTRFS_SUPER_FLAG_METADUMP_V2.
> BTRFS_SUPER_FLAG_METADUMP_V2 in kernel is defined but not supported.
> However since we are supporting BTRFS_SUPER_FLAG_CHANGING_FSID, so
> adding to BTRFS_SUPER_FLAG_SUPP make sense to me. ?
We still refuse to mount the fs if CHANGING_FSID is set, which I believe
should be called "unsupported"
On the other hand it would be better to include METADUMP_V2 into SUPP,
as we allow it to be mounted and now kernel knows that flag, output a
warning doesn't seems necessary now.
It would be even better to change the later SUPP flags checks, into
something not only output some meaningless numeric flags, but also
output the human readable flags if it's recognized but unsupported. (For
CHANGIND_FSID).
Thanks,
Qu
>
> Thanks, Anand
>
>
>> Thanks,
>> Qu
>>
>>> static const struct extent_io_ops btree_extent_io_ops;
>>> static void end_workqueue_fn(struct btrfs_work *work);
>>> @@ -3906,6 +3907,10 @@ 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_CHANGING_FSID) {
>>> + btrfs_err(fs_info, "SUPER_FLAG_CHANGING_FSID is set");
>>> + ret = -EINVAL;
>>> + }
>>> if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP)
>>> btrfs_warn(fs_info, "unrecognized super flag: %llu",
>>> btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP);
>>> 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 --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] btrfs: add support for SUPER_FLAG_CHANGING_FSID in btrfs.ko
2018-01-08 5:25 ` Qu Wenruo
@ 2018-01-08 5:58 ` Anand Jain
2018-01-08 7:50 ` Qu Wenruo
0 siblings, 1 reply; 12+ messages in thread
From: Anand Jain @ 2018-01-08 5:58 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs; +Cc: wqu
On 01/08/2018 01:25 PM, Qu Wenruo wrote:
>
>
> On 2018年01月08日 13:13, Anand Jain wrote:
>>
>>
>> On 01/08/2018 01:08 PM, Qu Wenruo wrote:
>>>
>>>
>>> On 2018年01月08日 13: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.
>>>>
>>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>>> cc: wqu@suse.com
>>>> ---
>>>> v1->v2: Oops. Fix cut and paste error, remove ~. My bad.
>>>>
>>>> fs/btrfs/disk-io.c | 7 ++++++-
>>>> include/uapi/linux/btrfs_tree.h | 1 +
>>>> 2 files changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>>>> index a69e5944dc08..0dd215258ff9 100644
>>>> --- a/fs/btrfs/disk-io.c
>>>> +++ b/fs/btrfs/disk-io.c
>>>> @@ -61,7 +61,8 @@
>>>> BTRFS_HEADER_FLAG_RELOC |\
>>>> BTRFS_SUPER_FLAG_ERROR |\
>>>> BTRFS_SUPER_FLAG_SEEDING |\
>>>> - BTRFS_SUPER_FLAG_METADUMP)
>>>> + BTRFS_SUPER_FLAG_METADUMP |\
>>>> + BTRFS_SUPER_FLAG_CHANGING_FSID)
>>>
>>> Still the same problem here.
>>>
>>> Since that super flag is excluded in btrfs_check_super_valid(), I didn't
>>> see any meaning including it into BTRFS_SUPER_FLAG_SUPP, and it will
>>> confuse later BTRFS_SUPER_FLAG_SUPP users.
>>
>>
>> Anyway BTRFS_SUPER_FLAG_SUPP as such does not stop to mount, it
>> just warns. So when there is a new flag which is not supported
>> we get this warning. As in the case with BTRFS_SUPER_FLAG_METADUMP_V2.
>> BTRFS_SUPER_FLAG_METADUMP_V2 in kernel is defined but not supported.
>> However since we are supporting BTRFS_SUPER_FLAG_CHANGING_FSID, so
>> adding to BTRFS_SUPER_FLAG_SUPP make sense to me. ?
>
> We still refuse to mount the fs if CHANGING_FSID is set, which I believe
> should be called "unsupported"
>
> On the other hand it would be better to include METADUMP_V2 into SUPP,
> as we allow it to be mounted and now kernel knows that flag, output a
> warning doesn't seems necessary now.
>
> It would be even better to change the later SUPP flags checks, into
> something not only output some meaningless numeric flags, but also
> output the human readable flags if it's recognized but unsupported. (For
> CHANGIND_FSID).
Originally [1] looks like there isn't any design specific reason not to
fail the mount (which I didn't want to alter to fail) but just to warn.
Will change it to fail.
[1]
commit 319e4d0661e5323c9f9945f0f8fb5905e5fe74c3
btrfs: Enhance super validation check
Thanks, Anand
> Thanks,
> Qu
>
>>
>> Thanks, Anand
>>
>>
>>> Thanks,
>>> Qu
>>>
>>>> static const struct extent_io_ops btree_extent_io_ops;
>>>> static void end_workqueue_fn(struct btrfs_work *work);
>>>> @@ -3906,6 +3907,10 @@ 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_CHANGING_FSID) {
>>>> + btrfs_err(fs_info, "SUPER_FLAG_CHANGING_FSID is set");
>>>> + ret = -EINVAL;
>>>> + }
>>>> if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP)
>>>> btrfs_warn(fs_info, "unrecognized super flag: %llu",
>>>> btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP);
>>>> 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)
>>>> /*
>>>>
>>>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] btrfs: add support for SUPER_FLAG_CHANGING_FSID in btrfs.ko
2018-01-08 5:58 ` Anand Jain
@ 2018-01-08 7:50 ` Qu Wenruo
0 siblings, 0 replies; 12+ messages in thread
From: Qu Wenruo @ 2018-01-08 7:50 UTC (permalink / raw)
To: Anand Jain, linux-btrfs; +Cc: wqu
[-- Attachment #1.1: Type: text/plain, Size: 4929 bytes --]
On 2018年01月08日 13:58, Anand Jain wrote:
>
>
> On 01/08/2018 01:25 PM, Qu Wenruo wrote:
>>
>>
>> On 2018年01月08日 13:13, Anand Jain wrote:
>>>
>>>
>>> On 01/08/2018 01:08 PM, Qu Wenruo wrote:
>>>>
>>>>
>>>> On 2018年01月08日 13: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.
>>>>>
>>>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>>>> cc: wqu@suse.com
>>>>> ---
>>>>> v1->v2: Oops. Fix cut and paste error, remove ~. My bad.
>>>>>
>>>>> fs/btrfs/disk-io.c | 7 ++++++-
>>>>> include/uapi/linux/btrfs_tree.h | 1 +
>>>>> 2 files changed, 7 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>>>>> index a69e5944dc08..0dd215258ff9 100644
>>>>> --- a/fs/btrfs/disk-io.c
>>>>> +++ b/fs/btrfs/disk-io.c
>>>>> @@ -61,7 +61,8 @@
>>>>> BTRFS_HEADER_FLAG_RELOC |\
>>>>> BTRFS_SUPER_FLAG_ERROR |\
>>>>> BTRFS_SUPER_FLAG_SEEDING |\
>>>>> - BTRFS_SUPER_FLAG_METADUMP)
>>>>> + BTRFS_SUPER_FLAG_METADUMP |\
>>>>> + BTRFS_SUPER_FLAG_CHANGING_FSID)
>>>>
>>>> Still the same problem here.
>>>>
>>>> Since that super flag is excluded in btrfs_check_super_valid(), I
>>>> didn't
>>>> see any meaning including it into BTRFS_SUPER_FLAG_SUPP, and it will
>>>> confuse later BTRFS_SUPER_FLAG_SUPP users.
>>>
>>>
>>> Anyway BTRFS_SUPER_FLAG_SUPP as such does not stop to mount, it
>>> just warns. So when there is a new flag which is not supported
>>> we get this warning. As in the case with BTRFS_SUPER_FLAG_METADUMP_V2.
>>> BTRFS_SUPER_FLAG_METADUMP_V2 in kernel is defined but not supported.
>>> However since we are supporting BTRFS_SUPER_FLAG_CHANGING_FSID, so
>>> adding to BTRFS_SUPER_FLAG_SUPP make sense to me. ?
>>
>> We still refuse to mount the fs if CHANGING_FSID is set, which I believe
>> should be called "unsupported"
>>
>> On the other hand it would be better to include METADUMP_V2 into SUPP,
>> as we allow it to be mounted and now kernel knows that flag, output a
>> warning doesn't seems necessary now.
>>
>> It would be even better to change the later SUPP flags checks, into
>> something not only output some meaningless numeric flags, but also
>> output the human readable flags if it's recognized but unsupported. (For
>> CHANGIND_FSID).
>
> Originally [1] looks like there isn't any design specific reason not to
> fail the mount (which I didn't want to alter to fail) but just to warn.
> Will change it to fail.
>
> [1]
> commit 319e4d0661e5323c9f9945f0f8fb5905e5fe74c3
> btrfs: Enhance super validation check
My fault, I should add the missing super flags at that time, and fail
the mount.
Thanks,
Qu
>
> Thanks, Anand
>
>> Thanks,
>> Qu
>>
>>>
>>> Thanks, Anand
>>>
>>>
>>>> Thanks,
>>>> Qu
>>>>
>>>>> static const struct extent_io_ops btree_extent_io_ops;
>>>>> static void end_workqueue_fn(struct btrfs_work *work);
>>>>> @@ -3906,6 +3907,10 @@ 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_CHANGING_FSID) {
>>>>> + btrfs_err(fs_info, "SUPER_FLAG_CHANGING_FSID is set");
>>>>> + ret = -EINVAL;
>>>>> + }
>>>>> if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP)
>>>>> btrfs_warn(fs_info, "unrecognized super flag: %llu",
>>>>> btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP);
>>>>> 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)
>>>>> /*
>>>>>
>>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-01-08 7:51 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-08 3:05 [PATCH 1/2] btrfs: add missing BTRFS_SUPER_FLAG define Anand Jain
2018-01-08 3:05 ` [PATCH 2/2] btrfs: add support for SUPER_FLAG_CHANGING_FSID in btrfs.ko Anand Jain
2018-01-08 4:55 ` Qu Wenruo
2018-01-08 5:05 ` Anand Jain
2018-01-08 5:10 ` Anand Jain
2018-01-08 5:04 ` [PATCH v2 1/2] " Anand Jain
2018-01-08 5:08 ` Qu Wenruo
2018-01-08 5:13 ` Anand Jain
2018-01-08 5:25 ` Qu Wenruo
2018-01-08 5:58 ` Anand Jain
2018-01-08 7:50 ` Qu Wenruo
2018-01-08 4:52 ` [PATCH 1/2] btrfs: add missing BTRFS_SUPER_FLAG define Qu Wenruo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).