Merge pull request #39011 from dillaman/wip-librbd-crypto-fixes-pacificv16.1.0

pacific: librbd: miscellaneous fixes for crypto layer

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

4 files changed, 75 insertions, 18 deletions

diff --git a/qa/tasks/qemu.py b/qa/tasks/qemu.py
index 5235c229b2f..673ccdb144f 100644
--- a/qa/tasks/qemu.py
+++ b/qa/tasks/qemu.py
@@ -20,6 +20,7 @@ log = logging.getLogger(__name__)
 DEFAULT_NUM_DISKS = 2
 DEFAULT_IMAGE_URL = 'http://download.ceph.com/qa/ubuntu-12.04.qcow2'
 DEFAULT_IMAGE_SIZE = 10240 # in megabytes
+ENCRYPTION_HEADER_SIZE = 16 # in megabytes
 DEFAULT_CPUS = 1
 DEFAULT_MEM = 4096 # in megabytes
 
@@ -81,11 +82,14 @@ def create_images(ctx, config, managers):
                     'image_url' in disk and
                     disk['encryption_format'] == 'none'):
                 continue
+            image_size = disk['image_size']
+            if disk['encryption_format'] != 'none':
+                image_size += ENCRYPTION_HEADER_SIZE
             create_config = {
                 client: {
                     'image_name': disk['image_name'],
                     'image_format': 2,
-                    'image_size': disk['image_size'],
+                    'image_size': image_size,
                     'encryption_format': disk['encryption_format'],
                     }
                 }
