librbd: validate data pool for self-managed snapshot support

Fixes: https://tracker.ceph.com/issues/24675
Signed-off-by: Mykola Golub <mgolub@suse.com>

diff --git a/qa/workunits/rbd/verify_pool.sh b/qa/workunits/rbd/verify_pool.sh
index 569b65a6d34b5017198a3301c4215cd33e46655a..08bcca5065627971b1cccab57f1d07b031102fab 100755 (executable)
--- a/qa/workunits/rbd/verify_pool.sh
+++ b/qa/workunits/rbd/verify_pool.sh
@@ -20,8 +20,8 @@ set_up
 # creating an image in a pool-managed snapshot pool should fail
 rbd create --pool $POOL_NAME --size 1 foo && exit 1 || true
 
-# should succeed if images already exist in the pool
-rados --pool $POOL_NAME create rbd_directory
+# should succeed if the pool already marked as validated
+printf "overwrite validated" | rados --pool $POOL_NAME put rbd_info -
 rbd create --pool $POOL_NAME --size 1 foo
 
 echo OK

diff --git a/src/librbd/image/CreateRequest.cc b/src/librbd/image/CreateRequest.cc
index c4e17921b722f03d071dd5b17084b23409b9e483..15aa19df68fecd8f34d2eb90efbf3e0f80c9bd63 100644 (file)
--- a/src/librbd/image/CreateRequest.cc
+++ b/src/librbd/image/CreateRequest.cc
@@ -78,25 +78,6 @@ int validate_striping(CephContext *cct, uint8_t order, uint64_t stripe_unit,
   return 0;
 }
 
