linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Btrfs-progs: save us an unnecessary ioctl call
@ 2014-05-13  9:05 Wang Shilong
  2014-05-13  9:05 ` [PATCH] Btrfs: set right total device count for seeding support Wang Shilong
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Wang Shilong @ 2014-05-13  9:05 UTC (permalink / raw)
  To: linux-btrfs

Btrfs device id start from 1, not 0.

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/utils.c b/utils.c
index 560c557..d480353 100644
--- a/utils.c
+++ b/utils.c
@@ -1765,7 +1765,7 @@ int get_fs_info(char *path, struct btrfs_ioctl_fs_info_args *fi_args,
 		goto out;
 	}
 
-	for (; i <= fi_args->max_id; ++i) {
+	for (i = 1; i <= fi_args->max_id; ++i) {
 		BUG_ON(ndevs >= fi_args->num_devices);
 		ret = get_device_info(fd, i, &di_args[ndevs]);
 		if (ret == -ENODEV)
-- 
1.8.2.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH] Btrfs: set right total device count for seeding support
  2014-05-13  9:05 [PATCH] Btrfs-progs: save us an unnecessary ioctl call Wang Shilong
@ 2014-05-13  9:05 ` Wang Shilong
  2014-05-16 14:14   ` Anand Jain
  2014-05-13 10:48 ` [PATCH] Btrfs-progs: save us an unnecessary ioctl call Stefan Behrens
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Wang Shilong @ 2014-05-13  9:05 UTC (permalink / raw)
  To: linux-btrfs

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);
-- 
1.8.2.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] Btrfs-progs: save us an unnecessary ioctl call
  2014-05-13  9:05 [PATCH] Btrfs-progs: save us an unnecessary ioctl call Wang Shilong
  2014-05-13  9:05 ` [PATCH] Btrfs: set right total device count for seeding support Wang Shilong
@ 2014-05-13 10:48 ` Stefan Behrens
  2014-05-13 11:26   ` Wang Shilong
  2014-05-14  5:32 ` Anand Jain
  2014-05-15 17:06 ` David Sterba
  3 siblings, 1 reply; 14+ messages in thread
From: Stefan Behrens @ 2014-05-13 10:48 UTC (permalink / raw)
  To: Wang Shilong, linux-btrfs

On Tue, 13 May 2014 17:05:05 +0800, Wang Shilong wrote:
> Btrfs device id start from 1, not 0.
> 
> Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
> ---
>  utils.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/utils.c b/utils.c
> index 560c557..d480353 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -1765,7 +1765,7 @@ int get_fs_info(char *path, struct btrfs_ioctl_fs_info_args *fi_args,
>  		goto out;
>  	}

You've not seen the two assignments in get_fs_info() above the lines
that you change:

        if (is_block_device(path)) {
                ...
                fi_args->max_id = devid;
                i = devid;

>  
> -	for (; i <= fi_args->max_id; ++i) {
> +	for (i = 1; i <= fi_args->max_id; ++i) {
>  		BUG_ON(ndevs >= fi_args->num_devices);
>  		ret = get_device_info(fd, i, &di_args[ndevs]);
>  		if (ret == -ENODEV)



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Btrfs-progs: save us an unnecessary ioctl call
  2014-05-13 10:48 ` [PATCH] Btrfs-progs: save us an unnecessary ioctl call Stefan Behrens