diff --git a/src/librbd/crypto/CryptoObjectDispatch.cc b/src/librbd/crypto/CryptoObjectDispatch.cc
index 9cac7140761..244f52dec2b 100644
--- a/src/librbd/crypto/CryptoObjectDispatch.cc
+++ b/src/librbd/crypto/CryptoObjectDispatch.cc
@@ -201,6 +201,7 @@ struct C_UnalignedObjectWriteRequest : public Context {
     int* object_dispatch_flags;
     uint64_t* journal_tid;
     Context* on_finish;
+    bool may_copyup;
     ceph::bufferlist aligned_data;
     io::ReadExtents extents;
     uint64_t version;
@@ -214,14 +215,15 @@ struct C_UnalignedObjectWriteRequest : public Context {
             IOContext io_context, int op_flags, int write_flags,
             std::optional<uint64_t> assert_version,
             const ZTracer::Trace &parent_trace, int* object_dispatch_flags,
-            uint64_t* journal_tid,Context* on_dispatched
+            uint64_t* journal_tid, Context* on_dispatched, bool may_copyup
             ) : image_ctx(image_ctx), crypto(crypto), object_no(object_no),
                 object_off(object_off), data(data), cmp_data(cmp_data),
                 mismatch_offset(mismatch_offset), io_context(io_context),
                 op_flags(op_flags), write_flags(write_flags),
                 assert_version(assert_version), parent_trace(parent_trace),
                 object_dispatch_flags(object_dispatch_flags),
-                journal_tid(journal_tid), on_finish(on_dispatched) {
+                journal_tid(journal_tid), on_finish(on_dispatched),
+                may_copyup(may_copyup) {
       // build read extents
       auto [pre_align, post_align] = crypto->get_pre_and_post_align(
               object_off, data.length());
@@ -337,7 +339,7 @@ struct C_UnalignedObjectWriteRequest : public Context {
       if (r < 0) {
         complete(r);
       } else {
-        restart_request();
+        restart_request(false);
       }
     }
 
@@ -345,13 +347,16 @@ struct C_UnalignedObjectWriteRequest : public Context {
       ldout(image_ctx->cct, 20) << "unaligned write r=" << r << dendl;
 
       if (r == -ENOENT) {
-        auto ctx = create_context_callback<
-                C_UnalignedObjectWriteRequest<I>,
-                &C_UnalignedObjectWriteRequest<I>::handle_copyup>(this);
-        if (io::util::trigger_copyup(image_ctx, object_no, io_context, ctx)) {
-          return;
+        if (may_copyup) {
+          auto ctx = create_context_callback<
+                  C_UnalignedObjectWriteRequest<I>,
+                  &C_UnalignedObjectWriteRequest<I>::handle_copyup>(this);
+          if (io::util::trigger_copyup(
+                  image_ctx, object_no, io_context, ctx)) {
+            return;
+          }
+          delete ctx;
         }
-        delete ctx;
         object_exists = false;
       } else if (r < 0) {
         complete(r);
@@ -388,13 +393,13 @@ struct C_UnalignedObjectWriteRequest : public Context {
       write_req->send();
     }
 
-    void restart_request() {
+    void restart_request(bool may_copyup) {
       auto req = new C_UnalignedObjectWriteRequest<I>(
               image_ctx, crypto, object_no, object_off,
               std::move(data), std::move(cmp_data),
               mismatch_offset, io_context, op_flags, write_flags,
               assert_version, parent_trace,
-              object_dispatch_flags, journal_tid, this);
+              object_dispatch_flags, journal_tid, this, may_copyup);
       req->send();
     }
 
@@ -409,7 +414,7 @@ struct C_UnalignedObjectWriteRequest : public Context {
       }
 
       if (restart) {
-        restart_request();
+        restart_request(may_copyup);
       } else {
         complete(r);
       }
@@ -491,7 +496,8 @@ bool CryptoObjectDispatch<I>::write(
     auto req = new C_UnalignedObjectWriteRequest<I>(
             m_image_ctx, m_crypto, object_no, object_off, std::move(data), {},
             nullptr, io_context, op_flags, write_flags, assert_version,
-            parent_trace, object_dispatch_flags, journal_tid, on_dispatched);
+            parent_trace, object_dispatch_flags, journal_tid, on_dispatched,
+            true);
     req->send();
   }
 
@@ -552,7 +558,7 @@ bool CryptoObjectDispatch<I>::compare_and_write(
           m_image_ctx, m_crypto, object_no, object_off, std::move(write_data),
           std::move(cmp_data), mismatch_offset, io_context, op_flags, 0,
           std::nullopt, parent_trace, object_dispatch_flags, journal_tid,
-          on_dispatched);
+          on_dispatched, true);
   req->send();
 
   return true;
diff --git a/src/test/librbd/crypto/test_mock_CryptoObjectDispatch.cc b/src/test/librbd/crypto/test_mock_CryptoObjectDispatch.cc
index 2f6972217fd..f0160f1074a 100644
--- a/src/test/librbd/crypto/test_mock_CryptoObjectDispatch.cc
+++ b/src/test/librbd/crypto/test_mock_CryptoObjectDispatch.cc
@@ -543,6 +543,50 @@ TEST_F(TestMockCryptoCryptoObjectDispatch, UnalignedWriteCopyup) {
   ASSERT_EQ(0, dispatched_cond.wait());
 }
 
+TEST_F(TestMockCryptoCryptoObjectDispatch, UnalignedWriteEmptyCopyup) {
+  MockObjectMap mock_object_map;
+  mock_image_ctx->object_map = &mock_object_map;
+  MockExclusiveLock mock_exclusive_lock;
+  mock_image_ctx->exclusive_lock = &mock_exclusive_lock;
+
+  ceph::bufferlist write_data;
+  write_data.append(std::string(8192, '1'));
+  io::ReadExtents extents = {{0, 4096}, {8192, 4096}};
+  expect_object_read(&extents);
+  ASSERT_TRUE(mock_crypto_object_dispatch->write(
+          0, 1, std::move(write_data), mock_image_ctx->get_data_io_context(),
+          0, 0, std::nullopt, {}, nullptr, nullptr, &dispatch_result,
+          &on_finish, on_dispatched));
+  ASSERT_EQ(dispatch_result, io::DISPATCH_RESULT_COMPLETE);
+  ASSERT_EQ(on_finish, &finished_cond);
+
+  expect_get_object_size();
+  expect_get_parent_overlap(mock_image_ctx->layout.object_size);
+  expect_remap_extents(0, mock_image_ctx->layout.object_size);
+  expect_prune_parent_extents(mock_image_ctx->layout.object_size);
+  EXPECT_CALL(mock_exclusive_lock, is_lock_owner()).WillRepeatedly(
+          Return(true));
+  EXPECT_CALL(*mock_image_ctx->object_map, object_may_exist(0)).WillOnce(
+          Return(false));
+  MockAbstractObjectWriteRequest *write_request = nullptr;
+  expect_copyup(&write_request, 0);
+
+  // unaligned write restarted
+  expect_object_read(&extents);
+  dispatcher_ctx->complete(-ENOENT); // complete first read
+  ASSERT_EQ(ETIMEDOUT, dispatched_cond.wait_for(0));
+
+  auto expected_data =
+        std::string(1, '\0') + std::string(8192, '1') +
+        std::string(4095, '\0');
+  expect_object_write(0, expected_data, io::OBJECT_WRITE_FLAG_CREATE_EXCLUSIVE,
+                      std::nullopt);
+  dispatcher_ctx->complete(-ENOENT); // complete second read
+  ASSERT_EQ(ETIMEDOUT, dispatched_cond.wait_for(0));
+  dispatcher_ctx->complete(0); // complete write
+  ASSERT_EQ(0, dispatched_cond.wait());
+}
+
 TEST_F(TestMockCryptoCryptoObjectDispatch, UnalignedWriteFailVersionCheck) {
   ceph::bufferlist write_data;
   uint64_t version = 1234;
diff --git a/src/tools/rbd_nbd/rbd-nbd.cc b/src/tools/rbd_nbd/rbd-nbd.cc
index c692e0ce089..3a13ff82fb1 100644
--- a/src/tools/rbd_nbd/rbd-nbd.cc
+++ b/src/tools/rbd_nbd/rbd-nbd.cc
@@ -1009,7 +1009,8 @@ static int parse_nbd_index(const std::string& devpath)
   return index;
 }
 
-static int try_ioctl_setup(Config *cfg, int fd, uint64_t size, uint64_t flags)
+static int try_ioctl_setup(Config *cfg, int fd, uint64_t size,
+                           uint64_t blksize, uint64_t flags)
 {
   int index = 0, r;
 
@@ -1072,7 +1073,7 @@ static int try_ioctl_setup(Config *cfg, int fd, uint64_t size, uint64_t flags)
     }
   }
 
-  r = ioctl(nbd, NBD_SET_BLKSIZE, RBD_NBD_BLKSIZE);
+  r = ioctl(nbd, NBD_SET_BLKSIZE, blksize);
   if (r < 0) {
     r = -errno;
     cerr << "rbd-nbd: NBD_SET_BLKSIZE failed" << std::endl;
@@ -1560,6 +1561,7 @@ static int do_map(int argc, const char *argv[], Config *cfg, bool reconnect)
   int read_only = 0;
   unsigned long flags;
   unsigned long size;
+  unsigned long blksize = RBD_NBD_BLKSIZE;
   bool use_netlink;
 
   int fd[2];
@@ -1677,6 +1679,7 @@ static int do_map(int argc, const char *argv[], Config *cfg, bool reconnect)
         opts.passphrase = passphrase;
         r = image.encryption_load(
                 RBD_ENCRYPTION_FORMAT_LUKS2, &opts, sizeof(opts));
+        blksize = 4096;
         break;
       }
       default:
@@ -1728,7 +1731,7 @@ static int do_map(int argc, const char *argv[], Config *cfg, bool reconnect)
   }
 
   if (!use_netlink) {
-    r = try_ioctl_setup(cfg, fd[0], size, flags);
+    r = try_ioctl_setup(cfg, fd[0], size, blksize, flags);
     if (r < 0)
       goto free_server;
   }