rbd: utilize '<name>@<snap>' format when creating/removing group snaps

Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/qa/workunits/rbd/rbd_groups.sh b/qa/workunits/rbd/rbd_groups.sh
index 5520b1578516009751e2bd550a04625447c81748..44bda52b875e74ed9ddd9dd0d3ff01216edfcc34 100755 (executable)
--- a/qa/workunits/rbd/rbd_groups.sh
+++ b/qa/workunits/rbd/rbd_groups.sh
@@ -100,14 +100,14 @@ create_snapshot()
 {
     local group_name=$1
     local snap_name=$2
-    rbd group snap create $group_name $snap_name
+    rbd group snap create $group_name@$snap_name
 }
 
 remove_snapshot()
 {
     local group_name=$1
     local snap_name=$2
-    rbd group snap remove $group_name $snap_name
+    rbd group snap remove $group_name@$snap_name
 }
 
 list_snapshots()

diff --git a/src/test/cli/rbd/help.t b/src/test/cli/rbd/help.t
index 2a722a9769a8392b31ddf5dac0cf937ef15bc959..532c26ae0e06d0cb35127f3afbdf431bfb6863b4 100644 (file)
--- a/src/test/cli/rbd/help.t
+++ b/src/test/cli/rbd/help.t
@@ -545,15 +545,13 @@ Skip test on FreeBSD as it generates different output there.
   
   rbd help group snap create
   usage: rbd group snap create [--pool <pool>] [--group <group>] [--snap <snap>] 
-                               <group-spec> <snap> 
+                               <group-spec> 
   
   Make a snapshot of a group.
   
   Positional arguments
     <group-spec>         group specification
-                         (example: [<pool-name>/]<group-name>)
-    <snap>               snapshot name
-                         (example: <snapshot-name>)
+                         (example: [<pool-name>/]<group-name>@<snap-name>)
   
   Optional arguments
     -p [ --pool ] arg    pool name
@@ -579,15 +577,13 @@ Skip test on FreeBSD as it generates different output there.
   
   rbd help group snap remove
   usage: rbd group snap remove [--pool <pool>] [--group <group>] [--snap <snap>] 
-                               <group-spec> <snap> 
+                               <group-spec> 
   
   Remove a snapshot from a group.
   
   Positional arguments
     <group-spec>         group specification
-                         (example: [<pool-name>/]<group-name>)
-    <snap>               snapshot name
-                         (example: <snapshot-name>)
+                         (example: [<pool-name>/]<group-name>@<snap-name>)
   
   Optional arguments
     -p [ --pool ] arg    pool name

