Re: [PATCH v4 12/13] btrfs: introduce device delete by devid

2016-02-18 Thread David Sterba 
On Thu, Feb 18, 2016 at 02:59:26PM +0800, Anand Jain wrote:
> #define BTRFS_PATH_NAME_MAX 4087
> #define BTRFS_SUBVOL_NAME_MAX 4039
> 
>   I am fine with using BTRFS_SUBVOL_NAME_MAX for now. But theoretical
>   anomaly is that add-device code path will use BTRFS_PATH_NAME_MAX and
>   delete device will use BTRFS_SUBVOL_NAME_MAX.. its only theoretical
>   as most of the devices path are well below 4k IMO. So its a good
>   trade off than other solutions like.. (just for the understanding),
> 
> - Update add device code as well to use btrfs_ioctl_vol_args_v2
>   Which means we need to introduce BTRFS_IOC_ADD_DEV_V2 (system
>   PATH_MAX is 4096).
> 
> OR
> 
> - Create new btrfs_ioctl_vol_args_v3 with name[BTRFS_PATH_NAME_MAX+1]
> (instead of name[BTRFS_SUBVOL_NAME_MAX+1]) and BTRFS_IOC_RM_DEV_V2
> will be the only consumer of btrfs_ioctl_vol_args_v3 as of now.

I'd rather not introduce any new structures or ioctls, the problem seems
to be marginal. As mentioned, paths are way shorter than 4k. A symlink
can be used as a workaround. We can document that.

The use of BTRFS_SUBVOL_NAME_MAX is confusing for devices, we can do

#define BTRFS_DEVICE_PATH_MAX   BTRFS_SUBVOL_NAME_MAX
--
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

Re: [PATCH v4 12/13] btrfs: introduce device delete by devid

2016-02-17 Thread Anand Jain 



On 02/17/2016 06:49 PM, David Sterba wrote:

On Sat, Feb 13, 2016 at 10:01:39AM +0800, Anand Jain wrote:

