On 11/17/2017 07:28 AM, Liu Bo wrote:
On Sun, Nov 12, 2017 at 06:56:50PM +0800, Anand Jain wrote:
If the device is not present at the time of (-o degrade) mount
the mount context will create a dummy missing struct btrfs_device.
Later this device may reappear after the FS is mounted.
This commit log doesn't explain what would happen when the missing
device re-appears.
Hm. You mean in the current design without this patch.?
Just nothing it gives a false impression to the user by
'btrfs dev ready' and 'btrfs fi show' that missing device
has been included. Will update change log.
So this
patch handles that case by going through the open_device steps
which this device missed and finally adds to the device alloc list.
So now with this patch, to bring back the missing device user can run,
btrfs dev scan <path-of-missing-device>
Most common distros already have some udev rules of btrfs to process a
reappeared disk.
Right. Do you see anything that will break ?
Planning to add comments [1] in v2 for more clarity around this.
Signed-off-by: Anand Jain <anand.j...@oracle.com>
---
This patch needs:
[PATCH 0/4] factor __btrfs_open_devices()
fs/btrfs/volumes.c | 43 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 41 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index d24e966ee29f..e7dd996831f2 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -769,8 +769,47 @@ static noinline int device_list_add(const char *path,
rcu_string_free(device->name);
rcu_assign_pointer(device->name, name);
if (device->missing) {
- fs_devices->missing_devices--;
- device->missing = 0;
+ int ret;
+ struct btrfs_fs_info *fs_info = fs_devices->fs_info;
+ fmode_t fmode = FMODE_READ | FMODE_WRITE | FMODE_EXCL;
+
+ if (btrfs_super_flags(disk_super) &
BTRFS_SUPER_FLAG_SEEDING)
+ fmode &= ~FMODE_WRITE;
+
+ /*
+ * Missing can be set only when FS is mounted.
+ * So here its always fs_devices->opened > 0 and most
+ * of the struct device members are already updated by
+ * the mount process even if this device was missing, so
+ * now follow the normal open device procedure for this
+ * device. The scrub will take care of filling the
+ * missing stripes for raid56 and balance for raid1 and
+ * raid10.
+ */
The idea looks good to me.
What if users skip balance/scrub given both could take a while?
Then btrfs takes the disks back and reads content from it, and it's OK
for raid1/10 as they may have a good copy,
Yes, except for the split brain situation for which patches are
in the ML, review comments appreciated.
but in case of raid56, it
might hit the reconstruction bug if two disks are missing and added
back, which I tried to address recently.
At least ->writable should not be set until balance/scrub completes
the re-sync job.
Hm. Why ?
Thanks,
-liubo
+ ASSERT(fs_devices->opened);
+ mutex_lock(&fs_devices->device_list_mutex);
+ mutex_lock(&fs_info->chunk_mutex);
[1]
/*
* By not failing for the
* reason that btrfs_open_one_device() could
* fail, it will keep the original behaviors as
* it is for now. That's fix me for later.
*/
+ ret = btrfs_open_one_device(fs_devices, device, fmode,
+ fs_info->bdev_holder);
+ if (!ret) {
/*
* Making sure missing is set to 0
* only when bdev != NULL is as expected
* at most of the places in the code.
* Further, what if user fixes the
* dev and reruns dev scan, then again
* we need to come here.
*/
+ fs_devices->missing_devices--;
+ device->missing = 0;
+ btrfs_clear_opt(fs_info->mount_opt, DEGRADED);
+ btrfs_warn(fs_info,
+ "BTRFS: device %s devid %llu uuid %pU
joined\n",
+ path, devid, device->uuid);
+ }
Also added check for missing as below in v2.
--------------
@@ -725,7 +725,9 @@ static noinline int device_list_add(const char *path,
ret = 1;
device->fs_devices = fs_devices;
- } else if (!device->name || strcmp(device->name->str, path)) {
+ } else if (!device->name ||
+ strcmp(device->name->str, path) ||
+ device->missing) {
/*
* When FS is already mounted.
* 1. If you are here and if the device->name is NULL that
------------
Thanks, Anand
+
+ if (device->writeable &&
+ !device->is_tgtdev_for_dev_replace) {
+ fs_devices->total_rw_bytes +=
device->total_bytes;
+ atomic64_add(device->total_bytes -
+ device->bytes_used,
+ &fs_info->free_chunk_space);
+ }
+ device->in_fs_metadata = 1;
+ mutex_unlock(&fs_devices->fs_info->chunk_mutex);
+ mutex_unlock(&fs_devices->device_list_mutex);
}
}
--
2.13.1
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
