librbd: image create state machine should validate namespace

Fixes: http://tracker.ceph.com/issues/24558
Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/librbd/image/CreateRequest.cc b/src/librbd/image/CreateRequest.cc
index 1418ef38f3caaeb4a0bd32908c4351c7bba05e49..5ebd6a6a718c2975da7af9e7f71a351b6d609485 100644 (file)
--- a/src/librbd/image/CreateRequest.cc
+++ b/src/librbd/image/CreateRequest.cc
@@ -140,12 +140,14 @@ CreateRequest<I>::CreateRequest(IoCtx &ioctx, const std::string &image_name,
                                 const std::string &primary_mirror_uuid,
                                 bool skip_mirror_enable,
                                 ContextWQ *op_work_queue, Context *on_finish)
-  : m_ioctx(ioctx), m_image_name(image_name), m_image_id(image_id),
+  : m_image_name(image_name), m_image_id(image_id),
     m_size(size), m_non_primary_global_image_id(non_primary_global_image_id),
     m_primary_mirror_uuid(primary_mirror_uuid),
     m_skip_mirror_enable(skip_mirror_enable),
     m_op_work_queue(op_work_queue), m_on_finish(on_finish) {
-  m_cct = reinterpret_cast<CephContext *>(m_ioctx.cct());
+
+  m_io_ctx.dup(ioctx);
+  m_cct = reinterpret_cast<CephContext *>(m_io_ctx.cct());
 
   m_id_obj = util::id_obj_name(m_image_name);
   m_header_obj = util::header_name(m_image_id);
@@ -259,7 +261,7 @@ void CreateRequest<I>::send() {
     return;
   }
 
-  r = validate_data_pool(m_cct, m_ioctx, m_features, m_data_pool,
+  r = validate_data_pool(m_cct, m_io_ctx, m_features, m_data_pool,
                          &m_data_pool_id);
   if (r < 0) {
     complete(r);
@@ -278,7 +280,7 @@ void CreateRequest<I>::send() {
 template<typename I>
 void CreateRequest<I>::validate_pool() {
   if (!m_cct->_conf->get_val<bool>("rbd_validate_pool")) {
-    create_id_object();
+    add_image_to_directory();
     return;
   }
 
@@ -292,7 +294,7 @@ void CreateRequest<I>::validate_pool() {
   op.stat(NULL, NULL, NULL);
 
   m_outbl.clear();
-  int r = m_ioctx.aio_operate(RBD_DIRECTORY, comp, &op, &m_outbl);
+  int r = m_io_ctx.aio_operate(RBD_DIRECTORY, comp, &op, &m_outbl);
   assert(r == 0);
   comp->release();
 }
@@ -318,7 +320,7 @@ void CreateRequest<I>::handle_validate_pool(int r) {
   // deadlocks).
 
   uint64_t snap_id;
-  r = m_ioctx.selfmanaged_snap_create(&snap_id);
+  r = m_io_ctx.selfmanaged_snap_create(&snap_id);
   if (r == -EINVAL) {
     lderr(m_cct) << "pool not configured for self-managed RBD snapshot support"
                  << dendl;
@@ -331,7 +333,7 @@ void CreateRequest<I>::handle_validate_pool(int r) {
     return;
   }
 
-  r = m_ioctx.selfmanaged_snap_remove(snap_id);
+  r = m_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.
@@ -346,9 +348,9 @@ template <typename I>
 void CreateRequest<I>::validate_overwrite() {
   ldout(m_cct, 20) << dendl;
 
-  m_data_io_ctx = m_ioctx;
+  m_data_io_ctx = m_io_ctx;
   if (m_data_pool_id != -1) {
-    librados::Rados rados(m_ioctx);
+    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;
@@ -378,7 +380,7 @@ void CreateRequest<I>::handle_validate_overwrite(int r) {
   bl.append("overwrite validated");
 
   if (r == 0 && m_outbl.contents_equal(bl)) {
-    create_id_object();
+    add_image_to_directory();
     return;
   } else if ((r < 0) && (r != -ENOENT)) {
     lderr(m_cct) << "failed to read RBD info: " << cpp_strerror(r) << dendl;
@@ -406,64 +408,83 @@ void CreateRequest<I>::handle_validate_overwrite(int r) {
     return;
   }
 
-  create_id_object();
+  add_image_to_directory();
 }
 
 template<typename I>
-void CreateRequest<I>::create_id_object() {
+void CreateRequest<I>::add_image_to_directory() {
   ldout(m_cct, 20) << dendl;
 
   librados::ObjectWriteOperation op;
-  op.create(true);
-  cls_client::set_id(&op, m_image_id);
+  if (!m_io_ctx.get_namespace().empty()) {
+    cls_client::dir_state_assert(&op, cls::rbd::DIRECTORY_STATE_READY);
+  }
+  cls_client::dir_add_image(&op, m_image_name, m_image_id);
 
   using klass = CreateRequest<I>;
   librados::AioCompletion *comp =
-    create_rados_callback<klass, &klass::handle_create_id_object>(this);
-  int r = m_ioctx.aio_operate(m_id_obj, comp, &op);
+    create_rados_callback<klass, &klass::handle_add_image_to_directory>(this);
+  int r = m_io_ctx.aio_operate(RBD_DIRECTORY, comp, &op);
   assert(r == 0);
   comp->release();
 }
 
 template<typename I>
-void CreateRequest<I>::handle_create_id_object(int r) {
+void CreateRequest<I>::handle_add_image_to_directory(int r) {
   ldout(m_cct, 20) << "r=" << r << dendl;
 
-  if (r < 0) {
-    lderr(m_cct) << "error creating RBD id object: " << cpp_strerror(r)
+  if (r == -EEXIST) {
+    ldout(m_cct, 5) << "directory entry for image " << m_image_name
+                    << " already exists" << dendl;
+    complete(r);
+    return;
+  } else if (!m_io_ctx.get_namespace().empty() && r == -ENOENT) {
+    ldout(m_cct, 5) << "namespace " << m_io_ctx.get_namespace()
+                    << " does not exist" << dendl;
+    complete(r);
+    return;
+  } else if (r < 0) {
+    lderr(m_cct) << "error adding image to directory: " << cpp_strerror(r)
                  << dendl;
     complete(r);
     return;
   }
 
-  add_image_to_directory();
+  create_id_object();
 }
 
 template<typename I>
-void CreateRequest<I>::add_image_to_directory() {
+void CreateRequest<I>::create_id_object() {
   ldout(m_cct, 20) << dendl;
 
   librados::ObjectWriteOperation op;
-  cls_client::dir_add_image(&op, m_image_name, m_image_id);
+  op.create(true);
+  cls_client::set_id(&op, m_image_id);
 
   using klass = CreateRequest<I>;
   librados::AioCompletion *comp =
-    create_rados_callback<klass, &klass::handle_add_image_to_directory>(this);
-  int r = m_ioctx.aio_operate(RBD_DIRECTORY, comp, &op);
+    create_rados_callback<klass, &klass::handle_create_id_object>(this);
+  int r = m_io_ctx.aio_operate(m_id_obj, comp, &op);
   assert(r == 0);
   comp->release();
 }
 
 template<typename I>
-void CreateRequest<I>::handle_add_image_to_directory(int r) {
+void CreateRequest<I>::handle_create_id_object(int r) {
   ldout(m_cct, 20) << "r=" << r << dendl;
 
-  if (r < 0) {
-    lderr(m_cct) << "error adding image to directory: " << cpp_strerror(r)
+  if (r == -EEXIST) {
+    ldout(m_cct, 5) << "id object for  " << m_image_name << " already exists"
+                    << dendl;
+    m_r_saved = r;
+    remove_from_dir();
+    return;
+  } else if (r < 0) {
+    lderr(m_cct) << "error creating RBD id object: " << cpp_strerror(r)
                  << dendl;
-
     m_r_saved = r;
-    remove_id_object();
+    remove_from_dir();
+    return;
   }
 
   negotiate_features();
@@ -486,7 +507,7 @@ void CreateRequest<I>::negotiate_features() {
     create_rados_callback<klass, &klass::handle_negotiate_features>(this);
 
   m_outbl.clear();
-  int r = m_ioctx.aio_operate(RBD_DIRECTORY, comp, &op, &m_outbl);
+  int r = m_io_ctx.aio_operate(RBD_DIRECTORY, comp, &op, &m_outbl);
   assert(r == 0);
   comp->release();
 }
@@ -520,12 +541,13 @@ void CreateRequest<I>::create_image() {
   ostringstream oss;
   oss << RBD_DATA_PREFIX;
   if (m_data_pool_id != -1) {
-    oss << stringify(m_ioctx.get_id()) << ".";
+    oss << stringify(m_io_ctx.get_id()) << ".";
   }
   oss << m_image_id;
   if (oss.str().length() > RBD_MAX_BLOCK_NAME_PREFIX_LENGTH) {
     lderr(m_cct) << "object prefix '" << oss.str() << "' too large" << dendl;
-    complete(-EINVAL);
+    m_r_saved = -EINVAL;
+    remove_id_object();
     return;
   }
 
@@ -537,7 +559,7 @@ void CreateRequest<I>::create_image() {
   using klass = CreateRequest<I>;
   librados::AioCompletion *comp =
     create_rados_callback<klass, &klass::handle_create_image>(this);
-  int r = m_ioctx.aio_operate(m_header_obj, comp, &op);
+  int r = m_io_ctx.aio_operate(m_header_obj, comp, &op);
   assert(r == 0);
   comp->release();
 }
@@ -549,7 +571,7 @@ void CreateRequest<I>::handle_create_image(int r) {
   if (r < 0) {
     lderr(m_cct) << "error writing header: " << cpp_strerror(r) << dendl;
     m_r_saved = r;
-    remove_from_dir();
+    remove_id_object();
     return;
   }
 
@@ -572,7 +594,7 @@ void CreateRequest<I>::set_stripe_unit_count() {
   using klass = CreateRequest<I>;
   librados::AioCompletion *comp =
     create_rados_callback<klass, &klass::handle_set_stripe_unit_count>(this);
-  int r = m_ioctx.aio_operate(m_header_obj, comp, &op);
+  int r = m_io_ctx.aio_operate(m_header_obj, comp, &op);
   assert(r == 0);
   comp->release();
 }
@@ -608,7 +630,7 @@ void CreateRequest<I>::object_map_resize() {
   using klass = CreateRequest<I>;
   librados::AioCompletion *comp =
     create_rados_callback<klass, &klass::handle_object_map_resize>(this);
-  int r = m_ioctx.aio_operate(m_objmap_name, comp, &op);
+  int r = m_io_ctx.aio_operate(m_objmap_name, comp, &op);
   assert(r == 0);
   comp->release();
 }
@@ -645,7 +667,7 @@ void CreateRequest<I>::fetch_mirror_mode() {
   librados::AioCompletion *comp =
     create_rados_callback<klass, &klass::handle_fetch_mirror_mode>(this);
   m_outbl.clear();
-  int r = m_ioctx.aio_operate(RBD_MIRRORING, comp, &op, &m_outbl);
+  int r = m_io_ctx.aio_operate(RBD_MIRRORING, comp, &op, &m_outbl);
   assert(r == 0);
   comp->release();
 }
@@ -708,7 +730,7 @@ void CreateRequest<I>::journal_create() {
 
   librbd::journal::CreateRequest<I> *req =
     librbd::journal::CreateRequest<I>::create(
-      m_ioctx, m_image_id, m_journal_order, m_journal_splay_width,
+      m_io_ctx, m_image_id, m_journal_order, m_journal_splay_width,
       m_journal_pool, cls::journal::Tag::TAG_CLASS_NEW, tag_data,
       librbd::Journal<I>::IMAGE_CLIENT_ID, m_op_work_queue, ctx);
   req->send();
@@ -741,7 +763,7 @@ void CreateRequest<I>::mirror_image_enable() {
   ldout(m_cct, 20) << dendl;
   auto ctx = create_context_callback<
     CreateRequest<I>, &CreateRequest<I>::handle_mirror_image_enable>(this);
-  auto req = mirror::EnableRequest<I>::create(m_ioctx, m_image_id,
+  auto req = mirror::EnableRequest<I>::create(m_io_ctx, m_image_id,
                                               m_non_primary_global_image_id,
                                               m_op_work_queue, ctx);
   req->send();
@@ -792,7 +814,7 @@ void CreateRequest<I>::journal_remove() {
 
   librbd::journal::RemoveRequest<I> *req =
     librbd::journal::RemoveRequest<I>::create(
-      m_ioctx, m_image_id, librbd::Journal<I>::IMAGE_CLIENT_ID, m_op_work_queue,
+      m_io_ctx, m_image_id, librbd::Journal<I>::IMAGE_CLIENT_ID, m_op_work_queue,
       ctx);
   req->send();
 }
@@ -821,7 +843,7 @@ void CreateRequest<I>::remove_object_map() {
   using klass = CreateRequest<I>;
   librados::AioCompletion *comp =
     create_rados_callback<klass, &klass::handle_remove_object_map>(this);
-  int r = m_ioctx.aio_remove(m_objmap_name, comp);
+  int r = m_io_ctx.aio_remove(m_objmap_name, comp);
   assert(r == 0);
   comp->release();
 }
@@ -845,7 +867,7 @@ void CreateRequest<I>::remove_header_object() {
   using klass = CreateRequest<I>;
   librados::AioCompletion *comp =
     create_rados_callback<klass, &klass::handle_remove_header_object>(this);
-  int r = m_ioctx.aio_remove(m_header_obj, comp);
+  int r = m_io_ctx.aio_remove(m_header_obj, comp);
   assert(r == 0);
   comp->release();
 }
@@ -859,55 +881,55 @@ void CreateRequest<I>::handle_remove_header_object(int r) {
                  << cpp_strerror(r) << dendl;
   }
 
-  remove_from_dir();
+  remove_id_object();
 }
 
 template<typename I>
-void CreateRequest<I>::remove_from_dir() {
+void CreateRequest<I>::remove_id_object() {
   ldout(m_cct, 20) << dendl;
 
-  librados::ObjectWriteOperation op;
-  cls_client::dir_remove_image(&op, m_image_name, m_image_id);
-
   using klass = CreateRequest<I>;
   librados::AioCompletion *comp =
-    create_rados_callback<klass, &klass::handle_remove_from_dir>(this);
-  int r = m_ioctx.aio_operate(RBD_DIRECTORY, comp, &op);
+    create_rados_callback<klass, &klass::handle_remove_id_object>(this);
+  int r = m_io_ctx.aio_remove(m_id_obj, comp);
   assert(r == 0);
   comp->release();
 }
 
 template<typename I>
-void CreateRequest<I>::handle_remove_from_dir(int r) {
+void CreateRequest<I>::handle_remove_id_object(int r) {
   ldout(m_cct, 20) << "r=" << r << dendl;
 
   if (r < 0) {
-    lderr(m_cct) << "error cleaning up image from rbd_directory object "
-                 << "after creation failed: " << cpp_strerror(r) << dendl;
+    lderr(m_cct) << "error cleaning up id object after creation failed: "
+                 << cpp_strerror(r) << dendl;
   }
 
-  remove_id_object();
+  remove_from_dir();
 }
 
 template<typename I>
-void CreateRequest<I>::remove_id_object() {
+void CreateRequest<I>::remove_from_dir() {
   ldout(m_cct, 20) << dendl;
 
+  librados::ObjectWriteOperation op;
+  cls_client::dir_remove_image(&op, m_image_name, m_image_id);
+
   using klass = CreateRequest<I>;
   librados::AioCompletion *comp =
-    create_rados_callback<klass, &klass::handle_remove_id_object>(this);
-  int r = m_ioctx.aio_remove(m_id_obj, comp);
+    create_rados_callback<klass, &klass::handle_remove_from_dir>(this);
+  int r = m_io_ctx.aio_operate(RBD_DIRECTORY, comp, &op);
   assert(r == 0);
   comp->release();
 }
 
 template<typename I>
-void CreateRequest<I>::handle_remove_id_object(int r) {
+void CreateRequest<I>::handle_remove_from_dir(int r) {
   ldout(m_cct, 20) << "r=" << r << dendl;
 
   if (r < 0) {
-    lderr(m_cct) << "error cleaning up id object after creation failed: "
-                 << cpp_strerror(r) << dendl;
+    lderr(m_cct) << "error cleaning up image from rbd_directory object "
+                 << "after creation failed: " << cpp_strerror(r) << dendl;
   }
 
   complete(m_r_saved);

diff --git a/src/librbd/image/CreateRequest.h b/src/librbd/image/CreateRequest.h
index 9464a5cb0a038c8235308feefed1c07d7a2bd89b..00a0818d406528c82e25484866f97cb08bd10487 100644 (file)
--- a/src/librbd/image/CreateRequest.h
+++ b/src/librbd/image/CreateRequest.h
@@ -60,18 +60,18 @@ private:
    *                             VALIDATE OVERWRITE             .
    *                                     |                      .
    *                                     v                      .
-   * (error: bottom up)           CREATE ID OBJECT. . < . . . . .
+   * (error: bottom up)         ADD IMAGE TO DIRECTORY  < . . . .
    *  _______<_______                    |
    * |               |                   v
-   * |               |          ADD IMAGE TO DIRECTORY
+   * |               |            CREATE ID OBJECT
    * |               |               /   |
-   * |      REMOVE ID OBJECT<-------/    v
+   * |      REMOVE FROM DIR <-------/    v
    * |               |           NEGOTIATE FEATURES (when using default features)
    * |               |                   |
    * |               |                   v         (stripingv2 disabled)
    * |               |              CREATE IMAGE. . . . > . . . .
    * v               |               /   |                      .
-   * |      REMOVE FROM DIR<--------/    v                      .
+   * |      REMOVE ID OBJ <---------/    v                      .
    * |               |          SET STRIPE UNIT COUNT           .
    * |               |               /   |  \ . . . . . > . . . .
    * |      REMOVE HEADER OBJ<------/    v                     /. (object-map
@@ -101,7 +101,7 @@ private:
                 bool skip_mirror_enable,
                 ContextWQ *op_work_queue, Context *on_finish);
 
-  IoCtx &m_ioctx;
+  IoCtx m_io_ctx;
   IoCtx m_data_io_ctx;
   std::string m_image_name;
   std::string m_image_id;
@@ -139,12 +139,12 @@ private:
   void validate_overwrite();
   void handle_validate_overwrite(int r);
 
-  void create_id_object();
-  void handle_create_id_object(int r);
-
   void add_image_to_directory();
   void handle_add_image_to_directory(int r);
 
+  void create_id_object();
+  void handle_create_id_object(int r);
+
   void negotiate_features();
   void handle_negotiate_features(int r);
 
@@ -178,11 +178,12 @@ private:
   void remove_header_object();
   void handle_remove_header_object(int r);
 
+  void remove_id_object();
+  void handle_remove_id_object(int r);
+
   void remove_from_dir();
   void handle_remove_from_dir(int r);
 
-  void remove_id_object();
-  void handle_remove_id_object(int r);
 };
 
 } //namespace image

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index a62b379b5e85ad3dcc54d352b1feabcf30a257f9..cea83d9c29a96dcf06c93f4093c13acdc534cf1e 100644 (file)
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -746,6 +746,11 @@ bool compare_by_name(const child_info_t& c1, const child_info_t& c2)
       return r;
     }
 
+    if (!io_ctx.get_namespace().empty()) {
+      lderr(cct) << "attempting to add v1 image to namespace" << dendl;
+      return -EINVAL;
+    }
+
     ldout(cct, 2) << "adding rbd image to directory..." << dendl;
     r = tmap_set(io_ctx, imgname);
     if (r < 0) {

diff --git a/src/test/librbd/test_librbd.cc b/src/test/librbd/test_librbd.cc
index 10666fcc09923964d59e82b83cf0fb0a4c8d9bbe..df671728eaf3a4443b4f2cfbb6c78d33204b05e1 100644 (file)
--- a/src/test/librbd/test_librbd.cc
+++ b/src/test/librbd/test_librbd.cc
@@ -6667,11 +6667,54 @@ TEST_F(TestLibRBD, NamespacesPP) {
 
   std::vector<std::string> names;
   ASSERT_EQ(0, rbd.namespace_list(ioctx, &names));
-  ASSERT_EQ(2U, names.size());
-
   ASSERT_EQ(2U, names.size());
   ASSERT_EQ("name1", names[0]);
   ASSERT_EQ("name3", names[1]);
+
+  librados::IoCtx ns_io_ctx;
+  ns_io_ctx.dup(ioctx);
+
+  std::string name = get_temp_image_name();
+  int order = 0;
+  uint64_t features = 0;
+  if (!get_features(&features)) {
+    // old format doesn't support namespaces
+    ns_io_ctx.set_namespace("name1");
+    ASSERT_EQ(-EINVAL, create_image_pp(rbd, ns_io_ctx, name.c_str(), 0,
+                                       &order));
+    return;
+  }
+
+  ns_io_ctx.set_namespace("missing");
+  ASSERT_EQ(-ENOENT, create_image_pp(rbd, ns_io_ctx, name.c_str(), 0, &order));
+
+  ns_io_ctx.set_namespace("name1");
+  ASSERT_EQ(0, create_image_pp(rbd, ns_io_ctx, name.c_str(), 0, &order));
+  ASSERT_EQ(-EBUSY, rbd.namespace_remove(ns_io_ctx, "name1"));
+
+  std::string image_id;
+  {
+    librbd::Image image;
+    ASSERT_EQ(-ENOENT, rbd.open(ioctx, image, name.c_str(), NULL));
+    ASSERT_EQ(0, rbd.open(ns_io_ctx, image, name.c_str(), NULL));
+    ASSERT_EQ(0, get_image_id(image, &image_id));
+  }
+
+  ASSERT_EQ(-ENOENT, rbd.trash_move(ioctx, name.c_str(), 0));
+  ASSERT_EQ(0, rbd.trash_move(ns_io_ctx, name.c_str(), 0));
+  ASSERT_EQ(-EBUSY, rbd.namespace_remove(ns_io_ctx, "name1"));
+
+  PrintProgress pp;
+  ASSERT_EQ(-ENOENT, rbd.trash_remove_with_progress(ioctx, image_id.c_str(),
+                                                    false, pp));
+  ASSERT_EQ(0, rbd.trash_remove_with_progress(ns_io_ctx, image_id.c_str(),
+                                              false, pp));
+  ASSERT_EQ(0, rbd.namespace_remove(ns_io_ctx, "name1"));
+
+  names.clear();
+  ASSERT_EQ(0, rbd.namespace_list(ioctx, &names));
+  ASSERT_EQ(1U, names.size());
+  ASSERT_EQ("name3", names[0]);
 }
 
 // poorman's assert()