@ 2014-05-13 11:26   ` Wang Shilong
  0 siblings, 0 replies; 14+ messages in thread
From: Wang Shilong @ 2014-05-13 11:26 UTC (permalink / raw)
  To: Stefan Behrens; +Cc: linux-btrfs

On 05/13/2014 06:48 PM, Stefan Behrens wrote:
> On Tue, 13 May 2014 17:05:05 +0800, Wang Shilong wrote:
>> Btrfs device id start from 1, not 0.
>>
>> Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
>> ---
>>   utils.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/utils.c b/utils.c
>> index 560c557..d480353 100644
>> --- a/utils.c
>> +++ b/utils.c
>> @@ -1765,7 +1765,7 @@ int get_fs_info(char *path, struct btrfs_ioctl_fs_info_args *fi_args,
>>   		goto out;
>>   	}
> You've not seen the two assignments in get_fs_info() above the lines
> that you change:
>
>          if (is_block_device(path)) {
>                  ...
>                  fi_args->max_id = devid;
>                  i = devid;
Oops, i did not see this one, but this condition may not meet, so
i guess the right fix is to set i=1 when declaration, something like:

int i = 1;

>
>>   
>> -	for (; i <= fi_args->max_id; ++i) {
>> +	for (i = 1; i <= fi_args->max_id; ++i) {
>>   		BUG_ON(ndevs >= fi_args->num_devices);
>>   		ret = get_device_info(fd, i, &di_args[ndevs]);
>>   		if (ret == -ENODEV)
>
> --
> 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
>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Btrfs-progs: save us an unnecessary ioctl call
  2014-05-13  9:05 [PATCH] Btrfs-progs: save us an unnecessary ioctl call Wang Shilong
  2014-05-13  9:05 ` [PATCH] Btrfs: set right total device count for seeding support Wang Shilong
  2014-05-13 10:48 ` [PATCH] Btrfs-progs: save us an unnecessary ioctl call Stefan Behrens
@ 2014-05-14  5:32 ` Anand Jain
  2014-05-15 17:06 ` David Sterba
  3 siblings, 0 replies; 14+ messages in thread
From: Anand Jain @ 2014-05-14  5:32 UTC (permalink / raw)
  To: Wang Shilong, linux-btrfs



 > Btrfs device id start from 1, not 0.

  That was an intentional change.

  50275ba btrfs-progs: there is devid 0 when replace is running




>
> Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
> ---
>   utils.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/utils.c b/utils.c
> index 560c557..d480353 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -1765,7 +1765,7 @@ int get_fs_info(char *path, struct btrfs_ioctl_fs_info_args *fi_args,
>   		goto out;
>   	}
>
> -	for (; i <= fi_args->max_id; ++i) {
> +	for (i = 1; i <= fi_args->max_id; ++i) {
>   		BUG_ON(ndevs >= fi_args->num_devices);
>   		ret = get_device_info(fd, i, &di_args[ndevs]);
>   		if (ret == -ENODEV)
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Btrfs-progs: save us an unnecessary ioctl call
  2014-05-13  9:05 [PATCH] Btrfs-progs: save us an unnecessary ioctl call Wang Shilong
                   ` (2 preceding siblings ...)
  2014-05-14  5:32 ` Anand Jain
@ 2014-05-15 17:06 ` David Sterba
  2014-05-16  4:32   ` Anand Jain
  3 siblings, 1 reply; 14+ messages in thread
From: David Sterba @ 2014-05-15 17:06 UTC (permalink / raw)
  To: Wang Shilong; +Cc: linux-btrfs

On Tue, May 13, 2014 at 05:05:05PM +0800, Wang Shilong wrote:
> Btrfs device id start from 1, not 0.

> --- a/utils.c
> +++ b/utils.c
> @@ -1765,7 +1765,7 @@ int get_fs_info(char *path, struct btrfs_ioctl_fs_info_args *fi_args,
>  		goto out;
>  	}
>  
> -	for (; i <= fi_args->max_id; ++i) {
> +	for (i = 1; i <= fi_args->max_id; ++i) {

You're right about the device id start, but forcing 1 here breaks the
case when get_fs_info is called with block device as an argument and 'i'
is set to the devid a few lines above.

Initializing i to 1 in the declaration block is the right fix IMO, and
as this is a trivial change I'll do that myself, no need to resend.

>  		BUG_ON(ndevs >= fi_args->num_devices);
>  		ret = get_device_info(fd, i, &di_args[ndevs]);
>  		if (ret == -ENODEV)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Btrfs-progs: save us an unnecessary ioctl call
  2014-05-15 17:06 ` David Sterba
