[PATCH 1/2] btrfs: Do per-chunk degrade mode check at mount time

[PATCH 1/2] btrfs: Do per-chunk degrade mode check at mount time

[Qu Wenruo]

Btrfs supports different raid profile for meta/data/sys, and as
different profile support different tolerated missing device, it's
better to check if it can be mounted degraded at a per-chunk base.

So this patch will add check for read_one_chunk() against its profile,
other than checking it against with the lowest duplication profile.

Reported-by: Zhao Lei <zhaolei@cn.fujitsu.com>
Reported-by: Anand Jain <anand.jain@oracle.com>
Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 fs/btrfs/volumes.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 644e070..3272187 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6164,12 +6164,15 @@ static int read_one_chunk(struct btrfs_root *root, struct btrfs_key *key,
 			  struct btrfs_chunk *chunk)
 {
 	struct btrfs_mapping_tree *map_tree = &root->fs_info->mapping_tree;
+	struct super_block *sb = root->fs_info->sb;
 	struct map_lookup *map;
 	struct extent_map *em;
 	u64 logical;
 	u64 length;
 	u64 devid;
 	u8 uuid[BTRFS_UUID_SIZE];
+	int missing = 0;
+	int max_tolerated;
 	int num_stripes;
 	int ret;
 	int i;
@@ -6238,7 +6241,21 @@ static int read_one_chunk(struct btrfs_root *root, struct btrfs_key *key,
 			btrfs_warn(root->fs_info, "devid %llu uuid %pU is missing",
 						devid, uuid);
 		}
+		if (map->stripes[i].dev->missing)
+			missing++;
 		map->stripes[i].dev->in_fs_metadata = 1;
+
+	}
+
+	/* XXX: Why the function name is SO LOOOOOOOOOOOOOOOOONG?! */
+	max_tolerated =
+		btrfs_get_num_tolerated_disk_barrier_failures(map->type);
+	if (missing > max_tolerated && !(sb->s_flags & MS_RDONLY)) {
+		free_extent_map(em);
+		btrfs_error(root->fs_info, -EIO,
+			"missing device(%d) exceeds the limit(%d), writeable mount is not allowed\n",
+			missing, max_tolerated);
+		return -EIO;
 	}
 
 	write_lock(&map_tree->map_tree.lock);
-- 
2.5.2

[PATCH 2/2] btrfs: Remove unneeded missing device number check

[Qu Wenruo]

As we do per-chunk missing device number check at read_one_chunk() time,
it's not needed to do global missing device number check.

Just remove it.

Now btrfs can handle the following case:
 # mkfs.btrfs -f -m raid1 -d single /dev/sdb /dev/sdc

 Data chunk will be located in sdb, so we should be safe to wipe sdc
 # wipefs -a /dev/sdc

 # mount /dev/sdb /mnt/btrfs -o degraded

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 fs/btrfs/disk-io.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 0b658d0..ac640ea 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2947,14 +2947,6 @@ retry_root_backup:
 	}
 	fs_info->num_tolerated_disk_barrier_failures =
 		btrfs_calc_num_tolerated_disk_barrier_failures(fs_info);
-	if (fs_info->fs_devices->missing_devices >
-	     fs_info->num_tolerated_disk_barrier_failures &&
-	    !(sb->s_flags & MS_RDONLY)) {
-		pr_warn("BTRFS: missing devices(%llu) exceeds the limit(%d), writeable mount is not allowed\n",
-			fs_info->fs_devices->missing_devices,
-			fs_info->num_tolerated_disk_barrier_failures);
-		goto fail_sysfs;
-	}
 
 	fs_info->cleaner_kthread = kthread_run(cleaner_kthread, tree_root,
 					       "btrfs-cleaner");
-- 
2.5.2

Re: [PATCH 1/2] btrfs: Do per-chunk degrade mode check at mount time

[Qu Wenruo]

To Anand Jain,

Any feedback on this method to allow single chunk still be degraded 
mountable?

It should be much better than allowing degraded mount for any missing 
device case.

Thanks,
Qu

