* [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: 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: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
* 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-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