From ea33368c1e6657fd02195e2994385230f520a37c Mon Sep 17 00:00:00 2001
From: Jason Dillaman <dillaman@redhat.com>
Date: Thu, 27 Jul 2017 07:48:59 -0400
Subject: [PATCH] osd: cmpext operator should ignore -ENOENT on read

The operator already handles the case where the object is truncated. If
an RBD user performs a read + cmpext/write of a sparse image, the read
of the missing object would return a zeroed buffer. Using that zeroed
buffer for the cmpext test would fail since it wasn't ignoring the
-ENOENT read failure.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
---
 src/osd/PrimaryLogPG.cc | 15 ++++++++++++---
 src/test/librados/io.cc | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/src/osd/PrimaryLogPG.cc b/src/osd/PrimaryLogPG.cc
index e94d8db1c8cf2..dbacb3c63c0f3 100644
--- a/src/osd/PrimaryLogPG.cc
+++ b/src/osd/PrimaryLogPG.cc
@@ -4694,7 +4694,13 @@ struct C_ExtentCmpRead : public Context {
   }
 
   void finish(int r) override {
-    fill_extent_ctx->complete(r);
+    if (r == -ENOENT) {
+      osd_op.rval = 0;
+      read_bl.clear();
+      delete fill_extent_ctx;
+    } else {
+      fill_extent_ctx->complete(r);
+    }
     fill_extent_ctx = nullptr;
 
     if (osd_op.rval >= 0) {
@@ -4708,7 +4714,10 @@ int PrimaryLogPG::do_extent_cmp(OpContext *ctx, OSDOp& osd_op)
   dout(20) << __func__ << dendl;
   ceph_osd_op& op = osd_op.op;
 
-  if (pool.info.require_rollback()) {
+  if (!ctx->obs->exists || ctx->obs->oi.is_whiteout()) {
+    dout(20) << __func__ << " object DNE" << dendl;
+    return finish_extent_cmp(osd_op, {});
+  } else if (pool.info.require_rollback()) {
     // If there is a data digest and it is possible we are reading
     // entire object, pass the digest.
     auto& oi = ctx->new_obs.oi;
@@ -4745,7 +4754,7 @@ int PrimaryLogPG::do_extent_cmp(OpContext *ctx, OSDOp& osd_op)
 
   int result = do_osd_ops(ctx, read_ops);
   if (result < 0) {
-    derr << "do_extent_cmp do_osd_ops failed " << result << dendl;
+    derr << __func__ << " failed " << result << dendl;
     return result;
   }
   return finish_extent_cmp(osd_op, read_op.outdata);
diff --git a/src/test/librados/io.cc b/src/test/librados/io.cc
index 347af5d288567..5094f756987af 100644
--- a/src/test/librados/io.cc
+++ b/src/test/librados/io.cc
@@ -1230,6 +1230,23 @@ TEST_F(LibRadosIoPP, CmpExtPP) {
   ASSERT_EQ(0, memcmp(bl.c_str(), "CEPH", 4));
 }
 
+TEST_F(LibRadosIoPP, CmpExtDNEPP) {
+  bufferlist bl;
+  bl.append(std::string(4, '\0'));
+
+  bufferlist new_bl;
+  new_bl.append("CEPH");
+  ObjectWriteOperation write;
+  write.cmpext(0, bl, nullptr);
+  write.write(0, new_bl);
+  ASSERT_EQ(0, ioctx.operate("foo", &write));
+
+  ObjectReadOperation read;
+  read.read(0, bl.length(), NULL, NULL);
+  ASSERT_EQ(0, ioctx.operate("foo", &read, &bl));
+  ASSERT_EQ(0, memcmp(bl.c_str(), "CEPH", 4));
+}
+
 TEST_F(LibRadosIoPP, CmpExtMismatchPP) {
   bufferlist bl;
   bl.append("ceph");
@@ -1270,6 +1287,23 @@ TEST_F(LibRadosIoECPP, CmpExtPP) {
   ASSERT_EQ(0, memcmp(bl.c_str(), "CEPH", 4));
 }
 
+TEST_F(LibRadosIoECPP, CmpExtDNEPP) {
+  bufferlist bl;
+  bl.append(std::string(4, '\0'));
+
+  bufferlist new_bl;
+  new_bl.append("CEPH");
+  ObjectWriteOperation write;
+  write.cmpext(0, bl, nullptr);
+  write.write_full(new_bl);
+  ASSERT_EQ(0, ioctx.operate("foo", &write));
+
+  ObjectReadOperation read;
+  read.read(0, bl.length(), NULL, NULL);
+  ASSERT_EQ(0, ioctx.operate("foo", &read, &bl));
+  ASSERT_EQ(0, memcmp(bl.c_str(), "CEPH", 4));
+}
+
 TEST_F(LibRadosIoECPP, CmpExtMismatchPP) {
   bufferlist bl;
   bl.append("ceph");
-- 
2.39.5