-int validate_data_pool(CephContext *cct, IoCtx &io_ctx, uint64_t features,
-                       const std::string &data_pool, int64_t *data_pool_id) {
-  if ((features & RBD_FEATURE_DATA_POOL) == 0) {
-    return 0;
-  }
-
-  librados::Rados rados(io_ctx);
-  librados::IoCtx data_io_ctx;
-  int r = rados.ioctx_create(data_pool.c_str(), data_io_ctx);
-  if (r < 0) {
-    lderr(cct) << "data pool " << data_pool << " does not exist" << dendl;
-    return -ENOENT;
-  }
-
-  *data_pool_id = data_io_ctx.get_id();
-  return 0;
-}
-
-
 bool validate_layout(CephContext *cct, uint64_t size, file_layout_t &layout) {
   if (!librbd::ObjectMap<>::is_compatible(layout, size)) {
     lderr(cct) << "image size not compatible with object map" << dendl;
@@ -261,24 +242,30 @@ void CreateRequest<I>::send() {
     return;
   }
 
-  r = validate_data_pool(m_cct, m_io_ctx, m_features, m_data_pool,
-                         &m_data_pool_id);
-  if (r < 0) {
-    complete(r);
-    return;
-  }
-
   if (((m_features & RBD_FEATURE_OBJECT_MAP) != 0) &&
       (!validate_layout(m_cct, m_size, m_layout))) {
     complete(-EINVAL);
     return;
   }
 
-  validate_pool();
+  validate_data_pool();
 }
 
-template<typename I>
-void CreateRequest<I>::validate_pool() {
+template <typename I>
+void CreateRequest<I>::validate_data_pool() {
+  m_data_io_ctx = m_io_ctx;
+  if ((m_features & RBD_FEATURE_DATA_POOL) != 0) {
+    librados::Rados rados(m_io_ctx);
+    int r = rados.ioctx_create(m_data_pool.c_str(), m_data_io_ctx);
+    if (r < 0) {
+      lderr(m_cct) << "data pool " << m_data_pool << " does not exist" << dendl;
+      complete(r);
+      return;
+    }
+    m_data_pool_id = m_data_io_ctx.get_id();
+    m_data_io_ctx.set_namespace(m_io_ctx.get_namespace());
+  }
+
   if (!m_cct->_conf.get_val<bool>("rbd_validate_pool")) {
     add_image_to_directory();
     return;
@@ -288,27 +275,29 @@ void CreateRequest<I>::validate_pool() {
 
   using klass = CreateRequest<I>;
   librados::AioCompletion *comp =
-    create_rados_callback<klass, &klass::handle_validate_pool>(this);
+    create_rados_callback<klass, &klass::handle_validate_data_pool>(this);
 
   librados::ObjectReadOperation op;
-  op.stat(NULL, NULL, NULL);
+  op.read(0, 0, nullptr, nullptr);
 
   m_outbl.clear();
-  int r = m_io_ctx.aio_operate(RBD_DIRECTORY, comp, &op, &m_outbl);
+  int r = m_data_io_ctx.aio_operate(RBD_INFO, comp, &op, &m_outbl);
   assert(r == 0);
   comp->release();
 }
 
-template<typename I>
-void CreateRequest<I>::handle_validate_pool(int r) {
+template <typename I>
+void CreateRequest<I>::handle_validate_data_pool(int r) {
   ldout(m_cct, 20) << "r=" << r << dendl;
 
-  if (r == 0) {
-    validate_overwrite();
+  bufferlist bl;
+  bl.append("overwrite validated");
+
+  if (r >= 0 && m_outbl.contents_equal(bl)) {
+    add_image_to_directory();
     return;
   } else if ((r < 0) && (r != -ENOENT)) {
-    lderr(m_cct) << "failed to stat RBD directory: " << cpp_strerror(r)
-                 << dendl;
+    lderr(m_cct) << "failed to read RBD info: " << cpp_strerror(r) << dendl;
     complete(r);
     return;
   }
@@ -319,8 +308,10 @@ void CreateRequest<I>::handle_validate_pool(int r) {
   // try hard to make it asynchronous (and it's pretty safe not to cause
   // deadlocks).
 
+  ldout(m_cct, 10) << "validating self-managed RBD snapshot support" << dendl;
+
   uint64_t snap_id;
-  r = m_io_ctx.selfmanaged_snap_create(&snap_id);
+  r = m_data_io_ctx.selfmanaged_snap_create(&snap_id);
   if (r == -EINVAL) {
     lderr(m_cct) << "pool not configured for self-managed RBD snapshot support"
                  << dendl;
@@ -333,7 +324,7 @@ void CreateRequest<I>::handle_validate_pool(int r) {
     return;
   }
 
-  r = m_io_ctx.selfmanaged_snap_remove(snap_id);
+  r = m_data_io_ctx.selfmanaged_snap_remove(snap_id);
   if (r < 0) {
     // we've already switched to self-managed snapshots -- no need to
     // error out in case of failure here.
@@ -341,57 +332,8 @@ void CreateRequest<I>::handle_validate_pool(int r) {
                      << ": " << cpp_strerror(r) << dendl;
   }
 
-  validate_overwrite();
-}
-
-template <typename I>
-void CreateRequest<I>::validate_overwrite() {
-  ldout(m_cct, 20) << dendl;
-
-  m_data_io_ctx = m_io_ctx;
-  if (m_data_pool_id != -1) {
-    librados::Rados rados(m_io_ctx);
-    int r = rados.ioctx_create2(m_data_pool_id, m_data_io_ctx);
-    if (r < 0) {
-      lderr(m_cct) << "data pool " << m_data_pool << " does not exist" << dendl;
-      complete(r);
-      return;
-    }
-    m_data_io_ctx.set_namespace(m_io_ctx.get_namespace());
-  }
-
-  using klass = CreateRequest<I>;
-  librados::AioCompletion *comp =
-    create_rados_callback<klass, &klass::handle_validate_overwrite>(this);
-
-  librados::ObjectReadOperation op;
-  op.read(0, 0, nullptr, nullptr);
-
-  m_outbl.clear();
-  int r = m_data_io_ctx.aio_operate(RBD_INFO, comp, &op, &m_outbl);
-  assert(r == 0);
-  comp->release();
-}
-
-template <typename I>
-void CreateRequest<I>::handle_validate_overwrite(int r) {
-  ldout(m_cct, 20) << "r=" << r << dendl;
-
-  bufferlist bl;
-  bl.append("overwrite validated");
-
-  if (r == 0 && m_outbl.contents_equal(bl)) {
-    add_image_to_directory();
-    return;
-  } else if ((r < 0) && (r != -ENOENT)) {
-    lderr(m_cct) << "failed to read RBD info: " << cpp_strerror(r) << dendl;
-    complete(r);
-    return;
-  }
-
-  // validate the pool supports overwrites. We cannot use rbd_directory
-  // since the v1 images store the directory as tmap data within the object.
   ldout(m_cct, 10) << "validating overwrite support" << dendl;
+
   bufferlist initial_bl;
   initial_bl.append("validate");
   r = m_data_io_ctx.write(RBD_INFO, initial_bl, initial_bl.length(), 0);

diff --git a/src/librbd/image/CreateRequest.h b/src/librbd/image/CreateRequest.h
index 00a0818d406528c82e25484866f97cb08bd10487..24aa0eb3f1b3c9dd500d8f5ec1744ed31152f45d 100644 (file)
--- a/src/librbd/image/CreateRequest.h
+++ b/src/librbd/image/CreateRequest.h
@@ -54,12 +54,9 @@ private:
    *                                  <start> . . . . > . . . . .
    *                                     |                      .
    *                                     v                      .
-   *                               VALIDATE POOL                v (pool validation
+   *                               VALIDATE DATA POOL           v (pool validation
    *                                     |                      .  disabled)
    *                                     v                      .
-   *                             VALIDATE OVERWRITE             .
-   *                                     |                      .
-   *                                     v                      .
    * (error: bottom up)         ADD IMAGE TO DIRECTORY  < . . . .
    *  _______<_______                    |
    * |               |                   v
@@ -133,11 +130,8 @@ private:
   rbd_mirror_mode_t m_mirror_mode = RBD_MIRROR_MODE_DISABLED;
   cls::rbd::MirrorImage m_mirror_image_internal;
 
-  void validate_pool();
-  void handle_validate_pool(int r);
-
-  void validate_overwrite();
-  void handle_validate_overwrite(int r);
+  void validate_data_pool();
+  void handle_validate_data_pool(int r);
 
   void add_image_to_directory();
   void handle_add_image_to_directory(int r);