All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v9] Add cli and ioctl to forget scanned device(s)
@ 2018-10-12  4:06 Anand Jain
  2018-10-12  4:06 ` [PATCH] btrfs: introduce feature to forget a btrfs device Anand Jain
                   ` (2 more replies)
  0 siblings, 3 replies; 21+ messages in thread
From: Anand Jain @ 2018-10-12  4:06 UTC (permalink / raw)
  To: linux-btrfs

v9:
 Make forget as a btrfs device scan option.
 Use forget in the fstests, now you can run fstests with btrfs as rootfs
  which helps to exercise the uuid_mutex lock.

v8:
 Change log update in the kernel patch.

v7:
 Use struct btrfs_ioctl_vol_args (instead of struct
  btrfs_ioctl_vol_args_v2) as its inline with other ioctl
  btrfs-control
 The CLI usage/features remains same. However internally the ioctl flag
  is not required to delete all the unmounted devices. Instead leave
  btrfs_ioctl_vol_args::name NULL.

v6:
 Use the changed fn name btrfs_free_stale_devices().

 Change in title:
 Old v5:
 Cover-letter:
  [PATCH v5] Add cli and ioctl to ignore a scanned device
 Kernel:
  [PATCH v5] btrfs: introduce feature to ignore a btrfs device
 Progs:
  [PATCH v5] btrfs-progs: add 'btrfs device ignore' cli

v5:
  Adds feature to delete all stale devices
  Reuses btrfs_free_stale_devices() fn and so depends on the
    patch-set [1] in the ML.
  Uses struct btrfs_ioctl_vol_args_v2 instead of
    struct btrfs_ioctl_vol_args as arg
  Does the device path matching instead of btrfs_device matching
    (we won't delete the mounted device as btrfs_free_stale_devices()
    checks for it)
v4:
  No change. But as the ML thread may be confusing, so resend.
v3:
  No change. Send to correct ML.
v2:
  Accepts review from Nikolay, details are in the specific patch.
  Patch 1/2 is renamed from
    [PATCH 1/2] btrfs: refactor btrfs_free_stale_device() to get device list delete
  to
    [PATCH 1/2] btrfs: add function to device list delete

Adds cli and ioctl to forget a scanned device or forget all stale
devices in the kernel.


Anand Jain (1):
  btrfs: introduce feature to forget a btrfs device

 fs/btrfs/super.c           | 3 +++
 fs/btrfs/volumes.c         | 9 +++++++++
 fs/btrfs/volumes.h         | 1 +
 include/uapi/linux/btrfs.h | 2 ++
 4 files changed, 15 insertions(+)

Anand Jain (1):
  btrfs-progs: add cli to forget one or all scanned devices

 cmds-device.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++---------
 ioctl.h       |  2 ++
 2 files changed, 56 insertions(+), 9 deletions(-)

[1]
Anand Jain (1):
  fstests: btrfs use forget if not reload

 common/btrfs    | 20 ++++++++++++++++++++
 tests/btrfs/124 |  6 +++---
 tests/btrfs/125 |  6 +++---
 tests/btrfs/154 |  6 +++---
 tests/btrfs/164 |  4 ++--
 5 files changed, 31 insertions(+), 11 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH] btrfs: introduce feature to forget a btrfs device
  2018-10-12  4:06 [PATCH v9] Add cli and ioctl to forget scanned device(s) Anand Jain
@ 2018-10-12  4:06 ` Anand Jain
  2018-10-12  8:48   ` Nikolay Borisov
  2018-10-12  4:06 ` [PATCH] btrfs-progs: add cli to forget one or all scanned devices Anand Jain
  2018-10-12  4:06 ` [PATCH] fstests: btrfs use forget if not reload Anand Jain
  2 siblings, 1 reply; 21+ messages in thread
From: Anand Jain @ 2018-10-12  4:06 UTC (permalink / raw)
  To: linux-btrfs

Support for a new command 'btrfs dev forget [dev]' is proposed here
to undo the effects of 'btrfs dev scan [dev]'. For this purpose
this patch proposes to use ioctl #5 as it was empty.
	IOW(BTRFS_IOCTL_MAGIC, 5, ..)
This patch adds new ioctl BTRFS_IOC_FORGET_DEV which can be sent from
the /dev/btrfs-control to forget one or all devices, (devices which are
not mounted) from the btrfs kernel.

The argument it takes is struct btrfs_ioctl_vol_args, and ::name can be
set to specify the device path. And all unmounted devices can be removed
from the kernel if no device path is provided.

Again, the devices are removed only if the relevant fsid aren't mounted.

This new cli can provide..
 . Release of unwanted btrfs_fs_devices and btrfs_devices memory if the
   device is not going to be mounted.
 . Ability to mount the device in degraded mode when one of the other
   device is corrupted like in split brain raid1.
 . Running test cases which requires btrfs.ko-reload if the rootfs
   is btrfs.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/super.c           | 3 +++
 fs/btrfs/volumes.c         | 9 +++++++++
 fs/btrfs/volumes.h         | 1 +
 include/uapi/linux/btrfs.h | 2 ++
 4 files changed, 15 insertions(+)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 6601c9aa5e35..f14610ef1c66 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2244,6 +2244,9 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
 		ret = PTR_ERR_OR_ZERO(device);
 		mutex_unlock(&uuid_mutex);
 		break;
+	case BTRFS_IOC_FORGET_DEV:
+		ret = btrfs_forget_devices(vol->name);
+		break;
 	case BTRFS_IOC_DEVICES_READY:
 		mutex_lock(&uuid_mutex);
 		device = btrfs_scan_one_device(vol->name, FMODE_READ,
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index f4405e430da6..e2ab203d9c1d 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1208,6 +1208,15 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr,
 	return 0;
 }
 
+int btrfs_forget_devices(const char *path)
+{
+	mutex_lock(&uuid_mutex);
+	btrfs_free_stale_devices(strlen(path) ? path:NULL, NULL);
+	mutex_unlock(&uuid_mutex);
+
+	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
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 23e9285d88de..394079d5d08d 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -406,6 +406,7 @@ int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
 		       fmode_t flags, void *holder);
 struct btrfs_device *btrfs_scan_one_device(const char *path,
 					   fmode_t flags, void *holder);
+int btrfs_forget_devices(const char *path);
 int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
 void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices, int step);
 void btrfs_assign_next_active_device(struct btrfs_device *device,
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index 5ca1d21fc4a7..b1be7f828cb4 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -836,6 +836,8 @@ enum btrfs_err_code {
 				   struct btrfs_ioctl_vol_args)
 #define BTRFS_IOC_SCAN_DEV _IOW(BTRFS_IOCTL_MAGIC, 4, \
 				   struct btrfs_ioctl_vol_args)
