[PATCH] btrfs: handle dynamically reappearing missing device

[PATCH] btrfs: handle dynamically reappearing missing device

[Anand Jain]

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. 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>

Signed-off-by: Anand Jain <anand.jain@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.
+			 */
+			ASSERT(fs_devices->opened);
+			mutex_lock(&fs_devices->device_list_mutex);
+			mutex_lock(&fs_info->chunk_mutex);
+			ret = btrfs_open_one_device(fs_devices, device, fmode,
+							fs_info->bdev_holder);
+			if (!ret) {
+				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);
+			}
+
+			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

Re: [PATCH] btrfs: handle dynamically reappearing missing device

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 7598 bytes --]

Hi Anand,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on btrfs/next]
[also build test ERROR on v4.14 next-20171114]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Anand-Jain/btrfs-handle-dynamically-reappearing-missing-device/20171115-143047
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mason/linux-btrfs.git next
config: sparc64-allyesconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sparc64 

All errors (new ones prefixed by >>):

   fs/btrfs/volumes.c: In function 'device_list_add':
>> fs/btrfs/volumes.c:732:10: error: implicit declaration of function 'btrfs_open_one_device'; did you mean 'btrfs_scan_one_device'? [-Werror=implicit-function-declaration]
       ret = btrfs_open_one_device(fs_devices, device, fmode,
             ^~~~~~~~~~~~~~~~~~~~~
             btrfs_scan_one_device
   cc1: some warnings being treated as errors

vim +732 fs/btrfs/volumes.c

   610	
   611	/*
   612	 * Add new device to list of registered devices
   613	 *
   614	 * Returns:
   615	 * 1   - first time device is seen
   616	 * 0   - device already known
   617	 * < 0 - error
   618	 */
   619	static noinline int device_list_add(const char *path,
   620				   struct btrfs_super_block *disk_super,
   621				   u64 devid, struct btrfs_fs_devices **fs_devices_ret)
   622	{
   623		struct btrfs_device *device;
   624		struct btrfs_fs_devices *fs_devices;
   625		struct rcu_string *name;
   626		int ret = 0;
   627		u64 found_transid = btrfs_super_generation(disk_super);
   628	
   629		fs_devices = find_fsid(disk_super->fsid);
   630		if (!fs_devices) {
   631			fs_devices = alloc_fs_devices(disk_super->fsid);
   632			if (IS_ERR(fs_devices))
   633				return PTR_ERR(fs_devices);
   634	
   635			list_add(&fs_devices->list, &fs_uuids);
   636	
   637			device = NULL;
   638		} else {
   639			device = __find_device(&fs_devices->devices, devid,
   640					       disk_super->dev_item.uuid);
   641		}
   642	
   643		if (!device) {
   644			if (fs_devices->opened)
   645				return -EBUSY;
   646	
   647			device = btrfs_alloc_device(NULL, &devid,
   648						    disk_super->dev_item.uuid);
   649			if (IS_ERR(device)) {
   650				/* we can safely leave the fs_devices entry around */
   651				return PTR_ERR(device);
   652			}
   653	
   654			name = rcu_string_strdup(path, GFP_NOFS);
   655			if (!name) {
   656				kfree(device);
   657				return -ENOMEM;
   658			}
   659			rcu_assign_pointer(device->name, name);
   660	
   661			mutex_lock(&fs_devices->device_list_mutex);
   662			list_add_rcu(&device->dev_list, &fs_devices->devices);
   663			fs_devices->num_devices++;
   664			mutex_unlock(&fs_devices->device_list_mutex);
   665	
   666			ret = 1;
   667			device->fs_devices = fs_devices;
   668		} else if (!device->name || strcmp(device->name->str, path)) {
   669			/*
   670			 * When FS is already mounted.
   671			 * 1. If you are here and if the device->name is NULL that
   672			 *    means this device was missing at time of FS mount.
   673			 * 2. If you are here and if the device->name is different
   674			 *    from 'path' that means either
   675			 *      a. The same device disappeared and reappeared with
   676			 *         different name. or
   677			 *      b. The missing-disk-which-was-replaced, has
   678			 *         reappeared now.
   679			 *
   680			 * We must allow 1 and 2a above. But 2b would be a spurious
   681			 * and unintentional.
   682			 *
   683			 * Further in case of 1 and 2a above, the disk at 'path'
   684			 * would have missed some transaction when it was away and
   685			 * in case of 2a the stale bdev has to be updated as well.
   686			 * 2b must not be allowed at all time.
   687			 */
   688	
   689			/*
   690			 * For now, we do allow update to btrfs_fs_device through the
   691			 * btrfs dev scan cli after FS has been mounted.  We're still
   692			 * tracking a problem where systems fail mount by subvolume id
   693			 * when we reject replacement on a mounted FS.
   694			 */
   695			if (!fs_devices->opened && found_transid < device->generation) {
   696				/*
   697				 * That is if the FS is _not_ mounted and if you
   698				 * are here, that means there is more than one
   699				 * disk with same uuid and devid.We keep the one
   700				 * with larger generation number or the last-in if
   701				 * generation are equal.
   702				 */
   703				return -EEXIST;
   704			}
   705	
   706			name = rcu_string_strdup(path, GFP_NOFS);
   707			if (!name)
   708				return -ENOMEM;
   709			rcu_string_free(device->name);
   710			rcu_assign_pointer(device->name, name);
   711			if (device->missing) {
   712				int ret;
   713				struct btrfs_fs_info *fs_info = fs_devices->fs_info;
   714				fmode_t fmode = FMODE_READ | FMODE_WRITE | FMODE_EXCL;
   715	
   716				if (btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_SEEDING)
   717					fmode &= ~FMODE_WRITE;
   718	
   719				/*
   720				 * Missing can be set only when FS is mounted.
   721				 * So here its always fs_devices->opened > 0 and most
   722				 * of the struct device members are already updated by
   723				 * the mount process even if this device was missing, so
   724				 * now follow the normal open device procedure for this
   725				 * device. The scrub will take care of filling the
   726				 * missing stripes for raid56 and balance for raid1 and
   727				 * raid10.
   728				 */
   729				ASSERT(fs_devices->opened);
   730				mutex_lock(&fs_devices->device_list_mutex);
   731				mutex_lock(&fs_info->chunk_mutex);
 > 732				ret = btrfs_open_one_device(fs_devices, device, fmode,
   733								fs_info->bdev_holder);
   734				if (!ret) {
   735					fs_devices->missing_devices--;
   736					device->missing = 0;
   737					btrfs_clear_opt(fs_info->mount_opt, DEGRADED);
   738					btrfs_warn(fs_info,
   739						"BTRFS: device %s devid %llu uuid %pU joined\n",
   740						path, devid, device->uuid);
   741				}
   742	
   743				if (device->writeable &&
   744						!device->is_tgtdev_for_dev_replace) {
   745					fs_devices->total_rw_bytes += device->total_bytes;
   746					atomic64_add(device->total_bytes -
   747							device->bytes_used,
   748							&fs_info->free_chunk_space);
   749				}
   750				device->in_fs_metadata = 1;
   751				mutex_unlock(&fs_devices->fs_info->chunk_mutex);
   752				mutex_unlock(&fs_devices->device_list_mutex);
   753			}
   754		}
   755	
   756		/*
   757		 * Unmount does not free the btrfs_device struct but would zero
   758		 * generation along with most of the other members. So just update
   759		 * it back. We need it to pick the disk with largest generation
   760		 * (as above).
   761		 */
   762		if (!fs_devices->opened)
   763			device->generation = found_transid;
   764	
   765		/*
   766		 * if there is new btrfs on an already registered device,
   767		 * then remove the stale device entry.
   768		 */
   769		if (ret > 0)
   770			btrfs_free_stale_device(device);
   771	
   772		*fs_devices_ret = fs_devices;
   773	
   774		return ret;
   775	}
   776	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 51512 bytes --]

