* [PATCH] btrfs: ioctl BTRFS_IOC_FS_INFO and BTRFS_IOC_DEV_INFO miss-matched with slots
@ 2014-05-16 14:06 Anand Jain
2014-05-16 14:40 ` Shilong Wang
0 siblings, 1 reply; 5+ messages in thread
From: Anand Jain @ 2014-05-16 14:06 UTC (permalink / raw)
To: linux-btrfs; +Cc: wangsl.fnst
BTRFS_IOC_FS_INFO return num_devices which does not include seed disks,
BTRFS_IOC_DEV_INFO fetches seed disk when probed. So in this case hits
the btrfs-progs bug:
get_fs_info()
::
BUG_ON(ndevs >= fi_args->num_devices);
which is very easy to hit by using btrfs filesystem show.
This patch will make BTRFS_IOC_DEV_INFO ioctl to provide disks only
of the FSID being probed (seed disks are under different FSID).
which means when seed is still not deleted from the sprout the btrfs
filesystem show command will show disks them under their
respective FSIDs
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
fs/btrfs/ioctl.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index ff27c08..902d279 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2584,7 +2584,8 @@ static long btrfs_ioctl_dev_info(struct btrfs_root *root, void __user *arg)
s_uuid = di_args->uuid;
mutex_lock(&fs_devices->device_list_mutex);
- dev = btrfs_find_device(root->fs_info, di_args->devid, s_uuid, NULL);
+ dev = btrfs_find_device(root->fs_info, di_args->devid, s_uuid,
+ fs_devices->fsid);
if (!dev) {
ret = -ENODEV;
--
1.7.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] btrfs: ioctl BTRFS_IOC_FS_INFO and BTRFS_IOC_DEV_INFO miss-matched with slots
2014-05-16 14:06 [PATCH] btrfs: ioctl BTRFS_IOC_FS_INFO and BTRFS_IOC_DEV_INFO miss-matched with slots Anand Jain
@ 2014-05-16 14:40 ` Shilong Wang
2014-05-16 15:13 ` Anand Jain
0 siblings, 1 reply; 5+ messages in thread
From: Shilong Wang @ 2014-05-16 14:40 UTC (permalink / raw)
To: Anand Jain; +Cc: linux-btrfs, Wang Shilong
2014-05-16 22:06 GMT+08:00 Anand Jain <anand.jain@oracle.com>:
> BTRFS_IOC_FS_INFO return num_devices which does not include seed disks,
> BTRFS_IOC_DEV_INFO fetches seed disk when probed. So in this case hits
> the btrfs-progs bug:
> get_fs_info()
> ::
> BUG_ON(ndevs >= fi_args->num_devices);
> which is very easy to hit by using btrfs filesystem show.
>
> This patch will make BTRFS_IOC_DEV_INFO ioctl to provide disks only
> of the FSID being probed (seed disks are under different FSID).
>
> which means when seed is still not deleted from the sprout the btrfs
> filesystem show command will show disks them under their
> respective FSIDs
Just a quick comment:
Newly created filesystem is based on existed seeding filesystem.
Though they have different fsid, newly created filesystem devices
should include seed devices, from seed device filesytem, they can not
see newly created filesystem devices! No?
Will the follow patch change the above behavior?
Thanks,
Wang
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> fs/btrfs/ioctl.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index ff27c08..902d279 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -2584,7 +2584,8 @@ static long btrfs_ioctl_dev_info(struct btrfs_root *root, void __user *arg)
> s_uuid = di_args->uuid;
>
> mutex_lock(&fs_devices->device_list_mutex);
> - dev = btrfs_find_device(root->fs_info, di_args->devid, s_uuid, NULL);
> + dev = btrfs_find_device(root->fs_info, di_args->devid, s_uuid,
> + fs_devices->fsid);
>
> if (!dev) {
> ret = -ENODEV;
> --
> 1.7.1
>
> --
> 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] 5+ messages in thread* Re: [PATCH] btrfs: ioctl BTRFS_IOC_FS_INFO and BTRFS_IOC_DEV_INFO miss-matched with slots
2014-05-16 14:40 ` Shilong Wang
@ 2014-05-16 15:13 ` Anand Jain
2014-05-16 15:23 ` Shilong Wang
0 siblings, 1 reply; 5+ messages in thread
From: Anand Jain @ 2014-05-16 15:13 UTC (permalink / raw)
To: Shilong Wang; +Cc: linux-btrfs, Wang Shilong
On 16/05/14 22:40, Shilong Wang wrote:
> 2014-05-16 22:06 GMT+08:00 Anand Jain <anand.jain@oracle.com>:
>> BTRFS_IOC_FS_INFO return num_devices which does not include seed disks,
>> BTRFS_IOC_DEV_INFO fetches seed disk when probed. So in this case hits
>> the btrfs-progs bug:
>> get_fs_info()
>> ::
>> BUG_ON(ndevs >= fi_args->num_devices);
>> which is very easy to hit by using btrfs filesystem show.
>>
>> This patch will make BTRFS_IOC_DEV_INFO ioctl to provide disks only
>> of the FSID being probed (seed disks are under different FSID).
>>
>> which means when seed is still not deleted from the sprout the btrfs
>> filesystem show command will show disks them under their
>> respective FSIDs
>
> Just a quick comment:
>
> Newly created filesystem is based on existed seeding filesystem.
> Though they have different fsid, newly created filesystem devices
> should include seed devices, from seed device filesytem, they can not
> see newly created filesystem devices! No?
>
> Will the follow patch change the above behavior?
Yes, as the dev info list does not contain the seed disk
so is the btrfs fi show output. Do you think its better
to show seed and sprout "disks together" ? other way is
indicate the seed fsid against the added disk until
seed is deleted.
Thanks. Anand
> Thanks,
> Wang
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>> fs/btrfs/ioctl.c | 3 ++-
>> 1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>> index ff27c08..902d279 100644
>> --- a/fs/btrfs/ioctl.c
>> +++ b/fs/btrfs/ioctl.c
>> @@ -2584,7 +2584,8 @@ static long btrfs_ioctl_dev_info(struct btrfs_root *root, void __user *arg)
>> s_uuid = di_args->uuid;
>>
>> mutex_lock(&fs_devices->device_list_mutex);
>> - dev = btrfs_find_device(root->fs_info, di_args->devid, s_uuid, NULL);
>> + dev = btrfs_find_device(root->fs_info, di_args->devid, s_uuid,
>> + fs_devices->fsid);
>>
>> if (!dev) {
>> ret = -ENODEV;
>> --
>> 1.7.1
>>
>> --
>> 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] 5+ messages in thread* Re: [PATCH] btrfs: ioctl BTRFS_IOC_FS_INFO and BTRFS_IOC_DEV_INFO miss-matched with slots
2014-05-16 15:13 ` Anand Jain
@ 2014-05-16 15:23 ` Shilong Wang
0 siblings, 0 replies; 5+ messages in thread
From: Shilong Wang @ 2014-05-16 15:23 UTC (permalink / raw)
To: Anand Jain; +Cc: linux-btrfs, Wang Shilong
2014-05-16 23:13 GMT+08:00 Anand Jain <Anand.Jain@oracle.com>:
>
>
> On 16/05/14 22:40, Shilong Wang wrote:
>>
>> 2014-05-16 22:06 GMT+08:00 Anand Jain <anand.jain@oracle.com>:
>>>
>>> BTRFS_IOC_FS_INFO return num_devices which does not include seed disks,
>>> BTRFS_IOC_DEV_INFO fetches seed disk when probed. So in this case hits
>>> the btrfs-progs bug:
>>> get_fs_info()
>>> ::
>>> BUG_ON(ndevs >= fi_args->num_devices);
>>> which is very easy to hit by using btrfs filesystem show.
>>>
>>> This patch will make BTRFS_IOC_DEV_INFO ioctl to provide disks only
>>> of the FSID being probed (seed disks are under different FSID).
>>>
>>> which means when seed is still not deleted from the sprout the btrfs
>>> filesystem show command will show disks them under their
>>> respective FSIDs
>>
>>
>> Just a quick comment:
>>
>> Newly created filesystem is based on existed seeding filesystem.
>> Though they have different fsid, newly created filesystem devices
>> should include seed devices, from seed device filesytem, they can not
>> see newly created filesystem devices! No?
>>
>> Will the follow patch change the above behavior?
>
>
> Yes, as the dev info list does not contain the seed disk
> so is the btrfs fi show output. Do you think its better
> to show seed and sprout "disks together" ? other way is
> indicate the seed fsid against the added disk until
> seed is deleted.
IMO, i think dev info list should contain seed disks, since new filesystem
can access their data.
Maybe a better way 'file show' will tell users which devices are seed devices..
>
> Thanks. Anand
>
>
>> Thanks,
>> Wang
>>>
>>>
>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>> ---
>>> fs/btrfs/ioctl.c | 3 ++-
>>> 1 files changed, 2 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>>> index ff27c08..902d279 100644
>>> --- a/fs/btrfs/ioctl.c
>>> +++ b/fs/btrfs/ioctl.c
>>> @@ -2584,7 +2584,8 @@ static long btrfs_ioctl_dev_info(struct btrfs_root
>>> *root, void __user *arg)
>>> s_uuid = di_args->uuid;
>>>
>>> mutex_lock(&fs_devices->device_list_mutex);
>>> - dev = btrfs_find_device(root->fs_info, di_args->devid, s_uuid,
>>> NULL);
>>> + dev = btrfs_find_device(root->fs_info, di_args->devid, s_uuid,
>>> + fs_devices->fsid);
>>>
>>> if (!dev) {
>>> ret = -ENODEV;
>>> --
>>> 1.7.1
>>>
>>> --
>>> 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] 5+ messages in thread
* [PATCH] make 'btrfs filesystem show' to work when seeding
@ 2014-08-16 3:08 Anand Jain
2014-08-16 3:08 ` [PATCH] btrfs: ioctl BTRFS_IOC_FS_INFO and BTRFS_IOC_DEV_INFO miss-matched with slots Anand Jain
0 siblings, 1 reply; 5+ messages in thread
From: Anand Jain @ 2014-08-16 3:08 UTC (permalink / raw)
To: linux-btrfs; +Cc: wangsl.fnst, lists
yeah. btrfs filesystem show didn't work any time before as in
the test case below.
mkfs.btrfs /dev/sdb -f
btrfstune -S 1 /dev/sdb
mount /dev/sdb /btrfs
btrfs dev add /dev/sdc /btrfs
btrfs fi show <-- fails.
kindly ref to the commit log for bug and its fix details.
Anand Jain (1):
btrfs-progs: do a separate probe for _transient_ replacing device
As mentioned in the kernel patch btrfs: ioctl
BTRFS_IOC_FS_INFO and BTRFS_IOC_DEV_INFO miss-matched with slots
utils.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
Anand Jain (1):
btrfs: ioctl BTRFS_IOC_FS_INFO and BTRFS_IOC_DEV_INFO miss-matched
with slots
fs/btrfs/ioctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--
2.0.0.153.g79dcccc
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH] btrfs: ioctl BTRFS_IOC_FS_INFO and BTRFS_IOC_DEV_INFO miss-matched with slots
2014-08-16 3:08 [PATCH] make 'btrfs filesystem show' to work when seeding Anand Jain
@ 2014-08-16 3:08 ` Anand Jain
0 siblings, 0 replies; 5+ messages in thread
From: Anand Jain @ 2014-08-16 3:08 UTC (permalink / raw)
To: linux-btrfs; +Cc: wangsl.fnst, lists
ioctl BTRFS_IOC_FS_INFO return num_devices which does _not_ include seed
device, But the following ioctl BTRFS_IOC_DEV_INFO counts and gets seed
disk when probed. So in the userland we hit a count-slot missmatch
bug..
get_fs_info()
::
BUG_ON(ndevs >= fi_args->num_devices);
which hits this bug when we have mounted a seed device.
So to fix this problem here in this patch ioctl BTRFS_IOC_FS_INFO
will provide total_devices instead of num_devices.
This would fix the problem partly. Partly because ealier num_devices
included the replacing device but now total_device does not include
the replacing device. Getting a count which includes a transient device
is rather too in efficient/wrong indeed, because there can be a race
condition where in the time between ioctl BTRFS_IOC_FS_INFO to
BTRFS_IOC_DEV_INFO the replace device operation might have been
completed. So to fix this problem its better that user land btrfs-progs
probes replacing device (at devid 0) separately.
v2:
Agree with Wang's comment. Its better to show seed disks under the
sprout fs, so that user can establish mapping of seed to sprout devices.
So here I am making BTRFS_IOC_FS_INFO to return the total_devices
which would count the seed devices (but not the replacing device).
A btrfs-progs patch will separately probe for devid 0 (which is
a replacing transient device when present).
v1:
This patch will make BTRFS_IOC_DEV_INFO ioctl to provide disks only
of the FSID being probed (seed disks are under different FSID).
which means when seed is still not deleted from the sprout the btrfs
filesystem show command will show disks them under their
respective FSIDs
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
fs/btrfs/ioctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index da73ab3..05dd88b 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2725,7 +2725,7 @@ static long btrfs_ioctl_fs_info(struct btrfs_root *root, void __user *arg)
return -ENOMEM;
mutex_lock(&fs_devices->device_list_mutex);
- fi_args->num_devices = fs_devices->num_devices;
+ fi_args->num_devices = fs_devices->total_devices;
memcpy(&fi_args->fsid, root->fs_info->fsid, sizeof(fi_args->fsid));
list_for_each_entry_safe(device, next, &fs_devices->devices, dev_list) {
--
2.0.0.153.g79dcccc
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-08-18 8:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-16 14:06 [PATCH] btrfs: ioctl BTRFS_IOC_FS_INFO and BTRFS_IOC_DEV_INFO miss-matched with slots Anand Jain
2014-05-16 14:40 ` Shilong Wang
2014-05-16 15:13 ` Anand Jain
2014-05-16 15:23 ` Shilong Wang
2014-08-16 3:08 [PATCH] make 'btrfs filesystem show' to work when seeding Anand Jain
2014-08-16 3:08 ` [PATCH] btrfs: ioctl BTRFS_IOC_FS_INFO and BTRFS_IOC_DEV_INFO miss-matched with slots Anand Jain
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).