+   if (vol_args->flags & BTRFS_DEVICE_BY_ID) {
+   ret = btrfs_rm_device(root, NULL, vol_args->devid);
+   } else {
+   vol_args->name[BTRFS_PATH_NAME_MAX] = '\0';


   BTRFS_SUBVOL_NAME_MAX

Spotted by Chris,

fs/btrfs/ioctl.c:2703: warning: array subscript is above array bounds

my gcc version does not report that. Fixed and for-next pushed.


 mine either. Sorry about that, thanks for the catch.

#define BTRFS_PATH_NAME_MAX 4087
#define BTRFS_SUBVOL_NAME_MAX 4039

 I am fine with using BTRFS_SUBVOL_NAME_MAX for now. But theoretical
 anomaly is that add-device code path will use BTRFS_PATH_NAME_MAX and
 delete device will use BTRFS_SUBVOL_NAME_MAX.. its only theoretical
 as most of the devices path are well below 4k IMO. So its a good
 trade off than other solutions like.. (just for the understanding),

   - Update add device code as well to use btrfs_ioctl_vol_args_v2
 Which means we need to introduce BTRFS_IOC_ADD_DEV_V2 (system
 PATH_MAX is 4096).

   OR

   - Create new btrfs_ioctl_vol_args_v3 with name[BTRFS_PATH_NAME_MAX+1]
   (instead of name[BTRFS_SUBVOL_NAME_MAX+1]) and BTRFS_IOC_RM_DEV_V2
   will be the only consumer of btrfs_ioctl_vol_args_v3 as of now.


Thanks, Anand




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

Re: [PATCH v4 12/13] btrfs: introduce device delete by devid

2016-02-17 Thread David Sterba 
On Sat, Feb 13, 2016 at 10:01:39AM +0800, Anand Jain wrote:
> + if (vol_args->flags & BTRFS_DEVICE_BY_ID) {
> + ret = btrfs_rm_device(root, NULL, vol_args->devid);
> + } else {
> + vol_args->name[BTRFS_PATH_NAME_MAX] = '\0';

   BTRFS_SUBVOL_NAME_MAX

Spotted by Chris,

fs/btrfs/ioctl.c:2703: warning: array subscript is above array bounds

my gcc version does not report that. Fixed and for-next pushed.
--
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

[PATCH v4 12/13] btrfs: introduce device delete by devid

2016-02-12 Thread Anand Jain 
This introduces new ioctl BTRFS_IOC_RM_DEV_V2, which uses enhanced struct
btrfs_ioctl_vol_args_v2 to carry devid as an user argument.

The patch won't delete the old ioctl interface and so kernel remains
backward compatible with user land progs.

Test case/script:
echo "0 $(blockdev --getsz /dev/sdf) linear /dev/sdf 0" | dmsetup create 
bad_disk
mkfs.btrfs -f -d raid1 -m raid1 /dev/sdd /dev/sde /dev/mapper/bad_disk
mount /dev/sdd /btrfs
dmsetup suspend bad_disk
echo "0 $(blockdev --getsz /dev/sdf) error /dev/sdf 0" | dmsetup load bad_disk
dmsetup resume bad_disk
echo "bad disk failed. now deleting/replacing"
btrfs dev del  3  /btrfs
echo $?
btrfs fi show /btrfs
umount /btrfs
btrfs-show-super /dev/sdd | egrep num_device
dmsetup remove bad_disk
wipefs -a /dev/sdf

Signed-off-by: Anand Jain 
Reported-by: Martin 
---
v4: enahnced btrfs_ioctl_vol_args_v2 to accept devid instead of
creating a new structure. Thanks to David.
commit update and title changed from..
Btrfs: device delete by devid

v3: commit update, included test case

v2: don't use device->name after free

 fs/btrfs/ioctl.c   | 58 +-
 fs/btrfs/volumes.c |  4 ++--
 fs/btrfs/volumes.h |  2 +-
 include/uapi/linux/btrfs.h | 14 ++-
 4 files changed, 73 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index e54a4e9..0585d2d 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2667,6 +2667,60 @@ out:
return ret;
 }
 
+static long btrfs_ioctl_rm_dev_v2(struct file *file, void __user *arg)
+{
+   struct btrfs_root *root = BTRFS_I(file_inode(file))->root;
+   struct btrfs_ioctl_vol_args_v2 *vol_args;
+   int ret;
+
+   if (!capable(CAP_SYS_ADMIN))
+   return -EPERM;
+
+   ret = mnt_want_write_file(file);
+   if (ret)
+   return ret;
+
+   vol_args = memdup_user(arg, sizeof(*vol_args));
+   if (IS_ERR(vol_args)) {
+   ret = PTR_ERR(vol_args);
+   goto err_drop;
+   }
+
+   /* Check for compatibility reject unknown flags */
+   if (vol_args->flags & ~BTRFS_VOL_ARG_V2_FLAGS)
+   return -ENOTTY;
+
+   if (atomic_xchg(>fs_info->mutually_exclusive_operation_running,
+   1)) {
+   ret = BTRFS_ERROR_DEV_EXCL_RUN_IN_PROGRESS;
+   goto out;
+   }
+
+   mutex_lock(>fs_info->volume_mutex);
+   if (vol_args->flags & BTRFS_DEVICE_BY_ID) {
+   ret = btrfs_rm_device(root, NULL, vol_args->devid);
+   } else {
+   vol_args->name[BTRFS_PATH_NAME_MAX] = '\0';
+   ret = btrfs_rm_device(root, vol_args->name, 0);
+   }
+
+   mutex_unlock(>fs_info->volume_mutex);
+   atomic_set(>fs_info->mutually_exclusive_operation_running, 0);
+
+   if (!ret) {
+   if (vol_args->flags & BTRFS_DEVICE_BY_ID)
+   btrfs_info(root->fs_info, "disk devid %llu deleted",
+   
vol_args->devid);
+   else
+   btrfs_info(root->fs_info, "disk deleted - %s", 
vol_args->name);
+   }
+out:
+   kfree(vol_args);
+err_drop:
+   mnt_drop_write_file(file);
+   return ret;
+}
+
 static long btrfs_ioctl_rm_dev(struct file *file, void __user *arg)
 {
struct btrfs_root *root = BTRFS_I(file_inode(file))->root;
@@ -2695,7 +2749,7 @@ static long btrfs_ioctl_rm_dev(struct file *file, void 
__user *arg)
}
 
mutex_lock(>fs_info->volume_mutex);
-   ret = btrfs_rm_device(root, vol_args->name);
+   ret = btrfs_rm_device(root, vol_args->name, 0);
mutex_unlock(>fs_info->volume_mutex);
atomic_set(>fs_info->mutually_exclusive_operation_running, 0);
 
@@ -5462,6 +5516,8 @@ long btrfs_ioctl(struct file *file, unsigned int
return btrfs_ioctl_add_dev(root, argp);
case BTRFS_IOC_RM_DEV:
return btrfs_ioctl_rm_dev(file, argp);
+   case BTRFS_IOC_RM_DEV_V2:
+   return btrfs_ioctl_rm_dev_v2(file, argp);
case BTRFS_IOC_FS_INFO:
return btrfs_ioctl_fs_info(root, argp);
case BTRFS_IOC_DEV_INFO:
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index b24cff2..f47bd0b 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1746,7 +1746,7 @@ static int __check_raid_min_devices(struct btrfs_fs_info 
*fs_info)
return 0;
 }
 
-int btrfs_rm_device(struct btrfs_root *root, char *device_path)
+int btrfs_rm_device(struct btrfs_root *root, char *device_path, u64 devid)
 {
struct btrfs_device *device;
struct btrfs_device *next_device;
@@ -1762,7 +1762,7 @@ int btrfs_rm_device(struct btrfs_root *root, char 
*device_path)
if (ret)
goto out;
 
-   ret = btrfs_find_device_by_user_input(root, 0, device_path,
+   ret =