Re: [PATCH] btrfs: handle dynamically reappearing missing device

[Anand Jain]

On 11/15/2017 03:38 PM, kbuild test robot wrote:
> Hi Anand,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on btrfs/next]
> [also build test ERROR on v4.14 next-20171114]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Anand-Jain/btrfs-handle-dynamically-reappearing-missing-device/20171115-143047
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/mason/linux-btrfs.git next
> config: sparc64-allyesconfig (attached as .config)
> compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
> reproduce:
>          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>          chmod +x ~/bin/make.cross
>          # save the attached .config to linux build tree
>          make.cross ARCH=sparc64
> 
> All errors (new ones prefixed by >>):
> 
>     fs/btrfs/volumes.c: In function 'device_list_add':
>>> fs/btrfs/volumes.c:732:10: error: implicit declaration of function 'btrfs_open_one_device'; did you mean 'btrfs_scan_one_device'? [-Werror=implicit-function-declaration]
>         ret = btrfs_open_one_device(fs_devices, device, fmode,
>               ^~~~~~~~~~~~~~~~~~~~~
>               btrfs_scan_one_device
>     cc1: some warnings being treated as errors


Its missing this patch set in the ML which was sent separately.
  [PATCH 0/4] factor __btrfs_open_devices()

Thanks, Anand


> vim +732 fs/btrfs/volumes.c
> 
>     610	
>     611	/*
>     612	 * Add new device to list of registered devices
>     613	 *
>     614	 * Returns:
>     615	 * 1   - first time device is seen
>     616	 * 0   - device already known
>     617	 * < 0 - error
>     618	 */
>     619	static noinline int device_list_add(const char *path,
>     620				   struct btrfs_super_block *disk_super,
>     621				   u64 devid, struct btrfs_fs_devices **fs_devices_ret)
>     622	{
>     623		struct btrfs_device *device;
>     624		struct btrfs_fs_devices *fs_devices;
>     625		struct rcu_string *name;
>     626		int ret = 0;
>     627		u64 found_transid = btrfs_super_generation(disk_super);
>     628	
>     629		fs_devices = find_fsid(disk_super->fsid);
>     630		if (!fs_devices) {
>     631			fs_devices = alloc_fs_devices(disk_super->fsid);
>     632			if (IS_ERR(fs_devices))
>     633				return PTR_ERR(fs_devices);
>     634	
>     635			list_add(&fs_devices->list, &fs_uuids);
>     636	
>     637			device = NULL;
>     638		} else {
>     639			device = __find_device(&fs_devices->devices, devid,
>     640					       disk_super->dev_item.uuid);
>     641		}
>     642	
>     643		if (!device) {
>     644			if (fs_devices->opened)
>     645				return -EBUSY;
>     646	
>     647			device = btrfs_alloc_device(NULL, &devid,
>     648						    disk_super->dev_item.uuid);
>     649			if (IS_ERR(device)) {
>     650				/* we can safely leave the fs_devices entry around */
>     651				return PTR_ERR(device);
>     652			}
>     653	
>     654			name = rcu_string_strdup(path, GFP_NOFS);
>     655			if (!name) {
>     656				kfree(device);
>     657				return -ENOMEM;
>     658			}
>     659			rcu_assign_pointer(device->name, name);
>     660	
>     661			mutex_lock(&fs_devices->device_list_mutex);
>     662			list_add_rcu(&device->dev_list, &fs_devices->devices);
>     663			fs_devices->num_devices++;
>     664			mutex_unlock(&fs_devices->device_list_mutex);
>     665	
>     666			ret = 1;
>     667			device->fs_devices = fs_devices;
>     668		} else if (!device->name || strcmp(device->name->str, path)) {
>     669			/*
>     670			 * When FS is already mounted.
>     671			 * 1. If you are here and if the device->name is NULL that
>     672			 *    means this device was missing at time of FS mount.
>     673			 * 2. If you are here and if the device->name is different
>     674			 *    from 'path' that means either
>     675			 *      a. The same device disappeared and reappeared with
>     676			 *         different name. or
>     677			 *      b. The missing-disk-which-was-replaced, has
>     678			 *         reappeared now.
>     679			 *
>     680			 * We must allow 1 and 2a above. But 2b would be a spurious
>     681			 * and unintentional.
>     682			 *
>     683			 * Further in case of 1 and 2a above, the disk at 'path'
>     684			 * would have missed some transaction when it was away and
>     685			 * in case of 2a the stale bdev has to be updated as well.
>     686			 * 2b must not be allowed at all time.
>     687			 */
>     688	
>     689			/*
>     690			 * For now, we do allow update to btrfs_fs_device through the
>     691			 * btrfs dev scan cli after FS has been mounted.  We're still
>     692			 * tracking a problem where systems fail mount by subvolume id
>     693			 * when we reject replacement on a mounted FS.
>     694			 */
>     695			if (!fs_devices->opened && found_transid < device->generation) {
>     696				/*
>     697				 * That is if the FS is _not_ mounted and if you
>     698				 * are here, that means there is more than one
>     699				 * disk with same uuid and devid.We keep the one
>     700				 * with larger generation number or the last-in if
>     701				 * generation are equal.
>     702				 */
>     703				return -EEXIST;
>     704			}
>     705	
>     706			name = rcu_string_strdup(path, GFP_NOFS);
>     707			if (!name)
>     708				return -ENOMEM;
>     709			rcu_string_free(device->name);
>     710			rcu_assign_pointer(device->name, name);
>     711			if (device->missing) {
>     712				int ret;
>     713				struct btrfs_fs_info *fs_info = fs_devices->fs_info;
>     714				fmode_t fmode = FMODE_READ | FMODE_WRITE | FMODE_EXCL;
>     715	
>     716				if (btrfs_super_flags(disk_super) & BTRFS_SUPER_FLAG_SEEDING)
>     717					fmode &= ~FMODE_WRITE;
>     718	
>     719				/*
>     720				 * Missing can be set only when FS is mounted.
>     721				 * So here its always fs_devices->opened > 0 and most
>     722				 * of the struct device members are already updated by
>     723				 * the mount process even if this device was missing, so
>     724				 * now follow the normal open device procedure for this
>     725				 * device. The scrub will take care of filling the
>     726				 * missing stripes for raid56 and balance for raid1 and
>     727				 * raid10.
>     728				 */
>     729				ASSERT(fs_devices->opened);
>     730				mutex_lock(&fs_devices->device_list_mutex);
>     731				mutex_lock(&fs_info->chunk_mutex);
>   > 732				ret = btrfs_open_one_device(fs_devices, device, fmode,
>     733								fs_info->bdev_holder);
>     734				if (!ret) {
>     735					fs_devices->missing_devices--;
>     736					device->missing = 0;
>     737					btrfs_clear_opt(fs_info->mount_opt, DEGRADED);
>     738					btrfs_warn(fs_info,
>     739						"BTRFS: device %s devid %llu uuid %pU joined\n",
>     740						path, devid, device->uuid);
>     741				}
>     742	
>     743				if (device->writeable &&
>     744						!device->is_tgtdev_for_dev_replace) {
>     745					fs_devices->total_rw_bytes += device->total_bytes;
>     746					atomic64_add(device->total_bytes -
>     747							device->bytes_used,
>     748							&fs_info->free_chunk_space);
>     749				}
>     750				device->in_fs_metadata = 1;
>     751				mutex_unlock(&fs_devices->fs_info->chunk_mutex);
>     752				mutex_unlock(&fs_devices->device_list_mutex);
>     753			}
>     754		}
>     755	
>     756		/*
>     757		 * Unmount does not free the btrfs_device struct but would zero
>     758		 * generation along with most of the other members. So just update
>     759		 * it back. We need it to pick the disk with largest generation
>     760		 * (as above).
>     761		 */
>     762		if (!fs_devices->opened)
>     763			device->generation = found_transid;
>     764	
>     765		/*
>     766		 * if there is new btrfs on an already registered device,
>     767		 * then remove the stale device entry.
>     768		 */
>     769		if (ret > 0)
>     770			btrfs_free_stale_device(device);
>     771	
>     772		*fs_devices_ret = fs_devices;
>     773	
>     774		return ret;
>     775	}
>     776	
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
> 

Re: [PATCH] btrfs: handle dynamically reappearing missing device

[Nikolay Borisov]

On 12.11.2017 12:56, 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. 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>
> 
> Signed-off-by: Anand Jain <anand.jain@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.
> +			 */
> +			ASSERT(fs_devices->opened);
> +			mutex_lock(&fs_devices->device_list_mutex);
> +			mutex_lock(&fs_info->chunk_mutex);
> +			ret = btrfs_open_one_device(fs_devices, device, fmode,
> +							fs_info->bdev_holder);
> +			if (!ret) {
> +				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);
> +			}
> +
> +			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);