diff --git a/src/tools/rbd/ArgumentTypes.cc b/src/tools/rbd/ArgumentTypes.cc
index 42b42a6fb37a99dc27b6b14c067abc048f7f40ee..13c31b7d8740bb18ecb15cd5ab064783284e6dea 100644 (file)
--- a/src/tools/rbd/ArgumentTypes.cc
+++ b/src/tools/rbd/ArgumentTypes.cc
@@ -214,13 +214,22 @@ void add_image_spec_options(po::options_description *pos,
 
 void add_group_spec_options(po::options_description *pos,
                            po::options_description *opt,
-                           ArgumentModifier modifier) {
-  pos->add_options()
-    ((get_name_prefix(modifier) + GROUP_SPEC).c_str(),
-     (get_description_prefix(modifier) + "group specification\n" +
-      "(example: [<pool-name>/]<group-name>)").c_str());
+                           ArgumentModifier modifier,
+                            bool snap) {
   add_pool_option(opt, modifier);
   add_group_option(opt, modifier);
+  if (!snap) {
+    pos->add_options()
+      ((get_name_prefix(modifier) + GROUP_SPEC).c_str(),
+       (get_description_prefix(modifier) + "group specification\n" +
+         "(example: [<pool-name>/]<group-name>)").c_str());
+  } else {
+    add_snap_option(opt, modifier);
+    pos->add_options()
+      ((get_name_prefix(modifier) + GROUP_SPEC).c_str(),
+       (get_description_prefix(modifier) + "group specification\n" +
+         "(example: [<pool-name>/]<group-name>@<snap-name>)").c_str());
+  }
 }
 
 void add_snap_spec_options(po::options_description *pos,

diff --git a/src/tools/rbd/ArgumentTypes.h b/src/tools/rbd/ArgumentTypes.h
index 1837637f94d2f1c2f7f4d3904be0ff2bafc41cf6..2496d94ff4583b433538f1b12d1e18a48a9d8b1c 100644 (file)
--- a/src/tools/rbd/ArgumentTypes.h
+++ b/src/tools/rbd/ArgumentTypes.h
@@ -163,7 +163,7 @@ void add_image_spec_options(boost::program_options::options_description *pos,
 
 void add_group_spec_options(boost::program_options::options_description *pos,
                            boost::program_options::options_description *opt,
-                           ArgumentModifier modifier);
+                           ArgumentModifier modifier, bool snap);
 
 void add_snap_spec_options(boost::program_options::options_description *pos,
                            boost::program_options::options_description *opt,

diff --git a/src/tools/rbd/Utils.cc b/src/tools/rbd/Utils.cc
index ffae4896f3bed535882251bb3d1a79d320c41219..07794344e9adc57d337035373cbedfc8b0a477fe 100644 (file)
--- a/src/tools/rbd/Utils.cc
+++ b/src/tools/rbd/Utils.cc
@@ -128,9 +128,14 @@ int extract_spec(const std::string &spec, std::string *pool_name,
 
 int extract_group_spec(const std::string &spec,
                       std::string *pool_name,
-                      std::string *group_name) {
+                      std::string *group_name,
+                       std::string *snap_name) {
   std::regex pattern;
-  pattern = "^(?:([^/]+)/)?(.+)?$";
+  if (snap_name == nullptr) {
+    pattern = "^(?:([^/@]+)/)?([^/@]+)?$";
+  } else {
+    pattern = "^(?:([^/@]+)/)?([^/@]+)(?:@([^/@]+))?$";
+  }
 
   std::smatch match;
   if (!std::regex_match(spec, match, pattern)) {
@@ -144,6 +149,9 @@ int extract_group_spec(const std::string &spec,
   if (group_name != nullptr) {
     *group_name = match[2];
   }
+  if (snap_name != nullptr && match[3].matched) {
+    *snap_name = match[3];
+  }
 
   return 0;
 }
@@ -227,7 +235,7 @@ int get_special_pool_group_names(const po::variables_map &vm,
   if (group_name->empty()) {
     std::string spec = utils::get_positional_argument(vm, (*arg_index)++);
     if (!spec.empty()) {
-      r = utils::extract_group_spec(spec, group_pool_name, group_name);
+      r = extract_group_spec(spec, group_pool_name, group_name, nullptr);
       if (r < 0) {
         return r;
       }
@@ -338,7 +346,8 @@ int get_pool_group_names(const po::variables_map &vm,
                         at::ArgumentModifier mod,
                         size_t *spec_arg_index,
                         std::string *pool_name,
-                        std::string *group_name) {
+                        std::string *group_name,
+                         std::string *snap_name) {
   std::string pool_key = (mod == at::ARGUMENT_MODIFIER_DEST ?
     at::DEST_POOL_NAME : at::POOL_NAME);
   std::string group_key = (mod == at::ARGUMENT_MODIFIER_DEST ?
@@ -351,12 +360,16 @@ int get_pool_group_names(const po::variables_map &vm,
     *group_name = vm[group_key].as<std::string>();
   }
 
+  if (vm.count(at::SNAPSHOT_NAME) && snap_name != nullptr) {
+    *snap_name = vm[at::SNAPSHOT_NAME].as<std::string>();
+  }
+
   int r;
   if (group_name != nullptr && spec_arg_index != nullptr &&
       group_name->empty()) {
     std::string spec = get_positional_argument(vm, (*spec_arg_index)++);
     if (!spec.empty()) {
-      r = extract_group_spec(spec, pool_name, group_name);
+      r = extract_group_spec(spec, pool_name, group_name, snap_name);
       if (r < 0) {
         return r;
       }
@@ -375,6 +388,11 @@ int get_pool_group_names(const po::variables_map &vm,
     return -EINVAL;
   }
 
+  if (snap_name != nullptr && snap_name->empty()) {
+    std::cerr << "rbd: snapshot name was not specified" << std::endl;
+    return -EINVAL;
+  }
+
   return 0;
 }
 

diff --git a/src/tools/rbd/Utils.h b/src/tools/rbd/Utils.h
index 4c46755303fd83442a1867876d7aeb97d7f801d7..c265249fb3799201a060225e430fd1f611702007 100644 (file)
--- a/src/tools/rbd/Utils.h
+++ b/src/tools/rbd/Utils.h
@@ -95,7 +95,8 @@ int extract_spec(const std::string &spec, std::string *pool_name,
 
 int extract_group_spec(const std::string &spec,
                       std::string *pool_name,
-                      std::string *group_name);
+                      std::string *group_name,
+                       std::string *snap_name);
 
 int extract_image_id_spec(const std::string &spec, std::string *pool_name,
                           std::string *image_id);
@@ -139,7 +140,8 @@ int get_pool_group_names(const boost::program_options::variables_map &vm,
                         argument_types::ArgumentModifier mod,
                         size_t *spec_arg_index,
                         std::string *pool_name,
-                        std::string *group_name);
+                        std::string *group_name,
+                         std::string *snap_name);
 
 int get_pool_journal_names(
     const boost::program_options::variables_map &vm,

diff --git a/src/tools/rbd/action/Group.cc b/src/tools/rbd/action/Group.cc
index 417ca4f0cd8d0cba95e2ebb3927a5ef8b4a540b3..968e317e4d95e2346345db046a9a413052436d57 100644 (file)
--- a/src/tools/rbd/action/Group.cc
+++ b/src/tools/rbd/action/Group.cc
@@ -26,7 +26,8 @@ int execute_create(const po::variables_map &vm) {
   std::string pool_name;
 
   int r = utils::get_pool_group_names(vm, at::ARGUMENT_MODIFIER_NONE,
-                                      &arg_index, &pool_name, &group_name);
+                                      &arg_index, &pool_name, &group_name,
+                                      nullptr);
   if (r < 0) {
     return r;
   }
@@ -96,7 +97,8 @@ int execute_remove(const po::variables_map &vm) {
   std::string pool_name;
 
   int r = utils::get_pool_group_names(vm, at::ARGUMENT_MODIFIER_NONE,
-                                      &arg_index, &pool_name, &group_name);
+                                      &arg_index, &pool_name, &group_name,
+                                      nullptr);
   if (r < 0) {
     return r;
   }
@@ -249,7 +251,8 @@ int execute_list_images(const po::variables_map &vm) {
   std::string pool_name;
 
   int r = utils::get_pool_group_names(vm, at::ARGUMENT_MODIFIER_NONE,
-                                      &arg_index, &pool_name, &group_name);
+                                      &arg_index, &pool_name, &group_name,
+                                      nullptr);
   if (r < 0) {
     return r;
   }
@@ -331,25 +334,12 @@ int execute_group_snap_create(const po::variables_map &vm) {
   std::string snap_name;
 
   int r = utils::get_pool_group_names(vm, at::ARGUMENT_MODIFIER_NONE,
-                                      &arg_index, &pool_name, &group_name);
+                                      &arg_index, &pool_name, &group_name,
+                                      &snap_name);
   if (r < 0) {
     return r;
   }
 
-  if (vm.count(at::SNAPSHOT_NAME)) {
-    snap_name = vm[at::SNAPSHOT_NAME].as<std::string>();
-  }
-
-  if (snap_name.empty()) {
-    snap_name = utils::get_positional_argument(vm, arg_index++);
-  }
-
-  if (snap_name.empty()) {
-    std::cerr << "rbd: "
-             << "snapshot name was not specified" << std::endl;
-    return -EINVAL;
-  }
-
   librados::IoCtx io_ctx;
   librados::Rados rados;
 
@@ -375,25 +365,12 @@ int execute_group_snap_remove(const po::variables_map &vm) {
   std::string snap_name;
 
   int r = utils::get_pool_group_names(vm, at::ARGUMENT_MODIFIER_NONE,
-                                      &arg_index, &pool_name, &group_name);
+                                      &arg_index, &pool_name, &group_name,
+                                      &snap_name);
   if (r < 0) {
     return r;
   }
 
-  if (vm.count(at::SNAPSHOT_NAME)) {
-    snap_name = vm[at::SNAPSHOT_NAME].as<std::string>();
-  }
-
-  if (snap_name.empty()) {
-    snap_name = utils::get_positional_argument(vm, arg_index++);
-  }
-
-  if (snap_name.empty()) {
-    std::cerr << "rbd: "
-             << "snapshot name was not specified" << std::endl;
-    return -EINVAL;
-  }
-
   librados::IoCtx io_ctx;
   librados::Rados rados;
 
@@ -414,7 +391,8 @@ int execute_group_snap_list(const po::variables_map &vm) {
   std::string pool_name;
 
   int r = utils::get_pool_group_names(vm, at::ARGUMENT_MODIFIER_NONE,
-                                      &arg_index, &pool_name, &group_name);
+                                      &arg_index, &pool_name, &group_name,
+                                      nullptr);
   if (r < 0) {
     return r;
   }
@@ -493,12 +471,14 @@ int execute_group_snap_list(const po::variables_map &vm) {
 
 void get_create_arguments(po::options_description *positional,
                           po::options_description *options) {
-  at::add_group_spec_options(positional, options, at::ARGUMENT_MODIFIER_NONE);
+  at::add_group_spec_options(positional, options, at::ARGUMENT_MODIFIER_NONE,
+                             false);
 }
 
 void get_remove_arguments(po::options_description *positional,
                           po::options_description *options) {
-  at::add_group_spec_options(positional, options, at::ARGUMENT_MODIFIER_NONE);
+  at::add_group_spec_options(positional, options, at::ARGUMENT_MODIFIER_NONE,
+                             false);
 }
 
 void get_list_arguments(po::options_description *positional,
@@ -555,33 +535,27 @@ void get_remove_image_arguments(po::options_description *positional,
 void get_list_images_arguments(po::options_description *positional,
                                po::options_description *options) {
   at::add_format_options(options);
-  at::add_group_spec_options(positional, options, at::ARGUMENT_MODIFIER_NONE);
+  at::add_group_spec_options(positional, options, at::ARGUMENT_MODIFIER_NONE,
+                             false);
 }
 
 void get_group_snap_create_arguments(po::options_description *positional,
                                  po::options_description *options) {
-  at::add_group_spec_options(positional, options, at::ARGUMENT_MODIFIER_NONE);
-
-  positional->add_options()
-    (at::SNAPSHOT_NAME.c_str(), "snapshot name\n(example: <snapshot-name>)");
-
-  at::add_snap_option(options, at::ARGUMENT_MODIFIER_NONE);
+  at::add_group_spec_options(positional, options, at::ARGUMENT_MODIFIER_NONE,
+                             true);
 }
 
 void get_group_snap_remove_arguments(po::options_description *positional,
                                  po::options_description *options) {
-  at::add_group_spec_options(positional, options, at::ARGUMENT_MODIFIER_NONE);
-
-  positional->add_options()
-    (at::SNAPSHOT_NAME.c_str(), "snapshot name\n(example: <snapshot-name>)");
-
-  at::add_snap_option(options, at::ARGUMENT_MODIFIER_NONE);
+  at::add_group_spec_options(positional, options, at::ARGUMENT_MODIFIER_NONE,
+                             true);
 }
 
 void get_group_snap_list_arguments(po::options_description *positional,
                              po::options_description *options) {
   at::add_format_options(options);
-  at::add_group_spec_options(positional, options, at::ARGUMENT_MODIFIER_NONE);
+  at::add_group_spec_options(positional, options, at::ARGUMENT_MODIFIER_NONE,
+                             false);
 }
 
 Shell::Action action_create(