librbd: skip partial discards by default

Avoid the IO expense of writing zeros mid-object when a discard
is attempting to free space.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/common/options.cc b/src/common/options.cc
index d5fc6d899095615de2dfd5a092635110d18d8ec0..e5752c2a2f0e94b20dfb0e61c89061111b9c0a95 100644 (file)
--- a/src/common/options.cc
+++ b/src/common/options.cc
@@ -7071,8 +7071,8 @@ static std::vector<Option> get_rbd_options() {
     .set_description("number of seconds before maintenance request times out"),
 
     Option("rbd_skip_partial_discard", Option::TYPE_BOOL, Option::LEVEL_ADVANCED)
-    .set_default(false)
-    .set_description("when trying to discard a range inside an object, set to true to skip zeroing the range"),
+    .set_default(true)
+    .set_description("skip discard (zero) of unaligned extents within an object"),
 
     Option("rbd_enable_alloc_hint", Option::TYPE_BOOL, Option::LEVEL_ADVANCED)
     .set_default(true)

diff --git a/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc b/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc
index 6b4b8929af389a42cfd4992bcacd1688b6975a78..8e70f153794a44ccd98f5aa03dcf2b92e7c059d1 100644 (file)
--- a/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc
+++ b/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc
@@ -749,6 +749,8 @@ TEST_F(TestMockDeepCopyObjectCopyRequest, WriteSnaps) {
 TEST_F(TestMockDeepCopyObjectCopyRequest, Trim) {
   ASSERT_EQ(0, m_src_image_ctx->operations->metadata_set(
               "conf_rbd_skip_partial_discard", "false"));
+  m_src_image_ctx->skip_partial_discard = false;
+
   // scribble some data
   interval_set<uint64_t> one;
   scribble(m_src_image_ctx, 10, 102400, &one);
@@ -757,7 +759,8 @@ TEST_F(TestMockDeepCopyObjectCopyRequest, Trim) {
   // trim the object
   uint64_t trim_offset = rand() % one.range_end();
   ASSERT_LE(0, m_src_image_ctx->io_work_queue->discard(
-    trim_offset, one.range_end() - trim_offset, m_src_image_ctx->skip_partial_discard));
+    trim_offset, one.range_end() - trim_offset,
+    m_src_image_ctx->skip_partial_discard));
   ASSERT_EQ(0, create_snap("copy"));
 
   librbd::MockTestImageCtx mock_src_image_ctx(*m_src_image_ctx);
@@ -809,7 +812,8 @@ TEST_F(TestMockDeepCopyObjectCopyRequest, Remove) {
 
   // remove the object
   uint64_t object_size = 1 << m_src_image_ctx->order;
-  ASSERT_LE(0, m_src_image_ctx->io_work_queue->discard(0, object_size, m_src_image_ctx->skip_partial_discard));
+  ASSERT_LE(0, m_src_image_ctx->io_work_queue->discard(
+    0, object_size, m_src_image_ctx->skip_partial_discard));
   ASSERT_EQ(0, create_snap("copy"));
   librbd::MockTestImageCtx mock_src_image_ctx(*m_src_image_ctx);
   librbd::MockTestImageCtx mock_dst_image_ctx(*m_dst_image_ctx);

diff --git a/src/test/librbd/test_Migration.cc b/src/test/librbd/test_Migration.cc
index f9ae5c3c274b7db3f73db41ee9dab2ad3f153399..ace266948fec3e8b1276fd0c4c5e0fbba292bd4d 100644 (file)
--- a/src/test/librbd/test_Migration.cc
+++ b/src/test/librbd/test_Migration.cc
@@ -157,11 +157,14 @@ struct TestMigration : public TestFixture {
     m_ictxs.insert(*ictx);
 
     ASSERT_EQ(0, (*ictx)->state->open(flags));
+    (*ictx)->skip_partial_discard = false;
   }
 
   void open_image(librados::IoCtx& io_ctx, const std::string &name,
                   librbd::ImageCtx **ictx) {
     open_image(io_ctx, name, "", false, 0, ictx);
+    ASSERT_EQ(0, (*ictx)->state->open(0));
+    (*ictx)->skip_partial_discard = false;
   }
 
   void migration_prepare(librados::IoCtx& dst_io_ctx,