Merge pull request ceph#62921 from idryomov/wip-71026

librbd: disallow "rbd trash mv" if image is in a group

Reviewed-by: Ramana Raja <[email protected]>

Author: idryomov

PendingReleaseNotes

@@ -133,6 +133,10 @@
 * RGW: Added support for the `RestrictPublicBuckets` property of the S3 `PublicAccessBlock`
   configuration.
 
+* RBD: Moving an image that is a member of a group to trash is no longer
+  allowed.  `rbd trash mv` command now behaves the same way as `rbd rm` in this
+  scenario.
+
 >=19.2.1
 
 * CephFS: Command `fs subvolume create` now allows tagging subvolumes through option

src/librbd/api/Trash.cc

@@ -217,6 +217,13 @@ int Trash<I>::move(librados::IoCtx &io_ctx, rbd_trash_image_source_t source,
       ictx->state->close();
       return -EBUSY;
     }
+    if (ictx->group_spec.is_valid() &&
+        source != RBD_TRASH_IMAGE_SOURCE_MIGRATION) {
+      lderr(cct) << "image is in a group - not moving to trash" << dendl;
+      ictx->image_lock.unlock_shared();
+      ictx->state->close();
+      return -EMLINK;
+    }
     ictx->image_lock.unlock_shared();
 
     if (mirror_r >= 0 &&

src/pybind/rbd/rbd.pyx

@@ -271,6 +271,12 @@ class ImageHasSnapshots(OSError):
                 "RBD image has snapshots (%s)" % message, errno)
 
 
+class ImageMemberOfGroup(OSError):
+    def __init__(self, message, errno=None):
+        super(ImageMemberOfGroup, self).__init__(
+                "RBD image is member of group (%s)" % message, errno)
+
+
 class FunctionNotSupported(OSError):
     def __init__(self, message, errno=None):
         super(FunctionNotSupported, self).__init__(
@@ -320,6 +326,7 @@ cdef errno_to_exception = {
     errno.EROFS      : ReadOnlyImage,
     errno.EBUSY      : ImageBusy,
     errno.ENOTEMPTY  : ImageHasSnapshots,
+    errno.EMLINK     : ImageMemberOfGroup,
     errno.ENOSYS     : FunctionNotSupported,
     errno.EDOM       : ArgumentOutOfRange,
     errno.ESHUTDOWN  : ConnectionShutdown,
@@ -339,6 +346,7 @@ cdef group_errno_to_exception = {
     errno.EROFS      : ReadOnlyImage,
     errno.EBUSY      : ImageBusy,
     errno.ENOTEMPTY  : ImageHasSnapshots,
+    errno.EMLINK     : ImageMemberOfGroup,
     errno.ENOSYS     : FunctionNotSupported,
     errno.EDOM       : ArgumentOutOfRange,
     errno.ESHUTDOWN  : ConnectionShutdown,

src/test/pybind/test_rbd.py

@@ -21,7 +21,7 @@
                    LIBRADOS_OP_FLAG_FADVISE_RANDOM)
 from rbd import (RBD, Group, Image, ImageNotFound, InvalidArgument, ImageExists,
                  ImageBusy, ImageHasSnapshots, ReadOnlyImage, ObjectNotFound,
-                 FunctionNotSupported, ArgumentOutOfRange,
+                 FunctionNotSupported, ArgumentOutOfRange, ImageMemberOfGroup,
                  ECANCELED, OperationCanceled,
                  DiskQuotaExceeded, ConnectionShutdown, PermissionError,
                  RBD_FEATURE_LAYERING, RBD_FEATURE_STRIPINGV2,
@@ -3069,13 +3069,19 @@ def test_group_image_list(self):
 
     def test_group_image_list_move_to_trash(self):
         eq([], list(self.group.list_images()))
-        with Image(ioctx, image_name) as image:
-            image_id = image.id()
         self.group.add_image(ioctx, image_name)
         eq([image_name], [img['name'] for img in self.group.list_images()])
-        RBD().trash_move(ioctx, image_name, 0)
+        assert_raises(ImageMemberOfGroup, RBD().trash_move, ioctx, image_name, 0)
+        eq([image_name], [img['name'] for img in self.group.list_images()])
+
+    def test_group_image_list_remove(self):
+        # need a closed image to get ImageMemberOfGroup instead of ImageBusy
+        self.image_names.append(create_image())
         eq([], list(self.group.list_images()))
-        RBD().trash_restore(ioctx, image_id, image_name)
+        self.group.add_image(ioctx, image_name)
+        eq([image_name], [img['name'] for img in self.group.list_images()])
+        assert_raises(ImageMemberOfGroup, RBD().remove, ioctx, image_name)
+        eq([image_name], [img['name'] for img in self.group.list_images()])
 
     def test_group_get_id(self):
         id = self.group.id()

src/tools/rbd/action/Trash.cc

@@ -97,14 +97,22 @@ int execute_move(const po::variables_map &vm,
   librbd::RBD rbd;
   r = rbd.trash_move(io_ctx, image_name.c_str(), dt);
   if (r < 0) {
-    std::cerr << "rbd: deferred delete error: " << cpp_strerror(r)
-              << std::endl;
+    if (r == -EMLINK) {
+      std::cerr << "rbd: error: image belongs to a group"
+                << std::endl
+                << "Remove the image from the group and try again."
+                << std::endl;
+    } else {
+      std::cerr << "rbd: deferred delete error: " << cpp_strerror(r)
+                << std::endl;
+    }
+    return r;
   }
 
   if (expires_at != "now") {
     std::cout << "rbd: image " << image_name << " will expire at " << exp_time << std::endl;
   }
-  return r;
+  return 0;
 }
 
 void get_remove_arguments(po::options_description *positional,
@@ -161,7 +169,10 @@ int execute_remove(const po::variables_map &vm,
                 << "waiting 30s for the crashed client to timeout."
                 << std::endl;
     } else if (r == -EMLINK) {
-      std::cerr << std::endl
+      // moving to trash an image that belongs to a group is no longer
+      // allowed, this is to handle any image that was trashed earlier
+      std::cerr << "rbd: error: image belongs to a group"
+                << std::endl
                 << "Remove the image from the group and try again."
                 << std::endl;
     } else if (r == -EPERM) {