Qu Wenruo wrote on 2015/09/16 11:43 +0800:
> Btrfs supports different raid profile for meta/data/sys, and as
> different profile support different tolerated missing device, it's
> better to check if it can be mounted degraded at a per-chunk base.
>
> So this patch will add check for read_one_chunk() against its profile,
> other than checking it against with the lowest duplication profile.
>
> Reported-by: Zhao Lei <zhaolei@cn.fujitsu.com>
> Reported-by: Anand Jain <anand.jain@oracle.com>
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> ---
>   fs/btrfs/volumes.c | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 644e070..3272187 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -6164,12 +6164,15 @@ static int read_one_chunk(struct btrfs_root *root, struct btrfs_key *key,
>   			  struct btrfs_chunk *chunk)
>   {
>   	struct btrfs_mapping_tree *map_tree = &root->fs_info->mapping_tree;
> +	struct super_block *sb = root->fs_info->sb;
>   	struct map_lookup *map;
>   	struct extent_map *em;
>   	u64 logical;
>   	u64 length;
>   	u64 devid;
>   	u8 uuid[BTRFS_UUID_SIZE];
> +	int missing = 0;
> +	int max_tolerated;
>   	int num_stripes;
>   	int ret;
>   	int i;
> @@ -6238,7 +6241,21 @@ static int read_one_chunk(struct btrfs_root *root, struct btrfs_key *key,
>   			btrfs_warn(root->fs_info, "devid %llu uuid %pU is missing",
>   						devid, uuid);
>   		}
> +		if (map->stripes[i].dev->missing)
> +			missing++;
>   		map->stripes[i].dev->in_fs_metadata = 1;
> +
> +	}
> +
> +	/* XXX: Why the function name is SO LOOOOOOOOOOOOOOOOONG?! */
> +	max_tolerated =
> +		btrfs_get_num_tolerated_disk_barrier_failures(map->type);
> +	if (missing > max_tolerated && !(sb->s_flags & MS_RDONLY)) {
> +		free_extent_map(em);
> +		btrfs_error(root->fs_info, -EIO,
> +			"missing device(%d) exceeds the limit(%d), writeable mount is not allowed\n",
> +			missing, max_tolerated);
> +		return -EIO;
>   	}
>
>   	write_lock(&map_tree->map_tree.lock);
>

Re: [PATCH 1/2] btrfs: Do per-chunk degrade mode check at mount time

[Anand Jain]

Hi Qu,

On 09/17/2015 09:48 AM, Qu Wenruo wrote:
> To Anand Jain,
>
> Any feedback on this method to allow single chunk still be degraded
> mountable?
>
> It should be much better than allowing degraded mount for any missing
> device case.

yeah. this changes the way missing devices are counted and its more fine 
grained. makes sense to me.


> Thanks,
> Qu
>
> Qu Wenruo wrote on 2015/09/16 11:43 +0800:
>> Btrfs supports different raid profile for meta/data/sys, and as
>> different profile support different tolerated missing device, it's
>> better to check if it can be mounted degraded at a per-chunk base.
>>
>> So this patch will add check for read_one_chunk() against its profile,
>> other than checking it against with the lowest duplication profile.
>>
>> Reported-by: Zhao Lei <zhaolei@cn.fujitsu.com>
>> Reported-by: Anand Jain <anand.jain@oracle.com>
>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>> ---
>>   fs/btrfs/volumes.c | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index 644e070..3272187 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -6164,12 +6164,15 @@ static int read_one_chunk(struct btrfs_root
>> *root, struct btrfs_key *key,
>>                 struct btrfs_chunk *chunk)
>>   {
>>       struct btrfs_mapping_tree *map_tree = &root->fs_info->mapping_tree;
>> +    struct super_block *sb = root->fs_info->sb;
>>       struct map_lookup *map;
>>       struct extent_map *em;
>>       u64 logical;
>>       u64 length;
>>       u64 devid;
>>       u8 uuid[BTRFS_UUID_SIZE];
>> +    int missing = 0;
>> +    int max_tolerated;
>>       int num_stripes;
>>       int ret;
>>       int i;
>> @@ -6238,7 +6241,21 @@ static int read_one_chunk(struct btrfs_root
>> *root, struct btrfs_key *key,
>>               btrfs_warn(root->fs_info, "devid %llu uuid %pU is missing",
>>                           devid, uuid);
>>           }
>> +        if (map->stripes[i].dev->missing)
>> +            missing++;
>>           map->stripes[i].dev->in_fs_metadata = 1;
>> +
>> +    }
>> +
>> +    /* XXX: Why the function name is SO LOOOOOOOOOOOOOOOOONG?! */
>> +    max_tolerated =
>> +        btrfs_get_num_tolerated_disk_barrier_failures(map->type);
>> +    if (missing > max_tolerated && !(sb->s_flags & MS_RDONLY)) {
>> +        free_extent_map(em);
>> +        btrfs_error(root->fs_info, -EIO,
>> +            "missing device(%d) exceeds the limit(%d), writeable
>> mount is not allowed\n",

  \n is not required.

Thanks, Anand

>> +            missing, max_tolerated);
>> +        return -EIO;
>>       }
>>
>>       write_lock(&map_tree->map_tree.lock);
>>