nit: You did add the fs_info local var, so for consistency's sake use that
>  		}
>  	}
>  
> 

Re: [PATCH] btrfs: handle dynamically reappearing missing device

[Liu Bo]

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.

> 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.

> 
> Signed-off-by: Anand Jain <anand.jain@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, 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.

Thanks,

-liubo

> +			ASSERT(fs_devices->opened);
> +			mutex_lock(&fs_devices->device_list_mutex);
> +			mutex_lock(&fs_info->chunk_mutex);
> +			ret = btrfs_open_one_device(fs_devices, device, fmode,
> +							fs_info->bdev_holder);
> +			if (!ret) {
> +				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);
> +			}
> +
> +			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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] btrfs: handle dynamically reappearing missing device

[Anand Jain]

On 11/17/2017 03:08 AM, Nikolay Borisov wrote:
> 
> 
> On 12.11.2017 12:56, 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. 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>
>>
>> Signed-off-by: Anand Jain <anand.jain@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.
>> +			 */
>> +			ASSERT(fs_devices->opened);
>> +			mutex_lock(&fs_devices->device_list_mutex);
>> +			mutex_lock(&fs_info->chunk_mutex);
>> +			ret = btrfs_open_one_device(fs_devices, device, fmode,
>> +							fs_info->bdev_holder);
>> +			if (!ret) {
>> +				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);
>> +			}
>> +
>> +			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);
> 
> nit: You did add the fs_info local var, so for consistency's sake use that

  Thanks. Will fix it.

