From: Ilya Dryomov <idryomov@gmail.com>
Date: Wed, 26 May 2021 12:21:22 +0000 (+0200)
Subject: librbd: don't stop at the first unremovable image when purging
X-Git-Tag: v14.2.22~17^2
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=f9437eb4f6fcc4ba00db609b405c9e9d84379edf;p=ceph.git

librbd: don't stop at the first unremovable image when purging

As there is no inherent ordering, there may be multiple removable
images past the unremovable image.  On top of that, removing a clone
may make its parent removable so perform an additional pass if any
image gets removed.

Fixes: https://tracker.ceph.com/issues/51021
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
(cherry picked from commit 16d9a68a3e863b5a819860abf0696fb76fc9341a)

Conflicts:
	qa/workunits/rbd/cli_generic.sh [ commit 6e1434eefc3d
	  ("librbd: optionally move parent image to trash on remove")
	  not in nautilus ]
---

diff --git a/qa/workunits/rbd/cli_generic.sh b/qa/workunits/rbd/cli_generic.sh
index 7f44d932d9534..3f5c2fd45f624 100755
--- a/qa/workunits/rbd/cli_generic.sh
+++ b/qa/workunits/rbd/cli_generic.sh
@@ -485,21 +485,148 @@ test_purge() {
     echo "testing trash purge..."
     remove_images
 
+    rbd trash ls | wc -l | grep 0
+    rbd trash purge
+
+    rbd create $RBD_CREATE_ARGS --size 256 testimg1
+    rbd create $RBD_CREATE_ARGS --size 256 testimg2
+    rbd trash mv testimg1
+    rbd trash mv testimg2
+    rbd trash ls | wc -l | grep 2
     rbd trash purge
     rbd trash ls | wc -l | grep 0
 
-    rbd create $RBD_CREATE_ARGS foo -s 1
-    rbd create $RBD_CREATE_ARGS bar -s 1
+    rbd create $RBD_CREATE_ARGS --size 256 testimg1
+    rbd create $RBD_CREATE_ARGS --size 256 testimg2
+    rbd trash mv testimg1 --expires-at "1 hour"
+    rbd trash mv testimg2 --expires-at "3 hours"
+    rbd trash ls | wc -l | grep 2
+    rbd trash purge
+    rbd trash ls | wc -l | grep 2
+    rbd trash purge --expired-before "now + 2 hours"
+    rbd trash ls | wc -l | grep 1
+    rbd trash ls | grep testimg2
+    rbd trash purge --expired-before "now + 4 hours"
+    rbd trash ls | wc -l | grep 0
+
+    rbd create $RBD_CREATE_ARGS --size 256 testimg1
+    rbd snap create testimg1@snap  # pin testimg1
+    rbd create $RBD_CREATE_ARGS --size 256 testimg2
+    rbd create $RBD_CREATE_ARGS --size 256 testimg3
+    rbd trash mv testimg1
+    rbd trash mv testimg2
+    rbd trash mv testimg3
+    rbd trash ls | wc -l | grep 3
+    rbd trash purge 2>&1 | grep 'some expired images could not be removed'
+    rbd trash ls | wc -l | grep 1
+    rbd trash ls | grep testimg1
+    ID=$(rbd trash ls | awk '{ print $1 }')
+    rbd snap purge --image-id $ID
+    rbd trash purge
+    rbd trash ls | wc -l | grep 0
+
+    rbd create $RBD_CREATE_ARGS --size 256 testimg1
+    rbd create $RBD_CREATE_ARGS --size 256 testimg2
+    rbd snap create testimg2@snap  # pin testimg2
+    rbd create $RBD_CREATE_ARGS --size 256 testimg3
+    rbd trash mv testimg1
+    rbd trash mv testimg2
+    rbd trash mv testimg3
+    rbd trash ls | wc -l | grep 3
+    rbd trash purge 2>&1 | grep 'some expired images could not be removed'
+    rbd trash ls | wc -l | grep 1
+    rbd trash ls | grep testimg2
+    ID=$(rbd trash ls | awk '{ print $1 }')
+    rbd snap purge --image-id $ID
+    rbd trash purge
+    rbd trash ls | wc -l | grep 0
 
-    rbd trash mv foo --expires-at "10 sec"
-    rbd trash mv bar --expires-at "30 sec"
+    rbd create $RBD_CREATE_ARGS --size 256 testimg1
+    rbd create $RBD_CREATE_ARGS --size 256 testimg2
+    rbd create $RBD_CREATE_ARGS --size 256 testimg3
+    rbd snap create testimg3@snap  # pin testimg3
+    rbd trash mv testimg1
+    rbd trash mv testimg2
+    rbd trash mv testimg3
+    rbd trash ls | wc -l | grep 3
+    rbd trash purge 2>&1 | grep 'some expired images could not be removed'
+    rbd trash ls | wc -l | grep 1
+    rbd trash ls | grep testimg3
+    ID=$(rbd trash ls | awk '{ print $1 }')
+    rbd snap purge --image-id $ID
+    rbd trash purge
+    rbd trash ls | wc -l | grep 0
 
-    rbd trash purge --expired-before "now + 10 sec"
-    rbd trash ls | grep -v foo | wc -l | grep 1
-    rbd trash ls | grep bar
+    # test purging a clone with a chain of parents
+    rbd create $RBD_CREATE_ARGS --size 256 testimg1
+    rbd snap create testimg1@snap
+    rbd clone --rbd-default-clone-format=2 testimg1@snap testimg2
+    rbd snap rm testimg1@snap
+    rbd create $RBD_CREATE_ARGS --size 256 testimg3
+    rbd snap create testimg2@snap
+    rbd clone --rbd-default-clone-format=2 testimg2@snap testimg4
+    rbd clone --rbd-default-clone-format=2 testimg2@snap testimg5
+    rbd snap rm testimg2@snap
+    rbd snap create testimg4@snap
+    rbd clone --rbd-default-clone-format=2 testimg4@snap testimg6
+    rbd snap rm testimg4@snap
+    rbd trash mv testimg1
+    rbd trash mv testimg2
+    rbd trash mv testimg3
+    rbd trash mv testimg4
+    rbd trash ls | wc -l | grep 4
+    rbd trash purge 2>&1 | grep 'some expired images could not be removed'
+    rbd trash ls | wc -l | grep 3
+    rbd trash ls | grep testimg1
+    rbd trash ls | grep testimg2
+    rbd trash ls | grep testimg4
+    rbd trash mv testimg6
+    rbd trash ls | wc -l | grep 4
+    rbd trash purge 2>&1 | grep 'some expired images could not be removed'
+    rbd trash ls | wc -l | grep 2
+    rbd trash ls | grep testimg1
+    rbd trash ls | grep testimg2
+    rbd trash mv testimg5
+    rbd trash ls | wc -l | grep 3
+    rbd trash purge
+    rbd trash ls | wc -l | grep 0
 
-    LAST_IMG=$(rbd trash ls | grep bar | awk '{print $1;}')
-    rbd trash rm $LAST_IMG --force --no-progress | grep -v '.' | wc -l | grep 0
+    rbd create $RBD_CREATE_ARGS --size 256 testimg1
+    rbd snap create testimg1@snap
+    rbd clone --rbd-default-clone-format=2 testimg1@snap testimg2
+    rbd snap rm testimg1@snap
+    rbd create $RBD_CREATE_ARGS --size 256 testimg3
+    rbd snap create testimg3@snap  # pin testimg3
+    rbd snap create testimg2@snap
+    rbd clone --rbd-default-clone-format=2 testimg2@snap testimg4
+    rbd clone --rbd-default-clone-format=2 testimg2@snap testimg5
+    rbd snap rm testimg2@snap
+    rbd snap create testimg4@snap
+    rbd clone --rbd-default-clone-format=2 testimg4@snap testimg6
+    rbd snap rm testimg4@snap
+    rbd trash mv testimg1
+    rbd trash mv testimg2
+    rbd trash mv testimg3
+    rbd trash mv testimg4
+    rbd trash ls | wc -l | grep 4
+    rbd trash purge 2>&1 | grep 'some expired images could not be removed'
+    rbd trash ls | wc -l | grep 4
+    rbd trash mv testimg6
+    rbd trash ls | wc -l | grep 5
+    rbd trash purge 2>&1 | grep 'some expired images could not be removed'
+    rbd trash ls | wc -l | grep 3
+    rbd trash ls | grep testimg1
+    rbd trash ls | grep testimg2
+    rbd trash ls | grep testimg3
+    rbd trash mv testimg5
+    rbd trash ls | wc -l | grep 4
+    rbd trash purge 2>&1 | grep 'some expired images could not be removed'
+    rbd trash ls | wc -l | grep 1
+    rbd trash ls | grep testimg3
+    ID=$(rbd trash ls | awk '{ print $1 }')
+    rbd snap purge --image-id $ID
+    rbd trash purge
+    rbd trash ls | wc -l | grep 0
 }
 
 test_deep_copy_clone() {
diff --git a/src/librbd/api/Trash.cc b/src/librbd/api/Trash.cc
index 678bf8c9f289d..52f79632d5e20 100644
--- a/src/librbd/api/Trash.cc
+++ b/src/librbd/api/Trash.cc
@@ -520,12 +520,48 @@ int Trash<I>::purge(IoCtx& io_ctx, time_t expire_ts,
 
   NoOpProgressContext remove_pctx;
   uint64_t list_size = to_be_removed.size(), i = 0;
-  for (const auto &entry_id : to_be_removed) {
-    r = librbd::api::Trash<I>::remove(io_ctx, entry_id, true, remove_pctx);
-    if (r < 0) {
-      return r;
+  int remove_err = 1;
+  while (!to_be_removed.empty() && remove_err == 1) {
+    remove_err = 0;
+    for (auto it = to_be_removed.begin(); it != to_be_removed.end(); ) {
+      trash_image_info_t trash_info;
+      r = Trash<I>::get(io_ctx, *it, &trash_info);
+      if (r == -ENOENT) {
+        // likely RBD_TRASH_IMAGE_SOURCE_USER_PARENT image removed as a side
+        // effect of a preceeding remove (last child detach)
+        pctx.update_progress(++i, list_size);
+        it = to_be_removed.erase(it);
+        continue;
+      } else if (r < 0) {
+        lderr(cct) << "error getting image id " << *it
+                   << " info: " << cpp_strerror(r) << dendl;
+        return r;
+      }
+
+      r = Trash<I>::remove(io_ctx, *it, true, remove_pctx);
+      if (r == -ENOTEMPTY || r == -EBUSY || r == -EMLINK || r == -EUCLEAN) {
+        if (!remove_err) {
+          remove_err = r;
+        }
+        ++it;
+        continue;
+      } else if (r < 0) {
+        lderr(cct) << "error removing image id " << *it
+                   << ": " << cpp_strerror(r) << dendl;
+        return r;
+      }
+      pctx.update_progress(++i, list_size);
+      it = to_be_removed.erase(it);
+      remove_err = 1;
     }
-    pctx.update_progress(++i, list_size);
+    ldout(cct, 20) << "remove_err=" << remove_err << dendl;
+  }
+
+  if (!to_be_removed.empty()) {
+    ceph_assert(remove_err < 0);
+    ldout(cct, 10) << "couldn't remove " << to_be_removed.size()
+                   << " expired images" << dendl;
+    return remove_err;
   }
 
   return 0;