librbd: clone copy-on-write operations should preserve sparseness

Signed-off-by: Mykola Golub <mgolub@suse.com>

diff --git a/src/librbd/ImageCtx.h b/src/librbd/ImageCtx.h
index ea41382a0e5e91656b92a3ad86b79ecc9c87c588..85afc5aecbbf4692c29c9c9d474ba068f18aa9c1 100644 (file)
--- a/src/librbd/ImageCtx.h
+++ b/src/librbd/ImageCtx.h
@@ -175,6 +175,7 @@ namespace librbd {
 
     bool ignore_migrating = false;
     bool disable_zero_copy = false;
+    bool enable_sparse_copyup = false;
 
     /// Cached latency-sensitive configuration settings
     bool non_blocking_aio;

diff --git a/src/librbd/image/RefreshRequest.cc b/src/librbd/image/RefreshRequest.cc
index 1f0bef418828461fc3009355d706823cb17ddaab..ba9be9beb8f82a0ac2c6d35858b1b74266b0f42b 100644 (file)
--- a/src/librbd/image/RefreshRequest.cc
+++ b/src/librbd/image/RefreshRequest.cc
@@ -1320,6 +1320,13 @@ void RefreshRequest<I>::apply() {
       m_image_ctx.parent_md = m_parent_md;
       m_image_ctx.migration_info = {};
     }
+
+    librados::Rados rados(m_image_ctx.md_ctx);
+    int8_t require_osd_release;
+    int r = rados.get_min_compatible_osd(&require_osd_release);
+    if (r == 0 && require_osd_release >= CEPH_RELEASE_OCTOPUS) {
+      m_image_ctx.enable_sparse_copyup = true;
+    }
   }
 
   for (size_t i = 0; i < m_snapc.snaps.size(); ++i) {

diff --git a/src/librbd/io/CopyupRequest.cc b/src/librbd/io/CopyupRequest.cc
index 4128a40a9e438f7b05380fa80f8170434fd7d2fd..2a49dc7fd0b32aa9b60cea23dc33112feabdc82e 100644 (file)
--- a/src/librbd/io/CopyupRequest.cc
+++ b/src/librbd/io/CopyupRequest.cc
@@ -172,9 +172,15 @@ void CopyupRequest<I>::read_from_parent() {
                  << "completion=" << comp << ", "
                  << "extents=" << m_image_extents
                  << dendl;
-  ImageRequest<I>::aio_read(m_image_ctx->parent, comp,
-                            std::move(m_image_extents),
-                            ReadResult{&m_copyup_data}, 0, m_trace);
+  if (m_image_ctx->enable_sparse_copyup) {
+    ImageRequest<I>::aio_read(
+      m_image_ctx->parent, comp, std::move(m_image_extents),
+      ReadResult{&m_copyup_extent_map, &m_copyup_data}, 0, m_trace);
+  } else {
+    ImageRequest<I>::aio_read(
+      m_image_ctx->parent, comp, std::move(m_image_extents),
+      ReadResult{&m_copyup_data}, 0, m_trace);
+  }
 }
 
 template <typename I>
@@ -383,12 +389,17 @@ void CopyupRequest<I>::copyup() {
   bool deep_copyup = !snapc.snaps.empty() && !m_copyup_is_zero;
   if (m_copyup_is_zero) {
     m_copyup_data.clear();
+    m_copyup_extent_map.clear();
   }
 
   int r;
   librados::ObjectWriteOperation copyup_op;
   if (copy_on_read || deep_copyup) {
-    copyup_op.exec("rbd", "copyup", m_copyup_data);
+    if (m_image_ctx->enable_sparse_copyup) {
+      cls_client::sparse_copyup(&copyup_op, m_copyup_extent_map, m_copyup_data);
+    } else {
+      cls_client::copyup(&copyup_op, m_copyup_data);
+    }
     ObjectRequest<I>::add_write_hint(*m_image_ctx, &copyup_op);
     ++m_pending_copyups;
   }
@@ -396,7 +407,12 @@ void CopyupRequest<I>::copyup() {
   librados::ObjectWriteOperation write_op;
   if (!copy_on_read) {
     if (!deep_copyup) {
-      write_op.exec("rbd", "copyup", m_copyup_data);
+      if (m_image_ctx->enable_sparse_copyup) {
+        cls_client::sparse_copyup(&write_op, m_copyup_extent_map,
+                                  m_copyup_data);
+      } else {
+        cls_client::copyup(&write_op, m_copyup_data);
+      }
       ObjectRequest<I>::add_write_hint(*m_image_ctx, &write_op);
     }
 

diff --git a/src/librbd/io/CopyupRequest.h b/src/librbd/io/CopyupRequest.h
index e4b3a2e7ffceabf1b122099456c667d2081cdd9d..d98d477620836d55181467f265bb2d7fa9cbfe81 100644 (file)
--- a/src/librbd/io/CopyupRequest.h
+++ b/src/librbd/io/CopyupRequest.h
@@ -12,6 +12,7 @@
 #include "librbd/io/AsyncOperation.h"
 #include "librbd/io/Types.h"
 
+#include <map>
 #include <string>
 #include <vector>
 
@@ -88,6 +89,7 @@ private:
   bool m_copyup_required = true;
   bool m_copyup_is_zero = true;
 
+  std::map<uint64_t, uint64_t> m_copyup_extent_map;
   ceph::bufferlist m_copyup_data;
 
   AsyncOperation m_async_op;

diff --git a/src/test/librbd/io/test_mock_CopyupRequest.cc b/src/test/librbd/io/test_mock_CopyupRequest.cc
index e9034918ee7bdc287d9038eb9fca31707bc09818..10c8ee0cf4bd6169cb2b78d29f1310547993d6a0 100644 (file)
--- a/src/test/librbd/io/test_mock_CopyupRequest.cc
+++ b/src/test/librbd/io/test_mock_CopyupRequest.cc
@@ -221,6 +221,28 @@ struct TestMockIoCopyupRequest : public TestMockFixture {
       .WillOnce(Return(r));
   }
 
+  void expect_sparse_copyup(MockTestImageCtx &mock_image_ctx, uint64_t snap_id,
+                            const std::string &oid,
+                            const std::map<uint64_t, uint64_t> &extent_map,
+                            const std::string &data, int r) {
+    bufferlist data_bl;
+    data_bl.append(data);
+
+    bufferlist in_bl;
+    encode(extent_map, in_bl);
+    encode(data_bl, in_bl);
+
+    SnapContext snapc;
+    if (snap_id == CEPH_NOSNAP) {
+      snapc = mock_image_ctx.snapc;
+    }
+
+    EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.data_ctx),
+                exec(oid, _, StrEq("rbd"), StrEq("sparse_copyup"),
+                     ContentsEqual(in_bl), _, snapc))
+      .WillOnce(Return(r));
+  }
+
   void expect_write(MockTestImageCtx& mock_image_ctx, uint64_t snap_id,
                     const std::string& oid, int r) {
     SnapContext snapc;
@@ -351,7 +373,8 @@ TEST_F(TestMockIoCopyupRequest, Standard) {
                            0);
 
   expect_add_copyup_ops(mock_write_request);
-  expect_copyup(mock_image_ctx, CEPH_NOSNAP, "oid", data, 0);
+  expect_sparse_copyup(mock_image_ctx, CEPH_NOSNAP, "oid", {{0, 4096}}, data,
+                       0);
   expect_write(mock_image_ctx, CEPH_NOSNAP, "oid", 0);
 
   auto req = new MockCopyupRequest(&mock_image_ctx, "oid", 0,
@@ -408,7 +431,7 @@ TEST_F(TestMockIoCopyupRequest, StandardWithSnaps) {
                            0);
 
   expect_add_copyup_ops(mock_write_request);
-  expect_copyup(mock_image_ctx, 0, "oid", data, 0);
+  expect_sparse_copyup(mock_image_ctx, 0, "oid", {{0, 4096}}, data, 0);
   expect_write(mock_image_ctx, CEPH_NOSNAP, "oid", 0);
 
   auto req = new MockCopyupRequest(&mock_image_ctx, "oid", 0,
@@ -449,7 +472,8 @@ TEST_F(TestMockIoCopyupRequest, CopyOnRead) {
   expect_object_map_update(mock_image_ctx, CEPH_NOSNAP, 0, OBJECT_EXISTS, true,
                            0);
 
-  expect_copyup(mock_image_ctx, CEPH_NOSNAP, "oid", data, 0);
+  expect_sparse_copyup(mock_image_ctx, CEPH_NOSNAP, "oid", {{0, 4096}}, data,
+                       0);
 
   auto req = new MockCopyupRequest(&mock_image_ctx, "oid", 0,
                                    {{0, 4096}}, {});
@@ -495,7 +519,7 @@ TEST_F(TestMockIoCopyupRequest, CopyOnReadWithSnaps) {
   expect_object_map_update(mock_image_ctx, CEPH_NOSNAP, 0, OBJECT_EXISTS_CLEAN,
                            true, 0);
 
-  expect_copyup(mock_image_ctx, 0, "oid", data, 0);
+  expect_sparse_copyup(mock_image_ctx, 0, "oid", {{0, 4096}}, data, 0);
 
   auto req = new MockCopyupRequest(&mock_image_ctx, "oid", 0,
                                    {{0, 4096}}, {});
@@ -538,7 +562,7 @@ TEST_F(TestMockIoCopyupRequest, DeepCopy) {
                            0);
 
   expect_add_copyup_ops(mock_write_request);
-  expect_copyup(mock_image_ctx, CEPH_NOSNAP, "oid", "", 0);
+  expect_sparse_copyup(mock_image_ctx, CEPH_NOSNAP, "oid", {}, "", 0);
   expect_write(mock_image_ctx, CEPH_NOSNAP, "oid", 0);
 
   auto req = new MockCopyupRequest(&mock_image_ctx, "oid", 0,
@@ -579,7 +603,7 @@ TEST_F(TestMockIoCopyupRequest, DeepCopyOnRead) {
   expect_object_map_update(mock_image_ctx, CEPH_NOSNAP, 0, OBJECT_EXISTS, true,
                            0);
 
-  expect_copyup(mock_image_ctx, CEPH_NOSNAP, "oid", "", 0);
+  expect_sparse_copyup(mock_image_ctx, CEPH_NOSNAP, "oid", {}, "", 0);
 
   auto req = new MockCopyupRequest(&mock_image_ctx, "oid", 0,
                                    {{0, 4096}}, {});
@@ -644,7 +668,7 @@ TEST_F(TestMockIoCopyupRequest, DeepCopyWithPostSnaps) {
                            0);
 
   expect_add_copyup_ops(mock_write_request);
-  expect_copyup(mock_image_ctx, CEPH_NOSNAP, "oid", "", 0);
+  expect_sparse_copyup(mock_image_ctx, CEPH_NOSNAP, "oid", {}, "", 0);
   expect_write(mock_image_ctx, CEPH_NOSNAP, "oid", 0);
 
   auto req = new MockCopyupRequest(&mock_image_ctx, "oid", 0,
@@ -715,7 +739,7 @@ TEST_F(TestMockIoCopyupRequest, DeepCopyWithPreAndPostSnaps) {
                            0);
 
   expect_add_copyup_ops(mock_write_request);
-  expect_copyup(mock_image_ctx, CEPH_NOSNAP, "oid", "", 0);
+  expect_sparse_copyup(mock_image_ctx, CEPH_NOSNAP, "oid", {}, "", 0);
   expect_write(mock_image_ctx, CEPH_NOSNAP, "oid", 0);
 
   auto req = new MockCopyupRequest(&mock_image_ctx, "oid", 0,
@@ -755,7 +779,7 @@ TEST_F(TestMockIoCopyupRequest, ZeroedCopyup) {
                            0);
 
   expect_add_copyup_ops(mock_write_request);
-  expect_copyup(mock_image_ctx, CEPH_NOSNAP, "oid", "", 0);
+  expect_sparse_copyup(mock_image_ctx, CEPH_NOSNAP, "oid", {}, "", 0);
   expect_write(mock_image_ctx, CEPH_NOSNAP, "oid", 0);
 
   auto req = new MockCopyupRequest(&mock_image_ctx, "oid", 0,
@@ -796,7 +820,7 @@ TEST_F(TestMockIoCopyupRequest, ZeroedCopyOnRead) {
   expect_object_map_update(mock_image_ctx, CEPH_NOSNAP, 0, OBJECT_EXISTS, true,
                            0);
 
-  expect_copyup(mock_image_ctx, CEPH_NOSNAP, "oid", "", 0);
+  expect_sparse_copyup(mock_image_ctx, CEPH_NOSNAP, "oid", {}, "", 0);
 
   auto req = new MockCopyupRequest(&mock_image_ctx, "oid", 0,
                                    {{0, 4096}}, {});
@@ -876,7 +900,8 @@ TEST_F(TestMockIoCopyupRequest, RestartWrite) {
   auto req = new MockCopyupRequest(&mock_image_ctx, "oid", 0,
                                    {{0, 4096}}, {});
   expect_add_copyup_ops(mock_write_request1);
-  expect_copyup(mock_image_ctx, CEPH_NOSNAP, "oid", data, 0);
+  expect_sparse_copyup(mock_image_ctx, CEPH_NOSNAP, "oid", {{0, 4096}}, data,
+                       0);
 
   MockAbstractObjectWriteRequest mock_write_request2;
   EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.data_ctx),
@@ -1049,7 +1074,7 @@ TEST_F(TestMockIoCopyupRequest, CopyupError) {
                            0);
 
   expect_add_copyup_ops(mock_write_request);
-  expect_copyup(mock_image_ctx, 0, "oid", data, -EPERM);
+  expect_sparse_copyup(mock_image_ctx, 0, "oid", {{0, 4096}}, data, -EPERM);
   expect_write(mock_image_ctx, CEPH_NOSNAP, "oid", 0);
 
   auto req = new MockCopyupRequest(&mock_image_ctx, "oid", 0,
@@ -1062,5 +1087,51 @@ TEST_F(TestMockIoCopyupRequest, CopyupError) {
   flush_async_operations(ictx);
 }
 
+TEST_F(TestMockIoCopyupRequest, SparseCopyupNotSupported) {
+  REQUIRE_FEATURE(RBD_FEATURE_LAYERING);
+
+  librbd::ImageCtx *ictx;
+  ASSERT_EQ(0, open_image(m_image_name, &ictx));
+
+  MockTestImageCtx mock_parent_image_ctx(*ictx->parent);
+  MockTestImageCtx mock_image_ctx(*ictx, &mock_parent_image_ctx);
+  mock_image_ctx.enable_sparse_copyup = false;
+
+  MockExclusiveLock mock_exclusive_lock;
+  MockJournal mock_journal;
+  MockObjectMap mock_object_map;
+  initialize_features(ictx, mock_image_ctx, mock_exclusive_lock, mock_journal,
+                      mock_object_map);
+
+  expect_op_work_queue(mock_image_ctx);
+  expect_is_lock_owner(mock_image_ctx);
+
+  InSequence seq;
+
+  MockImageRequest mock_image_request;
+  std::string data(4096, '1');
+  expect_read_parent(mock_parent_image_ctx, mock_image_request, {{0, 4096}},
+                     data, 0);
+
+  MockAbstractObjectWriteRequest mock_write_request;
+  expect_get_pre_write_object_map_state(mock_image_ctx, mock_write_request,
+                                        OBJECT_EXISTS);
+  expect_object_map_at(mock_image_ctx, 0, OBJECT_NONEXISTENT);
+  expect_object_map_update(mock_image_ctx, CEPH_NOSNAP, 0, OBJECT_EXISTS, true,
+                           0);
+
+  expect_add_copyup_ops(mock_write_request);
+  expect_copyup(mock_image_ctx, CEPH_NOSNAP, "oid", data, 0);
+  expect_write(mock_image_ctx, CEPH_NOSNAP, "oid", 0);
+
+  auto req = new MockCopyupRequest(&mock_image_ctx, "oid", 0,
+                                   {{0, 4096}}, {});
+  mock_image_ctx.copyup_list[0] = req;
+  req->append_request(&mock_write_request);
+  req->send();
+
+  ASSERT_EQ(0, mock_write_request.ctx.wait());
+}
+
 } // namespace io
 } // namespace librbd

diff --git a/src/test/librbd/mock/MockImageCtx.h b/src/test/librbd/mock/MockImageCtx.h
index 1975a842db00671e8bc8afc39be22fc6f2082ace..6e46042e53ba8768bcaf3e3796adde725cd02fa6 100644 (file)
--- a/src/test/librbd/mock/MockImageCtx.h
+++ b/src/test/librbd/mock/MockImageCtx.h
@@ -98,6 +98,7 @@ struct MockImageCtx {
       blkin_trace_all(image_ctx.blkin_trace_all),
       enable_alloc_hint(image_ctx.enable_alloc_hint),
       ignore_migrating(image_ctx.ignore_migrating),
+      enable_sparse_copyup(image_ctx.enable_sparse_copyup),
       mtime_update_interval(image_ctx.mtime_update_interval),
       atime_update_interval(image_ctx.atime_update_interval),
       cache(image_ctx.cache),
@@ -303,6 +304,7 @@ struct MockImageCtx {
   bool blkin_trace_all;
   bool enable_alloc_hint;
   bool ignore_migrating;
+  bool enable_sparse_copyup;
   uint64_t mtime_update_interval;
   uint64_t atime_update_interval;
   bool cache;

diff --git a/src/test/librbd/test_internal.cc b/src/test/librbd/test_internal.cc
index 763f18953711ca93c91c2f35704d3098df6c9b4f..193a4d39038049e778bcffc009b60c38fabfcf9b 100644 (file)
--- a/src/test/librbd/test_internal.cc
+++ b/src/test/librbd/test_internal.cc
@@ -576,9 +576,14 @@ TEST_F(TestInternal, SnapshotCopyup)
   librbd::ImageCtx *ictx;
   ASSERT_EQ(0, open_image(m_image_name, &ictx));
 
+  bool sparse_read_supported = is_sparse_read_supported(
+      ictx->data_ctx, ictx->get_object_name(10));
+
   bufferlist bl;
   bl.append(std::string(256, '1'));
   ASSERT_EQ(256, ictx->io_work_queue->write(0, bl.length(), bufferlist{bl}, 0));
+  ASSERT_EQ(256, ictx->io_work_queue->write(1024, bl.length(), bufferlist{bl},
+                                            0));
 
   ASSERT_EQ(0, snap_create(*ictx, "snap1"));
   ASSERT_EQ(0,
@@ -609,10 +614,11 @@ TEST_F(TestInternal, SnapshotCopyup)
   librados::snap_set_t snap_set;
   ASSERT_EQ(0, snap_ctx.list_snaps(ictx2->get_object_name(0), &snap_set));
 
+  uint64_t copyup_end = ictx2->enable_sparse_copyup ? 1024 + 256 : 1 << order;
   std::vector< std::pair<uint64_t,uint64_t> > expected_overlap =
     boost::assign::list_of(
       std::make_pair(0, 256))(
-      std::make_pair(512, 2096640));
+      std::make_pair(512, copyup_end - 512));
   ASSERT_EQ(2U, snap_set.clones.size());
   ASSERT_NE(CEPH_NOSNAP, snap_set.clones[0].cloneid);
   ASSERT_EQ(2U, snap_set.clones[0].snaps.size());
@@ -637,6 +643,12 @@ TEST_F(TestInternal, SnapshotCopyup)
                                          0));
     ASSERT_TRUE(bl.contents_equal(read_bl));
 
+    ASSERT_EQ(256,
+              ictx2->io_work_queue->read(1024, 256,
+                                         librbd::io::ReadResult{read_result},
+                                         0));
+    ASSERT_TRUE(bl.contents_equal(read_bl));
+
     ASSERT_EQ(256,
               ictx2->io_work_queue->read(256, 256,
                                          librbd::io::ReadResult{read_result},
@@ -647,6 +659,51 @@ TEST_F(TestInternal, SnapshotCopyup)
       ASSERT_TRUE(read_bl.is_zero());
     }
 
+    // verify sparseness was preserved
+    {
+      librados::IoCtx io_ctx;
+      io_ctx.dup(m_ioctx);
+      librados::Rados rados(io_ctx);
+      EXPECT_EQ(0, rados.conf_set("rbd_cache", "false"));
+      EXPECT_EQ(0, rados.conf_set("rbd_sparse_read_threshold_bytes", "256"));
+      auto ictx3 = new librbd::ImageCtx(clone_name, "", snap_name, io_ctx,
+                                        true);
+      ASSERT_EQ(0, ictx3->state->open(0));
+      BOOST_SCOPE_EXIT(ictx3) {
+        ictx3->state->close();
+      } BOOST_SCOPE_EXIT_END;
+      std::map<uint64_t, uint64_t> expected_m;
+      bufferlist expected_bl;
+      if (ictx3->enable_sparse_copyup && sparse_read_supported) {
+        if (snap_name == NULL) {
+          expected_m = {{0, 512}, {1024, 256}};
+          expected_bl.append(std::string(256 * 3, '1'));
+        } else {
+          expected_m = {{0, 256}, {1024, 256}};
+          expected_bl.append(std::string(256 * 2, '1'));
+        }
+      } else {
+        expected_m = {{0, 1024 + 256}};
+        if (snap_name == NULL) {
+          expected_bl.append(std::string(256 * 2, '1'));
+          expected_bl.append(std::string(256 * 2, '\0'));
+          expected_bl.append(std::string(256 * 1, '1'));
+        } else {
+          expected_bl.append(std::string(256 * 1, '1'));
+          expected_bl.append(std::string(256 * 3, '\0'));
+          expected_bl.append(std::string(256 * 1, '1'));
+        }
+      }
+      std::map<uint64_t, uint64_t> read_m;
+      librbd::io::ReadResult sparse_read_result{&read_m, &read_bl};
+      EXPECT_EQ(1024 + 256,
+                ictx3->io_work_queue->read(0, 1024 + 256,
+                                           librbd::io::ReadResult{sparse_read_result},
+                                           0));
+      EXPECT_EQ(expected_m, read_m);
+      EXPECT_TRUE(expected_bl.contents_equal(read_bl));
+    }
+
     // verify the object map was properly updated
     if ((ictx2->features & RBD_FEATURE_OBJECT_MAP) != 0) {
       uint8_t state = OBJECT_EXISTS;