-Anand

>>   		}
>>   	}
>>   
>>

Re: [PATCH] btrfs: handle dynamically reappearing missing device

[Anand Jain]

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.jain@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 majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] btrfs: handle dynamically reappearing missing device

[Anand Jain]

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 and
then device is included in the device list but it missed the
open_device part. 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>

Without this kernel patch, even though 'btrfs fi show' and 'btrfs
dev ready' would tell you that missing device has reappeared
successfully but actually in kernel FS layer it didn't.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
This patch needs:
 [PATCH 0/4]  factor __btrfs_open_devices()

v2:
Add more comments.
Add more change log.
Add to check if device missing is set, to handle the case
dev open fail and user will rerun the dev scan.

 fs/btrfs/volumes.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 60 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 5bd73edc2602..b3224baa6db4 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -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
@@ -769,8 +771,63 @@ 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.
+			 */
+			ASSERT(fs_devices->opened);
+			mutex_lock(&fs_devices->device_list_mutex);
+			mutex_lock(&fs_info->chunk_mutex);
+			/*
+			 * 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 the dev scan, then again
+				 * we need to come here to reset missing.
+				 */
+				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);
+			}
+
+			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_info->chunk_mutex);
+			mutex_unlock(&fs_devices->device_list_mutex);
 		}
 	}
 
