Should btrfs reuse the src_dev's dev UUID when doing dev replacing?

DISCUSSION Qu Wenruo 2 participants 5 messages May 22, 2014 – Jun 4, 2014 Btrfs file system

Share & tools

Qu Wenruo

author

2014-05-22 01:34 UTC #0 lore raw

Hi,

[Current dev replace]
As kernel codes show, 'btrfs dev replace' will swap tgt_dev's uuid with src_dev's uuid. This method works fine most of the time, since it doesn't need to change the chunk tree.

[Problem with re-appear missing device]
(Anand Jain reported the problem in Jan 2014)
Take the following suitiuation as example:
/dev/sda, /dev/sdb, /dev/sdc as btrfs RAID1.
1, 2, 3 as their dev id.

1)/dev/sdb is missing,
Mount them in degraded mode.

2) 'btrfs dev replace start 2 /dev/sdd' will replace missing /dev/sdb.

3) /dev/sdb is online again.

4) umount /BTRFS/MOUNT/POINT; mount /dev/sda
After mount, btrfs will still use /dev/sdb but not /dev/sdd

[Cause of the bug]
When this comes to missing device, since the src_dev is missing, neither UUID swap nor superblock wipe will work. So if the device reappears, next mount will scan the the fsid and dev uuid, and if btrfs scan the re-appeared device first, it will use the re-appeared device.

[Method to fix]
IMO there are 2 possible method to fix the bug.
1) Don't reuse the src_dev's dev UUID.
I don't think any of the UUID in btrfs should be reused, so if every device in btrfs has its own UUID, it is quite easy to distinguish different devices, and even don't need to wipe the superblock of src_dev. (But superblock wipe is still needed for other reasons)

2) Do generation check in device_list_add.
When multiple devices with same dev UUID is found, only add the one whose generation is the same with other deivces.
IMO this is just a workaround.

I think it is better to be decided before any related patch sent.

Any suggestions?

Thanks,
Qu

Anand Jain

2014-05-22 03:06 UTC #1 lore raw

Thanks Qu for bringing up this topic. We definitely need some focus on the btrfs volume management related bugs/features/enhancements.

more inline..

Quoted 17 lines · Qu Wenruo

Hi,
[Current dev replace]
As kernel codes show, 'btrfs dev replace' will swap tgt_dev's uuid with src_dev's uuid.
This method works fine most of the time, since it doesn't need to change the chunk tree.
[Problem with re-appear missing device]
(Anand Jain reported the problem in Jan 2014)
Take the following suitiuation as example:
/dev/sda, /dev/sdb, /dev/sdc as btrfs RAID1.
1, 2, 3 as their dev id.
1)/dev/sdb is missing,
Mount them in degraded mode.

Quoted 6 lines

2) 'btrfs dev replace start 2 /dev/sdd' will replace missing /dev/sdb.
3) /dev/sdb is online again.
4) umount /BTRFS/MOUNT/POINT; mount /dev/sda
After mount, btrfs will still use /dev/sdb but not /dev/sdd

Yeah its weird that grouping depends on the mercy of chronological oder of device probing. The _last_ device probed stays in the list. But the most weird is if FS is mounted and is followed with the dev scan it would just overwrite the btrfs_device struct.
I have sent out interim fix to both of these bugs a long time back.

Quoted 7 lines

[Cause of the bug]
When this comes to missing device, since the src_dev is missing, neither UUID swap nor superblock wipe will
work. So if the device reappears, next mount will scan the the fsid and dev uuid, and if btrfs scan the re-appeared
device first, it will use the re-appeared device.

Quoted 8 lines

[Method to fix]
IMO there are 2 possible method to fix the bug.
1) Don't reuse the src_dev's dev UUID.
I don't think any of the UUID in btrfs should be reused, so if every device in btrfs has its own UUID,
it is quite easy to distinguish different devices, and even don't need to wipe the superblock of src_dev.
(But superblock wipe is still needed for other reasons)

Yep that the right way IMO too. UUID must be unique to disk, even in the case of replace.

Quoted 5 lines

2) Do generation check in device_list_add.
When multiple devices with same dev UUID is found, only add the one whose generation is the same with
other deivces.
IMO this is just a workaround.

yes an interim fix, patch was sent out a long time back.

Quoted 10 lines

I think it is better to be decided before any related patch sent.
Any suggestions?
Thanks,
Qu
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Qu Wenruo

author

2014-06-03 06:27 UTC #2 lore raw

-------- Original Message --------
Subject: Re: Should btrfs reuse the src_dev's dev UUID when doing dev replacing?
From: Anand Jain <[email protected]>
To: Qu Wenruo <[email protected]>, linux-btrfs <[email protected]>
Date: 2014年05月22日 11:09

