librbd: copyup should use empty snapshot context

This allows child object to exist through all existing
snapshots as well as HEAD.  If the operation was a CoW,
issue the write ops as a separate RADOS op with the
correct snapshot context.

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

diff --git a/src/librbd/CopyupRequest.cc b/src/librbd/CopyupRequest.cc
index f96dfbbce96f4b5226df87b74a20030a1a88e273..83e432210cbeb0775ecc449612939e547584f478 100644 (file)
--- a/src/librbd/CopyupRequest.cc
+++ b/src/librbd/CopyupRequest.cc
@@ -52,40 +52,76 @@ namespace librbd {
   }
 
   bool CopyupRequest::send_copyup() {
+    bool add_copyup_op = !m_copyup_data.is_zero();
+    bool copy_on_read = m_pending_requests.empty();
+    if (!add_copyup_op && copy_on_read) {
+      // no copyup data and CoR operation
+      return true;
+    }
+
+    ldout(m_ictx->cct, 20) << __func__ << " " << this
+                          << ": oid " << m_oid << dendl;
+    m_state = STATE_COPYUP;
+
     m_ictx->snap_lock.get_read();
     ::SnapContext snapc = m_ictx->snapc;
     m_ictx->snap_lock.put_read();
 
     std::vector<librados::snap_t> snaps;
-    snaps.insert(snaps.end(), snapc.snaps.begin(), snapc.snaps.end());
 
-    librados::ObjectWriteOperation copyup_op;
-    if (!m_copyup_data.is_zero()) {
-      copyup_op.exec("rbd", "copyup", m_copyup_data);
+    if (!copy_on_read) {
+      m_pending_copyups.inc();
     }
 
-    // merge all pending write ops into this single RADOS op
-    for (size_t i=0; i<m_pending_requests.size(); ++i) {
-      AioRequest *req = m_pending_requests[i];
-      ldout(m_ictx->cct, 20) << __func__ << " add_copyup_ops " << req << dendl;
-      req->add_copyup_ops(&copyup_op);
-    }
+    int r;
+    if (copy_on_read || (!snapc.snaps.empty() && add_copyup_op)) {
+      assert(add_copyup_op);
+      add_copyup_op = false;
 
-    if (copyup_op.size() == 0) {
-      return true;
+      librados::ObjectWriteOperation copyup_op;
+      copyup_op.exec("rbd", "copyup", m_copyup_data);
+
+      // send only the copyup request with a blank snapshot context so that
+      // all snapshots are detected from the parent for this object.  If
+      // this is a CoW request, a second request will be created for the
+      // actual modification.
+      m_pending_copyups.inc();
+
+      ldout(m_ictx->cct, 20) << __func__ << " " << this << " copyup with "
+                             << "empty snapshot context" << dendl;
+      librados::AioCompletion *comp =
+        librados::Rados::aio_create_completion(create_callback_context(), NULL,
+                                               rados_ctx_cb);
+      r = m_ictx->md_ctx.aio_operate(m_oid, comp, &copyup_op, 0, snaps);
+      assert(r == 0);
+      comp->release();
     }
 
-    ldout(m_ictx->cct, 20) << __func__ << " " << this
-                          << ": oid " << m_oid << dendl;
-    m_state = STATE_COPYUP;
+    if (!copy_on_read) {
+      librados::ObjectWriteOperation write_op;
+      if (add_copyup_op) {
+        // CoW did not need to handle existing snapshots
+        write_op.exec("rbd", "copyup", m_copyup_data);
+      }
 
-    librados::AioCompletion *comp =
-      librados::Rados::aio_create_completion(create_callback_context(), NULL,
-                                             rados_ctx_cb);
-    int r = m_ictx->md_ctx.aio_operate(m_oid, comp, &copyup_op, snapc.seq.val,
-                                       snaps);
-    assert(r == 0);
-    comp->release();
+      // merge all pending write ops into this single RADOS op
+      for (size_t i=0; i<m_pending_requests.size(); ++i) {
+        AioRequest *req = m_pending_requests[i];
+        ldout(m_ictx->cct, 20) << __func__ << " add_copyup_ops " << req
+                               << dendl;
+        req->add_copyup_ops(&write_op);
+      }
+      assert(write_op.size() != 0);
+
+      snaps.insert(snaps.end(), snapc.snaps.begin(), snapc.snaps.end());
+      librados::AioCompletion *comp =
+        librados::Rados::aio_create_completion(create_callback_context(), NULL,
+                                               rados_ctx_cb);
+      r = m_ictx->md_ctx.aio_operate(m_oid, comp, &write_op, snapc.seq.val,
+                                     snaps);
+      assert(r == 0);
+      comp->release();
+    }
     return false;
   }
 
@@ -138,6 +174,7 @@ namespace librbd {
                   << ", extents " << m_image_extents
                   << ", r " << r << dendl;
 
+    uint64_t pending_copyups;
     switch (m_state) {
     case STATE_READ_FROM_PARENT:
       ldout(cct, 20) << "READ_FROM_PARENT" << dendl;
@@ -157,9 +194,15 @@ namespace librbd {
       break;
 
     case STATE_COPYUP:
-      ldout(cct, 20) << "COPYUP" << dendl;
-      complete_requests(r);
-      return true;
+      // invoked via a finisher in librados, so thread safe
+      pending_copyups = m_pending_copyups.dec();
+      ldout(cct, 20) << "COPYUP (" << pending_copyups << " pending)"
+                     << dendl;
+      if (pending_copyups == 0 || r < 0) {
+        complete_requests(r);
+        return (pending_copyups == 0);
+      }
+      break;
 
     default:
       lderr(cct) << "invalid state: " << m_state << dendl;

diff --git a/src/librbd/CopyupRequest.h b/src/librbd/CopyupRequest.h
index 667e14e7e8645f1f4811a2990b86bcd092056a41..aa07471be1ed9a45d329debac4691c285aa68452 100644 (file)
--- a/src/librbd/CopyupRequest.h
+++ b/src/librbd/CopyupRequest.h
@@ -63,6 +63,7 @@ namespace librbd {
     State m_state;
     ceph::bufferlist m_copyup_data;
     vector<AioRequest *> m_pending_requests;
+    atomic_t m_pending_copyups;
 
     AsyncOperation m_async_op;