]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
rbd/io: split IO check
authorlixiaoy1 <xiaoyan.li@intel.com>
Mon, 21 Sep 2020 15:03:09 +0000 (11:03 -0400)
committerlixiaoy1 <xiaoyan.li@intel.com>
Wed, 30 Sep 2020 07:56:58 +0000 (03:56 -0400)
Move IO check to interfaces in Utils.

Signed-off-by: Li, Xiaoyan <xiaoyan.li@intel.com>
src/librbd/internal.cc
src/librbd/io/ImageDispatcher.cc
src/librbd/io/ImageDispatcher.h
src/librbd/io/ImageRequest.cc
src/librbd/io/ImageRequest.h
src/librbd/io/Utils.cc
src/librbd/io/Utils.h
src/test/librbd/io/test_mock_ImageRequest.cc
src/test/librbd/test_librbd.cc

index d8bd8b7147e67f82b3845a6758f9395117416e30..d895d1f35e6d66c571d606da9b6be9c911d3788c 100644 (file)
@@ -1571,11 +1571,17 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
   int clip_io(ImageCtx *ictx, uint64_t off, uint64_t *len)
   {
     ceph_assert(ceph_mutex_is_locked(ictx->image_lock));
-    uint64_t image_size = ictx->get_image_size(ictx->snap_id);
-    bool snap_exists = ictx->snap_exists;
 
-    if (!snap_exists)
-      return -ENOENT;
+    uint64_t image_size;
+    if (ictx->snap_id == CEPH_NOSNAP) {
+      image_size = ictx->get_image_size(CEPH_NOSNAP);
+    } else {
+      auto snap_info = ictx->get_snap_info(ictx->snap_id);
+      if (snap_info == nullptr) {
+       return -ENOENT;
+      }
+      image_size = snap_info->size;
+    }
 
     // special-case "len == 0" requests: always valid
     if (*len == 0)
index 6c2863bccbb67653fa058d9688f4ff0ddacc2745..49f3bfc5ad04cfcbd85bd0f988a036fc971d95da 100644 (file)
@@ -14,6 +14,7 @@
 #include "librbd/io/QueueImageDispatch.h"
 #include "librbd/io/QosImageDispatch.h"
 #include "librbd/io/RefreshImageDispatch.h"
+#include "librbd/io/Utils.h"
 #include "librbd/io/WriteBlockImageDispatch.h"
 #include <boost/variant.hpp>
 
@@ -122,6 +123,59 @@ struct ImageDispatcher<I>::SendVisitor : public boost::static_visitor<bool> {
   }
 };
 
+template <typename I>
+struct ImageDispatcher<I>::PreprocessVisitor
+  : public boost::static_visitor<bool> {
+  ImageDispatcher<I>* image_dispatcher;
+  ImageDispatchSpec* image_dispatch_spec;
+
+  PreprocessVisitor(ImageDispatcher<I>* image_dispatcher,
+                    ImageDispatchSpec* image_dispatch_spec)
+    : image_dispatcher(image_dispatcher),
+      image_dispatch_spec(image_dispatch_spec) {
+  }
+
+  bool clip_request() const {
+    int r = util::clip_request(image_dispatcher->m_image_ctx,
+                               &image_dispatch_spec->image_extents);
+    if (r < 0) {
+      image_dispatch_spec->fail(r);
+      return true;
+    }
+    return false;
+  }
+
+  bool operator()(ImageDispatchSpec::Read& read) const {
+    if ((read.read_flags & READ_FLAG_DISABLE_CLIPPING) != 0) {
+      return false;
+    }
+    return clip_request();
+  }
+
+  bool operator()(ImageDispatchSpec::Flush&) const {
+    return clip_request();
+  }
+
+  bool operator()(ImageDispatchSpec::ListSnaps&) const {
+    return false;
+  }
+
+  template <typename T>
+  bool operator()(T&) const {
+    if (clip_request()) {
+      return true;
+    }
+
+    std::shared_lock image_locker{image_dispatcher->m_image_ctx->image_lock};
+    if (image_dispatcher->m_image_ctx->snap_id != CEPH_NOSNAP ||
+        image_dispatcher->m_image_ctx->read_only) {
+      image_dispatch_spec->fail(-EROFS);
+      return true;
+    }
+    return false;
+  }
+};
+
 template <typename I>
 ImageDispatcher<I>::ImageDispatcher(I* image_ctx)
   : Dispatcher<I, ImageDispatcherInterface>(image_ctx) {
@@ -203,6 +257,11 @@ bool ImageDispatcher<I>::send_dispatch(
     ImageDispatchSpec* image_dispatch_spec) {
   if (image_dispatch_spec->tid == 0) {
     image_dispatch_spec->tid = ++m_next_tid;
+
+    bool finished = preprocess(image_dispatch_spec);
+    if (finished) {
+      return true;
+    }
   }
 
   return boost::apply_visitor(
@@ -210,6 +269,14 @@ bool ImageDispatcher<I>::send_dispatch(
     image_dispatch_spec->request);
 }
 
+template <typename I>
+bool ImageDispatcher<I>::preprocess(
+    ImageDispatchSpec* image_dispatch_spec) {
+  return boost::apply_visitor(
+    PreprocessVisitor{this, image_dispatch_spec},
+    image_dispatch_spec->request);
+}
+
 } // namespace io
 } // namespace librbd
 