-- 
2.7.0

--
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

Re: [PATCH v2] btrfs: handle dynamically reappearing missing device

[Goffredo Baroncelli]

On 11/17/2017 01:36 PM, 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 and
> then device is included in the device list but it missed the
> open_device part. 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.

What happens if the first devices got writes before the "last device" is joined ?

Supposing to have a raid1 pair of devices: sda, sdb

- sda is dissappeared (usb detached ?)
- the filesystem is mounted as
	mount -o degraded /dev/sdb
- some writes happens on /dev/sdb
- the user reattach /dev/sda
- udev run "btrfs dev scan"
- the system joins /dev/sda to the filesystem disks pool

Because the filesystem is a raid1, btrfs may read from /dev/sdb (updated data) or /dev/sda (old data), or worse read something from the former and something from the later (metadata from sda and data from sdb ) ???

Am I missing something ?

BR
G.Baroncelli

-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [PATCH v2] btrfs: handle dynamically reappearing missing device

[Anand Jain]

On 11/18/2017 09:52 PM, Goffredo Baroncelli wrote:
> On 11/17/2017 01:36 PM, 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 and
>> then device is included in the device list but it missed the
>> open_device part. 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.
> 
> What happens if the first devices got writes before the "last device" is joined ?
> 
> Supposing to have a raid1 pair of devices: sda, sdb
> 
> - sda is dissappeared (usb detached ?)
> - the filesystem is mounted as
> 	mount -o degraded /dev/sdb
> - some writes happens on /dev/sdb
> - the user reattach /dev/sda
> - udev run "btrfs dev scan"
> - the system joins /dev/sda to the filesystem disks pool
> 
> Because the filesystem is a raid1, btrfs may read from /dev/sdb (updated data) or /dev/sda (old data), or worse read something from the former and something from the later (metadata from sda and data from sdb ) ???

  Thanks for the test scenario, this case is fine as its read from
  the disk having highest generation number, so we pick sdb in the
  case above.

Thanks, Anand


> Am I missing something ?
> 
> BR
> G.Baroncelli
> 

Re: [PATCH v2] btrfs: handle dynamically reappearing missing device

