From c16b06be8fb5ac21e902a796e4d3ec4d5b52496d Mon Sep 17 00:00:00 2001
From: Mykola Golub <mgolub@suse.com>
Date: Sun, 14 Feb 2021 11:48:35 +0000
Subject: [PATCH] librbd: always check on-disk image name when renaming

Signed-off-by: Mykola Golub <mgolub@suse.com>
(cherry picked from commit 950a7b5cfc66bdfea35304e6cb06e74c92dcaf5c)
---
 src/librbd/Operations.cc              | 16 ++++-----
 src/librbd/operation/RenameRequest.cc | 49 +++++++++++++++++++++++++--
 src/librbd/operation/RenameRequest.h  |  8 ++++-
 3 files changed, 62 insertions(+), 11 deletions(-)

diff --git a/src/librbd/Operations.cc b/src/librbd/Operations.cc
index d53d7637e3f13..9329786d6df0c 100644
--- a/src/librbd/Operations.cc
+++ b/src/librbd/Operations.cc
@@ -580,19 +580,19 @@ void Operations<I>::execute_rename(const std::string &dest_name,
     return;
   }
 
-  m_image_ctx.image_lock.lock_shared();
-  if (m_image_ctx.name == dest_name) {
-    m_image_ctx.image_lock.unlock_shared();
-    on_finish->complete(-EEXIST);
-    return;
-  }
-  m_image_ctx.image_lock.unlock_shared();
-
   CephContext *cct = m_image_ctx.cct;
   ldout(cct, 5) << this << " " << __func__ << ": dest_name=" << dest_name
                 << dendl;
 
   if (m_image_ctx.old_format) {
+    m_image_ctx.image_lock.lock_shared();
+    if (m_image_ctx.name == dest_name) {
+      m_image_ctx.image_lock.unlock_shared();
+      on_finish->complete(-EEXIST);
+      return;
+    }
+    m_image_ctx.image_lock.unlock_shared();
+
     // unregister watch before and register back after rename
     on_finish = new C_NotifyUpdate<I>(m_image_ctx, on_finish);
     on_finish = new LambdaContext([this, on_finish](int r) {
diff --git a/src/librbd/operation/RenameRequest.cc b/src/librbd/operation/RenameRequest.cc
index f62b549c6b9e8..15bcd819cf55a 100644
--- a/src/librbd/operation/RenameRequest.cc
+++ b/src/librbd/operation/RenameRequest.cc
@@ -22,6 +22,9 @@ template <typename I>
 std::ostream& operator<<(std::ostream& os,
                          const typename RenameRequest<I>::State& state) {
   switch(state) {
+  case RenameRequest<I>::STATE_READ_DIRECTORY:
+    os << "READ_DIRECTORY";
+    break;
   case RenameRequest<I>::STATE_READ_SOURCE_HEADER:
     os << "READ_SOURCE_HEADER";
     break;
@@ -55,7 +58,12 @@ RenameRequest<I>::RenameRequest(I &image_ctx, Context *on_finish,
 
 template <typename I>
 void RenameRequest<I>::send_op() {
-  send_read_source_header();
+  I &image_ctx = this->m_image_ctx;
+  if (image_ctx.old_format) {
+    send_read_source_header();
+    return;
+  }
+  send_read_directory();
 }
 
 template <typename I>
@@ -74,7 +82,24 @@ bool RenameRequest<I>::should_complete(int r) {
     return true;
   }
 
-  if (m_state == STATE_UPDATE_DIRECTORY) {
+  if (m_state == STATE_READ_DIRECTORY) {
+    std::string name;
+    auto it = m_source_name_bl.cbegin();
+    r = cls_client::dir_get_name_finish(&it, &name);
+    if (r < 0) {
+      lderr(cct) << "could not read directory: " << cpp_strerror(r) << dendl;
+      return true;
+    }
+    bool update = false;
+    {
+      std::shared_lock image_locker{image_ctx.image_lock};
+      update = image_ctx.name != name;
+    }
+    if (update) {
+      image_ctx.set_image_name(name);
+      m_source_oid = util::id_obj_name(name);
+    }
+  } else if (m_state == STATE_UPDATE_DIRECTORY) {
     // update in-memory name before removing source header
     apply();
   } else if (m_state == STATE_REMOVE_SOURCE_HEADER) {
@@ -83,6 +108,9 @@ bool RenameRequest<I>::should_complete(int r) {
 
   std::shared_lock owner_lock{image_ctx.owner_lock};
   switch (m_state) {
+  case STATE_READ_DIRECTORY:
+    send_read_source_header();
+    break;
   case STATE_READ_SOURCE_HEADER:
     send_write_destination_header();
     break;
@@ -120,6 +148,23 @@ int RenameRequest<I>::filter_return_code(int r) const {
   return r;
 }
 
+template <typename I>
+void RenameRequest<I>::send_read_directory() {
+  I &image_ctx = this->m_image_ctx;
+  CephContext *cct = image_ctx.cct;
+  ldout(cct, 5) << this << " " << __func__ << dendl;
+  m_state = STATE_READ_DIRECTORY;
+
+  librados::ObjectReadOperation op;
+  cls_client::dir_get_name_start(&op, image_ctx.id);
+
+  auto comp = this->create_callback_completion();
+  int r = image_ctx.md_ctx.aio_operate(RBD_DIRECTORY, comp, &op,
+                                       &m_source_name_bl);
+  ceph_assert(r == 0);
+  comp->release();
+}
+
 template <typename I>
 void RenameRequest<I>::send_read_source_header() {
   I &image_ctx = this->m_image_ctx;
diff --git a/src/librbd/operation/RenameRequest.h b/src/librbd/operation/RenameRequest.h
index 6534d36c09869..11fdec6488e18 100644
--- a/src/librbd/operation/RenameRequest.h
+++ b/src/librbd/operation/RenameRequest.h
@@ -27,6 +27,9 @@ public:
    * <start>
    *    |
    *    v
+   * STATE_READ_DIRECTORY
+   *    |
+   *    v
    * STATE_READ_SOURCE_HEADER
    *    |
    *    v
@@ -45,6 +48,7 @@ public:
    *
    */
   enum State {
+    STATE_READ_DIRECTORY,
     STATE_READ_SOURCE_HEADER,
     STATE_WRITE_DEST_HEADER,
     STATE_UPDATE_DIRECTORY,
@@ -69,10 +73,12 @@ private:
   std::string m_source_oid;
   std::string m_dest_oid;
 
-  State m_state = STATE_READ_SOURCE_HEADER;
+  State m_state = STATE_READ_DIRECTORY;
 
+  bufferlist m_source_name_bl;
   bufferlist m_header_bl;
 
+  void send_read_directory();
   void send_read_source_header();
   void send_write_destination_header();
   void send_update_directory();
-- 
2.39.5