+#define BTRFS_IOC_FORGET_DEV _IOW(BTRFS_IOCTL_MAGIC, 5, \
+				   struct btrfs_ioctl_vol_args)
 /* trans start and trans end are dangerous, and only for
  * use by applications that know how to avoid the
  * resulting deadlocks
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH] btrfs-progs: add cli to forget one or all scanned devices
  2018-10-12  4:06 [PATCH v9] Add cli and ioctl to forget scanned device(s) Anand Jain
  2018-10-12  4:06 ` [PATCH] btrfs: introduce feature to forget a btrfs device Anand Jain
@ 2018-10-12  4:06 ` Anand Jain
  2018-10-12  9:03   ` Nikolay Borisov
  2018-10-12  4:06 ` [PATCH] fstests: btrfs use forget if not reload Anand Jain
  2 siblings, 1 reply; 21+ messages in thread
From: Anand Jain @ 2018-10-12  4:06 UTC (permalink / raw)
  To: linux-btrfs

This patch adds cli
  btrfs device forget [dev]
to remove the given device structure in the kernel if the device
is unmounted. If no argument is given it shall remove all stale
(device which are not mounted) from the kernel.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-device.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++---------
 ioctl.h       |  2 ++
 2 files changed, 56 insertions(+), 9 deletions(-)

diff --git a/cmds-device.c b/cmds-device.c
index 2a05f70a76a9..ecc391ea01d8 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -254,10 +254,32 @@ static int cmd_device_delete(int argc, char **argv)
 	return _cmd_device_remove(argc, argv, cmd_device_delete_usage);
 }
 
+static int btrfs_forget_devices(char *path)
+{
+	struct btrfs_ioctl_vol_args args;
+	int ret;
+	int fd;
+
+	fd = open("/dev/btrfs-control", O_RDWR);
+	if (fd < 0)
+		return -errno;
+
+	memset(&args, 0, sizeof(args));
+	if (path)
+		strncpy_null(args.name, path);
+	ret = ioctl(fd, BTRFS_IOC_FORGET_DEV, &args);
+	if (ret)
+		ret = -errno;
+	close(fd);
+	return ret;
+}
+
 static const char * const cmd_device_scan_usage[] = {
-	"btrfs device scan [(-d|--all-devices)|<device> [<device>...]]",
-	"Scan devices for a btrfs filesystem",
+	"btrfs device scan [(-d|--all-devices)|-u|--forget|<device> "\
+							"[<device>...]]",
+	"Scan or forget (deregister) devices for a btrfs filesystem",
 	" -d|--all-devices (deprecated)",
+	" -u|--forget [<device> ..]",
 	NULL
 };
 
@@ -267,32 +289,40 @@ static int cmd_device_scan(int argc, char **argv)
 	int devstart;
 	int all = 0;
 	int ret = 0;
+	int forget = 0;
 
 	optind = 0;
 	while (1) {
 		int c;
 		static const struct option long_options[] = {
 			{ "all-devices", no_argument, NULL, 'd'},
+			{ "forget", no_argument, NULL, 'u'},
 			{ NULL, 0, NULL, 0}
 		};
 
-		c = getopt_long(argc, argv, "d", long_options, NULL);
+		c = getopt_long(argc, argv, "du", long_options, NULL);
 		if (c < 0)
 			break;
 		switch (c) {
 		case 'd':
 			all = 1;
 			break;
+		case 'u':
+			forget = 1;
+			break;
 		default:
 			usage(cmd_device_scan_usage);
 		}
 	}
 	devstart = optind;
 
+	if (all && forget)
+		usage(cmd_device_scan_usage);
+
 	if (all && check_argc_max(argc - optind, 1))
 		usage(cmd_device_scan_usage);
 
-	if (all || argc - optind == 0) {
+	if (!forget && (all || argc - optind == 0)) {
 		printf("Scanning for Btrfs filesystems\n");
 		ret = btrfs_scan_devices();
 		error_on(ret, "error %d while scanning", ret);
@@ -301,6 +331,13 @@ static int cmd_device_scan(int argc, char **argv)
 		goto out;
 	}
 
+	if (forget && (all || argc - optind == 0)) {
+		ret = btrfs_forget_devices(NULL);
+		if (ret)
+			error("Can't forget: %s", strerror(-ret));
+		goto out;
+	}
+
 	for( i = devstart ; i < argc ; i++ ){
 		char *path;
 
@@ -315,11 +352,19 @@ static int cmd_device_scan(int argc, char **argv)
 			ret = 1;
 			goto out;
 		}
-		printf("Scanning for Btrfs filesystems in '%s'\n", path);
-		if (btrfs_register_one_device(path) != 0) {
-			ret = 1;
-			free(path);
-			goto out;
+		if (forget) {
+			ret = btrfs_forget_devices(path);
+			if (ret)
+				error("Can't forget '%s': %s",
+							path, strerror(-ret));
+		} else {
+			printf("Scanning for Btrfs filesystems in '%s'\n",
+									path);
+			if (btrfs_register_one_device(path) != 0) {
+				ret = 1;
+				free(path);
+				goto out;
+			}
 		}
 		free(path);
 	}
diff --git a/ioctl.h b/ioctl.h
index 709e996f401c..e27d80e09392 100644
--- a/ioctl.h
+++ b/ioctl.h
@@ -721,6 +721,8 @@ static inline char *btrfs_err_str(enum btrfs_err_code err_code)
 				   struct btrfs_ioctl_vol_args)
 #define BTRFS_IOC_SCAN_DEV _IOW(BTRFS_IOCTL_MAGIC, 4, \
 				   struct btrfs_ioctl_vol_args)
+#define BTRFS_IOC_FORGET_DEV _IOW(BTRFS_IOCTL_MAGIC, 5, \
+				   struct btrfs_ioctl_vol_args)
 /* trans start and trans end are dangerous, and only for
  * use by applications that know how to avoid the
  * resulting deadlocks
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH] fstests: btrfs use forget if not reload
  2018-10-12  4:06 [PATCH v9] Add cli and ioctl to forget scanned device(s) Anand Jain
  2018-10-12  4:06 ` [PATCH] btrfs: introduce feature to forget a btrfs device Anand Jain
  2018-10-12  4:06 ` [PATCH] btrfs-progs: add cli to forget one or all scanned devices Anand Jain
@ 2018-10-12  4:06 ` Anand Jain
  2 siblings, 0 replies; 21+ messages in thread
From: Anand Jain @ 2018-10-12  4:06 UTC (permalink / raw)
  To: linux-btrfs

btrfs reload was introduced to cleanup the device list inside the btrfs
kernel module.

The problem with the reload approach is that you can't run btrfs test
cases 124,125, 154 and 164 on the system with btrfs as root fs.

Now as we are introducing the btrfs forget feature as an btrfs device
scan option [1], so here are its pertaining changes in the fstests.

So these changes ensures we use the forget feature where available and
if its not then falls back to the reload approach.

[1]
btrfs-progs: add cli to forget one or all scanned devices
btrfs: introduce feature to forget a btrfs device

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 common/btrfs    | 20 ++++++++++++++++++++
 tests/btrfs/124 |  6 +++---
 tests/btrfs/125 |  6 +++---
 tests/btrfs/154 |  6 +++---
 tests/btrfs/164 |  4 ++--
 5 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/common/btrfs b/common/btrfs
index 26dc0bb9600f..c7fbec11c8c1 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -374,3 +374,23 @@ _scratch_btrfs_sectorsize()
 	$BTRFS_UTIL_PROG inspect-internal dump-super $SCRATCH_DEV |\
 		grep sectorsize | awk '{print $2}'
 }
+
+_btrfs_supports_forget()
+{
+	$BTRFS_UTIL_PROG device scan --help | grep -wq forget && \
+		$BTRFS_UTIL_PROG device scan --forget > /dev/null 2>&1
+}
+
+_require_btrfs_forget_if_not_fs_loadable()
+{
+	_btrfs_supports_forget && return
+
+	_require_loadable_fs_module "btrfs"
+}
+
+_btrfs_forget_if_not_fs_reload()
+{
+	_btrfs_supports_forget && return
+
+	_reload_fs_module "btrfs"
+}
diff --git a/tests/btrfs/124 b/tests/btrfs/124
index ce3ad6aa3a58..edbeff1443f5 100755
--- a/tests/btrfs/124
+++ b/tests/btrfs/124
@@ -51,7 +51,7 @@ _supported_fs btrfs
 _supported_os Linux
 _require_scratch_dev_pool 2
 _test_unmount
-_require_loadable_fs_module "btrfs"
+_require_btrfs_forget_if_not_fs_loadable
 
 _scratch_dev_pool_get 2
 
@@ -86,7 +86,7 @@ echo "clean btrfs ko" >> $seqres.full
 _scratch_unmount
 
 # un-scan the btrfs devices
-_reload_fs_module "btrfs"
+_btrfs_forget_if_not_fs_reload
 
 echo >> $seqres.full
 echo "-----Write degraded mount fill upto $max_fs_sz bytes-----" >> $seqres.full
@@ -125,7 +125,7 @@ echo
 echo "Mount degraded with the other dev"
 _scratch_unmount
 # un-scan the btrfs devices
-_reload_fs_module "btrfs"
+_btrfs_forget_if_not_fs_reload
 _mount -o degraded $dev2 $SCRATCH_MNT >>$seqres.full 2>&1
 _run_btrfs_util_prog filesystem show
 checkpoint3=`md5sum $SCRATCH_MNT/tf2`
diff --git a/tests/btrfs/125 b/tests/btrfs/125
index e38de264b28e..9161a2ddeaad 100755
--- a/tests/btrfs/125
+++ b/tests/btrfs/125
@@ -50,7 +50,7 @@ _supported_fs btrfs
 _supported_os Linux
 _require_scratch_dev_pool 3
 _test_unmount
-_require_loadable_fs_module "btrfs"
+_require_btrfs_forget_if_not_fs_loadable
 
 _scratch_dev_pool_get 3
 
@@ -102,7 +102,7 @@ echo "unmount" >> $seqres.full
 _scratch_unmount
 echo "clean btrfs ko" >> $seqres.full
 # un-scan the btrfs devices
-_reload_fs_module "btrfs"
+_btrfs_forget_if_not_fs_reload
 _mount -o degraded,device=$dev2 $dev1 $SCRATCH_MNT >>$seqres.full 2>&1
 dd if=/dev/zero of="$SCRATCH_MNT"/tf2 bs=$bs count=$count \
 					>>$seqres.full 2>&1
@@ -138,7 +138,7 @@ echo "Mount degraded but with other dev"
 
 _scratch_unmount
 # un-scan the btrfs devices
-_reload_fs_module "btrfs"
+_btrfs_forget_if_not_fs_reload
 
 _mount -o degraded,device=${dev2} $dev3 $SCRATCH_MNT >>$seqres.full 2>&1
 
diff --git a/tests/btrfs/154 b/tests/btrfs/154
index 99ea232aba4c..01745d20e9fc 100755
--- a/tests/btrfs/154
+++ b/tests/btrfs/154
@@ -36,7 +36,7 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch_dev_pool 2
-_require_loadable_fs_module "btrfs"
+_require_btrfs_forget_if_not_fs_loadable
 
 _scratch_dev_pool_get 2
 
@@ -90,7 +90,7 @@ degrade_mount_write()
 
 	echo "clean btrfs ko" >> $seqres.full
 	# un-scan the btrfs devices
-	_reload_fs_module "btrfs"
+	_btrfs_forget_if_not_fs_reload
 	_mount -o degraded $DEV1 $SCRATCH_MNT >>$seqres.full 2>&1
 	cnt=$(( $COUNT/10 ))
 	dd if=/dev/urandom of="$SCRATCH_MNT"/tf1 bs=$bs count=$cnt \
@@ -142,7 +142,7 @@ verify()
 	echo "unmount" >> $seqres.full
 
 	_scratch_unmount
-	_reload_fs_module "btrfs"
+	_btrfs_forget_if_not_fs_reload
 	_mount -o degraded $DEV2 $SCRATCH_MNT >>$seqres.full 2>&1
 	verify_checkpoint1=`md5sum $SCRATCH_MNT/tf1`
 	verify_checkpoint2=`md5sum $SCRATCH_MNT/tf2`
diff --git a/tests/btrfs/164 b/tests/btrfs/164
index 097191a0e493..55042c4035e0 100755
--- a/tests/btrfs/164
+++ b/tests/btrfs/164
@@ -36,7 +36,7 @@ rm -f $seqres.full
 # Modify as appropriate.
 _supported_fs btrfs
 _supported_os Linux
-_require_loadable_fs_module "btrfs"
+_require_btrfs_forget_if_not_fs_loadable
 _require_scratch_dev_pool 2
 
 _scratch_dev_pool_get 2
@@ -69,7 +69,7 @@ delete_seed()
 {
 	_run_btrfs_util_prog device delete $dev_seed $SCRATCH_MNT
 	_scratch_unmount
-	_reload_fs_module "btrfs"
+	_btrfs_forget_if_not_fs_reload
 	run_check _mount $dev_sprout $SCRATCH_MNT
 	_run_btrfs_util_prog filesystem show -m $SCRATCH_MNT
 	echo -- sprout --
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH] btrfs: introduce feature to forget a btrfs device
  2018-10-12  4:06 ` [PATCH] btrfs: introduce feature to forget a btrfs device Anand Jain
@ 2018-10-12  8:48   ` Nikolay Borisov
  2018-10-24  4:31     ` Anand Jain
  0 siblings, 1 reply; 21+ messages in thread
From: Nikolay Borisov @ 2018-10-12  8:48 UTC (permalink / raw)
  To: Anand Jain, linux-btrfs



On 12.10.2018 07:06, Anand Jain wrote:
> Support for a new command 'btrfs dev forget [dev]' is proposed here
> to undo the effects of 'btrfs dev scan [dev]'. For this purpose
> this patch proposes to use ioctl #5 as it was empty.
> 	IOW(BTRFS_IOCTL_MAGIC, 5, ..)
> This patch adds new ioctl BTRFS_IOC_FORGET_DEV which can be sent from
> the /dev/btrfs-control to forget one or all devices, (devices which are
> not mounted) from the btrfs kernel.
> 
> The argument it takes is struct btrfs_ioctl_vol_args, and ::name can be
> set to specify the device path. And all unmounted devices can be removed
> from the kernel if no device path is provided.
> 
> Again, the devices are removed only if the relevant fsid aren't mounted.
> 
> This new cli can provide..
>  . Release of unwanted btrfs_fs_devices and btrfs_devices memory if the
>    device is not going to be mounted.
>  . Ability to mount the device in degraded mode when one of the other
>    device is corrupted like in split brain raid1.
>  . Running test cases which requires btrfs.ko-reload if the rootfs
>    is btrfs.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  fs/btrfs/super.c           | 3 +++
>  fs/btrfs/volumes.c         | 9 +++++++++
>  fs/btrfs/volumes.h         | 1 +
>  include/uapi/linux/btrfs.h | 2 ++
>  4 files changed, 15 insertions(+)
> 
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 6601c9aa5e35..f14610ef1c66 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -2244,6 +2244,9 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
>  		ret = PTR_ERR_OR_ZERO(device);
>  		mutex_unlock(&uuid_mutex);
>  		break;
> +	case BTRFS_IOC_FORGET_DEV:
> +		ret = btrfs_forget_devices(vol->name);
> +		break;
>  	case BTRFS_IOC_DEVICES_READY:
>  		mutex_lock(&uuid_mutex);
>  		device = btrfs_scan_one_device(vol->name, FMODE_READ,
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index f4405e430da6..e2ab203d9c1d 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1208,6 +1208,15 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr,
>  	return 0;
>  }
>  
> +int btrfs_forget_devices(const char *path)
> +{
> +	mutex_lock(&uuid_mutex);
> +	btrfs_free_stale_devices(strlen(path) ? path:NULL, NULL);

Checking the len of path is redundant since userspace will pass a zeroed
struct which will be NULL in any case. Additionally in
btrfs_free_stalce_devices there is existing check whether path is set or
not.

> +	mutex_unlock(&uuid_mutex);
> +
> +	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
> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
> index 23e9285d88de..394079d5d08d 100644
> --- a/fs/btrfs/volumes.h
> +++ b/fs/btrfs/volumes.h
> @@ -406,6 +406,7 @@ int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
>  		       fmode_t flags, void *holder);
>  struct btrfs_device *btrfs_scan_one_device(const char *path,
>  					   fmode_t flags, void *holder);
> +int btrfs_forget_devices(const char *path);
>  int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
>  void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices, int step);
>  void btrfs_assign_next_active_device(struct btrfs_device *device,
> diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
> index 5ca1d21fc4a7..b1be7f828cb4 100644
> --- a/include/uapi/linux/btrfs.h
> +++ b/include/uapi/linux/btrfs.h
> @@ -836,6 +836,8 @@ enum btrfs_err_code {
>  				   struct btrfs_ioctl_vol_args)
>  #define BTRFS_IOC_SCAN_DEV _IOW(BTRFS_IOCTL_MAGIC, 4, \
>  				   struct btrfs_ioctl_vol_args)
> +#define BTRFS_IOC_FORGET_DEV _IOW(BTRFS_IOCTL_MAGIC, 5, \
> +				   struct btrfs_ioctl_vol_args)
>  /* trans start and trans end are dangerous, and only for
>   * use by applications that know how to avoid the
>   * resulting deadlocks
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] btrfs-progs: add cli to forget one or all scanned devices
  2018-10-12  4:06 ` [PATCH] btrfs-progs: add cli to forget one or all scanned devices Anand Jain
@ 2018-10-12  9:03   ` Nikolay Borisov
  2018-10-24  4:31     ` Anand Jain
  0 siblings, 1 reply; 21+ messages in thread
From: Nikolay Borisov @ 2018-10-12  9:03 UTC (permalink / raw)
  To: Anand Jain, linux-btrfs



On 12.10.2018 07:06, Anand Jain wrote:
> This patch adds cli
>   btrfs device forget [dev]
> to remove the given device structure in the kernel if the device
> is unmounted. If no argument is given it shall remove all stale
> (device which are not mounted) from the kernel.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  cmds-device.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++---------
>  ioctl.h       |  2 ++
>  2 files changed, 56 insertions(+), 9 deletions(-)
> 
> diff --git a/cmds-device.c b/cmds-device.c
> index 2a05f70a76a9..ecc391ea01d8 100644
> --- a/cmds-device.c
> +++ b/cmds-device.c
> @@ -254,10 +254,32 @@ static int cmd_device_delete(int argc, char **argv)
>  	return _cmd_device_remove(argc, argv, cmd_device_delete_usage);
>  }
>  
> +static int btrfs_forget_devices(char *path)
> +{
> +	struct btrfs_ioctl_vol_args args;
> +	int ret;
> +	int fd;
> +
> +	fd = open("/dev/btrfs-control", O_RDWR);
> +	if (fd < 0)
> +		return -errno;
> +
> +	memset(&args, 0, sizeof(args));
> +	if (path)
> +		strncpy_null(args.name, path);
> +	ret = ioctl(fd, BTRFS_IOC_FORGET_DEV, &args);
> +	if (ret)
> +		ret = -errno;
> +	close(fd);
> +	return ret;
> +}
> +
>  static const char * const cmd_device_scan_usage[] = {
> -	"btrfs device scan [(-d|--all-devices)|<device> [<device>...]]",
> -	"Scan devices for a btrfs filesystem",
> +	"btrfs device scan [(-d|--all-devices)|-u|--forget|<device> "\
> +							"[<device>...]]",
> +	"Scan or forget (deregister) devices for a btrfs filesystem",
>  	" -d|--all-devices (deprecated)",
> +	" -u|--forget [<device> ..]",
>  	NULL
>  };
>  
> @@ -267,32 +289,40 @@ static int cmd_device_scan(int argc, char **argv)
>  	int devstart;
>  	int all = 0;
>  	int ret = 0;
> +	int forget = 0;
>  
>  	optind = 0;
>  	while (1) {
>  		int c;
>  		static const struct option long_options[] = {
>  			{ "all-devices", no_argument, NULL, 'd'},
> +			{ "forget", no_argument, NULL, 'u'},
>  			{ NULL, 0, NULL, 0}
>  		};
>  
> -		c = getopt_long(argc, argv, "d", long_options, NULL);
> +		c = getopt_long(argc, argv, "du", long_options, NULL);
>  		if (c < 0)
>  			break;
>  		switch (c) {
>  		case 'd':
>  			all = 1;
>  			break;
> +		case 'u':
> +			forget = 1;
> +			break;
>  		default:
>  			usage(cmd_device_scan_usage);
>  		}
>  	}
>  	devstart = optind;
>  
> +	if (all && forget)
> +		usage(cmd_device_scan_usage);
> +
>  	if (all && check_argc_max(argc - optind, 1))
>  		usage(cmd_device_scan_usage);
>  
> -	if (all || argc - optind == 0) {
> +	if (!forget && (all || argc - optind == 0)) {
>  		printf("Scanning for Btrfs filesystems\n");
>  		ret = btrfs_scan_devices();
>  		error_on(ret, "error %d while scanning", ret);
> @@ -301,6 +331,13 @@ static int cmd_device_scan(int argc, char **argv)
>  		goto out;
>  	}
>  
> +	if (forget && (all || argc - optind == 0)) {

You cannot ever have (forget && all) since you specifically call usage()
above, just remove the 'all'. Furthermore, I think the code will be more
legible if it's part of an else if rather than a plain if construct. In
fact the whole function will be more readable. I.e you will have the
error conditions at the to, followed by "all device scan" case, followed
by "all device forget" and in the final else branch you will have the
specific device handling. The increased indentation level won't be a
problem since the longest lines are string prints and we are not
wrapping those.


> +		ret = btrfs_forget_devices(NULL);
> +		if (ret)
> +			error("Can't forget: %s", strerror(-ret));
> +		goto out;
> +	}
> +
>  	for( i = devstart ; i < argc ; i++ ){
>  		char *path;
>  
> @@ -315,11 +352,19 @@ static int cmd_device_scan(int argc, char **argv)
>  			ret = 1;
>  			goto out;
>  		}
> -		printf("Scanning for Btrfs filesystems in '%s'\n", path);
> -		if (btrfs_register_one_device(path) != 0) {
> -			ret = 1;
> -			free(path);
> -			goto out;
> +		if (forget) {
> +			ret = btrfs_forget_devices(path);
> +			if (ret)
> +				error("Can't forget '%s': %s",
> +							path, strerror(-ret));

For consistency sake I think this error printout should be moved inside
btrfs_forget_devices() similarly to what btrfs_register_one_device does
on error.

> +		} else {
> +			printf("Scanning for Btrfs filesystems in '%s'\n",
> +									path);
> +			if (btrfs_register_one_device(path) != 0) {
> +				ret = 1;
> +				free(path);
> +				goto out;
> +			}
>  		}
>  		free(path);
>  	}
> diff --git a/ioctl.h b/ioctl.h
> index 709e996f401c..e27d80e09392 100644
> --- a/ioctl.h
> +++ b/ioctl.h
> @@ -721,6 +721,8 @@ static inline char *btrfs_err_str(enum btrfs_err_code err_code)
>  				   struct btrfs_ioctl_vol_args)
>  #define BTRFS_IOC_SCAN_DEV _IOW(BTRFS_IOCTL_MAGIC, 4, \
>  				   struct btrfs_ioctl_vol_args)
> +#define BTRFS_IOC_FORGET_DEV _IOW(BTRFS_IOCTL_MAGIC, 5, \
> +				   struct btrfs_ioctl_vol_args)
>  /* trans start and trans end are dangerous, and only for
>   * use by applications that know how to avoid the
>   * resulting deadlocks
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] btrfs: introduce feature to forget a btrfs device
  2018-10-12  8:48   ` Nikolay Borisov
@ 2018-10-24  4:31     ` Anand Jain
  0 siblings, 0 replies; 21+ messages in thread
From: Anand Jain @ 2018-10-24  4:31 UTC (permalink / raw)
  To: Nikolay Borisov, linux-btrfs



On 10/12/2018 04:48 PM, Nikolay Borisov wrote:
> 
> 
> On 12.10.2018 07:06, Anand Jain wrote:
>> Support for a new command 'btrfs dev forget [dev]' is proposed here
>> to undo the effects of 'btrfs dev scan [dev]'. For this purpose
>> this patch proposes to use ioctl #5 as it was empty.
>> 	IOW(BTRFS_IOCTL_MAGIC, 5, ..)
>> This patch adds new ioctl BTRFS_IOC_FORGET_DEV which can be sent from
>> the /dev/btrfs-control to forget one or all devices, (devices which are
>> not mounted) from the btrfs kernel.
>>
>> The argument it takes is struct btrfs_ioctl_vol_args, and ::name can be
>> set to specify the device path. And all unmounted devices can be removed
>> from the kernel if no device path is provided.
>>
>> Again, the devices are removed only if the relevant fsid aren't mounted.
>>
>> This new cli can provide..
>>   . Release of unwanted btrfs_fs_devices and btrfs_devices memory if the
>>     device is not going to be mounted.
>>   . Ability to mount the device in degraded mode when one of the other
>>     device is corrupted like in split brain raid1.
>>   . Running test cases which requires btrfs.ko-reload if the rootfs
>>     is btrfs.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>>   fs/btrfs/super.c           | 3 +++
>>   fs/btrfs/volumes.c         | 9 +++++++++
>>   fs/btrfs/volumes.h         | 1 +
>>   include/uapi/linux/btrfs.h | 2 ++
>>   4 files changed, 15 insertions(+)
>>
>> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
>> index 6601c9aa5e35..f14610ef1c66 100644
>> --- a/fs/btrfs/super.c
>> +++ b/fs/btrfs/super.c
>> @@ -2244,6 +2244,9 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
>>   		ret = PTR_ERR_OR_ZERO(device);
>>   		mutex_unlock(&uuid_mutex);
>>   		break;
>> +	case BTRFS_IOC_FORGET_DEV:
>> +		ret = btrfs_forget_devices(vol->name);
>> +		break;
>>   	case BTRFS_IOC_DEVICES_READY:
>>   		mutex_lock(&uuid_mutex);
>>   		device = btrfs_scan_one_device(vol->name, FMODE_READ,
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index f4405e430da6..e2ab203d9c1d 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -1208,6 +1208,15 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr,
>>   	return 0;
>>   }
>>   
>> +int btrfs_forget_devices(const char *path)
>> +{
>> +	mutex_lock(&uuid_mutex);
>> +	btrfs_free_stale_devices(strlen(path) ? path:NULL, NULL);
> 
> Checking the len of path is redundant since userspace will pass a zeroed
> struct which will be NULL in any case. Additionally in
> btrfs_free_stalce_devices there is existing check whether path is set or
> not.

That won't work. btrfs_free_stalce_devices() expects the pointer to be 
NULL. User space has zeroed the name which is char array and its pointer 
is not NULL.

Thanks, Anand

>> +	mutex_unlock(&uuid_mutex);
>> +
>> +	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
>> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
>> index 23e9285d88de..394079d5d08d 100644
>> --- a/fs/btrfs/volumes.h
>> +++ b/fs/btrfs/volumes.h
>> @@ -406,6 +406,7 @@ int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
>>   		       fmode_t flags, void *holder);
>>   struct btrfs_device *btrfs_scan_one_device(const char *path,
>>   					   fmode_t flags, void *holder);
>> +int btrfs_forget_devices(const char *path);
>>   int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
>>   void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices, int step);
>>   void btrfs_assign_next_active_device(struct btrfs_device *device,
>> diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
>> index 5ca1d21fc4a7..b1be7f828cb4 100644
>> --- a/include/uapi/linux/btrfs.h
>> +++ b/include/uapi/linux/btrfs.h
>> @@ -836,6 +836,8 @@ enum btrfs_err_code {
>>   				   struct btrfs_ioctl_vol_args)
>>   #define BTRFS_IOC_SCAN_DEV _IOW(BTRFS_IOCTL_MAGIC, 4, \
>>   				   struct btrfs_ioctl_vol_args)
>> +#define BTRFS_IOC_FORGET_DEV _IOW(BTRFS_IOCTL_MAGIC, 5, \
>> +				   struct btrfs_ioctl_vol_args)
>>   /* trans start and trans end are dangerous, and only for
>>    * use by applications that know how to avoid the
>>    * resulting deadlocks
>>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] btrfs-progs: add cli to forget one or all scanned devices
  2018-10-12  9:03   ` Nikolay Borisov
@ 2018-10-24  4:31     ` Anand Jain
  0 siblings, 0 replies; 21+ messages in thread
From: Anand Jain @ 2018-10-24  4:31 UTC (permalink / raw)
  To: Nikolay Borisov, linux-btrfs



On 10/12/2018 05:03 PM, Nikolay Borisov wrote:
> 
> 
> On 12.10.2018 07:06, Anand Jain wrote:
>> This patch adds cli
>>    btrfs device forget [dev]
>> to remove the given device structure in the kernel if the device
>> is unmounted. If no argument is given it shall remove all stale
>> (device which are not mounted) from the kernel.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>>   cmds-device.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++---------
>>   ioctl.h       |  2 ++
>>   2 files changed, 56 insertions(+), 9 deletions(-)
>>
>> diff --git a/cmds-device.c b/cmds-device.c
>> index 2a05f70a76a9..ecc391ea01d8 100644
>> --- a/cmds-device.c
>> +++ b/cmds-device.c
>> @@ -254,10 +254,32 @@ static int cmd_device_delete(int argc, char **argv)
>>   	return _cmd_device_remove(argc, argv, cmd_device_delete_usage);
>>   }
>>   
>> +static int btrfs_forget_devices(char *path)
>> +{
>> +	struct btrfs_ioctl_vol_args args;
>> +	int ret;
>> +	int fd;
>> +
>> +	fd = open("/dev/btrfs-control", O_RDWR);
>> +	if (fd < 0)
>> +		return -errno;
>> +
>> +	memset(&args, 0, sizeof(args));
>> +	if (path)
>> +		strncpy_null(args.name, path);
>> +	ret = ioctl(fd, BTRFS_IOC_FORGET_DEV, &args);
>> +	if (ret)
>> +		ret = -errno;
>> +	close(fd);
>> +	return ret;
>> +}
>> +
>>   static const char * const cmd_device_scan_usage[] = {
>> -	"btrfs device scan [(-d|--all-devices)|<device> [<device>...]]",
>> -	"Scan devices for a btrfs filesystem",
>> +	"btrfs device scan [(-d|--all-devices)|-u|--forget|<device> "\
>> +							"[<device>...]]",
>> +	"Scan or forget (deregister) devices for a btrfs filesystem",
>>   	" -d|--all-devices (deprecated)",
>> +	" -u|--forget [<device> ..]",
>>   	NULL
>>   };
>>   
>> @@ -267,32 +289,40 @@ static int cmd_device_scan(int argc, char **argv)
>>   	int devstart;
>>   	int all = 0;
>>   	int ret = 0;
>> +	int forget = 0;
>>   
>>   	optind = 0;
>>   	while (1) {
>>   		int c;
>>   		static const struct option long_options[] = {
>>   			{ "all-devices", no_argument, NULL, 'd'},
>> +			{ "forget", no_argument, NULL, 'u'},
>>   			{ NULL, 0, NULL, 0}
>>   		};
>>   
>> -		c = getopt_long(argc, argv, "d", long_options, NULL);
>> +		c = getopt_long(argc, argv, "du", long_options, NULL);
>>   		if (c < 0)
>>   			break;
>>   		switch (c) {
>>   		case 'd':
>>   			all = 1;
>>   			break;
>> +		case 'u':
>> +			forget = 1;
>> +			break;
>>   		default:
>>   			usage(cmd_device_scan_usage);
>>   		}
>>   	}
>>   	devstart = optind;
>>   
>> +	if (all && forget)
>> +		usage(cmd_device_scan_usage);
>> +
>>   	if (all && check_argc_max(argc - optind, 1))
>>   		usage(cmd_device_scan_usage);
>>   
>> -	if (all || argc - optind == 0) {
>> +	if (!forget && (all || argc - optind == 0)) {
>>   		printf("Scanning for Btrfs filesystems\n");
>>   		ret = btrfs_scan_devices();
>>   		error_on(ret, "error %d while scanning", ret);
>> @@ -301,6 +331,13 @@ static int cmd_device_scan(int argc, char **argv)
>>   		goto out;
>>   	}
>>   
>> +	if (forget && (all || argc - optind == 0)) {
> 
> You cannot ever have (forget && all) since you specifically call usage()
> above, just remove the 'all'. Furthermore, I think the code will be more
> legible if it's part of an else if rather than a plain if construct. > In fact the whole function will be more readable. I.e you will have the
> error conditions at the to, followed by "all device scan" case, followed
> by "all device forget" and in the final else branch you will have the
> specific device handling. 

  Originally the -d | --all option is bit broken.
  The -d | --all expects either no arguments OR only one device as an 
argument.

         if (all && check_argc_max(argc - optind, 1))
                 usage(cmd_device_scan_usage);

  Now because of

     if (all || argc - optind == 0) {

    The cli 'btrfs device scan --all /dev/sdb' which should have scanned
     only one device, ends up scanning all the devices and I am not
     trying to fix this bug in this patch because..
   . -d|--all is marked as deprecated, I hope -d option would go away
   . For now some script might be using this bug as a feature, and fixing
     this bug might lead to mount failure.

  However still there is some opportunity to make the my patch more 
readable, thanks for the comments pls see v10 if it is much better?

> The increased indentation level won't be a
> problem since the longest lines are string prints and we are not
> wrapping those.
> 
> 
>> +		ret = btrfs_forget_devices(NULL);
>> +		if (ret)
>> +			error("Can't forget: %s", strerror(-ret));
>> +		goto out;
>> +	}
>> +
>>   	for( i = devstart ; i < argc ; i++ ){
>>   		char *path;
>>   
>> @@ -315,11 +352,19 @@ static int cmd_device_scan(int argc, char **argv)
>>   			ret = 1;
>>   			goto out;
>>   		}
>> -		printf("Scanning for Btrfs filesystems in '%s'\n", path);
>> -		if (btrfs_register_one_device(path) != 0) {
>> -			ret = 1;
>> -			free(path);
>> -			goto out;
>> +		if (forget) {
>> +			ret = btrfs_forget_devices(path);
>> +			if (ret)
>> +				error("Can't forget '%s': %s",
>> +							path, strerror(-ret));
> 
> For consistency sake I think this error printout should be moved inside
> btrfs_forget_devices() similarly to what btrfs_register_one_device does
> on error.

  Where possible we are trying to move the printing to the parent
  functions, so that these helper functions can be used as a/part
  of the library function. As here we can successfully return the unique
  errno for the two possible errors, I would like to keep it as
  it is as of now.

Thanks, Anand

>> +		} else {
>> +			printf("Scanning for Btrfs filesystems in '%s'\n",
>> +									path);
>> +			if (btrfs_register_one_device(path) != 0) {
>> +				ret = 1;
>> +				free(path);
>> +				goto out;
>> +			}
>>   		}
>>   		free(path);
>>   	}
>> diff --git a/ioctl.h b/ioctl.h
>> index 709e996f401c..e27d80e09392 100644
>> --- a/ioctl.h
>> +++ b/ioctl.h
>> @@ -721,6 +721,8 @@ static inline char *btrfs_err_str(enum btrfs_err_code err_code)
>>   				   struct btrfs_ioctl_vol_args)
>>   #define BTRFS_IOC_SCAN_DEV _IOW(BTRFS_IOCTL_MAGIC, 4, \
>>   				   struct btrfs_ioctl_vol_args)
>> +#define BTRFS_IOC_FORGET_DEV _IOW(BTRFS_IOCTL_MAGIC, 5, \
>> +				   struct btrfs_ioctl_vol_args)
>>   /* trans start and trans end are dangerous, and only for
>>    * use by applications that know how to avoid the
>>    * resulting deadlocks
>>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] btrfs: introduce feature to forget a btrfs device
  2018-11-14 11:28     ` Filipe Manana
@ 2018-11-14 13:35       ` Anand Jain
  0 siblings, 0 replies; 21+ messages in thread
From: Anand Jain @ 2018-11-14 13:35 UTC (permalink / raw)
  To: fdmanana; +Cc: linux-btrfs



On 11/14/2018 07:28 PM, Filipe Manana wrote:
> On Wed, Nov 14, 2018 at 11:15 AM Filipe Manana <fdmanana@gmail.com> wrote:
>>
>> On Wed, Nov 14, 2018 at 9:14 AM Anand Jain <anand.jain@oracle.com> wrote:
>>>
>>> Support for a new command 'btrfs dev forget [dev]' is proposed here
>>> to undo the effects of 'btrfs dev scan [dev]'. For this purpose
>>> this patch proposes to use ioctl #5 as it was empty.
>>>          IOW(BTRFS_IOCTL_MAGIC, 5, ..)
>>> This patch adds new ioctl BTRFS_IOC_FORGET_DEV which can be sent from
>>> the /dev/btrfs-control to forget one or all devices, (devices which are
>>> not mounted) from the btrfs kernel.
>>>
>>> The argument it takes is struct btrfs_ioctl_vol_args, and ::name can be
>>> set to specify the device path. And all unmounted devices can be removed
>>> from the kernel if no device path is provided.
>>>
>>> Again, the devices are removed only if the relevant fsid aren't mounted.
>>>
>>> This new cli can provide..
>>>   . Release of unwanted btrfs_fs_devices and btrfs_devices memory if the
>>>     device is not going to be mounted.
>>>   . Ability to mount the device in degraded mode when one of the other
>>>     device is corrupted like in split brain raid1.
>>>   . Running test cases which requires btrfs.ko-reload if the rootfs
>>>     is btrfs.
>>>
>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
>>> ---
>>>   fs/btrfs/super.c           | 3 +++
>>>   fs/btrfs/volumes.c         | 9 +++++++++
>>>   fs/btrfs/volumes.h         | 1 +
>>>   include/uapi/linux/btrfs.h | 2 ++
>>>   4 files changed, 15 insertions(+)
>>>
>>> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
>>> index 345c64d810d4..f99db6899004 100644
>>> --- a/fs/btrfs/super.c
>>> +++ b/fs/btrfs/super.c
>>> @@ -2246,6 +2246,9 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
>>>                  ret = PTR_ERR_OR_ZERO(device);
>>>                  mutex_unlock(&uuid_mutex);
>>>                  break;
>>> +       case BTRFS_IOC_FORGET_DEV:
>>> +               ret = btrfs_forget_devices(vol->name);
>>> +               break;
>>>          case BTRFS_IOC_DEVICES_READY:
>>>                  mutex_lock(&uuid_mutex);
>>>                  device = btrfs_scan_one_device(vol->name, FMODE_READ,
>>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>>> index f435d397019e..e1365a122657 100644
>>> --- a/fs/btrfs/volumes.c
>>> +++ b/fs/btrfs/volumes.c
>>> @@ -1208,6 +1208,15 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr,
>>>          return 0;
>>>   }
>>>
>>> +int btrfs_forget_devices(const char *path)
>>> +{
>>> +       mutex_lock(&uuid_mutex);
>>> +       btrfs_free_stale_devices(strlen(path) ? path:NULL, NULL);
>>
>> One space before : and another one after it please.
>>
>> Now the more important: don't use strlen, use strnlen. Some malicious
>> or sloppy user might have passed a non-null terminated string, you
>> don't want strlen to go past the limits of btrfs_ioctl_vol_args for
>> obvious reasons.
> 
> In fact that's a problem for the entire use of vol->name in
> btrfs_control_ioctl. The name's last byte should be set to '\0' to
> avoid issues.
> I'll send a fix for that, so if David fixes the white spaces on commit
> there's no need for a v12.

   Ok. Thanks.

   David, wonder if white spaces can be fixed when integrating?

Thanks, Anand

>>
>> Also, please, not just to make a maintainer's life easier, but current
>> and future reviewers, add the patch version to each patch's subject
>> and not just the cover letter. Also list (after ---) what changes
>> between each patch version in the patch itself and not the cover
>> letter.
>>
>> V12, here we go.
>>
>>> +       mutex_unlock(&uuid_mutex);
>>> +
>>> +       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
>>> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
>>> index aefce895e994..180297d04938 100644
>>> --- a/fs/btrfs/volumes.h
>>> +++ b/fs/btrfs/volumes.h
>>> @@ -406,6 +406,7 @@ int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
>>>                         fmode_t flags, void *holder);
>>>   struct btrfs_device *btrfs_scan_one_device(const char *path,
>>>                                             fmode_t flags, void *holder);
>>> +int btrfs_forget_devices(const char *path);
>>>   int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
>>>   void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices, int step);
>>>   void btrfs_assign_next_active_device(struct btrfs_device *device,
>>> diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
>>> index 5ca1d21fc4a7..b1be7f828cb4 100644
>>> --- a/include/uapi/linux/btrfs.h
>>> +++ b/include/uapi/linux/btrfs.h
>>> @@ -836,6 +836,8 @@ enum btrfs_err_code {
>>>                                     struct btrfs_ioctl_vol_args)
>>>   #define BTRFS_IOC_SCAN_DEV _IOW(BTRFS_IOCTL_MAGIC, 4, \
>>>                                     struct btrfs_ioctl_vol_args)
>>> +#define BTRFS_IOC_FORGET_DEV _IOW(BTRFS_IOCTL_MAGIC, 5, \
>>> +                                  struct btrfs_ioctl_vol_args)
>>>   /* trans start and trans end are dangerous, and only for
>>>    * use by applications that know how to avoid the
>>>    * resulting deadlocks
>>> --
>>> 1.8.3.1
>>>
>>
>>
>> --
>> Filipe David Manana,
>>
>> “Whether you think you can, or you think you can't — you're right.”
> 
> 
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] btrfs: introduce feature to forget a btrfs device
  2018-11-14 11:15   ` Filipe Manana
  2018-11-14 11:28     ` Filipe Manana
@ 2018-11-14 13:26     ` Anand Jain
  1 sibling, 0 replies; 21+ messages in thread
From: Anand Jain @ 2018-11-14 13:26 UTC (permalink / raw)
  To: fdmanana; +Cc: linux-btrfs



On 11/14/2018 07:15 PM, Filipe Manana wrote:
> On Wed, Nov 14, 2018 at 9:14 AM Anand Jain <anand.jain@oracle.com> wrote:
>>
>> Support for a new command 'btrfs dev forget [dev]' is proposed here
>> to undo the effects of 'btrfs dev scan [dev]'. For this purpose
>> this patch proposes to use ioctl #5 as it was empty.
>>          IOW(BTRFS_IOCTL_MAGIC, 5, ..)
>> This patch adds new ioctl BTRFS_IOC_FORGET_DEV which can be sent from
>> the /dev/btrfs-control to forget one or all devices, (devices which are
>> not mounted) from the btrfs kernel.
>>
>> The argument it takes is struct btrfs_ioctl_vol_args, and ::name can be
>> set to specify the device path. And all unmounted devices can be removed
>> from the kernel if no device path is provided.
>>
>> Again, the devices are removed only if the relevant fsid aren't mounted.
>>
>> This new cli can provide..
>>   . Release of unwanted btrfs_fs_devices and btrfs_devices memory if the
>>     device is not going to be mounted.
>>   . Ability to mount the device in degraded mode when one of the other
>>     device is corrupted like in split brain raid1.
>>   . Running test cases which requires btrfs.ko-reload if the rootfs
>>     is btrfs.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>>   fs/btrfs/super.c           | 3 +++
>>   fs/btrfs/volumes.c         | 9 +++++++++
>>   fs/btrfs/volumes.h         | 1 +
>>   include/uapi/linux/btrfs.h | 2 ++
>>   4 files changed, 15 insertions(+)
>>
>> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
>> index 345c64d810d4..f99db6899004 100644
>> --- a/fs/btrfs/super.c
>> +++ b/fs/btrfs/super.c
>> @@ -2246,6 +2246,9 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
>>                  ret = PTR_ERR_OR_ZERO(device);
>>                  mutex_unlock(&uuid_mutex);
>>                  break;
>> +       case BTRFS_IOC_FORGET_DEV:
>> +               ret = btrfs_forget_devices(vol->name);
>> +               break;
>>          case BTRFS_IOC_DEVICES_READY:
>>                  mutex_lock(&uuid_mutex);
>>                  device = btrfs_scan_one_device(vol->name, FMODE_READ,
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index f435d397019e..e1365a122657 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -1208,6 +1208,15 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr,
>>          return 0;
>>   }
>>
>> +int btrfs_forget_devices(const char *path)
>> +{
>> +       mutex_lock(&uuid_mutex);
>> +       btrfs_free_stale_devices(strlen(path) ? path:NULL, NULL);
> 
> One space before : and another one after it please.

  will fix.

> Now the more important: don't use strlen, use strnlen. Some malicious
> or sloppy user might have passed a non-null terminated string, you
> don't want strlen to go past the limits of btrfs_ioctl_vol_args for
> obvious reasons.

  Makes sense. Will fix.

> Also, please, not just to make a maintainer's life easier, but current
> and future reviewers, add the patch version to each patch's subject
> and not just the cover letter. Also list (after ---) what changes
> between each patch version in the patch itself and not the cover
> letter.

  Sure. Thanks for the feedback.

-Anand

> V12, here we go.
> 
>> +       mutex_unlock(&uuid_mutex);
>> +
>> +       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
>> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
>> index aefce895e994..180297d04938 100644
>> --- a/fs/btrfs/volumes.h
>> +++ b/fs/btrfs/volumes.h
>> @@ -406,6 +406,7 @@ int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
>>                         fmode_t flags, void *holder);
>>   struct btrfs_device *btrfs_scan_one_device(const char *path,
>>                                             fmode_t flags, void *holder);
>> +int btrfs_forget_devices(const char *path);
>>   int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
>>   void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices, int step);
>>   void btrfs_assign_next_active_device(struct btrfs_device *device,
>> diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
>> index 5ca1d21fc4a7..b1be7f828cb4 100644
>> --- a/include/uapi/linux/btrfs.h
>> +++ b/include/uapi/linux/btrfs.h
>> @@ -836,6 +836,8 @@ enum btrfs_err_code {
>>                                     struct btrfs_ioctl_vol_args)
>>   #define BTRFS_IOC_SCAN_DEV _IOW(BTRFS_IOCTL_MAGIC, 4, \
>>                                     struct btrfs_ioctl_vol_args)
>> +#define BTRFS_IOC_FORGET_DEV _IOW(BTRFS_IOCTL_MAGIC, 5, \
>> +                                  struct btrfs_ioctl_vol_args)
>>   /* trans start and trans end are dangerous, and only for
>>    * use by applications that know how to avoid the
>>    * resulting deadlocks
>> --
>> 1.8.3.1
>>
> 
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] btrfs: introduce feature to forget a btrfs device
  2018-11-14 11:15   ` Filipe Manana
@ 2018-11-14 11:28     ` Filipe Manana
  2018-11-14 13:35       ` Anand Jain
  2018-11-14 13:26     ` Anand Jain
  1 sibling, 1 reply; 21+ messages in thread
From: Filipe Manana @ 2018-11-14 11:28 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs

On Wed, Nov 14, 2018 at 11:15 AM Filipe Manana <fdmanana@gmail.com> wrote:
>
> On Wed, Nov 14, 2018 at 9:14 AM Anand Jain <anand.jain@oracle.com> wrote:
> >
> > Support for a new command 'btrfs dev forget [dev]' is proposed here
> > to undo the effects of 'btrfs dev scan [dev]'. For this purpose
> > this patch proposes to use ioctl #5 as it was empty.
> >         IOW(BTRFS_IOCTL_MAGIC, 5, ..)
> > This patch adds new ioctl BTRFS_IOC_FORGET_DEV which can be sent from
> > the /dev/btrfs-control to forget one or all devices, (devices which are
> > not mounted) from the btrfs kernel.
> >
> > The argument it takes is struct btrfs_ioctl_vol_args, and ::name can be
> > set to specify the device path. And all unmounted devices can be removed
> > from the kernel if no device path is provided.
> >
> > Again, the devices are removed only if the relevant fsid aren't mounted.
> >
> > This new cli can provide..
> >  . Release of unwanted btrfs_fs_devices and btrfs_devices memory if the
> >    device is not going to be mounted.
> >  . Ability to mount the device in degraded mode when one of the other
> >    device is corrupted like in split brain raid1.
> >  . Running test cases which requires btrfs.ko-reload if the rootfs
> >    is btrfs.
> >
> > Signed-off-by: Anand Jain <anand.jain@oracle.com>
> > Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> > ---
> >  fs/btrfs/super.c           | 3 +++
> >  fs/btrfs/volumes.c         | 9 +++++++++
> >  fs/btrfs/volumes.h         | 1 +
> >  include/uapi/linux/btrfs.h | 2 ++
> >  4 files changed, 15 insertions(+)
> >
> > diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> > index 345c64d810d4..f99db6899004 100644
> > --- a/fs/btrfs/super.c
> > +++ b/fs/btrfs/super.c
> > @@ -2246,6 +2246,9 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
> >                 ret = PTR_ERR_OR_ZERO(device);
> >                 mutex_unlock(&uuid_mutex);
> >                 break;
> > +       case BTRFS_IOC_FORGET_DEV:
> > +               ret = btrfs_forget_devices(vol->name);
> > +               break;
> >         case BTRFS_IOC_DEVICES_READY:
> >                 mutex_lock(&uuid_mutex);
> >                 device = btrfs_scan_one_device(vol->name, FMODE_READ,
> > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> > index f435d397019e..e1365a122657 100644
> > --- a/fs/btrfs/volumes.c
> > +++ b/fs/btrfs/volumes.c
> > @@ -1208,6 +1208,15 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr,
> >         return 0;
> >  }
> >
> > +int btrfs_forget_devices(const char *path)
> > +{
> > +       mutex_lock(&uuid_mutex);
> > +       btrfs_free_stale_devices(strlen(path) ? path:NULL, NULL);
>
> One space before : and another one after it please.
>
> Now the more important: don't use strlen, use strnlen. Some malicious
> or sloppy user might have passed a non-null terminated string, you
> don't want strlen to go past the limits of btrfs_ioctl_vol_args for
> obvious reasons.

In fact that's a problem for the entire use of vol->name in
btrfs_control_ioctl. The name's last byte should be set to '\0' to
avoid issues.
I'll send a fix for that, so if David fixes the white spaces on commit
there's no need for a v12.

>
> Also, please, not just to make a maintainer's life easier, but current
> and future reviewers, add the patch version to each patch's subject
> and not just the cover letter. Also list (after ---) what changes
> between each patch version in the patch itself and not the cover
> letter.
>
> V12, here we go.
>
> > +       mutex_unlock(&uuid_mutex);
> > +
> > +       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
> > diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
> > index aefce895e994..180297d04938 100644
> > --- a/fs/btrfs/volumes.h
> > +++ b/fs/btrfs/volumes.h
> > @@ -406,6 +406,7 @@ int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
> >                        fmode_t flags, void *holder);
> >  struct btrfs_device *btrfs_scan_one_device(const char *path,
> >                                            fmode_t flags, void *holder);
> > +int btrfs_forget_devices(const char *path);
> >  int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
> >  void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices, int step);
> >  void btrfs_assign_next_active_device(struct btrfs_device *device,
> > diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
> > index 5ca1d21fc4a7..b1be7f828cb4 100644
> > --- a/include/uapi/linux/btrfs.h
> > +++ b/include/uapi/linux/btrfs.h
> > @@ -836,6 +836,8 @@ enum btrfs_err_code {
> >                                    struct btrfs_ioctl_vol_args)
> >  #define BTRFS_IOC_SCAN_DEV _IOW(BTRFS_IOCTL_MAGIC, 4, \
> >                                    struct btrfs_ioctl_vol_args)
> > +#define BTRFS_IOC_FORGET_DEV _IOW(BTRFS_IOCTL_MAGIC, 5, \
> > +                                  struct btrfs_ioctl_vol_args)
> >  /* trans start and trans end are dangerous, and only for
> >   * use by applications that know how to avoid the
> >   * resulting deadlocks
> > --
> > 1.8.3.1
> >
>
>
> --
> Filipe David Manana,
>
> “Whether you think you can, or you think you can't — you're right.”



-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] btrfs: introduce feature to forget a btrfs device
  2018-11-14  9:12 ` [PATCH] btrfs: introduce feature to forget a btrfs device Anand Jain
@ 2018-11-14 11:15   ` Filipe Manana
  2018-11-14 11:28     ` Filipe Manana
  2018-11-14 13:26     ` Anand Jain
  0 siblings, 2 replies; 21+ messages in thread
From: Filipe Manana @ 2018-11-14 11:15 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs

On Wed, Nov 14, 2018 at 9:14 AM Anand Jain <anand.jain@oracle.com> wrote:
>
> Support for a new command 'btrfs dev forget [dev]' is proposed here
> to undo the effects of 'btrfs dev scan [dev]'. For this purpose
> this patch proposes to use ioctl #5 as it was empty.
>         IOW(BTRFS_IOCTL_MAGIC, 5, ..)
> This patch adds new ioctl BTRFS_IOC_FORGET_DEV which can be sent from
> the /dev/btrfs-control to forget one or all devices, (devices which are
> not mounted) from the btrfs kernel.
>
> The argument it takes is struct btrfs_ioctl_vol_args, and ::name can be
> set to specify the device path. And all unmounted devices can be removed
> from the kernel if no device path is provided.
>
> Again, the devices are removed only if the relevant fsid aren't mounted.
>
> This new cli can provide..
>  . Release of unwanted btrfs_fs_devices and btrfs_devices memory if the
>    device is not going to be mounted.
>  . Ability to mount the device in degraded mode when one of the other
>    device is corrupted like in split brain raid1.
>  . Running test cases which requires btrfs.ko-reload if the rootfs
>    is btrfs.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  fs/btrfs/super.c           | 3 +++
>  fs/btrfs/volumes.c         | 9 +++++++++
>  fs/btrfs/volumes.h         | 1 +
>  include/uapi/linux/btrfs.h | 2 ++
>  4 files changed, 15 insertions(+)
>
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 345c64d810d4..f99db6899004 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -2246,6 +2246,9 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
>                 ret = PTR_ERR_OR_ZERO(device);
>                 mutex_unlock(&uuid_mutex);
>                 break;
> +       case BTRFS_IOC_FORGET_DEV:
> +               ret = btrfs_forget_devices(vol->name);
> +               break;
>         case BTRFS_IOC_DEVICES_READY:
>                 mutex_lock(&uuid_mutex);
>                 device = btrfs_scan_one_device(vol->name, FMODE_READ,
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index f435d397019e..e1365a122657 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1208,6 +1208,15 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr,
>         return 0;
>  }
>
> +int btrfs_forget_devices(const char *path)
> +{
> +       mutex_lock(&uuid_mutex);
> +       btrfs_free_stale_devices(strlen(path) ? path:NULL, NULL);

One space before : and another one after it please.

Now the more important: don't use strlen, use strnlen. Some malicious
or sloppy user might have passed a non-null terminated string, you
don't want strlen to go past the limits of btrfs_ioctl_vol_args for
obvious reasons.

Also, please, not just to make a maintainer's life easier, but current
and future reviewers, add the patch version to each patch's subject
and not just the cover letter. Also list (after ---) what changes
between each patch version in the patch itself and not the cover
letter.

V12, here we go.

> +       mutex_unlock(&uuid_mutex);
> +
> +       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
> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
> index aefce895e994..180297d04938 100644
> --- a/fs/btrfs/volumes.h
> +++ b/fs/btrfs/volumes.h
> @@ -406,6 +406,7 @@ int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
>                        fmode_t flags, void *holder);
>  struct btrfs_device *btrfs_scan_one_device(const char *path,
>                                            fmode_t flags, void *holder);
> +int btrfs_forget_devices(const char *path);
>  int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
>  void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices, int step);
>  void btrfs_assign_next_active_device(struct btrfs_device *device,
> diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
> index 5ca1d21fc4a7..b1be7f828cb4 100644
> --- a/include/uapi/linux/btrfs.h
> +++ b/include/uapi/linux/btrfs.h
> @@ -836,6 +836,8 @@ enum btrfs_err_code {
>                                    struct btrfs_ioctl_vol_args)
>  #define BTRFS_IOC_SCAN_DEV _IOW(BTRFS_IOCTL_MAGIC, 4, \
>                                    struct btrfs_ioctl_vol_args)
> +#define BTRFS_IOC_FORGET_DEV _IOW(BTRFS_IOCTL_MAGIC, 5, \
> +                                  struct btrfs_ioctl_vol_args)
>  /* trans start and trans end are dangerous, and only for
>   * use by applications that know how to avoid the
>   * resulting deadlocks
> --
> 1.8.3.1
>


-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH] btrfs: introduce feature to forget a btrfs device
  2018-11-14  9:12 [PATCH RESEND v11] Add cli and ioctl to forget scanned device(s) Anand Jain
@ 2018-11-14  9:12 ` Anand Jain
  2018-11-14 11:15   ` Filipe Manana
  0 siblings, 1 reply; 21+ messages in thread
From: Anand Jain @ 2018-11-14  9:12 UTC (permalink / raw)
  To: linux-btrfs

Support for a new command 'btrfs dev forget [dev]' is proposed here
to undo the effects of 'btrfs dev scan [dev]'. For this purpose
this patch proposes to use ioctl #5 as it was empty.
	IOW(BTRFS_IOCTL_MAGIC, 5, ..)
This patch adds new ioctl BTRFS_IOC_FORGET_DEV which can be sent from
the /dev/btrfs-control to forget one or all devices, (devices which are
not mounted) from the btrfs kernel.

The argument it takes is struct btrfs_ioctl_vol_args, and ::name can be
set to specify the device path. And all unmounted devices can be removed
from the kernel if no device path is provided.

Again, the devices are removed only if the relevant fsid aren't mounted.

This new cli can provide..
 . Release of unwanted btrfs_fs_devices and btrfs_devices memory if the
   device is not going to be mounted.
 . Ability to mount the device in degraded mode when one of the other
   device is corrupted like in split brain raid1.
 . Running test cases which requires btrfs.ko-reload if the rootfs
   is btrfs.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/super.c           | 3 +++
 fs/btrfs/volumes.c         | 9 +++++++++
 fs/btrfs/volumes.h         | 1 +
 include/uapi/linux/btrfs.h | 2 ++
 4 files changed, 15 insertions(+)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 345c64d810d4..f99db6899004 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2246,6 +2246,9 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
 		ret = PTR_ERR_OR_ZERO(device);
 		mutex_unlock(&uuid_mutex);
 		break;
+	case BTRFS_IOC_FORGET_DEV:
+		ret = btrfs_forget_devices(vol->name);
+		break;
 	case BTRFS_IOC_DEVICES_READY:
 		mutex_lock(&uuid_mutex);
 		device = btrfs_scan_one_device(vol->name, FMODE_READ,
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index f435d397019e..e1365a122657 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1208,6 +1208,15 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr,
 	return 0;
 }
 
+int btrfs_forget_devices(const char *path)
+{
+	mutex_lock(&uuid_mutex);
+	btrfs_free_stale_devices(strlen(path) ? path:NULL, NULL);
+	mutex_unlock(&uuid_mutex);
+
+	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
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index aefce895e994..180297d04938 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -406,6 +406,7 @@ int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
 		       fmode_t flags, void *holder);
 struct btrfs_device *btrfs_scan_one_device(const char *path,
 					   fmode_t flags, void *holder);
+int btrfs_forget_devices(const char *path);
 int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
 void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices, int step);
 void btrfs_assign_next_active_device(struct btrfs_device *device,
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index 5ca1d21fc4a7..b1be7f828cb4 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -836,6 +836,8 @@ enum btrfs_err_code {
 				   struct btrfs_ioctl_vol_args)
 #define BTRFS_IOC_SCAN_DEV _IOW(BTRFS_IOCTL_MAGIC, 4, \
 				   struct btrfs_ioctl_vol_args)
+#define BTRFS_IOC_FORGET_DEV _IOW(BTRFS_IOCTL_MAGIC, 5, \
+				   struct btrfs_ioctl_vol_args)
 /* trans start and trans end are dangerous, and only for
  * use by applications that know how to avoid the
  * resulting deadlocks
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH] btrfs: introduce feature to forget a btrfs device
  2018-10-26 14:27 ` [PATCH] btrfs: introduce feature to forget a btrfs device Anand Jain
@ 2018-10-26 14:34   ` Nikolay Borisov
  0 siblings, 0 replies; 21+ messages in thread
From: Nikolay Borisov @ 2018-10-26 14:34 UTC (permalink / raw)
  To: Anand Jain, linux-btrfs



On 26.10.18 г. 17:27 ч., Anand Jain wrote:
> Support for a new command 'btrfs dev forget [dev]' is proposed here
> to undo the effects of 'btrfs dev scan [dev]'. For this purpose
> this patch proposes to use ioctl #5 as it was empty.
> 	IOW(BTRFS_IOCTL_MAGIC, 5, ..)
> This patch adds new ioctl BTRFS_IOC_FORGET_DEV which can be sent from
> the /dev/btrfs-control to forget one or all devices, (devices which are
> not mounted) from the btrfs kernel.
> 
> The argument it takes is struct btrfs_ioctl_vol_args, and ::name can be
> set to specify the device path. And all unmounted devices can be removed
> from the kernel if no device path is provided.
> 
> Again, the devices are removed only if the relevant fsid aren't mounted.
> 
> This new cli can provide..
>  . Release of unwanted btrfs_fs_devices and btrfs_devices memory if the
>    device is not going to be mounted.
>  . Ability to mount the device in degraded mode when one of the other
>    device is corrupted like in split brain raid1.
>  . Running test cases which requires btrfs.ko-reload if the rootfs
>    is btrfs.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  fs/btrfs/super.c           | 3 +++
>  fs/btrfs/volumes.c         | 9 +++++++++
>  fs/btrfs/volumes.h         | 1 +
>  include/uapi/linux/btrfs.h | 2 ++
>  4 files changed, 15 insertions(+)
> 
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 345c64d810d4..f99db6899004 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -2246,6 +2246,9 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
>  		ret = PTR_ERR_OR_ZERO(device);
>  		mutex_unlock(&uuid_mutex);
>  		break;
> +	case BTRFS_IOC_FORGET_DEV:
> +		ret = btrfs_forget_devices(vol->name);
> +		break;
>  	case BTRFS_IOC_DEVICES_READY:
>  		mutex_lock(&uuid_mutex);
>  		device = btrfs_scan_one_device(vol->name, FMODE_READ,
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index f435d397019e..e1365a122657 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1208,6 +1208,15 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr,
>  	return 0;
>  }
>  
> +int btrfs_forget_devices(const char *path)
> +{
> +	mutex_lock(&uuid_mutex);
> +	btrfs_free_stale_devices(strlen(path) ? path:NULL, NULL);
> +	mutex_unlock(&uuid_mutex);
> +
> +	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
> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
> index aefce895e994..180297d04938 100644
> --- a/fs/btrfs/volumes.h
> +++ b/fs/btrfs/volumes.h
> @@ -406,6 +406,7 @@ int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
>  		       fmode_t flags, void *holder);
>  struct btrfs_device *btrfs_scan_one_device(const char *path,
>  					   fmode_t flags, void *holder);
> +int btrfs_forget_devices(const char *path);
>  int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
>  void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices, int step);
>  void btrfs_assign_next_active_device(struct btrfs_device *device,
> diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
> index 5ca1d21fc4a7..b1be7f828cb4 100644
> --- a/include/uapi/linux/btrfs.h
> +++ b/include/uapi/linux/btrfs.h
> @@ -836,6 +836,8 @@ enum btrfs_err_code {
>  				   struct btrfs_ioctl_vol_args)
>  #define BTRFS_IOC_SCAN_DEV _IOW(BTRFS_IOCTL_MAGIC, 4, \
>  				   struct btrfs_ioctl_vol_args)
> +#define BTRFS_IOC_FORGET_DEV _IOW(BTRFS_IOCTL_MAGIC, 5, \
> +				   struct btrfs_ioctl_vol_args)
>  /* trans start and trans end are dangerous, and only for
>   * use by applications that know how to avoid the
>   * resulting deadlocks
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH] btrfs: introduce feature to forget a btrfs device
  2018-10-26 14:27 [PATCH v11] Add cli and ioctl to forget scanned device(s) Anand Jain
@ 2018-10-26 14:27 ` Anand Jain
  2018-10-26 14:34   ` Nikolay Borisov
  0 siblings, 1 reply; 21+ messages in thread
From: Anand Jain @ 2018-10-26 14:27 UTC (permalink / raw)
  To: linux-btrfs

Support for a new command 'btrfs dev forget [dev]' is proposed here
to undo the effects of 'btrfs dev scan [dev]'. For this purpose
this patch proposes to use ioctl #5 as it was empty.
	IOW(BTRFS_IOCTL_MAGIC, 5, ..)
This patch adds new ioctl BTRFS_IOC_FORGET_DEV which can be sent from
the /dev/btrfs-control to forget one or all devices, (devices which are
not mounted) from the btrfs kernel.

The argument it takes is struct btrfs_ioctl_vol_args, and ::name can be
set to specify the device path. And all unmounted devices can be removed
from the kernel if no device path is provided.

Again, the devices are removed only if the relevant fsid aren't mounted.

This new cli can provide..
 . Release of unwanted btrfs_fs_devices and btrfs_devices memory if the
   device is not going to be mounted.
 . Ability to mount the device in degraded mode when one of the other
   device is corrupted like in split brain raid1.
 . Running test cases which requires btrfs.ko-reload if the rootfs
   is btrfs.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/super.c           | 3 +++
 fs/btrfs/volumes.c         | 9 +++++++++
 fs/btrfs/volumes.h         | 1 +
 include/uapi/linux/btrfs.h | 2 ++
 4 files changed, 15 insertions(+)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 345c64d810d4..f99db6899004 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2246,6 +2246,9 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
 		ret = PTR_ERR_OR_ZERO(device);
 		mutex_unlock(&uuid_mutex);
 		break;
+	case BTRFS_IOC_FORGET_DEV:
+		ret = btrfs_forget_devices(vol->name);
+		break;
 	case BTRFS_IOC_DEVICES_READY:
 		mutex_lock(&uuid_mutex);
 		device = btrfs_scan_one_device(vol->name, FMODE_READ,
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index f435d397019e..e1365a122657 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1208,6 +1208,15 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr,
 	return 0;
 }
 
+int btrfs_forget_devices(const char *path)
+{
+	mutex_lock(&uuid_mutex);
+	btrfs_free_stale_devices(strlen(path) ? path:NULL, NULL);
+	mutex_unlock(&uuid_mutex);
+
+	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
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index aefce895e994..180297d04938 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -406,6 +406,7 @@ int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
 		       fmode_t flags, void *holder);
 struct btrfs_device *btrfs_scan_one_device(const char *path,
 					   fmode_t flags, void *holder);
+int btrfs_forget_devices(const char *path);
 int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
 void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices, int step);
 void btrfs_assign_next_active_device(struct btrfs_device *device,
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index 5ca1d21fc4a7..b1be7f828cb4 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -836,6 +836,8 @@ enum btrfs_err_code {
 				   struct btrfs_ioctl_vol_args)
 #define BTRFS_IOC_SCAN_DEV _IOW(BTRFS_IOCTL_MAGIC, 4, \
 				   struct btrfs_ioctl_vol_args)
+#define BTRFS_IOC_FORGET_DEV _IOW(BTRFS_IOCTL_MAGIC, 5, \
+				   struct btrfs_ioctl_vol_args)
 /* trans start and trans end are dangerous, and only for
  * use by applications that know how to avoid the
  * resulting deadlocks
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH] btrfs: introduce feature to forget a btrfs device
  2018-10-24  4:31 [PATCH v10] Add cli and ioctl to forget scanned device(s) Anand Jain
@ 2018-10-24  4:31 ` Anand Jain
  0 siblings, 0 replies; 21+ messages in thread
From: Anand Jain @ 2018-10-24  4:31 UTC (permalink / raw)
  To: linux-btrfs

Support for a new command 'btrfs dev forget [dev]' is proposed here
to undo the effects of 'btrfs dev scan [dev]'. For this purpose
this patch proposes to use ioctl #5 as it was empty.
	IOW(BTRFS_IOCTL_MAGIC, 5, ..)
This patch adds new ioctl BTRFS_IOC_FORGET_DEV which can be sent from
the /dev/btrfs-control to forget one or all devices, (devices which are
not mounted) from the btrfs kernel.

The argument it takes is struct btrfs_ioctl_vol_args, and ::name can be
set to specify the device path. And all unmounted devices can be removed
from the kernel if no device path is provided.

Again, the devices are removed only if the relevant fsid aren't mounted.

This new cli can provide..
 . Release of unwanted btrfs_fs_devices and btrfs_devices memory if the
   device is not going to be mounted.
 . Ability to mount the device in degraded mode when one of the other
   device is corrupted like in split brain raid1.
 . Running test cases which requires btrfs.ko-reload if the rootfs
   is btrfs.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/super.c           | 3 +++
 fs/btrfs/volumes.c         | 9 +++++++++
 fs/btrfs/volumes.h         | 1 +
 include/uapi/linux/btrfs.h | 2 ++
 4 files changed, 15 insertions(+)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 345c64d810d4..f99db6899004 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2246,6 +2246,9 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
 		ret = PTR_ERR_OR_ZERO(device);
 		mutex_unlock(&uuid_mutex);
 		break;
+	case BTRFS_IOC_FORGET_DEV:
+		ret = btrfs_forget_devices(vol->name);
+		break;
 	case BTRFS_IOC_DEVICES_READY:
 		mutex_lock(&uuid_mutex);
 		device = btrfs_scan_one_device(vol->name, FMODE_READ,
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index f435d397019e..e1365a122657 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1208,6 +1208,15 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr,
 	return 0;
 }
 
+int btrfs_forget_devices(const char *path)
+{
+	mutex_lock(&uuid_mutex);
+	btrfs_free_stale_devices(strlen(path) ? path:NULL, NULL);
+	mutex_unlock(&uuid_mutex);
+
+	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
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index aefce895e994..180297d04938 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -406,6 +406,7 @@ int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
 		       fmode_t flags, void *holder);
 struct btrfs_device *btrfs_scan_one_device(const char *path,
 					   fmode_t flags, void *holder);
+int btrfs_forget_devices(const char *path);
 int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
 void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices, int step);
 void btrfs_assign_next_active_device(struct btrfs_device *device,
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index 5ca1d21fc4a7..b1be7f828cb4 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -836,6 +836,8 @@ enum btrfs_err_code {
 				   struct btrfs_ioctl_vol_args)
 #define BTRFS_IOC_SCAN_DEV _IOW(BTRFS_IOCTL_MAGIC, 4, \
 				   struct btrfs_ioctl_vol_args)
+#define BTRFS_IOC_FORGET_DEV _IOW(BTRFS_IOCTL_MAGIC, 5, \
+				   struct btrfs_ioctl_vol_args)
 /* trans start and trans end are dangerous, and only for
  * use by applications that know how to avoid the
  * resulting deadlocks
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH] btrfs: introduce feature to forget a btrfs device
  2018-08-06  1:09 [PATCH RESEND v8] Add cli and ioctl to forget scanned device(s) Anand Jain
@ 2018-08-06  1:09 ` Anand Jain
  0 siblings, 0 replies; 21+ messages in thread
From: Anand Jain @ 2018-08-06  1:09 UTC (permalink / raw)
  To: linux-btrfs

Support for a new command 'btrfs dev forget [dev]' is proposed here
to undo the effects of 'btrfs dev scan [dev]'. For this purpose
this patch proposes to use ioctl #5 as it was empty.
	IOW(BTRFS_IOCTL_MAGIC, 5, ..)
This patch adds new ioctl BTRFS_IOC_FORGET_DEV which can be sent from
the /dev/btrfs-control to forget one or all devices, (devices which are
not mounted) from the btrfs kernel.

The argument it takes is struct btrfs_ioctl_vol_args, and ::name can be
set to specify the device path. And all unmounted devices can be removed
from the kernel if no device path is provided.

Again, the devices are removed only if the relevant fsid aren't mounted.

This new cli can provide..
 . Release of unwanted btrfs_fs_devices and btrfs_devices memory if the
   device is not going to be mounted.
 . Ability to mount the device in degraded mode when one of the other
   device is corrupted like in split brain raid1.
 . Running test cases which requires btrfs.ko-reload if the rootfs
   is btrfs.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/super.c           | 3 +++
 fs/btrfs/volumes.c         | 9 +++++++++
 fs/btrfs/volumes.h         | 1 +
 include/uapi/linux/btrfs.h | 2 ++
 4 files changed, 15 insertions(+)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 67de3c0fc85b..470a32af474e 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2244,6 +2244,9 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
 		ret = PTR_ERR_OR_ZERO(device);
 		mutex_unlock(&uuid_mutex);
 		break;
+	case BTRFS_IOC_FORGET_DEV:
+		ret = btrfs_forget_devices(vol->name);
+		break;
 	case BTRFS_IOC_DEVICES_READY:
 		mutex_lock(&uuid_mutex);
 		device = btrfs_scan_one_device(vol->name, FMODE_READ,
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 8844904f9009..cd54a926141a 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1208,6 +1208,15 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr,
 	return 0;
 }
 
+int btrfs_forget_devices(const char *path)
+{
+	mutex_lock(&uuid_mutex);
+	btrfs_free_stale_devices(strlen(path) ? path:NULL, NULL);
+	mutex_unlock(&uuid_mutex);
+
+	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
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 049619176831..1602b5faa7e7 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -405,6 +405,7 @@ int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
 		       fmode_t flags, void *holder);
 struct btrfs_device *btrfs_scan_one_device(const char *path,
 					   fmode_t flags, void *holder);
+int btrfs_forget_devices(const char *path);
 int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
 void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices, int step);
 void btrfs_assign_next_active_device(struct btrfs_device *device,
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index 5ca1d21fc4a7..b1be7f828cb4 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -836,6 +836,8 @@ enum btrfs_err_code {
 				   struct btrfs_ioctl_vol_args)
 #define BTRFS_IOC_SCAN_DEV _IOW(BTRFS_IOCTL_MAGIC, 4, \
 				   struct btrfs_ioctl_vol_args)
+#define BTRFS_IOC_FORGET_DEV _IOW(BTRFS_IOCTL_MAGIC, 5, \
+				   struct btrfs_ioctl_vol_args)
 /* trans start and trans end are dangerous, and only for
  * use by applications that know how to avoid the
  * resulting deadlocks
-- 
2.7.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH] btrfs: introduce feature to forget a btrfs device
  2018-07-27  0:25 [PATCH v8] Add cli and ioctl to forget scanned device(s) Anand Jain
@ 2018-07-27  0:25 ` Anand Jain
  0 siblings, 0 replies; 21+ messages in thread
From: Anand Jain @ 2018-07-27  0:25 UTC (permalink / raw)
  To: linux-btrfs

Support for a new command 'btrfs dev forget [dev]' is proposed here
to undo the effects of 'btrfs dev scan [dev]'. For this purpose
this patch proposes to use ioctl #5 as it was empty.
	IOW(BTRFS_IOCTL_MAGIC, 5, ..)
This patch adds new ioctl BTRFS_IOC_FORGET_DEV which can be sent from
the /dev/btrfs-control to forget one or all devices, (devices which are
not mounted) from the btrfs kernel.

The argument it takes is struct btrfs_ioctl_vol_args, and ::name can be
set to specify the device path. And all unmounted devices can be removed
from the kernel if no device path is provided.

Again, the devices are removed only if the relevant fsid aren't mounted.

This new cli can provide..
 . Release of unwanted btrfs_fs_devices and btrfs_devices memory if the
   device is not going to be mounted.
 . Ability to mount the device in degraded mode when one of the other
   device is corrupted like in split brain raid1.
 . Running test cases which requires btrfs.ko-reload if the rootfs
   is btrfs.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/super.c           | 3 +++
 fs/btrfs/volumes.c         | 9 +++++++++
 fs/btrfs/volumes.h         | 1 +
 include/uapi/linux/btrfs.h | 2 ++
 4 files changed, 15 insertions(+)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 67de3c0fc85b..470a32af474e 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2244,6 +2244,9 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
 		ret = PTR_ERR_OR_ZERO(device);
 		mutex_unlock(&uuid_mutex);
 		break;
+	case BTRFS_IOC_FORGET_DEV:
+		ret = btrfs_forget_devices(vol->name);
+		break;
 	case BTRFS_IOC_DEVICES_READY:
 		mutex_lock(&uuid_mutex);
 		device = btrfs_scan_one_device(vol->name, FMODE_READ,
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 8844904f9009..cd54a926141a 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1208,6 +1208,15 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr,
 	return 0;
 }
 
+int btrfs_forget_devices(const char *path)
+{
+	mutex_lock(&uuid_mutex);
+	btrfs_free_stale_devices(strlen(path) ? path:NULL, NULL);
+	mutex_unlock(&uuid_mutex);
+
+	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
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 049619176831..1602b5faa7e7 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -405,6 +405,7 @@ int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
 		       fmode_t flags, void *holder);
 struct btrfs_device *btrfs_scan_one_device(const char *path,
 					   fmode_t flags, void *holder);
+int btrfs_forget_devices(const char *path);
 int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
 void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices, int step);
 void btrfs_assign_next_active_device(struct btrfs_device *device,
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index 5ca1d21fc4a7..b1be7f828cb4 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -836,6 +836,8 @@ enum btrfs_err_code {
 				   struct btrfs_ioctl_vol_args)
 #define BTRFS_IOC_SCAN_DEV _IOW(BTRFS_IOCTL_MAGIC, 4, \
 				   struct btrfs_ioctl_vol_args)
+#define BTRFS_IOC_FORGET_DEV _IOW(BTRFS_IOCTL_MAGIC, 5, \
+				   struct btrfs_ioctl_vol_args)
 /* trans start and trans end are dangerous, and only for
  * use by applications that know how to avoid the
  * resulting deadlocks
-- 
2.7.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH] btrfs: introduce feature to forget a btrfs device
  2018-07-26 15:25   ` Filipe Manana
@ 2018-07-26 23:51     ` Anand Jain
  0 siblings, 0 replies; 21+ messages in thread
From: Anand Jain @ 2018-07-26 23:51 UTC (permalink / raw)
  To: fdmanana; +Cc: linux-btrfs



On 07/26/2018 11:25 PM, Filipe Manana wrote:
> On Thu, Jul 26, 2018 at 12:32 PM, Anand Jain <anand.jain@oracle.com> wrote:
>> Support for a new command 'btrfs dev forget [dev]' is proposed here,
>> to undo the effects of 'btrfs dev scan [dev]'. For this purpose,
>> this patch proposes to use ioctl #5 as it was empty.
>>          IOW(BTRFS_IOCTL_MAGIC, 5, ..)
>> This patch adds new ioctl BTRFS_IOC_FORGET_DEV which can be sent from
>> the /dev/btrfs-control to forget one or all devices, (devices which are
>> not mounted) from the btrfs kernel.
>>
>> The argument it takes is struct btrfs_ioctl_vol_args, and ::name can be
>> set to specify the device path. And all unmounted devices can be removed
>> from the kernel if no device path is provided.
>>
>> Again, the devices are removed only if the relevant fsid aren't mounted.
> 
> And why is the feature needed? What problems does it solve?
> That is missing from the changelog, no matter how obvious it is to you
> (or anyone else), it should be mentioned in the changelog.

  There are few reasons..
  . Release of unwanted btrfs_fs_devices and btrfs_devices memory if the
    device is not going to be mounted.
  . Ability to mount the device in degraded mode when one of the other
    device is corrupted (patch [1] in the ML explained this).
      [1] btrfs: handle volume split brain scenario
  . Running test cases which requires btrfs.ko-reload if the rootfs
    is btrfs.

Thanks, Anand


> Thanks.
> 
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>>   fs/btrfs/super.c           | 3 +++
>>   fs/btrfs/volumes.c         | 9 +++++++++
>>   fs/btrfs/volumes.h         | 1 +
>>   include/uapi/linux/btrfs.h | 2 ++
>>   4 files changed, 15 insertions(+)
>>
>> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
>> index 67de3c0fc85b..470a32af474e 100644
>> --- a/fs/btrfs/super.c
>> +++ b/fs/btrfs/super.c
>> @@ -2244,6 +2244,9 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
>>                  ret = PTR_ERR_OR_ZERO(device);
>>                  mutex_unlock(&uuid_mutex);
>>                  break;
>> +       case BTRFS_IOC_FORGET_DEV:
>> +               ret = btrfs_forget_devices(vol->name);
>> +               break;
>>          case BTRFS_IOC_DEVICES_READY:
>>                  mutex_lock(&uuid_mutex);
>>                  device = btrfs_scan_one_device(vol->name, FMODE_READ,
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index 8844904f9009..cd54a926141a 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -1208,6 +1208,15 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr,
>>          return 0;
>>   }
>>
>> +int btrfs_forget_devices(const char *path)
>> +{
>> +       mutex_lock(&uuid_mutex);
>> +       btrfs_free_stale_devices(strlen(path) ? path:NULL, NULL);
>> +       mutex_unlock(&uuid_mutex);
>> +
>> +       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
>> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
>> index 049619176831..1602b5faa7e7 100644
>> --- a/fs/btrfs/volumes.h
>> +++ b/fs/btrfs/volumes.h
>> @@ -405,6 +405,7 @@ int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
>>                         fmode_t flags, void *holder);
>>   struct btrfs_device *btrfs_scan_one_device(const char *path,
>>                                             fmode_t flags, void *holder);
>> +int btrfs_forget_devices(const char *path);
>>   int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
>>   void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices, int step);
>>   void btrfs_assign_next_active_device(struct btrfs_device *device,
>> diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
>> index 5ca1d21fc4a7..b1be7f828cb4 100644
>> --- a/include/uapi/linux/btrfs.h
>> +++ b/include/uapi/linux/btrfs.h
>> @@ -836,6 +836,8 @@ enum btrfs_err_code {
>>                                     struct btrfs_ioctl_vol_args)
>>   #define BTRFS_IOC_SCAN_DEV _IOW(BTRFS_IOCTL_MAGIC, 4, \
>>                                     struct btrfs_ioctl_vol_args)
>> +#define BTRFS_IOC_FORGET_DEV _IOW(BTRFS_IOCTL_MAGIC, 5, \
>> +                                  struct btrfs_ioctl_vol_args)
>>   /* trans start and trans end are dangerous, and only for
>>    * use by applications that know how to avoid the
>>    * resulting deadlocks
>> --
>> 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-info.html
> 
> 
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH] btrfs: introduce feature to forget a btrfs device
  2018-07-26 11:32 ` [PATCH] btrfs: introduce feature to forget a btrfs device Anand Jain
@ 2018-07-26 15:25   ` Filipe Manana
  2018-07-26 23:51     ` Anand Jain
  0 siblings, 1 reply; 21+ messages in thread
From: Filipe Manana @ 2018-07-26 15:25 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs

On Thu, Jul 26, 2018 at 12:32 PM, Anand Jain <anand.jain@oracle.com> wrote:
> Support for a new command 'btrfs dev forget [dev]' is proposed here,
> to undo the effects of 'btrfs dev scan [dev]'. For this purpose,
> this patch proposes to use ioctl #5 as it was empty.
>         IOW(BTRFS_IOCTL_MAGIC, 5, ..)
> This patch adds new ioctl BTRFS_IOC_FORGET_DEV which can be sent from
> the /dev/btrfs-control to forget one or all devices, (devices which are
> not mounted) from the btrfs kernel.
>
> The argument it takes is struct btrfs_ioctl_vol_args, and ::name can be
> set to specify the device path. And all unmounted devices can be removed
> from the kernel if no device path is provided.
>
> Again, the devices are removed only if the relevant fsid aren't mounted.

And why is the feature needed? What problems does it solve?
That is missing from the changelog, no matter how obvious it is to you
(or anyone else), it should be mentioned in the changelog.

Thanks.

>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  fs/btrfs/super.c           | 3 +++
>  fs/btrfs/volumes.c         | 9 +++++++++
>  fs/btrfs/volumes.h         | 1 +
>  include/uapi/linux/btrfs.h | 2 ++
>  4 files changed, 15 insertions(+)
>
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 67de3c0fc85b..470a32af474e 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -2244,6 +2244,9 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
>                 ret = PTR_ERR_OR_ZERO(device);
>                 mutex_unlock(&uuid_mutex);
>                 break;
> +       case BTRFS_IOC_FORGET_DEV:
> +               ret = btrfs_forget_devices(vol->name);
> +               break;
>         case BTRFS_IOC_DEVICES_READY:
>                 mutex_lock(&uuid_mutex);
>                 device = btrfs_scan_one_device(vol->name, FMODE_READ,
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 8844904f9009..cd54a926141a 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1208,6 +1208,15 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr,
>         return 0;
>  }
>
> +int btrfs_forget_devices(const char *path)
> +{
> +       mutex_lock(&uuid_mutex);
> +       btrfs_free_stale_devices(strlen(path) ? path:NULL, NULL);
> +       mutex_unlock(&uuid_mutex);
> +
> +       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
> diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
> index 049619176831..1602b5faa7e7 100644
> --- a/fs/btrfs/volumes.h
> +++ b/fs/btrfs/volumes.h
> @@ -405,6 +405,7 @@ int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
>                        fmode_t flags, void *holder);
>  struct btrfs_device *btrfs_scan_one_device(const char *path,
>                                            fmode_t flags, void *holder);
> +int btrfs_forget_devices(const char *path);
>  int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
>  void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices, int step);
>  void btrfs_assign_next_active_device(struct btrfs_device *device,
> diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
> index 5ca1d21fc4a7..b1be7f828cb4 100644
> --- a/include/uapi/linux/btrfs.h
> +++ b/include/uapi/linux/btrfs.h
> @@ -836,6 +836,8 @@ enum btrfs_err_code {
>                                    struct btrfs_ioctl_vol_args)
>  #define BTRFS_IOC_SCAN_DEV _IOW(BTRFS_IOCTL_MAGIC, 4, \
>                                    struct btrfs_ioctl_vol_args)
> +#define BTRFS_IOC_FORGET_DEV _IOW(BTRFS_IOCTL_MAGIC, 5, \
> +                                  struct btrfs_ioctl_vol_args)
>  /* trans start and trans end are dangerous, and only for
>   * use by applications that know how to avoid the
>   * resulting deadlocks
> --
> 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-info.html



-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH] btrfs: introduce feature to forget a btrfs device
  2018-07-26 11:32 [PATCH RESEND v7] Add cli and ioctl to forget scanned device(s) Anand Jain
@ 2018-07-26 11:32 ` Anand Jain
  2018-07-26 15:25   ` Filipe Manana
  0 siblings, 1 reply; 21+ messages in thread
From: Anand Jain @ 2018-07-26 11:32 UTC (permalink / raw)
  To: linux-btrfs

Support for a new command 'btrfs dev forget [dev]' is proposed here,
to undo the effects of 'btrfs dev scan [dev]'. For this purpose,
this patch proposes to use ioctl #5 as it was empty.
	IOW(BTRFS_IOCTL_MAGIC, 5, ..)
This patch adds new ioctl BTRFS_IOC_FORGET_DEV which can be sent from
the /dev/btrfs-control to forget one or all devices, (devices which are
not mounted) from the btrfs kernel.

The argument it takes is struct btrfs_ioctl_vol_args, and ::name can be
set to specify the device path. And all unmounted devices can be removed
from the kernel if no device path is provided.

Again, the devices are removed only if the relevant fsid aren't mounted.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/super.c           | 3 +++
 fs/btrfs/volumes.c         | 9 +++++++++
 fs/btrfs/volumes.h         | 1 +
 include/uapi/linux/btrfs.h | 2 ++
 4 files changed, 15 insertions(+)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 67de3c0fc85b..470a32af474e 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2244,6 +2244,9 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
 		ret = PTR_ERR_OR_ZERO(device);
 		mutex_unlock(&uuid_mutex);
 		break;
+	case BTRFS_IOC_FORGET_DEV:
+		ret = btrfs_forget_devices(vol->name);
+		break;
 	case BTRFS_IOC_DEVICES_READY:
 		mutex_lock(&uuid_mutex);
 		device = btrfs_scan_one_device(vol->name, FMODE_READ,
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 8844904f9009..cd54a926141a 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1208,6 +1208,15 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr,
 	return 0;
 }
 
+int btrfs_forget_devices(const char *path)
+{
+	mutex_lock(&uuid_mutex);
+	btrfs_free_stale_devices(strlen(path) ? path:NULL, NULL);
+	mutex_unlock(&uuid_mutex);
+
+	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
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 049619176831..1602b5faa7e7 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -405,6 +405,7 @@ int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
 		       fmode_t flags, void *holder);
 struct btrfs_device *btrfs_scan_one_device(const char *path,
 					   fmode_t flags, void *holder);
+int btrfs_forget_devices(const char *path);
 int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
 void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices, int step);
 void btrfs_assign_next_active_device(struct btrfs_device *device,
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index 5ca1d21fc4a7..b1be7f828cb4 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -836,6 +836,8 @@ enum btrfs_err_code {
 				   struct btrfs_ioctl_vol_args)
 #define BTRFS_IOC_SCAN_DEV _IOW(BTRFS_IOCTL_MAGIC, 4, \
 				   struct btrfs_ioctl_vol_args)
+#define BTRFS_IOC_FORGET_DEV _IOW(BTRFS_IOCTL_MAGIC, 5, \
+				   struct btrfs_ioctl_vol_args)
 /* trans start and trans end are dangerous, and only for
  * use by applications that know how to avoid the
  * resulting deadlocks
-- 
2.7.0


^ permalink raw reply related	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2018-11-14 13:35 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-12  4:06 [PATCH v9] Add cli and ioctl to forget scanned device(s) Anand Jain
2018-10-12  4:06 ` [PATCH] btrfs: introduce feature to forget a btrfs device Anand Jain
2018-10-12  8:48   ` Nikolay Borisov
2018-10-24  4:31     ` Anand Jain
2018-10-12  4:06 ` [PATCH] btrfs-progs: add cli to forget one or all scanned devices Anand Jain
2018-10-12  9:03   ` Nikolay Borisov
2018-10-24  4:31     ` Anand Jain
2018-10-12  4:06 ` [PATCH] fstests: btrfs use forget if not reload Anand Jain
  -- strict thread matches above, loose matches on Subject: below --
2018-11-14  9:12 [PATCH RESEND v11] Add cli and ioctl to forget scanned device(s) Anand Jain
2018-11-14  9:12 ` [PATCH] btrfs: introduce feature to forget a btrfs device Anand Jain
2018-11-14 11:15   ` Filipe Manana
2018-11-14 11:28     ` Filipe Manana
2018-11-14 13:35       ` Anand Jain
2018-11-14 13:26     ` Anand Jain
2018-10-26 14:27 [PATCH v11] Add cli and ioctl to forget scanned device(s) Anand Jain
2018-10-26 14:27 ` [PATCH] btrfs: introduce feature to forget a btrfs device Anand Jain
2018-10-26 14:34   ` Nikolay Borisov
2018-10-24  4:31 [PATCH v10] Add cli and ioctl to forget scanned device(s) Anand Jain
2018-10-24  4:31 ` [PATCH] btrfs: introduce feature to forget a btrfs device Anand Jain
2018-08-06  1:09 [PATCH RESEND v8] Add cli and ioctl to forget scanned device(s) Anand Jain
2018-08-06  1:09 ` [PATCH] btrfs: introduce feature to forget a btrfs device Anand Jain
2018-07-27  0:25 [PATCH v8] Add cli and ioctl to forget scanned device(s) Anand Jain
2018-07-27  0:25 ` [PATCH] btrfs: introduce feature to forget a btrfs device Anand Jain
2018-07-26 11:32 [PATCH RESEND v7] Add cli and ioctl to forget scanned device(s) Anand Jain
2018-07-26 11:32 ` [PATCH] btrfs: introduce feature to forget a btrfs device Anand Jain
2018-07-26 15:25   ` Filipe Manana
2018-07-26 23:51     ` Anand Jain

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.