librbd: validate data pool for self-managed snapshot support

Fixes: https://tracker.ceph.com/issues/24675
Signed-off-by: Mykola Golub <mgolub@suse.com>
(cherry picked from commit 08ea7d62ba6eedf614d72ff9d33f2e6a1c0b81fe)

Conflicts:
	src/librbd/image/CreateRequest.cc
	src/librbd/image/CreateRequest.h
- luminous uses m_ioctx where master has m_io_ctx
- luminous IoCtx does not have get_namespace/set_namespace
- in luminuos CreateRequest state machine is a little different than in
the master (see the diagram in CreateRequest.h). In luminous the next
state after VALIDATE_DATA_POOL is CREATE_ID_OBJECT, so call
create_id_object() instead of add_image_to_directory().

diff --git a/qa/workunits/rbd/verify_pool.sh b/qa/workunits/rbd/verify_pool.sh
index f008fb6b3cfb34a0a1193c349278db7d66b7a501..65a61199bba434e1e9e885bec2bafea107a0f996 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 75af7061efc0e883b8aaed98d850d58dccdc1e90..b4e25e08425e86c881711bca4589de4dadb71874 100644 (file)
--- a/src/librbd/image/CreateRequest.cc
+++ b/src/librbd/image/CreateRequest.cc
@@ -73,25 +73,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;
@@ -250,24 +231,29 @@ void CreateRequest<I>::send() {
     return;
   }
 
-  r = validate_data_pool(m_cct, m_ioctx, 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_ioctx;
+  if ((m_features & RBD_FEATURE_DATA_POOL) != 0) {
+    librados::Rados rados(m_ioctx);
+    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();
+  }
+
   if (!m_cct->_conf->get_val<bool>("rbd_validate_pool")) {
     create_id_object();
     return;
@@ -277,27 +263,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_ioctx.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)) {
+    create_id_object();
     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;
   }
@@ -308,8 +296,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_ioctx.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;
@@ -322,7 +312,7 @@ void CreateRequest<I>::handle_validate_pool(int r) {
     return;
   }
 
-  r = m_ioctx.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.
@@ -330,56 +320,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_ioctx;
-  if (m_data_pool_id != -1) {
-    librados::Rados rados(m_ioctx);
-    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;
-    }
-  }
-
-  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)) {
-    create_id_object();
-    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 9d77838dd3180bd342cba10e5981f3a2db416809..42f50b8ff1a928686c1b5068df70714583202c94 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)           CREATE ID OBJECT. . < . . . . .
    *  _______<_______                    |
    * |               |                   v
@@ -133,11 +130,8 @@ private:
   rbd_mirror_mode_t m_mirror_mode;
   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 create_id_object();
   void handle_create_id_object(int r);