Re: [PATCH 2/2] btrfs: Remove unneeded missing device number check

[Anand Jain]

On 09/16/2015 11:43 AM, Qu Wenruo wrote:
> As we do per-chunk missing device number check at read_one_chunk() time,
> it's not needed to do global missing device number check.
>
> Just remove it.

However the missing device count, what we have during the remount is not 
fine grained per chunk.
-----------
btrfs_remount
::
                  if (fs_info->fs_devices->missing_devices >
                      fs_info->num_tolerated_disk_barrier_failures &&
                     !(*flags & MS_RDONLY ||
                         btrfs_test_opt(root, DEGRADED))) {
                         btrfs_warn(fs_info,
                                 "too many missing devices, writeable 
remount is not allowed");
                         ret = -EACCES;
                         goto restore;
                 }
---------

Thanks, Anand


> Now btrfs can handle the following case:
>   # mkfs.btrfs -f -m raid1 -d single /dev/sdb /dev/sdc
>
>   Data chunk will be located in sdb, so we should be safe to wipe sdc
>   # wipefs -a /dev/sdc
>
>   # mount /dev/sdb /mnt/btrfs -o degraded
>
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> ---
>   fs/btrfs/disk-io.c | 8 --------
>   1 file changed, 8 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 0b658d0..ac640ea 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -2947,14 +2947,6 @@ retry_root_backup:
>   	}
>   	fs_info->num_tolerated_disk_barrier_failures =
>   		btrfs_calc_num_tolerated_disk_barrier_failures(fs_info);
> -	if (fs_info->fs_devices->missing_devices >
> -	     fs_info->num_tolerated_disk_barrier_failures &&
> -	    !(sb->s_flags & MS_RDONLY)) {
> -		pr_warn("BTRFS: missing devices(%llu) exceeds the limit(%d), writeable mount is not allowed\n",
> -			fs_info->fs_devices->missing_devices,
> -			fs_info->num_tolerated_disk_barrier_failures);
> -		goto fail_sysfs;
> -	}
>
>   	fs_info->cleaner_kthread = kthread_run(cleaner_kthread, tree_root,
>   					       "btrfs-cleaner");
>

Re: [PATCH 2/2] btrfs: Remove unneeded missing device number check

[Qu Wenruo]

Thanks for pointing this out.

Although previous patch is small enough, but for remount case, we need 
to iterate all the existing chunk cache.

So fix for remount will take a little more time.

Thanks for reviewing.
Qu

在 2015年09月17日 17:43, Anand Jain 写道:
>
>
> On 09/16/2015 11:43 AM, Qu Wenruo wrote:
>> As we do per-chunk missing device number check at read_one_chunk() time,
>> it's not needed to do global missing device number check.
>>
>> Just remove it.
>
> However the missing device count, what we have during the remount is not
> fine grained per chunk.
> -----------
> btrfs_remount
> ::
>                   if (fs_info->fs_devices->missing_devices >
>                       fs_info->num_tolerated_disk_barrier_failures &&
>                      !(*flags & MS_RDONLY ||
>                          btrfs_test_opt(root, DEGRADED))) {
>                          btrfs_warn(fs_info,
>                                  "too many missing devices, writeable
> remount is not allowed");
>                          ret = -EACCES;
>                          goto restore;
>                  }
> ---------
>
> Thanks, Anand
>
>
>> Now btrfs can handle the following case:
>>   # mkfs.btrfs -f -m raid1 -d single /dev/sdb /dev/sdc
>>
>>   Data chunk will be located in sdb, so we should be safe to wipe sdc
>>   # wipefs -a /dev/sdc
>>
>>   # mount /dev/sdb /mnt/btrfs -o degraded
>>
>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>> ---
>>   fs/btrfs/disk-io.c | 8 --------
>>   1 file changed, 8 deletions(-)
>>
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index 0b658d0..ac640ea 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -2947,14 +2947,6 @@ retry_root_backup:
>>       }
>>       fs_info->num_tolerated_disk_barrier_failures =
>>           btrfs_calc_num_tolerated_disk_barrier_failures(fs_info);
>> -    if (fs_info->fs_devices->missing_devices >
>> -         fs_info->num_tolerated_disk_barrier_failures &&
>> -        !(sb->s_flags & MS_RDONLY)) {
>> -        pr_warn("BTRFS: missing devices(%llu) exceeds the limit(%d),
>> writeable mount is not allowed\n",
>> -            fs_info->fs_devices->missing_devices,
>> -            fs_info->num_tolerated_disk_barrier_failures);
>> -        goto fail_sysfs;
>> -    }
>>
>>       fs_info->cleaner_kthread = kthread_run(cleaner_kthread, tree_root,
>>                              "btrfs-cleaner");
>>
> --
> 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