Quoted 57 lines

Thanks Qu for bringing up this topic. We definitely need some focus on the btrfs volume management related bugs/features/enhancements.
more inline..
Quoted 17 lines · Qu Wenruo
Hi,
[Current dev replace]
As kernel codes show, 'btrfs dev replace' will swap tgt_dev's uuid with src_dev's uuid.
This method works fine most of the time, since it doesn't need to change the chunk tree.
[Problem with re-appear missing device]
(Anand Jain reported the problem in Jan 2014)
Take the following suitiuation as example:
/dev/sda, /dev/sdb, /dev/sdc as btrfs RAID1.
1, 2, 3 as their dev id.
1)/dev/sdb is missing,
Mount them in degraded mode.
Quoted 6 lines
2) 'btrfs dev replace start 2 /dev/sdd' will replace missing /dev/sdb.
3) /dev/sdb is online again.
4) umount /BTRFS/MOUNT/POINT; mount /dev/sda
After mount, btrfs will still use /dev/sdb but not /dev/sdd
Yeah its weird that grouping depends on the mercy of chronological oder of device probing. The _last_ device probed stays in the list. But the most weird is if FS is mounted and is followed with the dev scan it would just overwrite the btrfs_device struct.
I have sent out interim fix to both of these bugs a long time back.
Quoted 7 lines
[Cause of the bug]
When this comes to missing device, since the src_dev is missing, neither UUID swap nor superblock wipe will
work. So if the device reappears, next mount will scan the the fsid and dev uuid, and if btrfs scan the re-appeared
device first, it will use the re-appeared device.
Quoted 8 lines
[Method to fix]
IMO there are 2 possible method to fix the bug.
1) Don't reuse the src_dev's dev UUID.
I don't think any of the UUID in btrfs should be reused, so if every device in btrfs has its own UUID,
it is quite easy to distinguish different devices, and even don't need to wipe the superblock of src_dev.
(But superblock wipe is still needed for other reasons)
Yep that the right way IMO too. UUID must be unique to disk, even in the case of replace.

Quoted 7 lines

Quoted 5 lines
2) Do generation check in device_list_add.
When multiple devices with same dev UUID is found, only add the one whose generation is the same with
other deivces.
IMO this is just a workaround.
yes an interim fix, patch was sent out a long time back.

BTW, I haven't seen a new version patch fixing device_list_add() function after Wang's comment.

What is the process now?

If you are busy working on other bugs, would you mind me making the device_list_add() check patch?

Thanks,
Qu

Quoted 11 lines

Quoted 11 lines
I think it is better to be decided before any related patch sent.
Any suggestions?
Thanks,
Qu
Signature · 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Qu Wenruo

author

2014-06-03 06:27 UTC #3 lore raw

In reply to Anand Jain

Quoted 57 lines

Thanks Qu for bringing up this topic. We definitely need some focus on the btrfs volume management related bugs/features/enhancements.
more inline..
Quoted 17 lines · Qu Wenruo
Hi,
[Current dev replace]
As kernel codes show, 'btrfs dev replace' will swap tgt_dev's uuid with src_dev's uuid.
This method works fine most of the time, since it doesn't need to change the chunk tree.
[Problem with re-appear missing device]
(Anand Jain reported the problem in Jan 2014)
Take the following suitiuation as example:
/dev/sda, /dev/sdb, /dev/sdc as btrfs RAID1.
1, 2, 3 as their dev id.
1)/dev/sdb is missing,
Mount them in degraded mode.
Quoted 6 lines
2) 'btrfs dev replace start 2 /dev/sdd' will replace missing /dev/sdb.
3) /dev/sdb is online again.
4) umount /BTRFS/MOUNT/POINT; mount /dev/sda
After mount, btrfs will still use /dev/sdb but not /dev/sdd
Yeah its weird that grouping depends on the mercy of chronological oder of device probing. The _last_ device probed stays in the list. But the most weird is if FS is mounted and is followed with the dev scan it would just overwrite the btrfs_device struct.
I have sent out interim fix to both of these bugs a long time back.
Quoted 7 lines
[Cause of the bug]
When this comes to missing device, since the src_dev is missing, neither UUID swap nor superblock wipe will
work. So if the device reappears, next mount will scan the the fsid and dev uuid, and if btrfs scan the re-appeared
device first, it will use the re-appeared device.
Quoted 8 lines
[Method to fix]
IMO there are 2 possible method to fix the bug.
1) Don't reuse the src_dev's dev UUID.
I don't think any of the UUID in btrfs should be reused, so if every device in btrfs has its own UUID,
it is quite easy to distinguish different devices, and even don't need to wipe the superblock of src_dev.
(But superblock wipe is still needed for other reasons)
Yep that the right way IMO too. UUID must be unique to disk, even in the case of replace.

