[PATCH 00/15] Device delete by id

[PATCH 00/15] Device delete by id

[David Sterba]

Hi,

this patchset extends the ioctl arugments to take an id so we can delete a
device by it. It reuses the existing structure btrfs_ioctl_vol_args_v2 and
extends it in na backward-compatible way so that we don't need to introduce
another one.

The core patchset is from Anand, I did some cleanups as I went through the
series again and made some cleanups and minor naming tweaks to the interface.

I'll add the branch to for-next and if everything goes fine I'll send a pull
request for 4.6 in a week.

Anand Jain (9):
  btrfs: create a helper function to read the disk super
  btrfs: create helper function __check_raid_min_devices()
  btrfs: clean up and optimize __check_raid_min_device()
  btrfs: create helper btrfs_find_device_by_user_input()
  btrfs: make use of btrfs_find_device_by_user_input()
  btrfs: enhance btrfs_find_device_by_user_input() to check device path
  btrfs: make use of btrfs_scratch_superblocks() in btrfs_rm_device()
  btrfs: introduce device delete by devid
  btrfs: optimize check for stale device

David Sterba (6):
  btrfs: rename __check_raid_min_devices
  btrfs: pass number of devices to btrfs_check_raid_min_devices
  btrfs: indtroduce raid-type to error-code table, for minimum device
    constraint
  btrfs: use existing device constraints table btrfs_raid_array
  btrfs: rename btrfs_find_device_by_user_input
  btrfs: rename flags for vol args v2

 fs/btrfs/dev-replace.c     |  28 +---
 fs/btrfs/ioctl.c           |  58 +++++++-
 fs/btrfs/volumes.c         | 322 ++++++++++++++++++++++-----------------------
 fs/btrfs/volumes.h         |   7 +-
 include/uapi/linux/btrfs.h |  15 ++-
 5 files changed, 233 insertions(+), 197 deletions(-)

-- 
2.7.1

[PATCH 01/15] btrfs: create a helper function to read the disk super

[David Sterba]

From: Anand Jain <anand.jain@oracle.com>

A part of code from btrfs_scan_one_device() is moved to a new function
btrfs_read_disk_super(), so that former function looks cleaner. (In this
process it also moves the code which ensures null terminating label). So
this creates easy opportunity to merge various duplicate codes on read
disk super. Earlier attempt to merge duplicate codes highlighted that
there were some issues for which there are duplicate codes (to read disk
super), however it was not clear what was the issue. So until we figure
that out, its better to keep them in a separate functions.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
[ use GFP_KERNEL ]
Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/volumes.c | 87 ++++++++++++++++++++++++++++++++----------------------
 1 file changed, 52 insertions(+), 35 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 366b335946fa..fa119ea8c74b 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -987,6 +987,56 @@ int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
 	return ret;
 }
 
+void btrfs_release_disk_super(struct page *page)
+{
+	kunmap(page);
+	page_cache_release(page);
+}
+
+int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr,
+		struct page **page, struct btrfs_super_block **disk_super)
+{
+	void *p;
+	pgoff_t index;
+
+	/* make sure our super fits in the device */
+	if (bytenr + PAGE_CACHE_SIZE >= i_size_read(bdev->bd_inode))
+		return 1;
+
+	/* make sure our super fits in the page */
+	if (sizeof(**disk_super) > PAGE_CACHE_SIZE)
+		return 1;
+
+	/* make sure our super doesn't straddle pages on disk */
+	index = bytenr >> PAGE_CACHE_SHIFT;
+	if ((bytenr + sizeof(**disk_super) - 1) >> PAGE_CACHE_SHIFT != index)
+		return 1;
+
+	/* pull in the page with our super */
+	*page = read_cache_page_gfp(bdev->bd_inode->i_mapping,
+				   index, GFP_KERNEL);
+
+	if (IS_ERR_OR_NULL(*page))
+		return 1;
+
+	p = kmap(*page);
+
+	/* align our pointer to the offset of the super block */
+	*disk_super = p + (bytenr & ~PAGE_CACHE_MASK);
+
+	if (btrfs_super_bytenr(*disk_super) != bytenr ||
+	    btrfs_super_magic(*disk_super) != BTRFS_MAGIC) {
+		btrfs_release_disk_super(*page);
+		return 1;
+	}
+
+	if ((*disk_super)->label[0] &&
+		(*disk_super)->label[BTRFS_LABEL_SIZE - 1])
+		(*disk_super)->label[BTRFS_LABEL_SIZE - 1] = '\0';
+
+	return 0;
+}
+
 /*
  * Look for a btrfs signature on a device. This may be called out of the mount path
  * and we are not allowed to call set_blocksize during the scan. The superblock
@@ -998,13 +1048,11 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
 	struct btrfs_super_block *disk_super;
 	struct block_device *bdev;
 	struct page *page;
-	void *p;
 	int ret = -EINVAL;
 	u64 devid;
 	u64 transid;
 	u64 total_devices;
 	u64 bytenr;
-	pgoff_t index;
 
 	/*
 	 * we would like to check all the supers, but that would make
@@ -1017,41 +1065,14 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
 	mutex_lock(&uuid_mutex);
 
 	bdev = blkdev_get_by_path(path, flags, holder);
-
 	if (IS_ERR(bdev)) {
 		ret = PTR_ERR(bdev);
 		goto error;
 	}
 
-	/* make sure our super fits in the device */
-	if (bytenr + PAGE_CACHE_SIZE >= i_size_read(bdev->bd_inode))
-		goto error_bdev_put;
-
-	/* make sure our super fits in the page */
-	if (sizeof(*disk_super) > PAGE_CACHE_SIZE)
-		goto error_bdev_put;
-
-	/* make sure our super doesn't straddle pages on disk */
-	index = bytenr >> PAGE_CACHE_SHIFT;
-	if ((bytenr + sizeof(*disk_super) - 1) >> PAGE_CACHE_SHIFT != index)
-		goto error_bdev_put;
-
-	/* pull in the page with our super */
-	page = read_cache_page_gfp(bdev->bd_inode->i_mapping,
-				   index, GFP_NOFS);
-
-	if (IS_ERR_OR_NULL(page))
+	if (btrfs_read_disk_super(bdev, bytenr, &page, &disk_super))
 		goto error_bdev_put;
 
-	p = kmap(page);
-
-	/* align our pointer to the offset of the super block */
-	disk_super = p + (bytenr & ~PAGE_CACHE_MASK);
-
-	if (btrfs_super_bytenr(disk_super) != bytenr ||
-	    btrfs_super_magic(disk_super) != BTRFS_MAGIC)
-		goto error_unmap;
-
 	devid = btrfs_stack_device_id(&disk_super->dev_item);
 	transid = btrfs_super_generation(disk_super);
 	total_devices = btrfs_super_num_devices(disk_super);