Re: [PATCH 2/2] btrfs: Remove unneeded missing device number check

[Anand Jain]

On 09/17/2015 06:01 PM, Qu Wenruo wrote:
> Thanks for pointing this out.


> Although previous patch is small enough, but for remount case, we need
> to iterate all the existing chunk cache.

  yes indeed.

  thinking hard on this - is there any test-case that these two patches 
are solving, which the original patch [1] didn't solve ?

  I tried to break both the approaches (this patch set and [1]) but I 
wasn't successful. sorry if I am missing something.

Thanks, Anand

[1] [PATCH 23/23] Btrfs: allow -o rw,degraded for single group profile


> So fix for remount will take a little more time.

> Thanks for reviewing.
> Qu
>
> 在 2015年09月17日 17:43, Anand Jain 写道:
>>
>>
>> On 09/16/2015 11:43 AM, Qu Wenruo wrote:
>>> As we do per-chunk missing device number check at read_one_chunk() time,
>>> it's not needed to do global missing device number check.
>>>
>>> Just remove it.
>>
>> However the missing device count, what we have during the remount is not
>> fine grained per chunk.
>> -----------
>> btrfs_remount
>> ::
>>                   if (fs_info->fs_devices->missing_devices >
>>                       fs_info->num_tolerated_disk_barrier_failures &&
>>                      !(*flags & MS_RDONLY ||
>>                          btrfs_test_opt(root, DEGRADED))) {
>>                          btrfs_warn(fs_info,
>>                                  "too many missing devices, writeable
>> remount is not allowed");
>>                          ret = -EACCES;
>>                          goto restore;
>>                  }
>> ---------
>>
>> Thanks, Anand
>>
>>
>>> Now btrfs can handle the following case:
>>>   # mkfs.btrfs -f -m raid1 -d single /dev/sdb /dev/sdc
>>>
>>>   Data chunk will be located in sdb, so we should be safe to wipe sdc
>>>   # wipefs -a /dev/sdc
>>>
>>>   # mount /dev/sdb /mnt/btrfs -o degraded
>>>
>>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>>> ---
>>>   fs/btrfs/disk-io.c | 8 --------
>>>   1 file changed, 8 deletions(-)
>>>
>>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>>> index 0b658d0..ac640ea 100644
>>> --- a/fs/btrfs/disk-io.c
>>> +++ b/fs/btrfs/disk-io.c
>>> @@ -2947,14 +2947,6 @@ retry_root_backup:
>>>       }
>>>       fs_info->num_tolerated_disk_barrier_failures =
>>>           btrfs_calc_num_tolerated_disk_barrier_failures(fs_info);
>>> -    if (fs_info->fs_devices->missing_devices >
>>> -         fs_info->num_tolerated_disk_barrier_failures &&
>>> -        !(sb->s_flags & MS_RDONLY)) {
>>> -        pr_warn("BTRFS: missing devices(%llu) exceeds the limit(%d),
>>> writeable mount is not allowed\n",
>>> -            fs_info->fs_devices->missing_devices,
>>> -            fs_info->num_tolerated_disk_barrier_failures);
>>> -        goto fail_sysfs;
>>> -    }
>>>
>>>       fs_info->cleaner_kthread = kthread_run(cleaner_kthread, tree_root,
>>>                              "btrfs-cleaner");
>>>
>> --
>> 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
> --
> 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