Quoted 7 lines

Quoted 5 lines
2) Do generation check in device_list_add.
When multiple devices with same dev UUID is found, only add the one whose generation is the same with
other deivces.
IMO this is just a workaround.
yes an interim fix, patch was sent out a long time back.

BTW, I haven't seen a new version patch fixing device_list_add() function after Wang's comment.

What is the process now?

If you are busy working on other bugs, would you mind me making the device_list_add() check patch?

Thanks,
Qu

Quoted 11 lines

Quoted 11 lines
I think it is better to be decided before any related patch sent.
Any suggestions?
Thanks,
Qu
Signature · 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Anand Jain

2014-06-04 03:24 UTC #4 lore raw

Hi Qu,

in-line below.

Quoted 73 lines · Qu Wenruo

-------- Original Message --------
Subject: Re: Should btrfs reuse the src_dev's dev UUID when doing dev replacing?
From: Anand Jain <[email protected]>
To: Qu Wenruo <[email protected]>, linux-btrfs <[email protected]>
Date: 2014年05月22日 11:09
Quoted 57 lines
Thanks Qu for bringing up this topic. We definitely need some focus on the btrfs volume management related bugs/features/enhancements.
more inline..
Quoted 17 lines · Qu Wenruo
Hi,
[Current dev replace]
As kernel codes show, 'btrfs dev replace' will swap tgt_dev's uuid with src_dev's uuid.
This method works fine most of the time, since it doesn't need to change the chunk tree.
[Problem with re-appear missing device]
(Anand Jain reported the problem in Jan 2014)
Take the following suitiuation as example:
/dev/sda, /dev/sdb, /dev/sdc as btrfs RAID1.
1, 2, 3 as their dev id.
1)/dev/sdb is missing,
Mount them in degraded mode.
Quoted 6 lines
2) 'btrfs dev replace start 2 /dev/sdd' will replace missing /dev/sdb.
3) /dev/sdb is online again.
4) umount /BTRFS/MOUNT/POINT; mount /dev/sda
After mount, btrfs will still use /dev/sdb but not /dev/sdd
Yeah its weird that grouping depends on the mercy of chronological oder of device probing. The _last_ device probed stays in the list. But the most weird is if FS is mounted and is followed with the dev scan it would just overwrite the btrfs_device struct.
I have sent out interim fix to both of these bugs a long time back.
Quoted 7 lines
[Cause of the bug]
When this comes to missing device, since the src_dev is missing, neither UUID swap nor superblock wipe will
work. So if the device reappears, next mount will scan the the fsid and dev uuid, and if btrfs scan the re-appeared
device first, it will use the re-appeared device.
Quoted 8 lines
[Method to fix]
IMO there are 2 possible method to fix the bug.
1) Don't reuse the src_dev's dev UUID.
I don't think any of the UUID in btrfs should be reused, so if every device in btrfs has its own UUID,
it is quite easy to distinguish different devices, and even don't need to wipe the superblock of src_dev.
(But superblock wipe is still needed for other reasons)
Yep that the right way IMO too. UUID must be unique to disk, even in the case of replace.
Quoted 7 lines
Quoted 5 lines
2) Do generation check in device_list_add.
When multiple devices with same dev UUID is found, only add the one whose generation is the same with
other deivces.
IMO this is just a workaround.
yes an interim fix, patch was sent out a long time back.

Quoted 7 lines

BTW, I haven't seen a new version patch fixing device_list_add() function after Wang's comment.
What is the process now?
If you are busy working on other bugs, would you mind me making the device_list_add() check patch?

Yes. some challenges to get that based on the generation number. too many limitations. and patch created didn't pass all the tests. so I didn't send that patch.
But I was talking about this patch (sorry to confuse you).

Btrfs: device_list_add() should not update list when mounted

And as of now when its unmounted we expect user to wipe SB of the disk which should not belong to the fsid. which will solve the problem as well. but a bit of hard work though. (there is a chance to notice the _actual_ disks being used after the fs is mounted)

appreciate your follow up on this. Kindly let me know if this plan solves the problem reasonably well. (for now).

Thanks, Anand

Quoted 19 lines

Thanks,
Qu
Quoted 5 lines
Quoted 5 lines
I think it is better to be decided before any related patch sent.
Any suggestions?
Quoted 7 lines
Quoted 7 lines
Thanks,
Qu
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [email protected]
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html