rbd-nbd: allow to unmap by image or snap spec

(to match the behaviour of krbd unmap)

Signed-off-by: Mykola Golub <mgolub@suse.com>

diff --git a/qa/workunits/rbd/rbd-nbd.sh b/qa/workunits/rbd/rbd-nbd.sh
index e9547e92853a3e06da1f7924d5dc1690715e94b6..15b3d1706fa7f9ad4ca4bccf1a02564412af1bdb 100755 (executable)
--- a/qa/workunits/rbd/rbd-nbd.sh
+++ b/qa/workunits/rbd/rbd-nbd.sh
@@ -65,6 +65,7 @@ function cleanup()
            sleep $s
            rbd -p ${POOL} status ${IMAGE} | grep 'Watchers: none' && break
        done
+       rbd -p ${POOL} snap purge ${IMAGE}
        rbd -p ${POOL} remove ${IMAGE}
     fi
 }
@@ -173,6 +174,29 @@ _sudo rbd-nbd unmap ${DEV}
 DEV=
 rbd bench ${IMAGE} --io-type write --io-size=1024 --io-total=1024
 
+# unmap by image name test
+DEV=`_sudo rbd-nbd map ${POOL}/${IMAGE}`
+PID=$(rbd-nbd list-mapped | awk -v pool=${POOL} -v img=${IMAGE} -v dev=${DEV} \
+    '$2 == pool && $3 == img && $5 == dev {print $1}')
+test -n "${PID}"
+ps -p ${PID} -o cmd | grep rbd-nbd
+_sudo rbd-nbd unmap "${IMAGE}"
+rbd-nbd list-mapped | expect_false grep "${DEV} $"
+DEV=
+ps -p ${PID} -o cmd | expect_false grep rbd-nbd
+
+# map/unmap snap test
+rbd snap create ${POOL}/${IMAGE}@snap
+DEV=`_sudo rbd-nbd map ${POOL}/${IMAGE}@snap`
+PID=$(rbd-nbd list-mapped |
+      awk -v pool=${POOL} -v img=${IMAGE} -v snap=snap -v dev=${DEV} \
+          '$2 == pool && $3 == img && $4 == snap && $5 == dev {print $1}')
+test -n "${PID}"
+_sudo rbd-nbd unmap "${IMAGE}@snap"
+rbd-nbd list-mapped | expect_false grep "${DEV} $"
+DEV=
+ps -p ${PID} -o cmd | expect_false grep rbd-nbd
+
 # auto unmap test
 DEV=`_sudo rbd-nbd map ${POOL}/${IMAGE}`
 PID=$(rbd-nbd list-mapped | awk -v pool=${POOL} -v img=${IMAGE} -v dev=${DEV} \

diff --git a/src/test/cli/rbd/help.t b/src/test/cli/rbd/help.t
index 876aa88aa3ac9b35407b828865526aa5bb32786e..be8367893ed00d6eaa156fa2e8cdea5ac189d1aa 100644 (file)
--- a/src/test/cli/rbd/help.t
+++ b/src/test/cli/rbd/help.t
@@ -1169,13 +1169,20 @@ Skip test on FreeBSD as it generates different output there.
     --timeout arg         set nbd request timeout (seconds)
   
   rbd help nbd unmap
-  usage: rbd nbd unmap 
-                       <device-spec> 
+  usage: rbd nbd unmap [--pool <pool>] [--image <image>] [--snap <snap>] 
+                       <image-or-snap-or-device-spec> 
   
   Unmap a nbd device.
   
   Positional arguments
-    <device-spec>        specify nbd device
+    <image-or-snap-or-device-spec>  image, snapshot, or device specification
+                                    [<pool-name>/]<image-name>[@<snapshot-name>]
+                                    or <device-path>
+  
+  Optional arguments
+    -p [ --pool ] arg               pool name
+    --image arg                     image name
+    --snap arg                      snapshot name
   
   rbd help object-map check
   usage: rbd object-map check [--pool <pool>] [--image <image>] [--snap <snap>] 

diff --git a/src/tools/rbd/action/Nbd.cc b/src/tools/rbd/action/Nbd.cc
index 0a586353d5efb5cc2674fc1997bb713f14ded3c2..43f547704e5fea40db58a17040f52ea3fb450f90 100644 (file)
--- a/src/tools/rbd/action/Nbd.cc
+++ b/src/tools/rbd/action/Nbd.cc
@@ -80,6 +80,30 @@ static int call_nbd_cmd(const po::variables_map &vm,
   return 0;
 }
 
+int get_image_or_snap_spec(const po::variables_map &vm, std::string *spec) {
+  size_t arg_index = 0;
+  std::string pool_name;
+  std::string image_name;
+  std::string snap_name;
+  int r = utils::get_pool_image_snapshot_names(
+    vm, at::ARGUMENT_MODIFIER_NONE, &arg_index, &pool_name, &image_name,
+    &snap_name, utils::SNAPSHOT_PRESENCE_PERMITTED,
+    utils::SPEC_VALIDATION_NONE);
+  if (r < 0) {
+    return r;
+  }
+
+  spec->append(pool_name);
+  spec->append("/");
+  spec->append(image_name);
+  if (!snap_name.empty()) {
+    spec->append("@");
+    spec->append(snap_name);
+  }
+
+  return 0;
+}
+
 void get_show_arguments(po::options_description *positional,
                         po::options_description *options)
 { }
@@ -109,28 +133,13 @@ void get_map_arguments(po::options_description *positional,
 
 int execute_map(const po::variables_map &vm)
 {
-  size_t arg_index = 0;
-  std::string pool_name;
-  std::string image_name;
-  std::string snap_name;
-  int r = utils::get_pool_image_snapshot_names(
-    vm, at::ARGUMENT_MODIFIER_NONE, &arg_index, &pool_name, &image_name,
-    &snap_name, utils::SNAPSHOT_PRESENCE_PERMITTED,
-    utils::SPEC_VALIDATION_NONE);
-  if (r < 0) {
-    return r;
-  }
-
   std::vector<const char*> args;
 
   args.push_back("map");
   std::string img;
-  img.append(pool_name);
-  img.append("/");
-  img.append(image_name);
-  if (!snap_name.empty()) {
-    img.append("@");
-    img.append(snap_name);
+  int r = get_image_or_snap_spec(vm, &img);
+  if (r < 0) {
+    return r;
   }
   args.push_back(img.c_str());
 
@@ -164,7 +173,12 @@ void get_unmap_arguments(po::options_description *positional,
                    po::options_description *options)
 {
   positional->add_options()
-    ("device-spec", "specify nbd device");
+    ("image-or-snap-or-device-spec",
+     "image, snapshot, or device specification\n"
+     "[<pool-name>/]<image-name>[@<snapshot-name>] or <device-path>");
+  at::add_pool_option(options, at::ARGUMENT_MODIFIER_NONE);
+  at::add_image_option(options, at::ARGUMENT_MODIFIER_NONE);
+  at::add_snap_option(options, at::ARGUMENT_MODIFIER_NONE);
 }
 
 int execute_unmap(const po::variables_map &vm)
@@ -174,15 +188,25 @@ int execute_unmap(const po::variables_map &vm)
     device_name.clear();
   }
 
+  std::string image_name;
   if (device_name.empty()) {
-    std::cerr << "rbd: nbd unmap requires device path" << std::endl;
+    int r = get_image_or_snap_spec(vm, &image_name);
+    if (r < 0) {
+      return r;
+    }
+  }
+
+  if (device_name.empty() && image_name.empty()) {
+    std::cerr << "rbd: unmap requires either image name or device path"
+              << std::endl;
     return -EINVAL;
   }
 
   std::vector<const char*> args;
 
   args.push_back("unmap");
-  args.push_back(device_name.c_str());
+  args.push_back(device_name.empty() ? image_name.c_str() :
+                 device_name.c_str());
 
   return call_nbd_cmd(vm, args);
 }

diff --git a/src/tools/rbd_nbd/rbd-nbd.cc b/src/tools/rbd_nbd/rbd-nbd.cc
index bf133c13dd2dc32a66d2eb5ef69f113fcaf7b69b..e5006592e052a114452ff15de8e4298bc10867d8 100644 (file)
--- a/src/tools/rbd_nbd/rbd-nbd.cc
+++ b/src/tools/rbd_nbd/rbd-nbd.cc
@@ -35,6 +35,7 @@
 #include <iostream>
 #include <fstream>
 #include <boost/regex.hpp>
+#include <boost/algorithm/string/predicate.hpp>
 
 #include "mon/MonClient.h"
 #include "common/config.h"
@@ -78,7 +79,7 @@ struct Config {
 static void usage()
 {
   std::cout << "Usage: rbd-nbd [options] map <image-or-snap-spec>  Map an image to nbd device\n"
-            << "               unmap <device path>                 Unmap nbd device\n"
+            << "               unmap <device|image-or-snap-spec>   Unmap nbd device\n"
             << "               list-mapped                         List mapped nbd devices\n"
             << "Options:\n"
             << "  --device <device path>  Specify nbd device path\n"
@@ -93,12 +94,14 @@ static void usage()
 
 static int nbd = -1;
 
-static enum {
+enum Command {
   None,
   Connect,
   Disconnect,
   List
-} cmd = None;
+};
+
+static Command cmd = None;
 
 #define RBD_NBD_BLKSIZE 512UL
 
@@ -114,7 +117,8 @@ static enum {
 #endif
 #define htonll(a) ntohll(a)
 
-static int parse_args(vector<const char*>& args, std::ostream *err_msg, Config *cfg);
+static int parse_args(vector<const char*>& args, std::ostream *err_msg,
+                      Command *command, Config *cfg);
 
 static void handle_signal(int signum)
 {
@@ -491,11 +495,84 @@ public:
   }
 };
 
+class NBDListIterator {
+public:
+  bool get(int *pid, Config *cfg) {
+    while (true) {
+      std::string nbd_path = "/sys/block/nbd" + stringify(m_index);
+      if(access(nbd_path.c_str(), F_OK) != 0) {
+        return false;
+      }
+
+      *cfg = Config();
+      cfg->devpath = "/dev/nbd" + stringify(m_index++);
+
+      std::ifstream ifs;
+      ifs.open(nbd_path + "/pid", std::ifstream::in);
+      if (!ifs.is_open()) {
+        continue;
+      }
+      ifs >> *pid;
+
+      int r = get_mapped_info(*pid, cfg);
+      if (r < 0) {
+        continue;
+      }
+
+      return true;
+    }
+  }
+
+private:
+  int m_index = 0;
+
+  int get_mapped_info(int pid, Config *cfg) {
+    int r;
+    std::string path = "/proc/" + stringify(pid) + "/cmdline";
+    std::ifstream ifs;
+    std::string cmdline;
+    std::vector<const char*> args;
+
+    ifs.open(path.c_str(), std::ifstream::in);
+    if (!ifs.is_open())
+      return -1;
+    ifs >> cmdline;
+
+    for (unsigned i = 0; i < cmdline.size(); i++) {
+      const char *arg = &cmdline[i];
+      if (i == 0) {
+        if (strcmp(basename(arg) , "rbd-nbd") != 0) {
+          return -EINVAL;
+        }
+      } else {
+        args.push_back(arg);
+      }
+
+      while (cmdline[i] != '\0') {
+        i++;
+      }
+    }
+
+    std::ostringstream err_msg;
+    Command command;
+    r = parse_args(args, &err_msg, &command, cfg);
+    if (r < 0) {
+      return r;
+    }
+
+    if (command != Connect) {
+      return -ENOENT;
+    }
+
+    return 0;
+  }
+};
+
 static int open_device(const char* path, Config *cfg = nullptr, bool try_load_module = false)
 {
   int nbd = open(path, O_RDWR);
   bool loaded_module = false;
-  
+
   if (nbd < 0 && try_load_module && access("/sys/module/nbd", F_OK) != 0) {
     ostringstream param;
     int r;
@@ -520,7 +597,7 @@ static int open_device(const char* path, Config *cfg = nullptr, bool try_load_mo
     cerr << "rbd-nbd: ignoring kernel module parameter options: nbd module already loaded" 
          << std::endl;
   }
-  
+
   return nbd;
 }
 
@@ -777,14 +854,14 @@ static int do_map(int argc, const char *argv[], Config *cfg)
       NBDServer server(fd[1], image);
 
       server.start();
-      
+
       init_async_signal_handler();
       register_async_signal_handler(SIGHUP, sighup_handler);
       register_async_signal_handler_oneshot(SIGINT, handle_signal);
       register_async_signal_handler_oneshot(SIGTERM, handle_signal);
 
       ioctl(nbd, NBD_DO_IT);
-     
+
       unregister_async_signal_handler(SIGHUP, sighup_handler);
       unregister_async_signal_handler(SIGINT, handle_signal);
       unregister_async_signal_handler(SIGTERM, handle_signal);
@@ -834,12 +911,12 @@ static int do_unmap(const std::string &devpath)
   return r;
 }
 
-static int parse_imgpath(const std::string &imgpath, Config *cfg)
-{
+static int parse_imgpath(const std::string &imgpath, Config *cfg,
+                         std::ostream *err_msg) {
   boost::regex pattern("^(?:([^/@]+)/)?([^/@]+)(?:@([^/@]+))?$");
   boost::smatch match;
   if (!boost::regex_match(imgpath, match, pattern)) {
-    std::cerr << "rbd-nbd: invalid spec '" << imgpath << "'" << std::endl;
+    *err_msg << "rbd-nbd: invalid spec '" << imgpath << "'";
     return -EINVAL;
   }
 
@@ -855,60 +932,9 @@ static int parse_imgpath(const std::string &imgpath, Config *cfg)
   return 0;
 }
 
-static int get_mapped_info(int pid, Config *cfg)
-{
-  int r;
-  std::string path = "/proc/" + stringify(pid) + "/cmdline";
-  std::ifstream ifs;
-  std::string cmdline;
-  std::vector<const char*> args;
-
-  ifs.open(path.c_str(), std::ifstream::in);
-  if (!ifs.is_open())
-    return -1;
-  ifs >> cmdline;
-
-  for (unsigned i = 0; i < cmdline.size(); i++) {
-    const char *arg = &cmdline[i];
-    if (i == 0) {
-      if (strcmp(basename(arg) , "rbd-nbd") != 0) {
-        return -EINVAL;
-      }
-    } else {
-      args.push_back(arg);
-    }
- 
-    while (cmdline[i] != '\0') {
-      i++;
-    }
-  }
- 
-  std::ostringstream err_msg;
-  r = parse_args(args, &err_msg, cfg);
-  return r;
-}
-
-static int get_map_pid(const std::string& pid_path)
-{
-  int pid = 0;
-  std::ifstream ifs;
-  ifs.open(pid_path.c_str(), std::ifstream::in);
-  if (!ifs.is_open()) {
-    return 0;
-  }
-  ifs >> pid;
-  return pid;
-}
-
 static int do_list_mapped_devices()
 {
-  int r;
   bool should_print = false;
-  int index = 0;
-  int pid = 0;
-
-  std::string default_pool_name;
-
   TextTable tbl;
 
   tbl.define_column("pid", TextTable::LEFT, TextTable::LEFT);
@@ -916,31 +942,17 @@ static int do_list_mapped_devices()
   tbl.define_column("image", TextTable::LEFT, TextTable::LEFT);
   tbl.define_column("snap", TextTable::LEFT, TextTable::LEFT);
   tbl.define_column("device", TextTable::LEFT, TextTable::LEFT);
- 
-  while (true) {
-    std::string nbd_path = "/sys/block/nbd" + stringify(index);
-    if(access(nbd_path.c_str(), F_OK) != 0) {
-      break;
-    }
-    std::string pid_path = nbd_path + "/pid";
-    pid = get_map_pid(pid_path);
 
-    if(pid > 0) {
-      Config cfg;
-      r = get_mapped_info(pid, &cfg);
-      if (r < 0) {
-        index++;
-        continue;
-      }
-      should_print = true;
-      if (cfg.snapname.empty()) {
-        cfg.snapname = "-";
-      }
-      tbl << pid << cfg.poolname << cfg.imgname << cfg.snapname
-          << "/dev/nbd" + stringify(index) << TextTable::endrow;
+  int pid;
+  Config cfg;
+  NBDListIterator it;
+  while (it.get(&pid, &cfg)) {
+    should_print = true;
+    if (cfg.snapname.empty()) {
+      cfg.snapname = "-";
     }
-
-    index++;
+    tbl << pid << cfg.poolname << cfg.imgname << cfg.snapname << cfg.devpath
+        << TextTable::endrow;
   }
 
   if (should_print) {
@@ -949,8 +961,23 @@ static int do_list_mapped_devices()
   return 0;
 }
 
-static int parse_args(vector<const char*>& args, std::ostream *err_msg, Config *cfg)
-{
+static bool find_mapped_dev_by_spec(Config *cfg) {
+  int pid;
+  Config c;
+  NBDListIterator it;
+  while (it.get(&pid, &c)) {
+    if (c.poolname == cfg->poolname && c.imgname == cfg->imgname &&
+        c.snapname == cfg->snapname) {
+      *cfg = c;
+      return true;
+    }
+  }
+  return false;
+}
+
+
+static int parse_args(vector<const char*>& args, std::ostream *err_msg,
+                      Command *command, Config *cfg) {
   std::string conf_file_list;
   std::string cluster;
   CephInitParameters iparams = ceph_argparse_early_args(
@@ -1016,6 +1043,7 @@ static int parse_args(vector<const char*>& args, std::ostream *err_msg, Config *
     }
   }
 
+  Command cmd = None;
   if (args.begin() != args.end()) {
     if (strcmp(*args.begin(), "map") == 0) {
       cmd = Connect;
@@ -1041,16 +1069,27 @@ static int parse_args(vector<const char*>& args, std::ostream *err_msg, Config *
         *err_msg << "rbd-nbd: must specify image-or-snap-spec";
         return -EINVAL;
       }
-      if (parse_imgpath(string(*args.begin()), cfg) < 0)
+      if (parse_imgpath(*args.begin(), cfg, err_msg) < 0) {
         return -EINVAL;
+      }
       args.erase(args.begin());
       break;
     case Disconnect:
       if (args.begin() == args.end()) {
-        *err_msg << "rbd-nbd: must specify nbd device path";
+        *err_msg << "rbd-nbd: must specify nbd device or image-or-snap-spec";
         return -EINVAL;
       }
-      cfg->devpath = *args.begin();
+      if (boost::starts_with(*args.begin(), "/dev/")) {
+        cfg->devpath = *args.begin();
+      } else {
+        if (parse_imgpath(*args.begin(), cfg, err_msg) < 0) {
+          return -EINVAL;
+        }
+        if (!find_mapped_dev_by_spec(cfg)) {
+          *err_msg << "rbd-nbd: " << *args.begin() << " is not mapped";
+          return -ENOENT;
+        }
+      }
       args.erase(args.begin());
       break;
     default:
@@ -1063,6 +1102,7 @@ static int parse_args(vector<const char*>& args, std::ostream *err_msg, Config *
     return -EINVAL;
   }
 
+  *command = cmd;
   return 0;
 }
 
@@ -1074,7 +1114,7 @@ static int rbd_nbd(int argc, const char *argv[])
   argv_to_vec(argc, argv, args);
 
   std::ostringstream err_msg;
-  r = parse_args(args, &err_msg, &cfg);
+  r = parse_args(args, &err_msg, &cmd, &cfg);
   if (r == HELP_INFO) {
     usage();
     ceph_abort();