Re: [PATCH 2/2] btrfs: Remove unneeded missing device number check

[Qu Wenruo]

Anand Jain wrote on 2015/09/18 09:47 +0800:
>
>
> On 09/17/2015 06:01 PM, Qu Wenruo wrote:
>> Thanks for pointing this out.
>
>
>> Although previous patch is small enough, but for remount case, we need
>> to iterate all the existing chunk cache.
>
>   yes indeed.
>
>   thinking hard on this - is there any test-case that these two patches
> are solving, which the original patch [1] didn't solve ?

Yep, your patch is OK to fix single chunk on safe disk case.
But IMHO, it's a little aggressive and not safe as old codes.

For example, if one use single metadata for 2 disks, and each disk has 
one metadata chunk on it.

One device got missing later.

Then your patch will allow the fs to be mounted as rw, even some tree 
block can be in the missing device.
For RO case, it won't be too dangerous, but if we mounted it as RW, who 
knows what will happen.
(Normal tree COW thing should fail before real write, but I'm not sure 
about other RW operation like scrub/replace/balance and others)

And I think that's the original design concept behind the old missing 
device number check, and it's not a bad idea to follow it anyway.

For the patch size, I find a good idea to handle it, and should make the 
patch(set) size below 200 lines.

Further more, it's even possible to make btrfs change mount option to 
degraded for runtime device missing.

Thanks,
Qu
>
>   I tried to break both the approaches (this patch set and [1]) but I
> wasn't successful. sorry if I am missing something.
>
> Thanks, Anand
>
> [1] [PATCH 23/23] Btrfs: allow -o rw,degraded for single group profile
>
>
>> So fix for remount will take a little more time.
>
>> Thanks for reviewing.
>> Qu
>>
>> 在 2015年09月17日 17:43, Anand Jain 写道:
>>>
>>>
>>> On 09/16/2015 11:43 AM, Qu Wenruo wrote:
>>>> As we do per-chunk missing device number check at read_one_chunk()
>>>> time,
>>>> it's not needed to do global missing device number check.
>>>>
>>>> Just remove it.
>>>
>>> However the missing device count, what we have during the remount is not
>>> fine grained per chunk.
>>> -----------
>>> btrfs_remount
>>> ::
>>>                   if (fs_info->fs_devices->missing_devices >
>>>                       fs_info->num_tolerated_disk_barrier_failures &&
>>>                      !(*flags & MS_RDONLY ||
>>>                          btrfs_test_opt(root, DEGRADED))) {
>>>                          btrfs_warn(fs_info,
>>>                                  "too many missing devices, writeable
>>> remount is not allowed");
>>>                          ret = -EACCES;
>>>                          goto restore;
>>>                  }
>>> ---------
>>>
>>> Thanks, Anand
>>>
>>>
>>>> Now btrfs can handle the following case:
>>>>   # mkfs.btrfs -f -m raid1 -d single /dev/sdb /dev/sdc
>>>>
>>>>   Data chunk will be located in sdb, so we should be safe to wipe sdc
>>>>   # wipefs -a /dev/sdc
>>>>
>>>>   # mount /dev/sdb /mnt/btrfs -o degraded
>>>>
>>>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>>>> ---
>>>>   fs/btrfs/disk-io.c | 8 --------
>>>>   1 file changed, 8 deletions(-)
>>>>
>>>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>>>> index 0b658d0..ac640ea 100644
>>>> --- a/fs/btrfs/disk-io.c
>>>> +++ b/fs/btrfs/disk-io.c
>>>> @@ -2947,14 +2947,6 @@ retry_root_backup:
>>>>       }
>>>>       fs_info->num_tolerated_disk_barrier_failures =
>>>>           btrfs_calc_num_tolerated_disk_barrier_failures(fs_info);
>>>> -    if (fs_info->fs_devices->missing_devices >
>>>> -         fs_info->num_tolerated_disk_barrier_failures &&
>>>> -        !(sb->s_flags & MS_RDONLY)) {
>>>> -        pr_warn("BTRFS: missing devices(%llu) exceeds the limit(%d),
>>>> writeable mount is not allowed\n",
>>>> -            fs_info->fs_devices->missing_devices,
>>>> -            fs_info->num_tolerated_disk_barrier_failures);
>>>> -        goto fail_sysfs;
>>>> -    }
>>>>
>>>>       fs_info->cleaner_kthread = kthread_run(cleaner_kthread,
>>>> tree_root,
>>>>                              "btrfs-cleaner");
>>>>
>>> --
>>> 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
>> --
>> 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