@@ -1059,8 +1080,6 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
 	ret = device_list_add(path, disk_super, devid, fs_devices_ret);
 	if (ret > 0) {
 		if (disk_super->label[0]) {
-			if (disk_super->label[BTRFS_LABEL_SIZE - 1])
-				disk_super->label[BTRFS_LABEL_SIZE - 1] = '\0';
 			printk(KERN_INFO "BTRFS: device label %s ", disk_super->label);
 		} else {
 			printk(KERN_INFO "BTRFS: device fsid %pU ", disk_super->fsid);
@@ -1072,9 +1091,7 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
 	if (!ret && fs_devices_ret)
 		(*fs_devices_ret)->total_devices = total_devices;
 
-error_unmap:
-	kunmap(page);
-	page_cache_release(page);
+	btrfs_release_disk_super(page);
 
 error_bdev_put:
 	blkdev_put(bdev, flags);
-- 
2.7.1

[PATCH 02/15] btrfs: create helper function __check_raid_min_devices()

[David Sterba]

From: Anand Jain <anand.jain@oracle.com>

move a section of btrfs_rm_device() code to check for min number of the
devices into the function __check_raid_min_devices()

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/volumes.c | 51 ++++++++++++++++++++++++++++++++-------------------
 1 file changed, 32 insertions(+), 19 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index fa119ea8c74b..8511f917c469 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1704,23 +1704,20 @@ static int btrfs_rm_dev_item(struct btrfs_root *root,
 	return ret;
 }
 
-int btrfs_rm_device(struct btrfs_root *root, char *device_path)
+static int __check_raid_min_devices(struct btrfs_root *root)
 {
-	struct btrfs_device *device;
-	struct btrfs_device *next_device;
-	struct block_device *bdev;
-	struct buffer_head *bh = NULL;
-	struct btrfs_super_block *disk_super;
-	struct btrfs_fs_devices *cur_devices;
 	u64 all_avail;
-	u64 devid;
 	u64 num_devices;
-	u8 *dev_uuid;
 	unsigned seq;
 	int ret = 0;
-	bool clear_super = false;
 
-	mutex_lock(&uuid_mutex);
+	num_devices = root->fs_info->fs_devices->num_devices;
+	btrfs_dev_replace_lock(&root->fs_info->dev_replace);
+	if (btrfs_dev_replace_is_ongoing(&root->fs_info->dev_replace)) {
+		WARN_ON(num_devices < 1);
+		num_devices--;
+	}
+	btrfs_dev_replace_unlock(&root->fs_info->dev_replace);
 
 	do {
 		seq = read_seqbegin(&root->fs_info->profiles_lock);
@@ -1730,14 +1727,6 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path)
 			    root->fs_info->avail_metadata_alloc_bits;
 	} while (read_seqretry(&root->fs_info->profiles_lock, seq));
 
-	num_devices = root->fs_info->fs_devices->num_devices;
-	btrfs_dev_replace_lock(&root->fs_info->dev_replace);
-	if (btrfs_dev_replace_is_ongoing(&root->fs_info->dev_replace)) {
-		WARN_ON(num_devices < 1);
-		num_devices--;
-	}
-	btrfs_dev_replace_unlock(&root->fs_info->dev_replace);
-
 	if ((all_avail & BTRFS_BLOCK_GROUP_RAID10) && num_devices <= 4) {
 		ret = BTRFS_ERROR_DEV_RAID10_MIN_NOT_MET;
 		goto out;
@@ -1759,6 +1748,30 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path)
 		goto out;
 	}
 
+out:
+	return ret;
+}
+
+int btrfs_rm_device(struct btrfs_root *root, char *device_path)
+{
+	struct btrfs_device *device;
+	struct btrfs_device *next_device;
+	struct block_device *bdev;
+	struct buffer_head *bh = NULL;
+	struct btrfs_super_block *disk_super;
+	struct btrfs_fs_devices *cur_devices;
+	u64 devid;
+	u64 num_devices;
+	u8 *dev_uuid;
+	int ret = 0;
+	bool clear_super = false;
+
+	mutex_lock(&uuid_mutex);
+
+	ret = __check_raid_min_devices(root);
+	if (ret)
+		goto out;
+
 	if (strcmp(device_path, "missing") == 0) {
 		struct list_head *devices;
 		struct btrfs_device *tmp;
-- 
2.7.1

[PATCH 03/15] btrfs: clean up and optimize __check_raid_min_device()

[David Sterba]

From: Anand Jain <anand.jain@oracle.com>

__check_raid_min_device() which was pealed from btrfs_rm_device()
maintianed its original code to show the block move. This patch cleans up
__check_raid_min_device().

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/volumes.c | 43 +++++++++++++++++++------------------------
 1 file changed, 19 insertions(+), 24 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 8511f917c469..ffc9a215e6ec 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1704,52 +1704,47 @@ static int btrfs_rm_dev_item(struct btrfs_root *root,
 	return ret;
 }
 
-static int __check_raid_min_devices(struct btrfs_root *root)
+static int __check_raid_min_devices(struct btrfs_fs_info *fs_info)
 {
 	u64 all_avail;
 	u64 num_devices;
 	unsigned seq;
-	int ret = 0;
 
-	num_devices = root->fs_info->fs_devices->num_devices;
-	btrfs_dev_replace_lock(&root->fs_info->dev_replace);
-	if (btrfs_dev_replace_is_ongoing(&root->fs_info->dev_replace)) {
+	num_devices = fs_info->fs_devices->num_devices;
+	btrfs_dev_replace_lock(&fs_info->dev_replace);
+	if (btrfs_dev_replace_is_ongoing(&fs_info->dev_replace)) {
 		WARN_ON(num_devices < 1);
 		num_devices--;
 	}
-	btrfs_dev_replace_unlock(&root->fs_info->dev_replace);
+	btrfs_dev_replace_unlock(&fs_info->dev_replace);
 
 	do {
-		seq = read_seqbegin(&root->fs_info->profiles_lock);
+		seq = read_seqbegin(&fs_info->profiles_lock);
 
-		all_avail = root->fs_info->avail_data_alloc_bits |
-			    root->fs_info->avail_system_alloc_bits |
-			    root->fs_info->avail_metadata_alloc_bits;
-	} while (read_seqretry(&root->fs_info->profiles_lock, seq));
+		all_avail = fs_info->avail_data_alloc_bits |
+			    fs_info->avail_system_alloc_bits |
+			    fs_info->avail_metadata_alloc_bits;
+	} while (read_seqretry(&fs_info->profiles_lock, seq));
 
 	if ((all_avail & BTRFS_BLOCK_GROUP_RAID10) && num_devices <= 4) {
-		ret = BTRFS_ERROR_DEV_RAID10_MIN_NOT_MET;
-		goto out;
+		return BTRFS_ERROR_DEV_RAID10_MIN_NOT_MET;
 	}
 
 	if ((all_avail & BTRFS_BLOCK_GROUP_RAID1) && num_devices <= 2) {
-		ret = BTRFS_ERROR_DEV_RAID1_MIN_NOT_MET;
-		goto out;
+		return BTRFS_ERROR_DEV_RAID1_MIN_NOT_MET;
 	}
 
 	if ((all_avail & BTRFS_BLOCK_GROUP_RAID5) &&
-	    root->fs_info->fs_devices->rw_devices <= 2) {
-		ret = BTRFS_ERROR_DEV_RAID5_MIN_NOT_MET;
-		goto out;
+	    fs_info->fs_devices->rw_devices <= 2) {
+		return BTRFS_ERROR_DEV_RAID5_MIN_NOT_MET;
 	}
+
 	if ((all_avail & BTRFS_BLOCK_GROUP_RAID6) &&
-	    root->fs_info->fs_devices->rw_devices <= 3) {
-		ret = BTRFS_ERROR_DEV_RAID6_MIN_NOT_MET;
-		goto out;
+	    fs_info->fs_devices->rw_devices <= 3) {
+		return BTRFS_ERROR_DEV_RAID6_MIN_NOT_MET;
 	}
 
-out:
-	return ret;
+	return 0;
 }
 
 int btrfs_rm_device(struct btrfs_root *root, char *device_path)
@@ -1768,7 +1763,7 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path)
 
 	mutex_lock(&uuid_mutex);
 
-	ret = __check_raid_min_devices(root);
+	ret = __check_raid_min_devices(root->fs_info);
 	if (ret)
 		goto out;
 
-- 
2.7.1

[PATCH 04/15] btrfs: create helper btrfs_find_device_by_user_input()

[David Sterba]

From: Anand Jain <anand.jain@oracle.com>

The patch renames btrfs_dev_replace_find_srcdev() to
btrfs_find_device_by_user_input() and moves it to volumes.c, so that
delete device can use it.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/dev-replace.c | 24 +-----------------------
 fs/btrfs/volumes.c     | 19 +++++++++++++++++++
 fs/btrfs/volumes.h     |  3 +++
 3 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index cbb7dbfb3fff..12d941d9b0e8 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -44,9 +44,6 @@ static void btrfs_dev_replace_update_device_in_mapping_tree(
 						struct btrfs_fs_info *fs_info,
 						struct btrfs_device *srcdev,
 						struct btrfs_device *tgtdev);
-static int btrfs_dev_replace_find_srcdev(struct btrfs_root *root, u64 srcdevid,
-					 char *srcdev_name,
-					 struct btrfs_device **device);
 static u64 __btrfs_dev_replace_cancel(struct btrfs_fs_info *fs_info);
 static int btrfs_dev_replace_kthread(void *data);
 static int btrfs_dev_replace_continue_on_mount(struct btrfs_fs_info *fs_info);
@@ -329,7 +326,7 @@ int btrfs_dev_replace_start(struct btrfs_root *root,
 
 	/* the disk copy procedure reuses the scrub code */
 	mutex_lock(&fs_info->volume_mutex);
-	ret = btrfs_dev_replace_find_srcdev(root, args->start.srcdevid,
+	ret = btrfs_find_device_by_user_input(root, args->start.srcdevid,
 					    args->start.srcdev_name,
 					    &src_device);
 	if (ret) {
@@ -624,25 +621,6 @@ static void btrfs_dev_replace_update_device_in_mapping_tree(
 	write_unlock(&em_tree->lock);
 }
 
-static int btrfs_dev_replace_find_srcdev(struct btrfs_root *root, u64 srcdevid,
-					 char *srcdev_name,
-					 struct btrfs_device **device)
-{
-	int ret;
-
-	if (srcdevid) {
-		ret = 0;
-		*device = btrfs_find_device(root->fs_info, srcdevid, NULL,
-					    NULL);
-		if (!*device)
-			ret = -ENOENT;
-	} else {
-		ret = btrfs_find_device_missing_or_by_path(root, srcdev_name,
-							   device);
-	}
-	return ret;
-}
-
 void btrfs_dev_replace_status(struct btrfs_fs_info *fs_info,
 			      struct btrfs_ioctl_dev_replace_args *args)
 {
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index ffc9a215e6ec..ffaa1306a35f 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2125,6 +2125,25 @@ int btrfs_find_device_missing_or_by_path(struct btrfs_root *root,
 	}
 }
 
+int btrfs_find_device_by_user_input(struct btrfs_root *root, u64 srcdevid,
+					 char *srcdev_name,
+					 struct btrfs_device **device)
+{
+	int ret;
+
+	if (srcdevid) {
+		ret = 0;
+		*device = btrfs_find_device(root->fs_info, srcdevid, NULL,
+					    NULL);
+		if (!*device)
+			ret = -ENOENT;
+	} else {
+		ret = btrfs_find_device_missing_or_by_path(root, srcdev_name,
+							   device);
+	}
+	return ret;
+}
+
 /*
  * does all the dirty work required for changing file system's UUID.
  */
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 1939ebde63df..508739314e43 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -448,6 +448,9 @@ void btrfs_close_extra_devices(struct btrfs_fs_devices *fs_devices, int step);
 int btrfs_find_device_missing_or_by_path(struct btrfs_root *root,
 					 char *device_path,
 					 struct btrfs_device **device);
+int btrfs_find_device_by_user_input(struct btrfs_root *root, u64 srcdevid,
+					 char *srcdev_name,
+					 struct btrfs_device **device);
 struct btrfs_device *btrfs_alloc_device(struct btrfs_fs_info *fs_info,
 					const u64 *devid,
 					const u8 *uuid);
-- 
2.7.1

[PATCH 05/15] btrfs: make use of btrfs_find_device_by_user_input()

[David Sterba]

From: Anand Jain <anand.jain@oracle.com>

btrfs_rm_device() has a section of the code which can be replaced
btrfs_find_device_by_user_input()

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/volumes.c | 100 ++++++++++++++++++++---------------------------------
 1 file changed, 37 insertions(+), 63 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index ffaa1306a35f..6d8b78ffb0a0 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1751,13 +1751,11 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path)
 {
 	struct btrfs_device *device;
 	struct btrfs_device *next_device;
-	struct block_device *bdev;
+	struct block_device *bdev = NULL;
 	struct buffer_head *bh = NULL;
-	struct btrfs_super_block *disk_super;
+	struct btrfs_super_block *disk_super = NULL;
 	struct btrfs_fs_devices *cur_devices;
-	u64 devid;
 	u64 num_devices;
-	u8 *dev_uuid;
 	int ret = 0;
 	bool clear_super = false;
 
@@ -1767,57 +1765,19 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path)
 	if (ret)
 		goto out;
 
-	if (strcmp(device_path, "missing") == 0) {
-		struct list_head *devices;
-		struct btrfs_device *tmp;
-
-		device = NULL;
-		devices = &root->fs_info->fs_devices->devices;
-		/*
-		 * It is safe to read the devices since the volume_mutex
-		 * is held.
-		 */
-		list_for_each_entry(tmp, devices, dev_list) {
-			if (tmp->in_fs_metadata &&
-			    !tmp->is_tgtdev_for_dev_replace &&
-			    !tmp->bdev) {
-				device = tmp;
-				break;
-			}
-		}
-		bdev = NULL;
-		bh = NULL;
-		disk_super = NULL;
-		if (!device) {
-			ret = BTRFS_ERROR_DEV_MISSING_NOT_FOUND;
-			goto out;
-		}
-	} else {
-		ret = btrfs_get_bdev_and_sb(device_path,
-					    FMODE_WRITE | FMODE_EXCL,
-					    root->fs_info->bdev_holder, 0,
-					    &bdev, &bh);
-		if (ret)
-			goto out;
-		disk_super = (struct btrfs_super_block *)bh->b_data;
-		devid = btrfs_stack_device_id(&disk_super->dev_item);
-		dev_uuid = disk_super->dev_item.uuid;
-		device = btrfs_find_device(root->fs_info, devid, dev_uuid,
-					   disk_super->fsid);
-		if (!device) {
-			ret = -ENOENT;
-			goto error_brelse;
-		}
-	}
+	ret = btrfs_find_device_by_user_input(root, 0, device_path,
+				&device);
+	if (ret)
+		goto out;
 
 	if (device->is_tgtdev_for_dev_replace) {
 		ret = BTRFS_ERROR_DEV_TGT_REPLACE;
-		goto error_brelse;
+		goto out;
 	}
 
 	if (device->writeable && root->fs_info->fs_devices->rw_devices == 1) {
 		ret = BTRFS_ERROR_DEV_ONLY_WRITABLE;
-		goto error_brelse;
+		goto out;
 	}
 
 	if (device->writeable) {
@@ -1907,16 +1867,33 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path)
 	 * at this point, the device is zero sized.  We want to
 	 * remove it from the devices list and zero out the old super
 	 */
-	if (clear_super && disk_super) {
+	if (clear_super) {
 		u64 bytenr;
 		int i;
 
+		if (!disk_super) {
+			ret = btrfs_get_bdev_and_sb(device_path,
+					FMODE_WRITE | FMODE_EXCL,
+					root->fs_info->bdev_holder, 0,
+					&bdev, &bh);
+			if (ret) {
+				/*
+				 * It could be a failed device ok for clear_super
+				 * to fail. So return success
+				 */
+				ret = 0;
+				goto out;
+			}
+
+			disk_super = (struct btrfs_super_block *)bh->b_data;
+		}
 		/* make sure this device isn't detected as part of
 		 * the FS anymore
 		 */
 		memset(&disk_super->magic, 0, sizeof(disk_super->magic));
 		set_buffer_dirty(bh);
 		sync_dirty_buffer(bh);
+		brelse(bh);
 
 		/* clear the mirror copies of super block on the disk
 		 * being removed, 0th copy is been taken care above and
@@ -1928,7 +1905,6 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path)
 					i_size_read(bdev->bd_inode))
 				break;
 
-			brelse(bh);
 			bh = __bread(bdev, bytenr / 4096,
 					BTRFS_SUPER_INFO_SIZE);
 			if (!bh)
@@ -1938,32 +1914,30 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path)
 
 			if (btrfs_super_bytenr(disk_super) != bytenr ||
 				btrfs_super_magic(disk_super) != BTRFS_MAGIC) {
+				brelse(bh);
 				continue;
 			}
 			memset(&disk_super->magic, 0,
 						sizeof(disk_super->magic));
 			set_buffer_dirty(bh);
 			sync_dirty_buffer(bh);
+			brelse(bh);
 		}
-	}
 
-	ret = 0;
-
-	if (bdev) {
-		/* Notify udev that device has changed */
-		btrfs_kobject_uevent(bdev, KOBJ_CHANGE);
+		if (bdev) {
+			/* Notify udev that device has changed */
+			btrfs_kobject_uevent(bdev, KOBJ_CHANGE);
 
-		/* Update ctime/mtime for device path for libblkid */
-		update_dev_time(device_path);
+			/* Update ctime/mtime for device path for libblkid */
+			update_dev_time(device_path);
+			blkdev_put(bdev, FMODE_READ | FMODE_EXCL);
+		}
 	}
 
-error_brelse:
-	brelse(bh);
-	if (bdev)
-		blkdev_put(bdev, FMODE_READ | FMODE_EXCL);
 out:
 	mutex_unlock(&uuid_mutex);
 	return ret;
+
 error_undo:
 	if (device->writeable) {
 		lock_chunks(root);
@@ -1972,7 +1946,7 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path)
 		device->fs_devices->rw_devices++;
 		unlock_chunks(root);
 	}
-	goto error_brelse;
+	goto out;
 }
 
 void btrfs_rm_dev_replace_remove_srcdev(struct btrfs_fs_info *fs_info,
-- 
2.7.1

[PATCH 06/15] btrfs: enhance btrfs_find_device_by_user_input() to check device path

[David Sterba]

From: Anand Jain <anand.jain@oracle.com>

The operation of device replace and device delete follows same steps upto
some depth with in btrfs kernel, however they don't share codes. This
enhancement will help replace and delete to share codes.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/dev-replace.c | 4 ----
 fs/btrfs/volumes.c     | 3 +++
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index 12d941d9b0e8..3e2616d151d9 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -320,10 +320,6 @@ int btrfs_dev_replace_start(struct btrfs_root *root,
 		return -EINVAL;
 	}
 
-	if ((args->start.srcdevid == 0 && args->start.srcdev_name[0] == '\0') ||
-	    args->start.tgtdev_name[0] == '\0')
-		return -EINVAL;
-
 	/* the disk copy procedure reuses the scrub code */
 	mutex_lock(&fs_info->volume_mutex);
 	ret = btrfs_find_device_by_user_input(root, args->start.srcdevid,
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 6d8b78ffb0a0..fede69134368 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2112,6 +2112,9 @@ int btrfs_find_device_by_user_input(struct btrfs_root *root, u64 srcdevid,
 		if (!*device)
 			ret = -ENOENT;
 	} else {
+		if (!srcdev_name || !srcdev_name[0])
+			return -EINVAL;
+
 		ret = btrfs_find_device_missing_or_by_path(root, srcdev_name,
 							   device);
 	}
-- 
2.7.1

[PATCH 07/15] btrfs: make use of btrfs_scratch_superblocks() in btrfs_rm_device()

[David Sterba]

From: Anand Jain <anand.jain@oracle.com>

With the previous patches now the btrfs_scratch_superblocks() is ready to
be used in btrfs_rm_device() so use it.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
[ use GFP_KERNEL ]
Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/volumes.c | 78 ++++++++++--------------------------------------------
 1 file changed, 14 insertions(+), 64 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index fede69134368..0b1fa2faa4eb 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1751,13 +1751,11 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path)
 {
 	struct btrfs_device *device;
 	struct btrfs_device *next_device;
-	struct block_device *bdev = NULL;
-	struct buffer_head *bh = NULL;
-	struct btrfs_super_block *disk_super = NULL;
 	struct btrfs_fs_devices *cur_devices;
 	u64 num_devices;
 	int ret = 0;
 	bool clear_super = false;
+	char *dev_name = NULL;
 
 	mutex_lock(&uuid_mutex);
 
@@ -1785,6 +1783,11 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path)
 		list_del_init(&device->dev_alloc_list);
 		device->fs_devices->rw_devices--;
 		unlock_chunks(root);
+		dev_name = kstrdup(device->name->str, GFP_KERNEL);
+		if (!dev_name) {
+			ret = -ENOMEM;
+			goto error_undo;
+		}
 		clear_super = true;
 	}
 
@@ -1868,73 +1871,20 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path)
 	 * remove it from the devices list and zero out the old super
 	 */
 	if (clear_super) {
-		u64 bytenr;
-		int i;
-
-		if (!disk_super) {
-			ret = btrfs_get_bdev_and_sb(device_path,
-					FMODE_WRITE | FMODE_EXCL,
-					root->fs_info->bdev_holder, 0,
-					&bdev, &bh);
-			if (ret) {
-				/*
-				 * It could be a failed device ok for clear_super
-				 * to fail. So return success
-				 */
-				ret = 0;
-				goto out;
-			}
-
-			disk_super = (struct btrfs_super_block *)bh->b_data;
-		}
-		/* make sure this device isn't detected as part of
-		 * the FS anymore
-		 */
-		memset(&disk_super->magic, 0, sizeof(disk_super->magic));
-		set_buffer_dirty(bh);
-		sync_dirty_buffer(bh);
-		brelse(bh);
-
-		/* clear the mirror copies of super block on the disk
-		 * being removed, 0th copy is been taken care above and
-		 * the below would take of the rest
-		 */
-		for (i = 1; i < BTRFS_SUPER_MIRROR_MAX; i++) {
-			bytenr = btrfs_sb_offset(i);
-			if (bytenr + BTRFS_SUPER_INFO_SIZE >=
-					i_size_read(bdev->bd_inode))
-				break;
-
-			bh = __bread(bdev, bytenr / 4096,
-					BTRFS_SUPER_INFO_SIZE);
-			if (!bh)
-				continue;
+		struct block_device *bdev;
 
-			disk_super = (struct btrfs_super_block *)bh->b_data;
-
-			if (btrfs_super_bytenr(disk_super) != bytenr ||
-				btrfs_super_magic(disk_super) != BTRFS_MAGIC) {
-				brelse(bh);
-				continue;
-			}
-			memset(&disk_super->magic, 0,
-						sizeof(disk_super->magic));
-			set_buffer_dirty(bh);
-			sync_dirty_buffer(bh);
-			brelse(bh);
-		}
-
-		if (bdev) {
-			/* Notify udev that device has changed */
-			btrfs_kobject_uevent(bdev, KOBJ_CHANGE);
-
-			/* Update ctime/mtime for device path for libblkid */
-			update_dev_time(device_path);
+		bdev = blkdev_get_by_path(dev_name, FMODE_READ | FMODE_EXCL,
+						root->fs_info->bdev_holder);
+		if (!IS_ERR(bdev)) {
+			btrfs_scratch_superblocks(bdev, dev_name);
 			blkdev_put(bdev, FMODE_READ | FMODE_EXCL);
 		}
 	}
 
 out:
+	if (dev_name)
+		kfree(dev_name);
+
 	mutex_unlock(&uuid_mutex);
 	return ret;
 
-- 
2.7.1

[PATCH 08/15] btrfs: introduce device delete by devid

[David Sterba]

From: Anand Jain <anand.jain@oracle.com>

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 <anand.jain@oracle.com>
Reported-by: Martin <m_btrfs@ml1.co.uk>
[ adjust messages, s/disk/device/ ]
Signed-off-by: David Sterba <dsterba@suse.com>
---
 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 952172ca7e45..57fb05960435 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2669,6 +2669,60 @@ static long btrfs_ioctl_add_dev(struct btrfs_root *root, void __user *arg)
 	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(&root->fs_info->mutually_exclusive_operation_running,
+			1)) {
+		ret = BTRFS_ERROR_DEV_EXCL_RUN_IN_PROGRESS;
+		goto out;
+	}
+
+	mutex_lock(&root->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(&root->fs_info->volume_mutex);
+	atomic_set(&root->fs_info->mutually_exclusive_operation_running, 0);
+
+	if (!ret) {
+		if (vol_args->flags & BTRFS_DEVICE_BY_ID)
+			btrfs_info(root->fs_info, "device deleted: id %llu",
+					vol_args->devid);
+		else
+			btrfs_info(root->fs_info, "device 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;
@@ -2697,7 +2751,7 @@ static long btrfs_ioctl_rm_dev(struct file *file, void __user *arg)
 	}
 
 	mutex_lock(&root->fs_info->volume_mutex);
-	ret = btrfs_rm_device(root, vol_args->name);
+	ret = btrfs_rm_device(root, vol_args->name, 0);
 	mutex_unlock(&root->fs_info->volume_mutex);
 	atomic_set(&root->fs_info->mutually_exclusive_operation_running, 0);
 
@@ -5383,6 +5437,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 0b1fa2faa4eb..5e49adb34b2e 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1747,7 +1747,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;
@@ -1763,7 +1763,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 = btrfs_find_device_by_user_input(root, devid, device_path,
 				&device);
 	if (ret)
 		goto out;
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 508739314e43..c73d027e2f8b 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -454,7 +454,7 @@ int btrfs_find_device_by_user_input(struct btrfs_root *root, u64 srcdevid,
 struct btrfs_device *btrfs_alloc_device(struct btrfs_fs_info *fs_info,
 					const u64 *devid,
 					const u8 *uuid);
-int btrfs_rm_device(struct btrfs_root *root, char *device_path);
+int btrfs_rm_device(struct btrfs_root *root, char *device_path, u64 devid);
 void btrfs_cleanup_fs_uuids(void);
 int btrfs_num_copies(struct btrfs_fs_info *fs_info, u64 logical, u64 len);
 int btrfs_grow_device(struct btrfs_trans_handle *trans,
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index dea893199257..396a4efca775 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -36,6 +36,13 @@ struct btrfs_ioctl_vol_args {
 #define BTRFS_SUBVOL_CREATE_ASYNC	(1ULL << 0)
 #define BTRFS_SUBVOL_RDONLY		(1ULL << 1)
 #define BTRFS_SUBVOL_QGROUP_INHERIT	(1ULL << 2)
+#define BTRFS_DEVICE_BY_ID		(1ULL << 3)
+#define BTRFS_VOL_ARG_V2_FLAGS				\
+			(BTRFS_SUBVOL_CREATE_ASYNC |	\
+			BTRFS_SUBVOL_RDONLY |		\
+			BTRFS_SUBVOL_QGROUP_INHERIT |	\
+			BTRFS_DEVICE_BY_ID)
+
 #define BTRFS_FSID_SIZE 16
 #define BTRFS_UUID_SIZE 16
 #define BTRFS_UUID_UNPARSED_SIZE	37
@@ -76,7 +83,10 @@ struct btrfs_ioctl_vol_args_v2 {
 		};
 		__u64 unused[4];
 	};
-	char name[BTRFS_SUBVOL_NAME_MAX + 1];
+	union {
+		char name[BTRFS_SUBVOL_NAME_MAX + 1];
+		u64 devid;
+	};
 };
 
 /*
@@ -659,5 +669,7 @@ static inline char *btrfs_err_str(enum btrfs_err_code err_code)
 				   struct btrfs_ioctl_feature_flags[2])
 #define BTRFS_IOC_GET_SUPPORTED_FEATURES _IOR(BTRFS_IOCTL_MAGIC, 57, \
 				   struct btrfs_ioctl_feature_flags[3])
+#define BTRFS_IOC_RM_DEV_V2 _IOW(BTRFS_IOCTL_MAGIC, 58, \
+				   struct btrfs_ioctl_vol_args_v2)
 
 #endif /* _UAPI_LINUX_BTRFS_H */
-- 
2.7.1

[PATCH 09/15] btrfs: optimize check for stale device

[David Sterba]

From: Anand Jain <anand.jain@oracle.com>

Optimize check for stale device to only be checked when there is device
added or changed. If there is no update to the device, there is no need
to call btrfs_free_stale_device().

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/volumes.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 5e49adb34b2e..20af20b0eaee 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -699,7 +699,8 @@ static noinline int device_list_add(const char *path,
 	 * if there is new btrfs on an already registered device,
 	 * then remove the stale device entry.
 	 */
-	btrfs_free_stale_device(device);
+	if (ret > 0)
+		btrfs_free_stale_device(device);
 
 	*fs_devices_ret = fs_devices;
 
-- 
2.7.1

[PATCH 10/15] btrfs: rename __check_raid_min_devices

[David Sterba]

Underscores are for special functions, use the full prefix for better
stacktrace recognition.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/volumes.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 20af20b0eaee..4fa4a836a072 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1705,7 +1705,7 @@ static int btrfs_rm_dev_item(struct btrfs_root *root,
 	return ret;
 }
 
-static int __check_raid_min_devices(struct btrfs_fs_info *fs_info)
+static int btrfs_check_raid_min_devices(struct btrfs_fs_info *fs_info)
 {
 	u64 all_avail;
 	u64 num_devices;
@@ -1760,7 +1760,7 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path, u64 devid)
 
 	mutex_lock(&uuid_mutex);
 
-	ret = __check_raid_min_devices(root->fs_info);
+	ret = btrfs_check_raid_min_devices(root->fs_info);
 	if (ret)
 		goto out;
 
-- 
2.7.1

Re: [PATCH 10/15] btrfs: rename __check_raid_min_devices

[Anand Jain]

  thanks.

Reviewed-by: Anand Jain <anand.jain@oracle.com>


On 02/16/2016 01:34 AM, David Sterba wrote:
> Underscores are for special functions, use the full prefix for better
> stacktrace recognition.
>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>   fs/btrfs/volumes.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 20af20b0eaee..4fa4a836a072 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1705,7 +1705,7 @@ static int btrfs_rm_dev_item(struct btrfs_root *root,
>   	return ret;
>   }
>
> -static int __check_raid_min_devices(struct btrfs_fs_info *fs_info)
> +static int btrfs_check_raid_min_devices(struct btrfs_fs_info *fs_info)
>   {
>   	u64 all_avail;
>   	u64 num_devices;
> @@ -1760,7 +1760,7 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path, u64 devid)
>
>   	mutex_lock(&uuid_mutex);
>
> -	ret = __check_raid_min_devices(root->fs_info);
> +	ret = btrfs_check_raid_min_devices(root->fs_info);
>   	if (ret)
>   		goto out;
>
>

[PATCH 11/15] btrfs: pass number of devices to btrfs_check_raid_min_devices

[David Sterba]

Before this patch, btrfs_check_raid_min_devices would do an off-by-one
check of the constraints and not the miminmum check, as its name
suggests. This is not a problem if the only caller is device remove, but
would be confusing for others.

Add an argument with the exact number and let the caller(s) decide if
this needs any adjustments, like when device replace is running.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/volumes.c | 35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 4fa4a836a072..ae94e06f3e61 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1705,20 +1705,17 @@ static int btrfs_rm_dev_item(struct btrfs_root *root,
 	return ret;
 }
 
-static int btrfs_check_raid_min_devices(struct btrfs_fs_info *fs_info)
+/*
+ * Verify that @num_devices satisfies the RAID profile constraints in the whole
+ * filesystem. It's up to the caller to adjust that number regarding eg. device
+ * replace.
+ */
+static int btrfs_check_raid_min_devices(struct btrfs_fs_info *fs_info,
+		u64 num_devices)
 {
 	u64 all_avail;
-	u64 num_devices;
 	unsigned seq;
 
-	num_devices = fs_info->fs_devices->num_devices;
-	btrfs_dev_replace_lock(&fs_info->dev_replace);
-	if (btrfs_dev_replace_is_ongoing(&fs_info->dev_replace)) {
-		WARN_ON(num_devices < 1);
-		num_devices--;
-	}
-	btrfs_dev_replace_unlock(&fs_info->dev_replace);
-
 	do {
 		seq = read_seqbegin(&fs_info->profiles_lock);
 
@@ -1727,21 +1724,21 @@ static int btrfs_check_raid_min_devices(struct btrfs_fs_info *fs_info)
 			    fs_info->avail_metadata_alloc_bits;
 	} while (read_seqretry(&fs_info->profiles_lock, seq));
 
-	if ((all_avail & BTRFS_BLOCK_GROUP_RAID10) && num_devices <= 4) {
+	if ((all_avail & BTRFS_BLOCK_GROUP_RAID10) && num_devices < 4) {
 		return BTRFS_ERROR_DEV_RAID10_MIN_NOT_MET;
 	}
 
-	if ((all_avail & BTRFS_BLOCK_GROUP_RAID1) && num_devices <= 2) {
+	if ((all_avail & BTRFS_BLOCK_GROUP_RAID1) && num_devices < 2) {
 		return BTRFS_ERROR_DEV_RAID1_MIN_NOT_MET;
 	}
 
 	if ((all_avail & BTRFS_BLOCK_GROUP_RAID5) &&
-	    fs_info->fs_devices->rw_devices <= 2) {
+	    fs_info->fs_devices->rw_devices < 2) {
 		return BTRFS_ERROR_DEV_RAID5_MIN_NOT_MET;
 	}
 
 	if ((all_avail & BTRFS_BLOCK_GROUP_RAID6) &&
-	    fs_info->fs_devices->rw_devices <= 3) {
+	    fs_info->fs_devices->rw_devices < 3) {
 		return BTRFS_ERROR_DEV_RAID6_MIN_NOT_MET;
 	}
 
@@ -1760,7 +1757,15 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path, u64 devid)
 
 	mutex_lock(&uuid_mutex);
 
-	ret = btrfs_check_raid_min_devices(root->fs_info);
+	num_devices = root->fs_info->fs_devices->num_devices;
+	btrfs_dev_replace_lock(&root->fs_info->dev_replace);
+	if (btrfs_dev_replace_is_ongoing(&root->fs_info->dev_replace)) {
+		WARN_ON(num_devices < 1);
+		num_devices--;
+	}
+	btrfs_dev_replace_unlock(&root->fs_info->dev_replace);
+
+	ret = btrfs_check_raid_min_devices(root->fs_info, num_devices - 1);
 	if (ret)
 		goto out;
 
-- 
2.7.1

Re: [PATCH 11/15] btrfs: pass number of devices to btrfs_check_raid_min_devices

[Anand Jain]

looks good.

Reviewed-by: Anand Jain <anand.jain@oracle.com>
Tested-by: Anand Jain <anand.jain@oracle.com>

Thanks.

On 02/16/2016 01:34 AM, David Sterba wrote:
> Before this patch, btrfs_check_raid_min_devices would do an off-by-one
> check of the constraints and not the miminmum check, as its name
> suggests. This is not a problem if the only caller is device remove, but
> would be confusing for others.
>
> Add an argument with the exact number and let the caller(s) decide if
> this needs any adjustments, like when device replace is running.
>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>   fs/btrfs/volumes.c | 35 ++++++++++++++++++++---------------
>   1 file changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 4fa4a836a072..ae94e06f3e61 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1705,20 +1705,17 @@ static int btrfs_rm_dev_item(struct btrfs_root *root,
>   	return ret;
>   }
>
> -static int btrfs_check_raid_min_devices(struct btrfs_fs_info *fs_info)
> +/*
> + * Verify that @num_devices satisfies the RAID profile constraints in the whole
> + * filesystem. It's up to the caller to adjust that number regarding eg. device
> + * replace.
> + */
> +static int btrfs_check_raid_min_devices(struct btrfs_fs_info *fs_info,
> +		u64 num_devices)
>   {
>   	u64 all_avail;
> -	u64 num_devices;
>   	unsigned seq;
>
> -	num_devices = fs_info->fs_devices->num_devices;
> -	btrfs_dev_replace_lock(&fs_info->dev_replace);
> -	if (btrfs_dev_replace_is_ongoing(&fs_info->dev_replace)) {
> -		WARN_ON(num_devices < 1);
> -		num_devices--;
> -	}
> -	btrfs_dev_replace_unlock(&fs_info->dev_replace);
> -
>   	do {
>   		seq = read_seqbegin(&fs_info->profiles_lock);
>
> @@ -1727,21 +1724,21 @@ static int btrfs_check_raid_min_devices(struct btrfs_fs_info *fs_info)
>   			    fs_info->avail_metadata_alloc_bits;
>   	} while (read_seqretry(&fs_info->profiles_lock, seq));
>
> -	if ((all_avail & BTRFS_BLOCK_GROUP_RAID10) && num_devices <= 4) {
> +	if ((all_avail & BTRFS_BLOCK_GROUP_RAID10) && num_devices < 4) {
>   		return BTRFS_ERROR_DEV_RAID10_MIN_NOT_MET;
>   	}
>
> -	if ((all_avail & BTRFS_BLOCK_GROUP_RAID1) && num_devices <= 2) {
> +	if ((all_avail & BTRFS_BLOCK_GROUP_RAID1) && num_devices < 2) {
>   		return BTRFS_ERROR_DEV_RAID1_MIN_NOT_MET;
>   	}
>
>   	if ((all_avail & BTRFS_BLOCK_GROUP_RAID5) &&
> -	    fs_info->fs_devices->rw_devices <= 2) {
> +	    fs_info->fs_devices->rw_devices < 2) {
>   		return BTRFS_ERROR_DEV_RAID5_MIN_NOT_MET;
>   	}
>
>   	if ((all_avail & BTRFS_BLOCK_GROUP_RAID6) &&
> -	    fs_info->fs_devices->rw_devices <= 3) {
> +	    fs_info->fs_devices->rw_devices < 3) {
>   		return BTRFS_ERROR_DEV_RAID6_MIN_NOT_MET;
>   	}
>
> @@ -1760,7 +1757,15 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path, u64 devid)
>
>   	mutex_lock(&uuid_mutex);
>
> -	ret = btrfs_check_raid_min_devices(root->fs_info);
> +	num_devices = root->fs_info->fs_devices->num_devices;
> +	btrfs_dev_replace_lock(&root->fs_info->dev_replace);
> +	if (btrfs_dev_replace_is_ongoing(&root->fs_info->dev_replace)) {
> +		WARN_ON(num_devices < 1);
> +		num_devices--;
> +	}
> +	btrfs_dev_replace_unlock(&root->fs_info->dev_replace);
> +
> +	ret = btrfs_check_raid_min_devices(root->fs_info, num_devices - 1);
>   	if (ret)
>   		goto out;
>
>

[PATCH 12/15] btrfs: indtroduce raid-type to error-code table, for minimum device constraint

[David Sterba]

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/volumes.c | 15 +++++++++++++++
 fs/btrfs/volumes.h |  2 +-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index ae94e06f3e61..a67249582a6f 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -118,6 +118,21 @@ const u64 btrfs_raid_group[BTRFS_NR_RAID_TYPES] = {
 	[BTRFS_RAID_RAID6]  = BTRFS_BLOCK_GROUP_RAID6,
 };
 
+/*
+ * Table to convert BTRFS_RAID_* to the error code if minimum number of devices
+ * condition is not met. Zero means there's no corresponding
+ * BTRFS_ERROR_DEV_*_NOT_MET value.
+ */
+const int btrfs_raid_mindev_error[BTRFS_NR_RAID_TYPES] = {
+	[BTRFS_RAID_RAID10] = BTRFS_ERROR_DEV_RAID10_MIN_NOT_MET,
+	[BTRFS_RAID_RAID1]  = BTRFS_ERROR_DEV_RAID1_MIN_NOT_MET,
+	[BTRFS_RAID_DUP]    = 0,
+	[BTRFS_RAID_RAID0]  = 0,
+	[BTRFS_RAID_SINGLE] = 0,
+	[BTRFS_RAID_RAID5]  = BTRFS_ERROR_DEV_RAID5_MIN_NOT_MET,
+	[BTRFS_RAID_RAID6]  = BTRFS_ERROR_DEV_RAID6_MIN_NOT_MET,
+};
+
 static int init_first_rw_device(struct btrfs_trans_handle *trans,
 				struct btrfs_root *root,
 				struct btrfs_device *device);
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index c73d027e2f8b..a13a538cb01e 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -340,7 +340,7 @@ struct btrfs_raid_attr {
 };
 
 extern const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES];
-
+extern const int btrfs_raid_mindev_error[BTRFS_NR_RAID_TYPES];
 extern const u64 btrfs_raid_group[BTRFS_NR_RAID_TYPES];
 
 struct map_lookup {
-- 
2.7.1

Re: [PATCH 12/15] btrfs: indtroduce raid-type to error-code table, for minimum device constraint

[Anand Jain]

Nice fix. thanks

Reviewed-by: Anand Jain <anand.jain@oracle.com>


On 02/16/2016 01:34 AM, David Sterba wrote:
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>   fs/btrfs/volumes.c | 15 +++++++++++++++
>   fs/btrfs/volumes.h |  2 +-
>   2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index ae94e06f3e61..a67249582a6f 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -118,6 +118,21 @@ const u64 btrfs_raid_group[BTRFS_NR_RAID_TYPES] = {
>   	[BTRFS_RAID_RAID6]  = BTRFS_BLOCK_GROUP_RAID6,
>   };
>
> +/*
> + * Table to convert BTRFS_RAID_* to the error code if minimum number of devices
> + * condition is not met. Zero means there's no corresponding
> + * BTRFS_ERROR_DEV_*_NOT_MET value.
> + */
> +const int btrfs_raid_mindev_error[BTRFS_NR_RAID_TYPES] = {
> +	[BTRFS_RAID_RAID10] = BTRFS_ERROR_DEV_RAID10_MIN_NOT_MET,
> +	[BTRFS_RAID_RAID1]  = BTRFS_ERROR_DEV_RAID1_MIN_NOT_MET,
> +	[BTRFS_RAID_DUP]    = 0,
> +	[BTRFS_RAID_RAID0]  = 0,
> +	[BTRFS_RAID_SINGLE] = 0,
> +	[BTRFS_RAID_RAID5]  = BTRFS_ERROR_DEV_RAID5_MIN_NOT_MET,
> +	[BTRFS_RAID_RAID6]  = BTRFS_ERROR_DEV_RAID6_MIN_NOT_MET,
> +};
> +
>   static int init_first_rw_device(struct btrfs_trans_handle *trans,
>   				struct btrfs_root *root,
>   				struct btrfs_device *device);
> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
> index c73d027e2f8b..a13a538cb01e 100644
> --- a/fs/btrfs/volumes.h
> +++ b/fs/btrfs/volumes.h
> @@ -340,7 +340,7 @@ struct btrfs_raid_attr {
>   };
>
>   extern const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES];
> -
> +extern const int btrfs_raid_mindev_error[BTRFS_NR_RAID_TYPES];
>   extern const u64 btrfs_raid_group[BTRFS_NR_RAID_TYPES];
>
>   struct map_lookup {
>

[PATCH 13/15] btrfs: use existing device constraints table btrfs_raid_array

[David Sterba]

We should avoid duplicating the device constraints, let's use the
btrfs_raid_array in btrfs_check_raid_min_devices.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/volumes.c | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index a67249582a6f..8fee24f92574 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1730,6 +1730,7 @@ static int btrfs_check_raid_min_devices(struct btrfs_fs_info *fs_info,
 {
 	u64 all_avail;
 	unsigned seq;
+	int i;
 
 	do {
 		seq = read_seqbegin(&fs_info->profiles_lock);
@@ -1739,22 +1740,16 @@ static int btrfs_check_raid_min_devices(struct btrfs_fs_info *fs_info,
 			    fs_info->avail_metadata_alloc_bits;
 	} while (read_seqretry(&fs_info->profiles_lock, seq));
 
-	if ((all_avail & BTRFS_BLOCK_GROUP_RAID10) && num_devices < 4) {
-		return BTRFS_ERROR_DEV_RAID10_MIN_NOT_MET;
-	}
-
-	if ((all_avail & BTRFS_BLOCK_GROUP_RAID1) && num_devices < 2) {
-		return BTRFS_ERROR_DEV_RAID1_MIN_NOT_MET;
-	}
+	for (i = 0; i < BTRFS_NR_RAID_TYPES; i++) {
+		if (!(all_avail & btrfs_raid_group[i]))
+			continue;
 
-	if ((all_avail & BTRFS_BLOCK_GROUP_RAID5) &&
-	    fs_info->fs_devices->rw_devices < 2) {
-		return BTRFS_ERROR_DEV_RAID5_MIN_NOT_MET;
-	}
+		if (num_devices < btrfs_raid_array[i].devs_min) {
+			int ret = btrfs_raid_mindev_error[i];
 
-	if ((all_avail & BTRFS_BLOCK_GROUP_RAID6) &&
-	    fs_info->fs_devices->rw_devices < 3) {
-		return BTRFS_ERROR_DEV_RAID6_MIN_NOT_MET;
+			if (ret)
+				return ret;
+		}
 	}
 
 	return 0;
-- 
2.7.1

Re: [PATCH 13/15] btrfs: use existing device constraints table btrfs_raid_array

[Anand Jain]

yep required optimization. Deleting from my todo list.

Reviewed-by: Anand Jain <anand.jain@oracle.com>


On 02/16/2016 01:34 AM, David Sterba wrote:
> We should avoid duplicating the device constraints, let's use the
> btrfs_raid_array in btrfs_check_raid_min_devices.
>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>   fs/btrfs/volumes.c | 23 +++++++++--------------
>   1 file changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index a67249582a6f..8fee24f92574 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1730,6 +1730,7 @@ static int btrfs_check_raid_min_devices(struct btrfs_fs_info *fs_info,
>   {
>   	u64 all_avail;
>   	unsigned seq;
> +	int i;
>
>   	do {
>   		seq = read_seqbegin(&fs_info->profiles_lock);
> @@ -1739,22 +1740,16 @@ static int btrfs_check_raid_min_devices(struct btrfs_fs_info *fs_info,
>   			    fs_info->avail_metadata_alloc_bits;
>   	} while (read_seqretry(&fs_info->profiles_lock, seq));
>
> -	if ((all_avail & BTRFS_BLOCK_GROUP_RAID10) && num_devices < 4) {
> -		return BTRFS_ERROR_DEV_RAID10_MIN_NOT_MET;
> -	}
> -
> -	if ((all_avail & BTRFS_BLOCK_GROUP_RAID1) && num_devices < 2) {
> -		return BTRFS_ERROR_DEV_RAID1_MIN_NOT_MET;
> -	}
> +	for (i = 0; i < BTRFS_NR_RAID_TYPES; i++) {
> +		if (!(all_avail & btrfs_raid_group[i]))
> +			continue;
>
> -	if ((all_avail & BTRFS_BLOCK_GROUP_RAID5) &&
> -	    fs_info->fs_devices->rw_devices < 2) {
> -		return BTRFS_ERROR_DEV_RAID5_MIN_NOT_MET;
> -	}
> +		if (num_devices < btrfs_raid_array[i].devs_min) {
> +			int ret = btrfs_raid_mindev_error[i];
>
> -	if ((all_avail & BTRFS_BLOCK_GROUP_RAID6) &&
> -	    fs_info->fs_devices->rw_devices < 3) {
> -		return BTRFS_ERROR_DEV_RAID6_MIN_NOT_MET;
> +			if (ret)
> +				return ret;
> +		}
>   	}
>
>   	return 0;
>

[PATCH 14/15] btrfs: rename btrfs_find_device_by_user_input

[David Sterba]

For clarity how we are going to find the device, let's call it a device
specifier, devspec for short. Also rename the arguments that are a
leftover from previous function purpose.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/dev-replace.c |  2 +-
 fs/btrfs/volumes.c     | 17 ++++++++++-------
 fs/btrfs/volumes.h     |  4 ++--
 3 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index 3e2616d151d9..1731f92b6247 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -322,7 +322,7 @@ int btrfs_dev_replace_start(struct btrfs_root *root,
 
 	/* the disk copy procedure reuses the scrub code */
 	mutex_lock(&fs_info->volume_mutex);
-	ret = btrfs_find_device_by_user_input(root, args->start.srcdevid,
+	ret = btrfs_find_device_by_devspec(root, args->start.srcdevid,
 					    args->start.srcdev_name,
 					    &src_device);
 	if (ret) {
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 8fee24f92574..05d9bc0cdd49 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1779,7 +1779,7 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path, u64 devid)
 	if (ret)
 		goto out;
 
-	ret = btrfs_find_device_by_user_input(root, devid, device_path,
+	ret = btrfs_find_device_by_devspec(root, devid, device_path,
 				&device);
 	if (ret)
 		goto out;
@@ -2065,23 +2065,26 @@ int btrfs_find_device_missing_or_by_path(struct btrfs_root *root,
 	}
 }
 
-int btrfs_find_device_by_user_input(struct btrfs_root *root, u64 srcdevid,
-					 char *srcdev_name,
+/*
+ * Lookup a device given by device id, or the path if the id is 0.
+ */
+int btrfs_find_device_by_devspec(struct btrfs_root *root, u64 devid,
+					 char *devpath,
 					 struct btrfs_device **device)
 {
 	int ret;
 
-	if (srcdevid) {
+	if (devid) {
 		ret = 0;
-		*device = btrfs_find_device(root->fs_info, srcdevid, NULL,
+		*device = btrfs_find_device(root->fs_info, devid, NULL,
 					    NULL);
 		if (!*device)
 			ret = -ENOENT;
 	} else {
-		if (!srcdev_name || !srcdev_name[0])
+		if (!devpath || !devpath[0])
 			return -EINVAL;
 
-		ret = btrfs_find_device_missing_or_by_path(root, srcdev_name,
+		ret = btrfs_find_device_missing_or_by_path(root, devpath,
 							   device);
 	}
 	return ret;
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index a13a538cb01e..febdb7bc9370 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -448,8 +448,8 @@ void btrfs_close_extra_devices(struct btrfs_fs_devices *fs_devices, int step);
 int btrfs_find_device_missing_or_by_path(struct btrfs_root *root,
 					 char *device_path,
 					 struct btrfs_device **device);
-int btrfs_find_device_by_user_input(struct btrfs_root *root, u64 srcdevid,
-					 char *srcdev_name,
+int btrfs_find_device_by_devspec(struct btrfs_root *root, u64 devid,
+					 char *devpath,
 					 struct btrfs_device **device);
 struct btrfs_device *btrfs_alloc_device(struct btrfs_fs_info *fs_info,
 					const u64 *devid,
-- 
2.7.1

Re: [PATCH 14/15] btrfs: rename btrfs_find_device_by_user_input

[Anand Jain]

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Thanks, Anand


On 02/16/2016 01:34 AM, David Sterba wrote:
> For clarity how we are going to find the device, let's call it a device
> specifier, devspec for short. Also rename the arguments that are a
> leftover from previous function purpose.
>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>   fs/btrfs/dev-replace.c |  2 +-
>   fs/btrfs/volumes.c     | 17 ++++++++++-------
>   fs/btrfs/volumes.h     |  4 ++--
>   3 files changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
> index 3e2616d151d9..1731f92b6247 100644
> --- a/fs/btrfs/dev-replace.c
> +++ b/fs/btrfs/dev-replace.c
> @@ -322,7 +322,7 @@ int btrfs_dev_replace_start(struct btrfs_root *root,
>
>   	/* the disk copy procedure reuses the scrub code */
>   	mutex_lock(&fs_info->volume_mutex);
> -	ret = btrfs_find_device_by_user_input(root, args->start.srcdevid,
> +	ret = btrfs_find_device_by_devspec(root, args->start.srcdevid,
>   					    args->start.srcdev_name,
>   					    &src_device);
>   	if (ret) {
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 8fee24f92574..05d9bc0cdd49 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1779,7 +1779,7 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path, u64 devid)
>   	if (ret)
>   		goto out;
>
> -	ret = btrfs_find_device_by_user_input(root, devid, device_path,
> +	ret = btrfs_find_device_by_devspec(root, devid, device_path,
>   				&device);
>   	if (ret)
>   		goto out;
> @@ -2065,23 +2065,26 @@ int btrfs_find_device_missing_or_by_path(struct btrfs_root *root,
>   	}
>   }
>
> -int btrfs_find_device_by_user_input(struct btrfs_root *root, u64 srcdevid,
> -					 char *srcdev_name,
> +/*
> + * Lookup a device given by device id, or the path if the id is 0.
> + */
> +int btrfs_find_device_by_devspec(struct btrfs_root *root, u64 devid,
> +					 char *devpath,
>   					 struct btrfs_device **device)
>   {
>   	int ret;
>
> -	if (srcdevid) {
> +	if (devid) {
>   		ret = 0;
> -		*device = btrfs_find_device(root->fs_info, srcdevid, NULL,
> +		*device = btrfs_find_device(root->fs_info, devid, NULL,
>   					    NULL);
>   		if (!*device)
>   			ret = -ENOENT;
>   	} else {
> -		if (!srcdev_name || !srcdev_name[0])
> +		if (!devpath || !devpath[0])
>   			return -EINVAL;
>
> -		ret = btrfs_find_device_missing_or_by_path(root, srcdev_name,
> +		ret = btrfs_find_device_missing_or_by_path(root, devpath,
>   							   device);
>   	}
>   	return ret;
> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
> index a13a538cb01e..febdb7bc9370 100644
> --- a/fs/btrfs/volumes.h
> +++ b/fs/btrfs/volumes.h
> @@ -448,8 +448,8 @@ void btrfs_close_extra_devices(struct btrfs_fs_devices *fs_devices, int step);
>   int btrfs_find_device_missing_or_by_path(struct btrfs_root *root,
>   					 char *device_path,
>   					 struct btrfs_device **device);
> -int btrfs_find_device_by_user_input(struct btrfs_root *root, u64 srcdevid,
> -					 char *srcdev_name,
> +int btrfs_find_device_by_devspec(struct btrfs_root *root, u64 devid,
> +					 char *devpath,
>   					 struct btrfs_device **device);
>   struct btrfs_device *btrfs_alloc_device(struct btrfs_fs_info *fs_info,
>   					const u64 *devid,
>

[PATCH 15/15] btrfs: rename flags for vol args v2

[David Sterba]

Rename BTRFS_DEVICE_BY_ID so it's more descriptive that we specify the
device by id, it'll be part of the public API. The mask of supported
flags is also renamed, only for internal use.

The error code for unknown flags is EOPNOTSUPP, fixed.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/ioctl.c           | 8 ++++----
 include/uapi/linux/btrfs.h | 7 ++++---
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 57fb05960435..6bcd7700b9fd 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -2689,8 +2689,8 @@ static long btrfs_ioctl_rm_dev_v2(struct file *file, void __user *arg)
 	}
 
 	/* Check for compatibility reject unknown flags */
-	if (vol_args->flags & ~BTRFS_VOL_ARG_V2_FLAGS)
-		return -ENOTTY;
+	if (vol_args->flags & ~BTRFS_VOL_ARG_V2_FLAGS_SUPPORTED)
+		return -EOPNOTSUPP;
 
 	if (atomic_xchg(&root->fs_info->mutually_exclusive_operation_running,
 			1)) {
@@ -2699,7 +2699,7 @@ static long btrfs_ioctl_rm_dev_v2(struct file *file, void __user *arg)
 	}
 
 	mutex_lock(&root->fs_info->volume_mutex);
-	if (vol_args->flags & BTRFS_DEVICE_BY_ID) {
+	if (vol_args->flags & BTRFS_DEVICE_SPEC_BY_ID) {
 		ret = btrfs_rm_device(root, NULL, vol_args->devid);
 	} else {
 		vol_args->name[BTRFS_PATH_NAME_MAX] = '\0';
@@ -2709,7 +2709,7 @@ static long btrfs_ioctl_rm_dev_v2(struct file *file, void __user *arg)
 	atomic_set(&root->fs_info->mutually_exclusive_operation_running, 0);
 
 	if (!ret) {
-		if (vol_args->flags & BTRFS_DEVICE_BY_ID)
+		if (vol_args->flags & BTRFS_DEVICE_SPEC_BY_ID)
 			btrfs_info(root->fs_info, "device deleted: id %llu",
 					vol_args->devid);
 		else
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index 396a4efca775..3975e683af72 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -36,12 +36,13 @@ struct btrfs_ioctl_vol_args {
 #define BTRFS_SUBVOL_CREATE_ASYNC	(1ULL << 0)
 #define BTRFS_SUBVOL_RDONLY		(1ULL << 1)
 #define BTRFS_SUBVOL_QGROUP_INHERIT	(1ULL << 2)
-#define BTRFS_DEVICE_BY_ID		(1ULL << 3)
-#define BTRFS_VOL_ARG_V2_FLAGS				\
+#define BTRFS_DEVICE_SPEC_BY_ID		(1ULL << 3)
+
+#define BTRFS_VOL_ARG_V2_FLAGS_SUPPORTED		\
 			(BTRFS_SUBVOL_CREATE_ASYNC |	\
 			BTRFS_SUBVOL_RDONLY |		\
 			BTRFS_SUBVOL_QGROUP_INHERIT |	\
-			BTRFS_DEVICE_BY_ID)
+			BTRFS_DEVICE_SPEC_BY_ID)
 
 #define BTRFS_FSID_SIZE 16
 #define BTRFS_UUID_SIZE 16
-- 
2.7.1

Re: [PATCH 15/15] btrfs: rename flags for vol args v2

[Anand Jain]

..

On 02/16/2016 01:34 AM, David Sterba wrote:
> Rename BTRFS_DEVICE_BY_ID so it's more descriptive that we specify the
> device by id, it'll be part of the public API. The mask of supported
> flags is also renamed, only for internal use.
>
> The error code for unknown flags is EOPNOTSUPP, fixed.
>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>   fs/btrfs/ioctl.c           | 8 ++++----
>   include/uapi/linux/btrfs.h | 7 ++++---
>   2 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 57fb05960435..6bcd7700b9fd 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -2689,8 +2689,8 @@ static long btrfs_ioctl_rm_dev_v2(struct file *file, void __user *arg)
>   	}
>
>   	/* Check for compatibility reject unknown flags */
> -	if (vol_args->flags & ~BTRFS_VOL_ARG_V2_FLAGS)
> -		return -ENOTTY;
> +	if (vol_args->flags & ~BTRFS_VOL_ARG_V2_FLAGS_SUPPORTED)
> +		return -EOPNOTSUPP;
>
>   	if (atomic_xchg(&root->fs_info->mutually_exclusive_operation_running,
>   			1)) {
> @@ -2699,7 +2699,7 @@ static long btrfs_ioctl_rm_dev_v2(struct file *file, void __user *arg)
>   	}
>
>   	mutex_lock(&root->fs_info->volume_mutex);
> -	if (vol_args->flags & BTRFS_DEVICE_BY_ID) {
> +	if (vol_args->flags & BTRFS_DEVICE_SPEC_BY_ID) {
>   		ret = btrfs_rm_device(root, NULL, vol_args->devid);
>   	} else {
>   		vol_args->name[BTRFS_PATH_NAME_MAX] = '\0';
> @@ -2709,7 +2709,7 @@ static long btrfs_ioctl_rm_dev_v2(struct file *file, void __user *arg)
>   	atomic_set(&root->fs_info->mutually_exclusive_operation_running, 0);
>
>   	if (!ret) {
> -		if (vol_args->flags & BTRFS_DEVICE_BY_ID)
> +		if (vol_args->flags & BTRFS_DEVICE_SPEC_BY_ID)
>   			btrfs_info(root->fs_info, "device deleted: id %llu",
>   					vol_args->devid);
>   		else
> diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
> index 396a4efca775..3975e683af72 100644
> --- a/include/uapi/linux/btrfs.h
> +++ b/include/uapi/linux/btrfs.h
> @@ -36,12 +36,13 @@ struct btrfs_ioctl_vol_args {
>   #define BTRFS_SUBVOL_CREATE_ASYNC	(1ULL << 0)
>   #define BTRFS_SUBVOL_RDONLY		(1ULL << 1)
>   #define BTRFS_SUBVOL_QGROUP_INHERIT	(1ULL << 2)
> -#define BTRFS_DEVICE_BY_ID		(1ULL << 3)
> -#define BTRFS_VOL_ARG_V2_FLAGS				\
> +#define BTRFS_DEVICE_SPEC_BY_ID		(1ULL << 3)
> +

  Just checked next/delete-by-id-v3.
  You may consider to update progs as well.

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Thanks, Anand



> +#define BTRFS_VOL_ARG_V2_FLAGS_SUPPORTED		\
>   			(BTRFS_SUBVOL_CREATE_ASYNC |	\
>   			BTRFS_SUBVOL_RDONLY |		\
>   			BTRFS_SUBVOL_QGROUP_INHERIT |	\
> -			BTRFS_DEVICE_BY_ID)
> +			BTRFS_DEVICE_SPEC_BY_ID)
>
>   #define BTRFS_FSID_SIZE 16
>   #define BTRFS_UUID_SIZE 16
>

Re: [PATCH 15/15] btrfs: rename flags for vol args v2

[David Sterba]

On Tue, Feb 16, 2016 at 05:18:12PM +0800, Anand Jain wrote:
> 
>   Just checked next/delete-by-id-v3.
>   You may consider to update progs as well.
> 
> Reviewed-by: Anand Jain <anand.jain@oracle.com>

Thanks for the reviews, I'll update the patches and push to next. Progs
update will follow.

Re: [PATCH 00/15] Device delete by id

[David Sterba]

On Mon, Feb 15, 2016 at 06:33:56PM +0100, David Sterba wrote:
> this patchset extends the ioctl arugments to take an id so we can delete a
> device by it. It reuses the existing structure btrfs_ioctl_vol_args_v2 and
> extends it in na backward-compatible way so that we don't need to introduce
> another one.
> 
> The core patchset is from Anand, I did some cleanups as I went through the
> series again and made some cleanups and minor naming tweaks to the interface.
> 
> I'll add the branch to for-next and if everything goes fine I'll send a pull
> request for 4.6 in a week.

This patchset is causing crashes in my setup, based on linux-next tree
with my for-next. I did a test round without it and it passes, so I'm
removing it from 4.6 queue. We need more time to find and fix it.

Re: [PATCH 00/15] Device delete by id

[Anand Jain]

On 02/26/2016 01:59 AM, David Sterba wrote:
> On Mon, Feb 15, 2016 at 06:33:56PM +0100, David Sterba wrote:
>> this patchset extends the ioctl arugments to take an id so we can delete a
>> device by it. It reuses the existing structure btrfs_ioctl_vol_args_v2 and
>> extends it in na backward-compatible way so that we don't need to introduce
>> another one.
>>
>> The core patchset is from Anand, I did some cleanups as I went through the
>> series again and made some cleanups and minor naming tweaks to the interface.
>>
>> I'll add the branch to for-next and if everything goes fine I'll send a pull
>> request for 4.6 in a week.
>
> This patchset is causing crashes in my setup, based on linux-next tree
> with my for-next. I did a test round without it and it passes, so I'm
> removing it from 4.6 queue. We need more time to find and fix it.

  (crash.. are you ref to the below ?)
  BUG: unable to handle kernel NULL pointer dereference
  scrub_setup_ctx.isra.19+0x1f6/0x260 [btrfs]

  Current status: as of now there is nothing that tells me the
  above crash was due to the set of patches in here, if there
  are further data I will be happy to review, also this wasn't
  reproducible.


  Also:
  Most of the xfstest btrfs/tests aren't designed to generate a
  consistent result, it can be consistent with a sequence of tests,
  but when this sequence of tests is altered, it may generate
  different results.

  As xfstest results also depends on the factors like state of
  stale FSID on the devices and state of btrfs_device list with
  in the kernel, which depends on the previous tests.

  And looks like following changes can make it consistent.
    Before each test/sub-test:
    . wipefs -a of all scratch pool dev
    . modprobe -r btrfs && modprobe btrfs
  So that kernel will have a clean state of btrfs_device list.

  Which means all the btrfs tests especially which are grouped
  under replace should be updated with the above change, So that
  we can ensure kernel and devices are in the same state each
  time the test is run.

  Any idea?

Thanks, Anand