Btrfs: set right total device count for seeding support
diff mbox

Message ID 1399971906-1237-2-git-send-email-wangsl.fnst@cn.fujitsu.com
State Accepted
Headers show

Commit Message

Wang Shilong May 13, 2014, 9:05 a.m. UTC
Seeding device support allows us to create a new filesystem
based on existed filesystem.

However newly created filesystem's @total_devices should include seed
devices. This patch fix the following problem:

 # mkfs.btrfs -f /dev/sdb
 # btrfstune -S 1 /dev/sdb
 # mount /dev/sdb /mnt
 # btrfs device add -f /dev/sdc /mnt --->fs_devices->total_devices = 1
 # umount /mnt
 # mount /dev/sdc /mnt               --->fs_devices->total_devices = 2

This is because we record right @total_devices in superblock, but
@fs_devices->total_devices is reset to be 0 in btrfs_prepare_sprout().

Fix this problem by not resetting @fs_devices->total_devices.

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 fs/btrfs/volumes.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Anand Jain May 16, 2014, 2:14 p.m. UTC | #1
Wang,

  There seems to be a problem - after we delete the seed
  disk, the total_devices didn't decrement back to 1.
  reproducer as in the below test case. (I used btrfs-devlist
  (posted) to check fs_devices).

  > # mkfs.btrfs -f /dev/sdb
  > # btrfstune -S 1 /dev/sdb
  > # mount /dev/sdb /mnt
  > # btrfs device add -f /dev/sdc /mnt --->fs_devices->total_devices = 1
  mount -o rw,remount /mnt
  btrfs dev del /dev/sdb /mnt --> fs_devices->total_devices is still 2

Thanks, Anand