[Goffredo Baroncelli]

On 11/20/2017 09:19 AM, Anand Jain wrote:
> 
> 
> On 11/18/2017 09:52 PM, Goffredo Baroncelli wrote:
>> On 11/17/2017 01:36 PM, 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 and
>>> then device is included in the device list but it missed the
>>> open_device part. 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.
>>
>> What happens if the first devices got writes before the "last device" is joined ?
>>
>> Supposing to have a raid1 pair of devices: sda, sdb
>>
>> - sda is dissappeared (usb detached ?)
>> - the filesystem is mounted as
>>     mount -o degraded /dev/sdb
>> - some writes happens on /dev/sdb
>> - the user reattach /dev/sda
>> - udev run "btrfs dev scan"
>> - the system joins /dev/sda to the filesystem disks pool
>>
>> Because the filesystem is a raid1, btrfs may read from /dev/sdb (updated data) or /dev/sda (old data), or worse read something from the former and something from the later (metadata from sda and data from sdb ) ???
> 
>  Thanks for the test scenario, this case is fine as its read from
>  the disk having highest generation number, so we pick sdb in the
>  case above.

Ok, so in this case which is the benefit to add a disk ? With a lover number generation, the added will be used at all ?
In this case, would be better an explicit user intervention instead of an automatic one ?


> 
> Thanks, Anand
> 
> 
>> Am I missing something ?
>>
>> BR
>> G.Baroncelli
>>
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [PATCH v2] btrfs: handle dynamically reappearing missing device

[Anand Jain]

On 11/21/2017 03:28 AM, Goffredo Baroncelli wrote:
> On 11/20/2017 09:19 AM, Anand Jain wrote:
>>
>>
>> On 11/18/2017 09:52 PM, Goffredo Baroncelli wrote:
>>> On 11/17/2017 01:36 PM, 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 and
>>>> then device is included in the device list but it missed the
>>>> open_device part. 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.
>>>
>>> What happens if the first devices got writes before the "last device" is joined ?
>>>
>>> Supposing to have a raid1 pair of devices: sda, sdb
>>>
>>> - sda is dissappeared (usb detached ?)
>>> - the filesystem is mounted as
>>>      mount -o degraded /dev/sdb
>>> - some writes happens on /dev/sdb
>>> - the user reattach /dev/sda
>>> - udev run "btrfs dev scan"
>>> - the system joins /dev/sda to the filesystem disks pool
>>>
>>> Because the filesystem is a raid1, btrfs may read from /dev/sdb (updated data) or /dev/sda (old data), or worse read something from the former and something from the later (metadata from sda and data from sdb ) ???
>>
>>   Thanks for the test scenario, this case is fine as its read from
>>   the disk having highest generation number, so we pick sdb in the
>>   case above.
> 
> Ok, so in this case which is the benefit to add a disk ?
> With a lover number generation, the added will be used at all ?

  It will be used for new writes, it will stripe across both the disks.
  Balance is only for the older single chunks, which in the long term
  would go away when we are able to create a degraded raid1 chunks.

> In this case, would be better an explicit user intervention instead of an automatic one ?

   Without this patch the disk already joins the raid group.

   But it had a bug that it missed joining the alloc group.

Thanks, Anand
> 
>>
>> Thanks, Anand
>>
>>
>>> Am I missing something ?
>>>
>>> BR
>>> G.Baroncelli
>>>
>>
> 
> 

Re: [PATCH] btrfs: handle dynamically reappearing missing device

[Liu Bo]

On Fri, Nov 17, 2017 at 07:53:29PM +0800, Anand Jain wrote:
> 
> 
> 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.jain@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 ?
>

(Sorry for the late reply.)

On second thought, write is OK since parity will be calculated along
writes.

Will check the v2.


Thanks,

-liubo

Re: [PATCH v2] btrfs: handle dynamically reappearing missing device

[Anand Jain]

> [..]  would be better an explicit user intervention instead of an automatic one ?

  What is the user intervention method steps that you have in mind ?
  Just curious. Pls remember downtime is not a choice of recovery
  from this context which means FS should be available for the
  applications to perform read/write.

  -o remount was one this which I had skipped for sometime as
  theoretically it can't solve the problem (to bring back missing
  device) as well, but now I have verified it, it won't.

  There is no record of what is the original idea to perform this
  basic volume manager step.

Thanks, Anand