Re: [PATCH] btrfs-progs: add cli to forget one or all scanned devices

[Anand Jain]

On 10/26/2018 08:21 PM, Nikolay Borisov wrote:

On 24.10.2018 07:31, 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.j...@oracle.com>
---
  cmds-device.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++--------
  ioctl.h       |  2 ++
  2 files changed, 53 insertions(+), 8 deletions(-)

diff --git a/cmds-device.c b/cmds-device.c
index 2a05f70a76a9..c2a2b7f304b8 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,45 @@ 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) {
+                       ret = btrfs_forget_devices(NULL);
+                       error_on(ret, "error %d while running forget", ret);
+                       goto out;
+               }

nit: I would prefer to also have an if () {} else {} construct here,
similar to what you do below.

 Will fix.

 Also, I think I can use sterror() instead of ret, in the error_on().

 Fixed both of these in v11.

Thanks, Anand

                printf("Scanning for Btrfs filesystems\n");
                ret = btrfs_scan_devices();
                error_on(ret, "error %d while scanning", ret);
@@ -315,11 +350,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