Hi Miao, Anand,

(2014/07/03 19:22), Miao Xie wrote:
> From: Anand Jain <[email protected]>
> 
> device_list_add() is called when user runs btrfs dev scan, which would add
> any btrfs device into the btrfs_fs_devices list.

Reviewed-by: Satoru Takeuchi <[email protected]>

Thanks,
Satoru

> 
> Now think of a mounted btrfs. And a new device which contains the a SB
> from the mounted btrfs devices.
> 
> In this situation when user runs btrfs dev scan, the current code would
> just replace existing device with the new device.
> 
> Which is to note that old device is neither closed nor gracefully
> removed from the btrfs.
> 
> The FS is still operational with the old bdev however the device name
> is the btrfs_device is new which is provided by the btrfs dev scan.
> 
> reproducer:
> 
> devmgt[1] detach /dev/sdc
> 
> replace the missing disk /dev/sdc
> 
> btrfs rep start -f 1 /dev/sde /btrfs
> Label: none  uuid: 5dc0aaf4-4683-4050-b2d6-5ebe5f5cd120
>          Total devices 2 FS bytes used 32.00KiB
>          devid    1 size 958.94MiB used 115.88MiB path /dev/sde
>          devid    2 size 958.94MiB used 103.88MiB path /dev/sdd
> 
> make /dev/sdc to reappear
> 
> devmgt attach host2
> 
> btrfs dev scan
> 
> btrfs fi show -m
> Label: none  uuid: 5dc0aaf4-4683-4050-b2d6-5ebe5f5cd120^M
>          Total devices 2 FS bytes used 32.00KiB^M
>          devid    1 size 958.94MiB used 115.88MiB path /dev/sdc <- Wrong.
>          devid    2 size 958.94MiB used 103.88MiB path /dev/sdd
> 
> since /dev/sdc has been replaced with /dev/sde, the /dev/sdc shouldn't be
> part of the btrfs-fsid when it reappears. If user want it to be part of it
> then sys admin should be using btrfs device add instead.
> 
> [1] github.com/anajain/devmgt.git
> 
> Signed-off-by: Anand Jain <[email protected]>
> Signed-off-by: Wang Shilong <[email protected]>
> Signed-off-by: Miao Xie <[email protected]>
> ---
> Changelog v3->v4:
> - Fix the over-80-charactor problem
> ---
>   fs/btrfs/volumes.c | 27 +++++++++++++++++++++++++++
>   1 file changed, 27 insertions(+)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index a9c11a0..16e71a1 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -508,6 +508,33 @@ 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)) {
> +             /*
> +              * When FS is already mounted.
> +              * 1. If you are here and if the device->name is NULL that
> +              *    means this device was missing at time of FS mount.
> +              * 2. If you are here and if the device->name is different
> +              *    from 'path' that means either
> +              *      a. The same device disappeared and reappeared with
> +              *         different name. or
> +              *      b. The missing-disk-which-was-replaced, has
> +              *         reappeared now.
> +              *
> +              * We must allow 1 and 2a above. But 2b would be a spurious
> +              * and unintentional.
> +              *
> +              * Further in case of 1 and 2a above, the disk at 'path'
> +              * would have missed some transaction when it was away and
> +              * in case of 2a the stale bdev has to be updated as well.
> +              * 2b must not be allowed at all time.
> +              */
> +
> +             /*
> +              * As of now don't allow update to btrfs_fs_device through
> +              * the btrfs dev scan cli, after FS has been mounted.
> +              */
> +             if (fs_devices->opened)
> +                     return -EBUSY;
> +
>               name = rcu_string_strdup(path, GFP_NOFS);
>               if (!name)
>                       return -ENOMEM;
> 

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

Reply via email to