Re: [PATCH 2/2] btrfs: Remove unneeded missing device number check

[Anand Jain]

Hi Qu,

  Thanks for the comments on patch [1].

> For example, if one use single metadata for 2 disks,
 > and each disk has one metadata chunk on it.

  how that can be achieved ?

> One device got missing later.

  it would surely depend on which one of the device ? (initial only 
devid 1 mountable, with other missing)


Thanks, Anand

> Then your patch will allow the fs to be mounted as rw, even some tree
> block can be in the missing device.
> For RO case, it won't be too dangerous, but if we mounted it as RW, who
> knows what will happen.
> (Normal tree COW thing should fail before real write, but I'm not sure
> about other RW operation like scrub/replace/balance and others)
>
> And I think that's the original design concept behind the old missing
> device number check, and it's not a bad idea to follow it anyway.
>
> For the patch size, I find a good idea to handle it, and should make the
> patch(set) size below 200 lines.
>
> Further more, it's even possible to make btrfs change mount option to
> degraded for runtime device missing.
>
> Thanks,
> Qu
>>
>>   I tried to break both the approaches (this patch set and [1]) but I
>> wasn't successful. sorry if I am missing something.
>>
>> Thanks, Anand
>>
>> [1] [PATCH 23/23] Btrfs: allow -o rw,degraded for single group profile

Re: [PATCH 2/2] btrfs: Remove unneeded missing device number check

[Qu Wenruo]

在 2015年09月18日 14:45, Anand Jain 写道:
>
> Hi Qu,
>
>   Thanks for the comments on patch [1].
>
>> For example, if one use single metadata for 2 disks,
>  > and each disk has one metadata chunk on it.
>
>   how that can be achieved ?
By this, I mean, the metadata profile is SINGLE,
and there is 2 metadata chunks.

One on disk1 and one on disk2.

As btrfs chunk allocate will always use device by avaiable space order,
it should be quite easy to archieve that situation.

In that case, any missing device will be a disaster, and it's better not 
to allow RW mount.

Thanks,
Qu
>
>> One device got missing later.
>
>   it would surely depend on which one of the device ? (initial only
> devid 1 mountable, with other missing)
>
>
> Thanks, Anand
>
>> Then your patch will allow the fs to be mounted as rw, even some tree
>> block can be in the missing device.
>> For RO case, it won't be too dangerous, but if we mounted it as RW, who
>> knows what will happen.
>> (Normal tree COW thing should fail before real write, but I'm not sure
>> about other RW operation like scrub/replace/balance and others)
>>
>> And I think that's the original design concept behind the old missing
>> device number check, and it's not a bad idea to follow it anyway.
>>
>> For the patch size, I find a good idea to handle it, and should make the
>> patch(set) size below 200 lines.
>>
>> Further more, it's even possible to make btrfs change mount option to
>> degraded for runtime device missing.
>>
>> Thanks,
>> Qu
>>>
>>>   I tried to break both the approaches (this patch set and [1]) but I
>>> wasn't successful. sorry if I am missing something.
>>>
>>> Thanks, Anand
>>>
>>> [1] [PATCH 23/23] Btrfs: allow -o rw,degraded for single group profile
>
>
>
> --
> 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

Re: [PATCH 2/2] btrfs: Remove unneeded missing device number check

[Anand Jain]

On 09/20/2015 08:31 AM, Qu Wenruo wrote:
>
>
> 在 2015年09月18日 14:45, Anand Jain 写道:
>>
>> Hi Qu,
>>
>>   Thanks for the comments on patch [1].
>>
>>> For example, if one use single metadata for 2 disks,
>>  > and each disk has one metadata chunk on it.
>>
>>   how that can be achieved ?
> By this, I mean, the metadata profile is SINGLE,
> and there is 2 metadata chunks.
>
> One on disk1 and one on disk2.
>
> As btrfs chunk allocate will always use device by avaiable space order,
> it should be quite easy to archieve that situation.
>
> In that case, any missing device will be a disaster,

  in this case the read chunk would anyway fail, right ?
  and that will lead to mount fail.