index 176b2a229fe9cab25b1feffb99dea384f9003e27..7ab248abdf7c4dc72a9e5873e7b9494fb460715e 100644 (file)
@@ -50,12 +50,15 @@ protected:
 
 private:
   struct SendVisitor;
+  struct PreprocessVisitor;
 
   std::atomic<uint64_t> m_next_tid{0};
 
   QosImageDispatch<ImageCtxT>* m_qos_image_dispatch = nullptr;
   WriteBlockImageDispatch<ImageCtxT>* m_write_block_dispatch = nullptr;
 
+  bool preprocess(ImageDispatchSpec* image_dispatch_spec);
+
 };
 
 } // namespace io
index 09a37c50bf4d1f4e0a4c674a8153e39b9933a5bb..fc88357e20634172b7c753cc368069f71cc1dbb7 100644 (file)
@@ -313,16 +313,6 @@ void ImageRequest<I>::send() {
   ldout(cct, 20) << get_request_type() << ": ictx=" << &image_ctx << ", "
                  << "completion=" << aio_comp << dendl;
 
-  int r = clip_request();
-  if (r < 0) {
-    m_aio_comp->fail(r);
-    return;
-  }
-
-  if (finish_request_early()) {
-    return;
-  }
-
   if (m_bypass_image_cache || m_image_ctx.image_cache == nullptr) {
     update_timestamp();
     send_request();
@@ -331,41 +321,6 @@ void ImageRequest<I>::send() {
   }
 }
 
-template <typename I>
-int ImageRequest<I>::clip_request() {
-  std::shared_lock image_locker{m_image_ctx.image_lock};
-  for (auto &image_extent : m_image_extents) {
-    auto clip_len = image_extent.second;
-    int r = clip_io(get_image_ctx(&m_image_ctx), image_extent.first, &clip_len);
-    if (r < 0) {
-      return r;
-    }
-
-    image_extent.second = clip_len;
-  }
-  return 0;
-}
-
-template <typename I>
-bool ImageRequest<I>::finish_request_early() {
-  auto total_bytes = get_total_length();
-  if (total_bytes == 0) {
-    auto *aio_comp = this->m_aio_comp;
-    aio_comp->set_request_count(0);
-    return true;
-  }
-  return false;
-}
-
-template <typename I>
-uint64_t ImageRequest<I>::get_total_length() const {
-  uint64_t total_bytes = 0;
-  for (auto& image_extent : this->m_image_extents) {
-    total_bytes += image_extent.second;
-  }
-  return total_bytes;
-}
-
 template <typename I>
 void ImageRequest<I>::update_timestamp() {
   bool modify = (get_aio_type() != AIO_TYPE_READ);
@@ -431,20 +386,6 @@ ImageReadRequest<I>::ImageReadRequest(I &image_ctx, AioCompletion *aio_comp,
   aio_comp->read_result = std::move(read_result);
 }
 
-template <typename I>
-int ImageReadRequest<I>::clip_request() {
-  if ((m_read_flags & READ_FLAG_DISABLE_CLIPPING) != 0) {
-    return 0;
-  }
-
-  int r = ImageRequest<I>::clip_request();
-  if (r < 0) {
-    return r;
-  }
-
-  return 0;
-}
-
 template <typename I>
 void ImageReadRequest<I>::send_request() {
   I &image_ctx = this->m_image_ctx;
@@ -508,21 +449,6 @@ void ImageReadRequest<I>::send_image_cache_request() {
                                   req_comp);
 }
 
-template <typename I>
-bool AbstractImageWriteRequest<I>::finish_request_early() {
-  AioCompletion *aio_comp = this->m_aio_comp;
-  {
-    std::shared_lock image_locker{this->m_image_ctx.image_lock};
-    if (this->m_image_ctx.snap_id != CEPH_NOSNAP ||
-        this->m_image_ctx.read_only) {
-      aio_comp->fail(-EROFS);
-      return true;
-    }
-  }
-
-  return ImageRequest<I>::finish_request_early();
-}
-
 template <typename I>
 void AbstractImageWriteRequest<I>::send_request() {
   I &image_ctx = this->m_image_ctx;
@@ -988,13 +914,6 @@ ImageListSnapsRequest<I>::ImageListSnapsRequest(
                     parent_trace),
     m_snap_ids(std::move(snap_ids)), m_list_snaps_flags(list_snaps_flags),
     m_snapshot_delta(snapshot_delta) {
-  this->set_bypass_image_cache();
-}
-
-template <typename I>
-int ImageListSnapsRequest<I>::clip_request() {
-  // permit arbitrary list-snaps requests (internal API)
-  return 0;
 }
 
 template <typename I>
@@ -1042,11 +961,6 @@ void ImageListSnapsRequest<I>::send_request() {
   }
 }
 
-template <typename I>
-void ImageListSnapsRequest<I>::send_image_cache_request() {
-  ceph_abort();
-}
-
 } // namespace io
 } // namespace librbd
 