@ 2014-05-16  4:32   ` Anand Jain
  2014-05-16  4:58     ` Wang Shilong
  0 siblings, 1 reply; 14+ messages in thread
From: Anand Jain @ 2014-05-16  4:32 UTC (permalink / raw)
  To: dsterba, Wang Shilong, linux-btrfs


David,

  As mentioned, this patch will back-out the earlier patch

  50275bacab0f62b91453fbfa29e75c2bb77bf9b6

  I am confused on what I am missing ? Any comment?

Thanks, Anand


On 16/05/14 01:06, David Sterba wrote:
> On Tue, May 13, 2014 at 05:05:05PM +0800, Wang Shilong wrote:
>> Btrfs device id start from 1, not 0.
>
>> --- a/utils.c
>> +++ b/utils.c
>> @@ -1765,7 +1765,7 @@ int get_fs_info(char *path, struct btrfs_ioctl_fs_info_args *fi_args,
>>   		goto out;
>>   	}
>>
>> -	for (; i <= fi_args->max_id; ++i) {
>> +	for (i = 1; i <= fi_args->max_id; ++i) {
>
> You're right about the device id start, but forcing 1 here breaks the
> case when get_fs_info is called with block device as an argument and 'i'
> is set to the devid a few lines above.
>
> Initializing i to 1 in the declaration block is the right fix IMO, and
> as this is a trivial change I'll do that myself, no need to resend.
>
>>   		BUG_ON(ndevs >= fi_args->num_devices);
>>   		ret = get_device_info(fd, i, &di_args[ndevs]);
>>   		if (ret == -ENODEV)
> --
> 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
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Btrfs-progs: save us an unnecessary ioctl call
  2014-05-16  4:32   ` Anand Jain
@ 2014-05-16  4:58     ` Wang Shilong
  2014-05-16 12:14       ` David Sterba
  0 siblings, 1 reply; 14+ messages in thread
From: Wang Shilong @ 2014-05-16  4:58 UTC (permalink / raw)
  To: Anand Jain; +Cc: dsterba, linux-btrfs

Hi Anand,

On 05/16/2014 12:32 PM, Anand Jain wrote:
>
> David,
>
>  As mentioned, this patch will back-out the earlier patch
>
>  50275bacab0f62b91453fbfa29e75c2bb77bf9b6
>
>  I am confused on what I am missing ? Any comment?
You are right, i guess dave just missed your previous thread.:-)
dave, please ignore my patch, sorry for noise.

Thanks,
Wang
>
> Thanks, Anand
>
>
> On 16/05/14 01:06, David Sterba wrote:
>> On Tue, May 13, 2014 at 05:05:05PM +0800, Wang Shilong wrote:
>>> Btrfs device id start from 1, not 0.
>>
>>> --- a/utils.c
>>> +++ b/utils.c
>>> @@ -1765,7 +1765,7 @@ int get_fs_info(char *path, struct 
>>> btrfs_ioctl_fs_info_args *fi_args,
>>>           goto out;
>>>       }
>>>
>>> -    for (; i <= fi_args->max_id; ++i) {
>>> +    for (i = 1; i <= fi_args->max_id; ++i) {
>>
>> You're right about the device id start, but forcing 1 here breaks the
>> case when get_fs_info is called with block device as an argument and 'i'
>> is set to the devid a few lines above.
>>
>> Initializing i to 1 in the declaration block is the right fix IMO, and
>> as this is a trivial change I'll do that myself, no need to resend.
>>
>>>           BUG_ON(ndevs >= fi_args->num_devices);
>>>           ret = get_device_info(fd, i, &di_args[ndevs]);
>>>           if (ret == -ENODEV)
>> -- 
>> 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
>>
> .
>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Btrfs-progs: save us an unnecessary ioctl call
  2014-05-16  4:58     ` Wang Shilong
@ 2014-05-16 12:14       ` David Sterba
  0 siblings, 0 replies; 14+ messages in thread
From: David Sterba @ 2014-05-16 12:14 UTC (permalink / raw)
  To: Wang Shilong; +Cc: Anand Jain, dsterba, linux-btrfs

On Fri, May 16, 2014 at 12:58:46PM +0800, Wang Shilong wrote:
> Hi Anand,
> 
> On 05/16/2014 12:32 PM, Anand Jain wrote:
> >
> >David,
> >
> > As mentioned, this patch will back-out the earlier patch
> >
> > 50275bacab0f62b91453fbfa29e75c2bb77bf9b6
> >
> > I am confused on what I am missing ? Any comment?
> You are right, i guess dave just missed your previous thread.:-)

Oh I did, sorry.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Btrfs: set right total device count for seeding support
  2014-05-13  9:05 ` [PATCH] Btrfs: set right total device count for seeding support Wang Shilong
@ 2014-05-16 14:14   ` Anand Jain
  2014-05-16 14:44     ` Anand Jain
  2014-05-16 14:45     ` Shilong Wang
  0 siblings, 2 replies; 14+ messages in thread
From: Anand Jain @ 2014-05-16 14:14 UTC (permalink / raw)
  To: Wang Shilong, linux-btrfs


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);
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Btrfs: set right total device count for seeding support
  2014-05-16 14:14   ` Anand Jain
@ 2014-05-16 14:44     ` Anand Jain
  2014-05-16 14:50       ` Shilong Wang
  2014-05-16 14:45     ` Shilong Wang
  1 sibling, 1 reply; 14+ messages in thread
From: Anand Jain @ 2014-05-16 14:44 UTC (permalink / raw)
  To: Wang Shilong, linux-btrfs



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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Btrfs: set right total device count for seeding support
  2014-05-16 14:14   ` Anand Jain
  2014-05-16 14:44     ` Anand Jain
@ 2014-05-16 14:45     ` Shilong Wang
  1 sibling, 0 replies; 14+ messages in thread
From: Shilong Wang @ 2014-05-16 14:45 UTC (permalink / raw)
  To: Anand Jain; +Cc: Wang Shilong, linux-btrfs

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Btrfs: set right total device count for seeding support
  2014-05-16 14:44     ` Anand Jain
@ 2014-05-16 14:50       ` Shilong Wang
  2014-05-16 15:06         ` Anand Jain
  0 siblings, 1 reply; 14+ messages in thread
From: Shilong Wang @ 2014-05-16 14:50 UTC (permalink / raw)
  To: Anand Jain; +Cc: Wang Shilong, linux-btrfs

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Btrfs: set right total device count for seeding support
  2014-05-16 14:50       ` Shilong Wang
@ 2014-05-16 15:06         ` Anand Jain
  0 siblings, 0 replies; 14+ messages in thread
From: Anand Jain @ 2014-05-16 15:06 UTC (permalink / raw)
  To: Shilong Wang; +Cc: Wang Shilong, linux-btrfs



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
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2014-05-16 15:03 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-13  9:05 [PATCH] Btrfs-progs: save us an unnecessary ioctl call Wang Shilong
2014-05-13  9:05 ` [PATCH] Btrfs: set right total device count for seeding support Wang Shilong
2014-05-16 14:14   ` Anand Jain
2014-05-16 14:44     ` Anand Jain
2014-05-16 14:50       ` Shilong Wang
2014-05-16 15:06         ` Anand Jain
2014-05-16 14:45     ` Shilong Wang
2014-05-13 10:48 ` [PATCH] Btrfs-progs: save us an unnecessary ioctl call Stefan Behrens
2014-05-13 11:26   ` Wang Shilong
2014-05-14  5:32 ` Anand Jain
2014-05-15 17:06 ` David Sterba
2014-05-16  4:32   ` Anand Jain
2014-05-16  4:58     ` Wang Shilong
2014-05-16 12:14       ` David Sterba

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).