| Message ID | 1399971906-1237-2-git-send-email-wangsl.fnst@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
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
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
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
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
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
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);
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(-)