index dabda6b6d85a415df628c7ea6fd2c06aa65056a2..49787874c50165c3f4ba6b7ada8fc88e2d7ef382 100644 (file)
@@ -91,11 +91,6 @@ protected:
     m_trace.event("start");
   }
 
-  uint64_t get_total_length() const;
-
-  virtual int clip_request();
-  virtual bool finish_request_early();
-
   virtual void update_timestamp();
   virtual void send_request() = 0;
   virtual void send_image_cache_request() = 0;
@@ -115,8 +110,6 @@ public:
                    const ZTracer::Trace &parent_trace);
 
 protected:
-  int clip_request() override;
-
   void send_request() override;
   void send_image_cache_request() override;
 
@@ -151,8 +144,6 @@ protected:
       m_synchronous(false) {
   }
 
-  bool finish_request_early() override;
-
   void send_request() override;
 
   virtual int prune_object_extents(
@@ -269,13 +260,6 @@ public:
 protected:
   using typename ImageRequest<ImageCtxT>::ObjectRequests;
 
-  int clip_request() override {
-    return 0;
-  }
-  bool finish_request_early() override {
-    return false;
-  }
-
   void update_timestamp() override {
   }
   void send_request() override;
@@ -387,11 +371,8 @@ public:
       SnapshotDelta* snapshot_delta, const ZTracer::Trace& parent_trace);
 
 protected:
-  int clip_request() override;
-
   void update_timestamp() override {}
   void send_request() override;
-  void send_image_cache_request() override;
 
   aio_type_t get_aio_type() const override {
     return AIO_TYPE_GENERIC;
index 797006471ec3f710c2b2281fa6e38ca9564b5470..ffa5de47b93116b397763ffdca1f2d0170b095a3 100644 (file)
@@ -6,6 +6,8 @@
 #include "include/buffer.h"
 #include "include/rados/librados.hpp"
 #include "include/neorados/RADOS.hpp"
+#include "librbd/internal.h"
+#include "librbd/Utils.h"
 #include "librbd/io/AioCompletion.h"
 #include "librbd/io/ImageRequest.h"
 #include "osd/osd_types.h"
@@ -122,6 +124,30 @@ void read_parent(I *image_ctx, uint64_t object_no, const Extents &extents,
     image_ctx->parent->get_data_io_context(), 0, 0, trace);
 }
 
+template <typename I>
+int clip_request(I *image_ctx, Extents *image_extents) {
+  std::shared_lock image_locker{image_ctx->image_lock};
+  for (auto &image_extent : *image_extents) {
+    auto clip_len = image_extent.second;
+    int r = clip_io(librbd::util::get_image_ctx(image_ctx),
+                    image_extent.first, &clip_len);
+    if (r < 0) {
+      return r;
+    }
+
+    image_extent.second = clip_len;
+  }
+  return 0;
+}
+
+uint64_t extents_length(Extents &extents) {
+  uint64_t total_bytes = 0;
+  for (auto& image_extent : extents) {
+    total_bytes += image_extent.second;
+  }
+  return total_bytes;
+}
+
 } // namespace util
 } // namespace io
 } // namespace librbd
@@ -130,3 +156,5 @@ template void librbd::io::util::read_parent(
     librbd::ImageCtx *image_ctx, uint64_t object_no, const Extents &extents,
     librados::snap_t snap_id, const ZTracer::Trace &trace,
     ceph::bufferlist* data, Context* on_finish);