On 13/05/14 17:05, Wang Shilong wrote:
> Seeding device support allows us to create a new filesystem
> based on existed filesystem.
>
> However newly created filesystem's @total_devices should include seed
> devices. This patch fix the following problem:
>
>   # mkfs.btrfs -f /dev/sdb
>   # btrfstune -S 1 /dev/sdb
>   # mount /dev/sdb /mnt
>   # btrfs device add -f /dev/sdc /mnt --->fs_devices->total_devices = 1
>   # umount /mnt
>   # mount /dev/sdc /mnt               --->fs_devices->total_devices = 2
>
> This is because we record right @total_devices in superblock, but
> @fs_devices->total_devices is reset to be 0 in btrfs_prepare_sprout().
>
> Fix this problem by not resetting @fs_devices->total_devices.
>
> Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
> ---
>   fs/btrfs/volumes.c | 1 -
>   1 file changed, 1 deletion(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 6fd7fe6..19b2d32 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1883,7 +1883,6 @@ static int btrfs_prepare_sprout(struct btrfs_root *root)
>   	fs_devices->seeding = 0;
>   	fs_devices->num_devices = 0;
>   	fs_devices->open_devices = 0;
> -	fs_devices->total_devices = 0;
>   	fs_devices->seed = seed_devices;
>
>   	generate_random_uuid(fs_devices->fsid);
>
--
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
Anand Jain May 16, 2014, 2:44 p.m. UTC | #2
Wang,

  when we unmount && mount (instead of remount) and followed
  with device del <seed> it ends up with null pointer deref at
  btrfs_shrink_dev. Thats because the btrfs_root is not set for
  seed disk as we mounted the writable sprout disk. I am writing
  a separate fix for that as its a new bug.

Thanks, Anand


On 16/05/14 22:14, Anand Jain wrote:
>
> Wang,
>
>   There seems to be a problem - after we delete the seed
>   disk, the total_devices didn't decrement back to 1.
>   reproducer as in the below test case. (I used btrfs-devlist
>   (posted) to check fs_devices).
>
>   > # mkfs.btrfs -f /dev/sdb
>   > # btrfstune -S 1 /dev/sdb
>   > # mount /dev/sdb /mnt
>   > # btrfs device add -f /dev/sdc /mnt --->fs_devices->total_devices = 1
>   mount -o rw,remount /mnt
>   btrfs dev del /dev/sdb /mnt --> fs_devices->total_devices is still 2
>
> Thanks, Anand
>
>
> On 13/05/14 17:05, Wang Shilong wrote:
>> Seeding device support allows us to create a new filesystem
>> based on existed filesystem.
>>
>> However newly created filesystem's @total_devices should include seed
>> devices. This patch fix the following problem:
>>
>>   # mkfs.btrfs -f /dev/sdb
>>   # btrfstune -S 1 /dev/sdb
>>   # mount /dev/sdb /mnt
>>   # btrfs device add -f /dev/sdc /mnt --->fs_devices->total_devices = 1
>>   # umount /mnt
>>   # mount /dev/sdc /mnt               --->fs_devices->total_devices = 2
>>
>> This is because we record right @total_devices in superblock, but
>> @fs_devices->total_devices is reset to be 0 in btrfs_prepare_sprout().
>>
>> Fix this problem by not resetting @fs_devices->total_devices.
>>
>> Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
>> ---
>>   fs/btrfs/volumes.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index 6fd7fe6..19b2d32 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -1883,7 +1883,6 @@ static int btrfs_prepare_sprout(struct
>> btrfs_root *root)
>>       fs_devices->seeding = 0;
>>       fs_devices->num_devices = 0;
>>       fs_devices->open_devices = 0;
>> -    fs_devices->total_devices = 0;
>>       fs_devices->seed = seed_devices;
>>
>>       generate_random_uuid(fs_devices->fsid);
>>
> --
> 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
Wang Shilong May 16, 2014, 2:45 p.m. UTC | #3
2014-05-16 22:14 GMT+08:00 Anand Jain <Anand.Jain@oracle.com>:
>
> Wang,
>
>  There seems to be a problem - after we delete the seed
>  disk, the total_devices didn't decrement back to 1.
>  reproducer as in the below test case. (I used btrfs-devlist
>  (posted) to check fs_devices).

There should be other problems flighting or i missed something ,
i'll take a look at this. Thanks Anand for finding this.

>
>
>  > # mkfs.btrfs -f /dev/sdb
>  > # btrfstune -S 1 /dev/sdb
>  > # mount /dev/sdb /mnt
>  > # btrfs device add -f /dev/sdc /mnt --->fs_devices->total_devices = 1
>  mount -o rw,remount /mnt
>  btrfs dev del /dev/sdb /mnt --> fs_devices->total_devices is still 2
>
> Thanks, Anand
>
>
>
> On 13/05/14 17:05, Wang Shilong wrote:
>>
>> Seeding device support allows us to create a new filesystem
>> based on existed filesystem.
>>
>> However newly created filesystem's @total_devices should include seed
>> devices. This patch fix the following problem:
>>
>>   # mkfs.btrfs -f /dev/sdb
>>   # btrfstune -S 1 /dev/sdb
>>   # mount /dev/sdb /mnt
>>   # btrfs device add -f /dev/sdc /mnt --->fs_devices->total_devices = 1
>>   # umount /mnt
>>   # mount /dev/sdc /mnt               --->fs_devices->total_devices = 2
>>
>> This is because we record right @total_devices in superblock, but
>> @fs_devices->total_devices is reset to be 0 in btrfs_prepare_sprout().
>>
>> Fix this problem by not resetting @fs_devices->total_devices.
>>
>> Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
>> ---
>>   fs/btrfs/volumes.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index 6fd7fe6..19b2d32 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -1883,7 +1883,6 @@ static int btrfs_prepare_sprout(struct btrfs_root
>> *root)
>>         fs_devices->seeding = 0;
>>         fs_devices->num_devices = 0;
>>         fs_devices->open_devices = 0;
>> -       fs_devices->total_devices = 0;
>>         fs_devices->seed = seed_devices;
>>
>>         generate_random_uuid(fs_devices->fsid);
>>
> --
> 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
Wang Shilong May 16, 2014, 2:50 p.m. UTC | #4
2014-05-16 22:44 GMT+08:00 Anand Jain <Anand.Jain@oracle.com>:
>
>
> Wang,
>
>  when we unmount && mount (instead of remount) and followed
>  with device del <seed> it ends up with null pointer deref at
>  btrfs_shrink_dev. Thats because the btrfs_root is not set for
>  seed disk as we mounted the writable sprout disk. I am writing
>  a separate fix for that as its a new bug.

You means this one which was addressed by Liu few days ago?
https://patchwork.kernel.org/patch/4150471/


>
> Thanks, Anand
>
>
>
> On 16/05/14 22:14, Anand Jain wrote:
>>
>>
>> Wang,
>>
>>   There seems to be a problem - after we delete the seed
>>   disk, the total_devices didn't decrement back to 1.
>>   reproducer as in the below test case. (I used btrfs-devlist
>>   (posted) to check fs_devices).
>>
>>   > # mkfs.btrfs -f /dev/sdb
>>   > # btrfstune -S 1 /dev/sdb
>>   > # mount /dev/sdb /mnt
>>   > # btrfs device add -f /dev/sdc /mnt --->fs_devices->total_devices = 1
>>   mount -o rw,remount /mnt
>>   btrfs dev del /dev/sdb /mnt --> fs_devices->total_devices is still 2
>>
>> Thanks, Anand
>>
>>
>> On 13/05/14 17:05, Wang Shilong wrote:
>>>
>>> Seeding device support allows us to create a new filesystem
>>> based on existed filesystem.
>>>
>>> However newly created filesystem's @total_devices should include seed
>>> devices. This patch fix the following problem:
>>>
>>>   # mkfs.btrfs -f /dev/sdb
>>>   # btrfstune -S 1 /dev/sdb
>>>   # mount /dev/sdb /mnt
>>>   # btrfs device add -f /dev/sdc /mnt --->fs_devices->total_devices = 1
>>>   # umount /mnt
>>>   # mount /dev/sdc /mnt               --->fs_devices->total_devices = 2
>>>
>>> This is because we record right @total_devices in superblock, but
>>> @fs_devices->total_devices is reset to be 0 in btrfs_prepare_sprout().
>>>
>>> Fix this problem by not resetting @fs_devices->total_devices.
>>>
>>> Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
>>> ---
>>>   fs/btrfs/volumes.c | 1 -
>>>   1 file changed, 1 deletion(-)
>>>
>>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>>> index 6fd7fe6..19b2d32 100644
>>> --- a/fs/btrfs/volumes.c
>>> +++ b/fs/btrfs/volumes.c
>>> @@ -1883,7 +1883,6 @@ static int btrfs_prepare_sprout(struct
>>> btrfs_root *root)
>>>       fs_devices->seeding = 0;
>>>       fs_devices->num_devices = 0;
>>>       fs_devices->open_devices = 0;
>>> -    fs_devices->total_devices = 0;
>>>       fs_devices->seed = seed_devices;
>>>
>>>       generate_random_uuid(fs_devices->fsid);
>>>
>> --
>> 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
--
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
Anand Jain May 16, 2014, 3:06 p.m. UTC | #5
On 16/05/14 22:50, Shilong Wang wrote:
> 2014-05-16 22:44 GMT+08:00 Anand Jain <Anand.Jain@oracle.com>:
>>
>>
>> Wang,
>>
>>   when we unmount && mount (instead of remount) and followed
>>   with device del <seed> it ends up with null pointer deref at
>>   btrfs_shrink_dev. Thats because the btrfs_root is not set for
>>   seed disk as we mounted the writable sprout disk. I am writing
>>   a separate fix for that as its a new bug.
>
> You means this one which was addressed by Liu few days ago?
> https://patchwork.kernel.org/patch/4150471/

  Ah there is already a patch. yes thats the one. Thanks.

>>>> Thanks, Anand
>>
>>
>>
>> On 16/05/14 22:14, Anand Jain wrote:
>>>
>>>
>>> Wang,
>>>
>>>    There seems to be a problem - after we delete the seed
>>>    disk, the total_devices didn't decrement back to 1.
>>>    reproducer as in the below test case. (I used btrfs-devlist
>>>    (posted) to check fs_devices).
>>>
>>>    > # mkfs.btrfs -f /dev/sdb
>>>    > # btrfstune -S 1 /dev/sdb
>>>    > # mount /dev/sdb /mnt
>>>    > # btrfs device add -f /dev/sdc /mnt --->fs_devices->total_devices = 1
>>>    mount -o rw,remount /mnt
>>>    btrfs dev del /dev/sdb /mnt --> fs_devices->total_devices is still 2
>>>
>>> Thanks, Anand
>>>
>>>
>>> On 13/05/14 17:05, Wang Shilong wrote:
>>>>
>>>> Seeding device support allows us to create a new filesystem
>>>> based on existed filesystem.
>>>>
>>>> However newly created filesystem's @total_devices should include seed
>>>> devices. This patch fix the following problem:
>>>>
>>>>    # mkfs.btrfs -f /dev/sdb
>>>>    # btrfstune -S 1 /dev/sdb
>>>>    # mount /dev/sdb /mnt
>>>>    # btrfs device add -f /dev/sdc /mnt --->fs_devices->total_devices = 1
>>>>    # umount /mnt
>>>>    # mount /dev/sdc /mnt               --->fs_devices->total_devices = 2
>>>>
>>>> This is because we record right @total_devices in superblock, but
>>>> @fs_devices->total_devices is reset to be 0 in btrfs_prepare_sprout().
>>>>
>>>> Fix this problem by not resetting @fs_devices->total_devices.
>>>>
>>>> Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
>>>> ---
>>>>    fs/btrfs/volumes.c | 1 -
>>>>    1 file changed, 1 deletion(-)
>>>>
>>>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>>>> index 6fd7fe6..19b2d32 100644
>>>> --- a/fs/btrfs/volumes.c
>>>> +++ b/fs/btrfs/volumes.c
>>>> @@ -1883,7 +1883,6 @@ static int btrfs_prepare_sprout(struct
>>>> btrfs_root *root)
>>>>        fs_devices->seeding = 0;
>>>>        fs_devices->num_devices = 0;
>>>>        fs_devices->open_devices = 0;
>>>> -    fs_devices->total_devices = 0;
>>>>        fs_devices->seed = seed_devices;
>>>>
>>>>        generate_random_uuid(fs_devices->fsid);
>>>>
>>> --
>>> 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
> --
> 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

Patch
diff mbox

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 6fd7fe6..19b2d32 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1883,7 +1883,6 @@  static int btrfs_prepare_sprout(struct btrfs_root *root)
 	fs_devices->seeding = 0;
 	fs_devices->num_devices = 0;
 	fs_devices->open_devices = 0;
-	fs_devices->total_devices = 0;
 	fs_devices->seed = seed_devices;
 
 	generate_random_uuid(fs_devices->fsid);