linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Anand Jain <Anand.Jain@oracle.com>
To: Qu Wenruo <quwenruo@cn.fujitsu.com>,
	linux-btrfs <linux-btrfs@vger.kernel.org>
Subject: Re: Should btrfs reuse the src_dev's dev UUID when doing dev replacing?
Date: Wed, 04 Jun 2014 11:27:34 +0800	[thread overview]
Message-ID: <538E9226.5090509@oracle.com> (raw)
In-Reply-To: <538D6B0E.70903@cn.fujitsu.com>


Hi Qu,

  in-line below.

On 03/06/14 14:28, Qu Wenruo wrote:
>
> -------- Original Message --------
> Subject: Re: Should btrfs reuse the src_dev's dev UUID when doing dev
> replacing?
> From: Anand Jain <Anand.Jain@oracle.com>
> To: Qu Wenruo <quwenruo@cn.fujitsu.com>, linux-btrfs
> <linux-btrfs@vger.kernel.org>
> Date: 2014年05月22日 11:09
>>
>>
>>  Thanks Qu for bringing up this topic. We definitely need some focus
>>  on the btrfs volume management related bugs/features/enhancements.
>>
>>  more inline..
>>
>> On 22/05/14 09:35, Qu Wenruo wrote:
>>> 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
>>
>>  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.
>>
>>> [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)
>>
>>  Yep that the right way IMO too. UUID must be unique to disk, even in
>>  the case of replace.
>>
>>> 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?

  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



> Thanks,
> Qu
>>
>>> 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 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

      reply	other threads:[~2014-06-04  3:24 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-22  1:35 Should btrfs reuse the src_dev's dev UUID when doing dev replacing? Qu Wenruo
2014-05-22  3:09 ` Anand Jain
2014-06-03  6:28   ` Qu Wenruo
2014-06-03  6:28   ` Qu Wenruo
2014-06-04  3:27     ` Anand Jain [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=538E9226.5090509@oracle.com \
    --to=anand.jain@oracle.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=quwenruo@cn.fujitsu.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).