+template int librbd::io::util::clip_request(
+    librbd::ImageCtx *image_ctx, Extents *image_extents);
index 81161adc8313b85f8bb24c04014971fdf35bf499..63e0affcd1b4267f2181bc1aeb46238e49d59308 100644 (file)
@@ -34,6 +34,11 @@ void read_parent(ImageCtxT *image_ctx, uint64_t object_no, const Extents &extent
                  librados::snap_t snap_id, const ZTracer::Trace &trace,
                  ceph::bufferlist* data, Context* on_finish);
 
+template <typename ImageCtxT = librbd::ImageCtx>
+int clip_request(ImageCtxT *image_ctx, Extents *image_extents);
+
+uint64_t extents_length(Extents &extents);
+
 } // namespace util
 } // namespace io
 } // namespace librbd
index 968207fb218202c7090a6802a11d6138c7c6e7be..e39409e68a801c058adddc172aa427a802e0342a 100644 (file)
@@ -196,40 +196,6 @@ TEST_F(TestMockIoImageRequest, AioWriteModifyTimestamp) {
   ASSERT_EQ(0, aio_comp_ctx_2.wait());
 }
 
-TEST_F(TestMockIoImageRequest, AioWriteEarlyFinish) {
-  librbd::ImageCtx *ictx;
-  ASSERT_EQ(0, open_image(m_image_name, &ictx));
-
-  MockTestImageCtx mock_image_ctx(*ictx);
-
-  C_SaferCond aio_comp_ctx_1, aio_comp_ctx_2;
-  AioCompletion *aio_comp_1 = AioCompletion::create_and_start(
-    &aio_comp_ctx_1, ictx, AIO_TYPE_WRITE);
-  AioCompletion *aio_comp_2 = AioCompletion::create_and_start(
-    &aio_comp_ctx_2, ictx, AIO_TYPE_WRITE);
-
-  bufferlist bl;
-  MockImageWriteRequest mock_aio_image_write_1(
-    mock_image_ctx, aio_comp_1, {{0, 0}}, std::move(bl),
-    mock_image_ctx.get_data_io_context(), 0, {});
-  {
-    std::shared_lock owner_locker{mock_image_ctx.owner_lock};
-    mock_aio_image_write_1.send();
-  }
-  ASSERT_EQ(0, aio_comp_ctx_1.wait());
-
-  mock_image_ctx.snap_id = 123;
-  bl.append("1");
-  MockImageWriteRequest mock_aio_image_write_2(
-    mock_image_ctx, aio_comp_2, {{0, 1}}, std::move(bl),
-    mock_image_ctx.get_data_io_context(), 0, {});
-  {
-    std::shared_lock owner_locker{mock_image_ctx.owner_lock};
-    mock_aio_image_write_2.send();
-  }
-  ASSERT_EQ(-EROFS, aio_comp_ctx_2.wait());
-}
-
 TEST_F(TestMockIoImageRequest, AioReadAccessTimestamp) {
   REQUIRE_FORMAT_V2();
 
index f62065099fad90a58d8091ed3c5f162849c90139..5c3e27fd67b4d3a17d5ad18e9d2c9f5dff319e6d 100644 (file)
@@ -2964,6 +2964,36 @@ TEST_F(TestLibRBD, TestIOToSnapshot)
   rados_ioctx_destroy(ioctx);
 }
 
+TEST_F(TestLibRBD, TestSnapshotDeletedIo)
+{
+  rados_ioctx_t ioctx;
+  rados_ioctx_create(_cluster, m_pool_name.c_str(), &ioctx);
+
+  rbd_image_t image;
+  int order = 0;
+  std::string name = get_temp_image_name();
+  uint64_t isize = 2 << 20;
+
+  int r;
+
+  ASSERT_EQ(0, create_image(ioctx, name.c_str(), isize, &order));
+  ASSERT_EQ(0, rbd_open(ioctx, name.c_str(), &image, NULL));
+  ASSERT_EQ(0, rbd_snap_create(image, "orig"));
+
+  r = rbd_snap_set(image, "orig");
+  ASSERT_EQ(r, 0);
+
+  ASSERT_EQ(0, rbd_snap_remove(image, "orig"));
+  char test[20];
+  ASSERT_EQ(-ENOENT, rbd_read(image, 20, 20, test));
+
+  r = rbd_snap_set(image, NULL);
+  ASSERT_EQ(r, 0);
+
+  ASSERT_EQ(0, rbd_close(image));
+  rados_ioctx_destroy(ioctx);
+}
+
 TEST_F(TestLibRBD, TestClone)
 {
   REQUIRE_FEATURE(RBD_FEATURE_LAYERING);