Thanks, Anand

> and it's better not
> to allow RW mount.
>
> Thanks,
> Qu
>>
>>> One device got missing later.
>>
>>   it would surely depend on which one of the device ? (initial only
>> devid 1 mountable, with other missing)
>>
>>
>> Thanks, Anand
>>
>>> Then your patch will allow the fs to be mounted as rw, even some tree
>>> block can be in the missing device.
>>> For RO case, it won't be too dangerous, but if we mounted it as RW, who
>>> knows what will happen.
>>> (Normal tree COW thing should fail before real write, but I'm not sure
>>> about other RW operation like scrub/replace/balance and others)
>>>
>>> And I think that's the original design concept behind the old missing
>>> device number check, and it's not a bad idea to follow it anyway.
>>>
>>> For the patch size, I find a good idea to handle it, and should make the
>>> patch(set) size below 200 lines.
>>>
>>> Further more, it's even possible to make btrfs change mount option to
>>> degraded for runtime device missing.
>>>
>>> Thanks,
>>> Qu
>>>>
>>>>   I tried to break both the approaches (this patch set and [1]) but I
>>>> wasn't successful. sorry if I am missing something.
>>>>
>>>> Thanks, Anand
>>>>
>>>> [1] [PATCH 23/23] Btrfs: allow -o rw,degraded for single group profile
>>
>>
>>
>> --
>> 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
> --
> 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

Re: [PATCH 2/2] btrfs: Remove unneeded missing device number check

[Qu Wenruo]

Anand Jain wrote on 2015/09/20 13:37 +0800:
>
>
> On 09/20/2015 08:31 AM, Qu Wenruo wrote:
>>
>>
>> 在 2015年09月18日 14:45, Anand Jain 写道:
>>>
>>> Hi Qu,
>>>
>>>   Thanks for the comments on patch [1].
>>>
>>>> For example, if one use single metadata for 2 disks,
>>>  > and each disk has one metadata chunk on it.
>>>
>>>   how that can be achieved ?
>> By this, I mean, the metadata profile is SINGLE,
>> and there is 2 metadata chunks.
>>
>> One on disk1 and one on disk2.
>>
>> As btrfs chunk allocate will always use device by avaiable space order,
>> it should be quite easy to archieve that situation.
>>
>> In that case, any missing device will be a disaster,
>
>   in this case the read chunk would anyway fail, right ?
>   and that will lead to mount fail.
>
> Thanks, Anand

Yes, read will cause fail, but I'm not completely sure other operation 
can handle it well or not.
Like chunk/extent allocation or scrub/replace/balance.

So I'd still keep it allow RO mount only.

Thanks,
Qu

>
>> and it's better not
>> to allow RW mount.
>>
>> Thanks,
>> Qu
>>>
>>>> One device got missing later.
>>>
>>>   it would surely depend on which one of the device ? (initial only
>>> devid 1 mountable, with other missing)
>>>
>>>
>>> Thanks, Anand
>>>
>>>> Then your patch will allow the fs to be mounted as rw, even some tree
>>>> block can be in the missing device.
>>>> For RO case, it won't be too dangerous, but if we mounted it as RW, who
>>>> knows what will happen.
>>>> (Normal tree COW thing should fail before real write, but I'm not sure
>>>> about other RW operation like scrub/replace/balance and others)
>>>>
>>>> And I think that's the original design concept behind the old missing
>>>> device number check, and it's not a bad idea to follow it anyway.
>>>>
>>>> For the patch size, I find a good idea to handle it, and should make
>>>> the
>>>> patch(set) size below 200 lines.
>>>>
>>>> Further more, it's even possible to make btrfs change mount option to
>>>> degraded for runtime device missing.
>>>>
>>>> Thanks,
>>>> Qu
>>>>>
>>>>>   I tried to break both the approaches (this patch set and [1]) but I
>>>>> wasn't successful. sorry if I am missing something.
>>>>>
>>>>> Thanks, Anand
>>>>>
>>>>> [1] [PATCH 23/23] Btrfs: allow -o rw,degraded for single group profile
>>>
>>>
>>>
>>> --
>>> 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
>> --
>> 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