Merge pull request ceph#61155 from idryomov/wip-69324

rbd: handle --{group,image}-namespace in "rbd group image {add,rm}" Reviewed-by: Ramana Raja <[email protected]>
gbregman · Dec 22, 2024 · 610a87a · 610a87a
2 parents 738ac44 + 1f71671
commit 610a87a
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 53 deletions.
diff --git a/qa/workunits/rbd/cli_generic.sh b/qa/workunits/rbd/cli_generic.sh
@@ -914,6 +914,11 @@ test_namespace() {
 
     rbd group create rbd/test1/group1
     rbd group image add rbd/test1/group1 rbd/test1/image1
+    rbd group image add --group-pool rbd --group-namespace test1 --group group1 \
+        --image-pool rbd --image-namespace test1 --image image2
+    rbd group image rm --group-pool rbd --group-namespace test1 --group group1 \
+        --image-pool rbd --image-namespace test1 --image image1
+    rbd group image rm rbd/test1/group1 rbd/test1/image2
     rbd group rm rbd/test1/group1
 
     rbd trash move rbd/test1/image1

diff --git a/src/test/cli/rbd/help.t b/src/test/cli/rbd/help.t
@@ -916,7 +916,7 @@
                              [--group-namespace <group-namespace>] 
                              [--group <group>] [--image-pool <image-pool>] 
                              [--image-namespace <image-namespace>] 
-                             [--image <image>] [--pool <pool>] 
+                             [--image <image>] 
                              <group-spec> <image-spec> 
 
   Add an image to a group.
@@ -934,7 +934,6 @@
     --image-pool arg      image pool name
     --image-namespace arg image namespace name
     --image arg           image name
-    -p [ --pool ] arg     pool name unless overridden
 
   rbd help group image list
   usage: rbd group image list [--format <format>] [--pretty-format] 
@@ -960,8 +959,7 @@
                                 [--group-namespace <group-namespace>] 
                                 [--group <group>] [--image-pool <image-pool>] 
                                 [--image-namespace <image-namespace>] 
-                                [--image <image>] [--pool <pool>] 
-                                [--image-id <image-id>] 
+                                [--image <image>] [--image-id <image-id>] 
                                 <group-spec> <image-spec> 
 
   Remove an image from a group.
@@ -979,7 +977,6 @@
     --image-pool arg      image pool name
     --image-namespace arg image namespace name
     --image arg           image name
-    -p [ --pool ] arg     pool name unless overridden
     --image-id arg        image id
 
   rbd help group info

diff --git a/src/tools/rbd/Utils.cc b/src/tools/rbd/Utils.cc
@@ -337,11 +337,14 @@ int get_pool_image_snapshot_names(const po::variables_map &vm,
                                   SpecValidation spec_validation) {
   std::string pool_key = (mod == at::ARGUMENT_MODIFIER_DEST ?
     at::DEST_POOL_NAME : at::POOL_NAME);
+  std::string namespace_key = (mod == at::ARGUMENT_MODIFIER_DEST ?
+    at::DEST_NAMESPACE_NAME : at::NAMESPACE_NAME);
   std::string image_key = (mod == at::ARGUMENT_MODIFIER_DEST ?
     at::DEST_IMAGE_NAME : at::IMAGE_NAME);
+
   return get_pool_generic_snapshot_names(vm, mod, spec_arg_index, pool_key,
-                                         pool_name, namespace_name, image_key,
-                                         "image", image_name, snap_name,
+                                         pool_name, namespace_key, namespace_name,
+                                         image_key, "image", image_name, snap_name,
                                          image_name_required, snapshot_presence,
                                          spec_validation);
 }
@@ -351,6 +354,7 @@ int get_pool_generic_snapshot_names(const po::variables_map &vm,
                                     size_t *spec_arg_index,
                                     const std::string& pool_key,
                                     std::string *pool_name,
+                                    const std::string& namespace_key,
                                     std::string *namespace_name,
                                     const std::string& generic_key,
                                     const std::string& generic_key_desc,
@@ -359,8 +363,6 @@ int get_pool_generic_snapshot_names(const po::variables_map &vm,
                                     bool generic_name_required,
                                     SnapshotPresence snapshot_presence,
                                     SpecValidation spec_validation) {
-  std::string namespace_key = (mod == at::ARGUMENT_MODIFIER_DEST ?
-    at::DEST_NAMESPACE_NAME : at::NAMESPACE_NAME);
   std::string snap_key = (mod == at::ARGUMENT_MODIFIER_DEST ?
     at::DEST_SNAPSHOT_NAME : at::SNAPSHOT_NAME);
 

diff --git a/src/tools/rbd/Utils.h b/src/tools/rbd/Utils.h
@@ -163,10 +163,11 @@ int get_pool_generic_snapshot_names(
     const boost::program_options::variables_map &vm,
     argument_types::ArgumentModifier mod, size_t *spec_arg_index,
     const std::string& pool_key, std::string *pool_name,
-    std::string *namespace_name, const std::string& generic_key,
-    const std::string& generic_key_desc, std::string *generic_name,
-    std::string *snap_name, bool generic_name_required,
-    SnapshotPresence snapshot_presence, SpecValidation spec_validation);
+    const std::string& namespace_key, std::string *namespace_name,
+    const std::string& generic_key, const std::string& generic_key_desc,
+    std::string *generic_name, std::string *snap_name,
+    bool generic_name_required, SnapshotPresence snapshot_presence,
+    SpecValidation spec_validation);
 
 int get_pool_image_id(const boost::program_options::variables_map &vm,
                       size_t *spec_arg_index,

diff --git a/src/tools/rbd/action/Group.cc b/src/tools/rbd/action/Group.cc
@@ -28,6 +28,9 @@ static const std::string DEST_GROUP_NAME("dest-group");
 static const std::string GROUP_POOL_NAME("group-" + at::POOL_NAME);
 static const std::string IMAGE_POOL_NAME("image-" + at::POOL_NAME);
 
+static const std::string GROUP_NAMESPACE_NAME("group-" + at::NAMESPACE_NAME);
+static const std::string IMAGE_NAMESPACE_NAME("image-" + at::NAMESPACE_NAME);
+
 void add_group_option(po::options_description *opt,
 		      at::ArgumentModifier modifier) {
   std::string name = GROUP_NAME;
@@ -107,8 +110,8 @@ int execute_create(const po::variables_map &vm,
 
   int r = utils::get_pool_generic_snapshot_names(
     vm, at::ARGUMENT_MODIFIER_NONE, &arg_index, at::POOL_NAME, &pool_name,
-    &namespace_name, GROUP_NAME, "group", &group_name, nullptr, true,
-    utils::SNAPSHOT_PRESENCE_NONE, utils::SPEC_VALIDATION_FULL);
+    at::NAMESPACE_NAME, &namespace_name, GROUP_NAME, "group", &group_name,
+    nullptr, true, utils::SNAPSHOT_PRESENCE_NONE, utils::SPEC_VALIDATION_FULL);
   if (r < 0) {
     return r;
   }
@@ -187,8 +190,8 @@ int execute_remove(const po::variables_map &vm,
 
   int r = utils::get_pool_generic_snapshot_names(
     vm, at::ARGUMENT_MODIFIER_NONE, &arg_index, at::POOL_NAME, &pool_name,
-    &namespace_name, GROUP_NAME, "group", &group_name, nullptr, true,
-    utils::SNAPSHOT_PRESENCE_NONE, utils::SPEC_VALIDATION_FULL);
+    at::NAMESPACE_NAME, &namespace_name, GROUP_NAME, "group", &group_name,
+    nullptr, true, utils::SNAPSHOT_PRESENCE_NONE, utils::SPEC_VALIDATION_FULL);
   if (r < 0) {
     return r;
   }
@@ -221,8 +224,8 @@ int execute_rename(const po::variables_map &vm,
 
   int r = utils::get_pool_generic_snapshot_names(
     vm, at::ARGUMENT_MODIFIER_NONE, &arg_index, at::POOL_NAME, &pool_name,
-    &namespace_name, GROUP_NAME, "group", &group_name, nullptr, true,
-    utils::SNAPSHOT_PRESENCE_NONE, utils::SPEC_VALIDATION_FULL);
+    at::NAMESPACE_NAME, &namespace_name, GROUP_NAME, "group", &group_name,
+    nullptr, true, utils::SNAPSHOT_PRESENCE_NONE, utils::SPEC_VALIDATION_FULL);
   if (r < 0) {
     return r;
   }
@@ -233,9 +236,9 @@ int execute_rename(const po::variables_map &vm,
 
   r = utils::get_pool_generic_snapshot_names(
     vm, at::ARGUMENT_MODIFIER_DEST, &arg_index, at::DEST_POOL_NAME,
-    &dest_pool_name, &dest_namespace_name, DEST_GROUP_NAME, "group",
-    &dest_group_name, nullptr, true, utils::SNAPSHOT_PRESENCE_NONE,
-    utils::SPEC_VALIDATION_FULL);
+    &dest_pool_name, at::DEST_NAMESPACE_NAME, &dest_namespace_name,
+    DEST_GROUP_NAME, "group", &dest_group_name, nullptr, true,
+    utils::SNAPSHOT_PRESENCE_NONE, utils::SPEC_VALIDATION_FULL);
   if (r < 0) {
     return r;
   }
@@ -283,8 +286,8 @@ int execute_info(const po::variables_map &vm,
 
   int r = utils::get_pool_generic_snapshot_names(
     vm, at::ARGUMENT_MODIFIER_NONE, &arg_index, at::POOL_NAME, &pool_name,
-    &namespace_name, GROUP_NAME, "group", &group_name, nullptr, true,
-    utils::SNAPSHOT_PRESENCE_NONE, utils::SPEC_VALIDATION_FULL);
+    at::NAMESPACE_NAME, &namespace_name, GROUP_NAME, "group", &group_name,
+    nullptr, true, utils::SNAPSHOT_PRESENCE_NONE, utils::SPEC_VALIDATION_FULL);
   if (r < 0) {
     return r;
   }
@@ -335,8 +338,9 @@ int execute_add(const po::variables_map &vm,
 
   int r = utils::get_pool_generic_snapshot_names(
     vm, at::ARGUMENT_MODIFIER_NONE, &arg_index, GROUP_POOL_NAME,
-    &group_pool_name, &group_namespace_name, GROUP_NAME, "group", &group_name,
-    nullptr, true, utils::SNAPSHOT_PRESENCE_NONE, utils::SPEC_VALIDATION_FULL);
+    &group_pool_name, GROUP_NAMESPACE_NAME, &group_namespace_name,
+    GROUP_NAME, "group", &group_name, nullptr, true,
+    utils::SNAPSHOT_PRESENCE_NONE, utils::SPEC_VALIDATION_FULL);
   if (r < 0) {
     return r;
   }
@@ -347,9 +351,9 @@ int execute_add(const po::variables_map &vm,
 
   r = utils::get_pool_generic_snapshot_names(
     vm, at::ARGUMENT_MODIFIER_NONE, &arg_index, IMAGE_POOL_NAME,
-    &image_pool_name, &image_namespace_name, at::IMAGE_NAME, "image",
-    &image_name, nullptr, true, utils::SNAPSHOT_PRESENCE_NONE,
-    utils::SPEC_VALIDATION_FULL);
+    &image_pool_name, IMAGE_NAMESPACE_NAME, &image_namespace_name,
+    at::IMAGE_NAME, "image", &image_name, nullptr, true,
+    utils::SNAPSHOT_PRESENCE_NONE, utils::SPEC_VALIDATION_FULL);
   if (r < 0) {
     return r;
   }
@@ -393,8 +397,9 @@ int execute_remove_image(const po::variables_map &vm,
 
   int r = utils::get_pool_generic_snapshot_names(
     vm, at::ARGUMENT_MODIFIER_NONE, &arg_index, GROUP_POOL_NAME,
-    &group_pool_name, &group_namespace_name, GROUP_NAME, "group", &group_name,
-    nullptr, true, utils::SNAPSHOT_PRESENCE_NONE, utils::SPEC_VALIDATION_FULL);
+    &group_pool_name, GROUP_NAMESPACE_NAME, &group_namespace_name,
+    GROUP_NAME, "group", &group_name, nullptr, true,
+    utils::SNAPSHOT_PRESENCE_NONE, utils::SPEC_VALIDATION_FULL);
   if (r < 0) {
     return r;
   }
@@ -410,9 +415,9 @@ int execute_remove_image(const po::variables_map &vm,
 
   r = utils::get_pool_generic_snapshot_names(
     vm, at::ARGUMENT_MODIFIER_NONE, &arg_index, IMAGE_POOL_NAME,
-    &image_pool_name, &image_namespace_name, at::IMAGE_NAME, "image",
-    &image_name, nullptr, image_id.empty(), utils::SNAPSHOT_PRESENCE_NONE,
-    utils::SPEC_VALIDATION_FULL);
+    &image_pool_name, IMAGE_NAMESPACE_NAME, &image_namespace_name,
+    at::IMAGE_NAME, "image", &image_name, nullptr, image_id.empty(),
+    utils::SNAPSHOT_PRESENCE_NONE, utils::SPEC_VALIDATION_FULL);
   if (r < 0) {
     return r;
   }
@@ -464,8 +469,8 @@ int execute_list_images(const po::variables_map &vm,
 
   int r = utils::get_pool_generic_snapshot_names(
     vm, at::ARGUMENT_MODIFIER_NONE, &arg_index, at::POOL_NAME, &pool_name,
-    &namespace_name, GROUP_NAME, "group", &group_name, nullptr, true,
-    utils::SNAPSHOT_PRESENCE_NONE, utils::SPEC_VALIDATION_FULL);
+    at::NAMESPACE_NAME, &namespace_name, GROUP_NAME, "group", &group_name,
+    nullptr, true, utils::SNAPSHOT_PRESENCE_NONE, utils::SPEC_VALIDATION_FULL);
   if (r < 0) {
     return r;
   }
@@ -563,8 +568,9 @@ int execute_group_snap_create(const po::variables_map &vm,
 
   int r = utils::get_pool_generic_snapshot_names(
     vm, at::ARGUMENT_MODIFIER_NONE, &arg_index, at::POOL_NAME, &pool_name,
-    &namespace_name, GROUP_NAME, "group", &group_name, &snap_name, true,
-    utils::SNAPSHOT_PRESENCE_REQUIRED, utils::SPEC_VALIDATION_FULL);
+    at::NAMESPACE_NAME, &namespace_name, GROUP_NAME, "group", &group_name,
+    &snap_name, true, utils::SNAPSHOT_PRESENCE_REQUIRED,
+    utils::SPEC_VALIDATION_FULL);
   if (r < 0) {
     return r;
   }
@@ -604,8 +610,9 @@ int execute_group_snap_remove(const po::variables_map &vm,
 
   int r = utils::get_pool_generic_snapshot_names(
     vm, at::ARGUMENT_MODIFIER_NONE, &arg_index, at::POOL_NAME, &pool_name,
-    &namespace_name, GROUP_NAME, "group", &group_name, &snap_name, true,
-    utils::SNAPSHOT_PRESENCE_REQUIRED, utils::SPEC_VALIDATION_FULL);
+    at::NAMESPACE_NAME, &namespace_name, GROUP_NAME, "group", &group_name,
+    &snap_name, true, utils::SNAPSHOT_PRESENCE_REQUIRED,
+    utils::SPEC_VALIDATION_FULL);
   if (r < 0) {
     return r;
   }
@@ -640,8 +647,9 @@ int execute_group_snap_rename(const po::variables_map &vm,
 
   int r = utils::get_pool_generic_snapshot_names(
     vm, at::ARGUMENT_MODIFIER_NONE, &arg_index, at::POOL_NAME, &pool_name,
-    &namespace_name, GROUP_NAME, "group", &group_name, &source_snap_name, true,
-    utils::SNAPSHOT_PRESENCE_REQUIRED, utils::SPEC_VALIDATION_FULL);
+    at::NAMESPACE_NAME, &namespace_name, GROUP_NAME, "group", &group_name,
+    &source_snap_name, true, utils::SNAPSHOT_PRESENCE_REQUIRED,
+    utils::SPEC_VALIDATION_FULL);
   if (r < 0) {
     return r;
   }
@@ -696,8 +704,8 @@ int execute_group_snap_list(const po::variables_map &vm,
 
   int r = utils::get_pool_generic_snapshot_names(
     vm, at::ARGUMENT_MODIFIER_NONE, &arg_index, at::POOL_NAME, &pool_name,
-    &namespace_name, GROUP_NAME, "group", &group_name, nullptr, true,
-    utils::SNAPSHOT_PRESENCE_NONE, utils::SPEC_VALIDATION_FULL);
+    at::NAMESPACE_NAME, &namespace_name, GROUP_NAME, "group", &group_name,
+    nullptr, true, utils::SNAPSHOT_PRESENCE_NONE, utils::SPEC_VALIDATION_FULL);
   if (r < 0) {
     return r;
   }
@@ -764,8 +772,9 @@ int execute_group_snap_info(const po::variables_map &vm,
 
   int r = utils::get_pool_generic_snapshot_names(
     vm, at::ARGUMENT_MODIFIER_NONE, &arg_index, at::POOL_NAME, &pool_name,
-    &namespace_name, GROUP_NAME, "group", &group_name, &group_snap_name, true,
-    utils::SNAPSHOT_PRESENCE_REQUIRED, utils::SPEC_VALIDATION_FULL);
+    at::NAMESPACE_NAME, &namespace_name, GROUP_NAME, "group", &group_name,
+    &group_snap_name, true, utils::SNAPSHOT_PRESENCE_REQUIRED,
+    utils::SPEC_VALIDATION_FULL);
   if (r < 0) {
     return r;
   }
@@ -872,8 +881,9 @@ int execute_group_snap_rollback(const po::variables_map &vm,
 
   int r = utils::get_pool_generic_snapshot_names(
     vm, at::ARGUMENT_MODIFIER_NONE, &arg_index, at::POOL_NAME, &pool_name,
-    &namespace_name, GROUP_NAME, "group", &group_name, &snap_name, true,
-    utils::SNAPSHOT_PRESENCE_REQUIRED, utils::SPEC_VALIDATION_FULL);
+    at::NAMESPACE_NAME, &namespace_name, GROUP_NAME, "group", &group_name,
+    &snap_name, true, utils::SNAPSHOT_PRESENCE_REQUIRED,
+    utils::SPEC_VALIDATION_FULL);
   if (r < 0) {
     return r;
   }
@@ -954,9 +964,6 @@ void get_add_arguments(po::options_description *positional,
   add_prefixed_pool_option(options, "image");
   add_prefixed_namespace_option(options, "image");
   at::add_image_option(options, at::ARGUMENT_MODIFIER_NONE);
-
-  at::add_pool_option(options, at::ARGUMENT_MODIFIER_NONE,
-	       " unless overridden");
 }
 
 void get_remove_image_arguments(po::options_description *positional,
@@ -979,8 +986,6 @@ void get_remove_image_arguments(po::options_description *positional,
   add_prefixed_namespace_option(options, "image");
   at::add_image_option(options, at::ARGUMENT_MODIFIER_NONE);
 
-  at::add_pool_option(options, at::ARGUMENT_MODIFIER_NONE,
-	       " unless overridden");
   at::add_image_id_option(options);
 }