librbd: assert header lock ownership for maint operations

The resize, flatten, and snapshot maintenance operations now
use the new assert_lock feature to ensure that the current
client still owns the header lock when making changes.

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

diff --git a/src/cls/rbd/cls_rbd_client.cc b/src/cls/rbd/cls_rbd_client.cc
index 8bbb1ccc2c427cad3d4c5f6304c3e872bd1acc02..66dc03b3fdfedccd2fe8a5af8dbdcb65ca421c71 100644 (file)
--- a/src/cls/rbd/cls_rbd_client.cc
+++ b/src/cls/rbd/cls_rbd_client.cc
@@ -194,12 +194,18 @@ namespace librbd {
     }
 
     int set_size(librados::IoCtx *ioctx, const std::string &oid,
-                uint64_t size)
+                 uint64_t size)
     {
-      bufferlist bl, bl2;
-      ::encode(size, bl);
+      librados::ObjectWriteOperation op;
+      set_size(&op, size);
+      return ioctx->operate(oid, &op);
+    }
 
-      return ioctx->exec(oid, "rbd", "set_size", bl, bl2);
+    void set_size(librados::ObjectWriteOperation *op, uint64_t size)
+    {
+      bufferlist bl;
+      ::encode(size, bl);
+      op->exec("rbd", "set_size", bl);
     }
 
     int get_parent(librados::IoCtx *ioctx, const std::string &oid,
@@ -240,8 +246,15 @@ namespace librbd {
 
     int remove_parent(librados::IoCtx *ioctx, const std::string &oid)
     {
-      bufferlist inbl, outbl;
-      return ioctx->exec(oid, "rbd", "remove_parent", inbl, outbl);
+      librados::ObjectWriteOperation op;
+      remove_parent(&op);
+      return ioctx->operate(oid, &op);
+    }
+
+    void remove_parent(librados::ObjectWriteOperation *op)
+    {
+      bufferlist inbl;
+      op->exec("rbd", "remove_parent", inbl);
     }
 
     int add_child(librados::IoCtx *ioctx, const std::string &oid,
@@ -291,11 +304,18 @@ namespace librbd {
     int snapshot_add(librados::IoCtx *ioctx, const std::string &oid,
                     snapid_t snap_id, const std::string &snap_name)
     {
-      bufferlist bl, bl2;
+      librados::ObjectWriteOperation op;
+      snapshot_add(&op, snap_id, snap_name);
+      return ioctx->operate(oid, &op);
+    }
+
+    void snapshot_add(librados::ObjectWriteOperation *op, snapid_t snap_id,
+                     const std::string &snap_name)
+    {
+      bufferlist bl;
       ::encode(snap_name, bl);
       ::encode(snap_id, bl);
-
-      return ioctx->exec(oid, "rbd", "snapshot_add", bl, bl2);
+      op->exec("rbd", "snapshot_add", bl);
     }
 
     int snapshot_remove(librados::IoCtx *ioctx, const std::string &oid,

diff --git a/src/cls/rbd/cls_rbd_client.h b/src/cls/rbd/cls_rbd_client.h
index 10073e578c4886d42f0520348d2c9a53a0d3256d..19f0b2dabce269b0651f6af819fc5e02082a443a 100644 (file)
--- a/src/cls/rbd/cls_rbd_client.h
+++ b/src/cls/rbd/cls_rbd_client.h
@@ -40,14 +40,14 @@ namespace librbd {
                 snapid_t snap_id, uint64_t *size, uint8_t *order);
     int set_size(librados::IoCtx *ioctx, const std::string &oid,
                 uint64_t size);
-    int set_size(librados::IoCtx *ioctx, const std::string &oid,
-                uint64_t size);
+    void set_size(librados::ObjectWriteOperation *op, uint64_t size);
     int get_parent(librados::IoCtx *ioctx, const std::string &oid,
                   snapid_t snap_id, parent_spec *pspec,
                   uint64_t *parent_overlap);
     int set_parent(librados::IoCtx *ioctx, const std::string &oid,
                   parent_spec pspec, uint64_t parent_overlap);
     int remove_parent(librados::IoCtx *ioctx, const std::string &oid);
+    void remove_parent(librados::ObjectWriteOperation *op);
     int add_child(librados::IoCtx *ioctx, const std::string &oid,
                  parent_spec pspec, const std::string &c_imageid);
     int remove_child(librados::IoCtx *ioctx, const std::string &oid,
@@ -56,6 +56,8 @@ namespace librbd {
                     parent_spec pspec, set<string>& children);
     int snapshot_add(librados::IoCtx *ioctx, const std::string &oid,
                     snapid_t snap_id, const std::string &snap_name);
+    void snapshot_add(librados::ObjectWriteOperation *op, snapid_t snap_id,
+                     const std::string &snap_name);
     int snapshot_remove(librados::IoCtx *ioctx, const std::string &oid,
                        snapid_t snap_id);
     int get_snapcontext(librados::IoCtx *ioctx, const std::string &oid,

diff --git a/src/librbd/ImageWatcher.cc b/src/librbd/ImageWatcher.cc
index b02c6828364d1517517adf9c3259311125b1398b..9f95918ce278aa40b9516dd94f563fa99899f73a 100644 (file)
--- a/src/librbd/ImageWatcher.cc
+++ b/src/librbd/ImageWatcher.cc
@@ -408,6 +408,11 @@ void ImageWatcher::finalize_header_update() {
                        &m_image_ctx);
 }
 
+void ImageWatcher::assert_header_locked(librados::ObjectWriteOperation *op) {
+  rados::cls::lock::assert_locked(op, RBD_LOCK_NAME, LOCK_EXCLUSIVE,
+                                  encode_lock_cookie(), WATCHER_LOCK_TAG);
+}
+
 int ImageWatcher::notify_async_progress(const RemoteAsyncRequest &request,
                                        uint64_t offset, uint64_t total) {
   ldout(m_image_ctx.cct, 20) << "remote async request progress: "
@@ -495,6 +500,7 @@ void ImageWatcher::notify_header_update(librados::IoCtx &io_ctx,
 }
 
 std::string ImageWatcher::encode_lock_cookie() const {
+  RWLock::RLocker l(m_watch_lock);
   std::ostringstream ss;
   ss << WATCHER_LOCK_COOKIE_PREFIX << " " << m_handle;
   return ss.str();

diff --git a/src/librbd/ImageWatcher.h b/src/librbd/ImageWatcher.h
index 1cf4dee7c08a1e2b494a52a1d050cf1f5be13df9..b30ab5ffcf7fa7edcc5cdc168fe9fa7876467962 100644 (file)
--- a/src/librbd/ImageWatcher.h
+++ b/src/librbd/ImageWatcher.h
@@ -56,6 +56,8 @@ namespace librbd {
                     AioCompletion* c);
     int unlock();
 
+    void assert_header_locked(librados::ObjectWriteOperation *op);
+
     int notify_async_progress(const RemoteAsyncRequest &remote_async_request,
                              uint64_t offset, uint64_t total);
     int notify_async_complete(const RemoteAsyncRequest &remote_async_request,

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index e089cfa16471b89d90826bcaa25bb7a5569e12cd..5087a92ed7f4640ba6c41a0d635dabcc7a7ea02d 100644 (file)
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -1675,8 +1675,12 @@ reprotect_and_return_err:
        bl.append((const char *)&m_ictx->header, sizeof(m_ictx->header));
        r = m_ictx->md_ctx.write(m_ictx->header_oid, bl, bl.length(), 0);
       } else {
-       r = cls_client::set_size(&m_ictx->md_ctx, m_ictx->header_oid,
-                                m_new_size);
+       librados::ObjectWriteOperation op;
+        if (m_ictx->image_watcher->is_lock_supported()) {
+         m_ictx->image_watcher->assert_header_locked(&op);
+        }
+       cls_client::set_size(&op, m_new_size);
+       r = m_ictx->md_ctx.operate(m_ictx->header_oid, &op);
       }
 
       if (r < 0) {
@@ -1895,6 +1899,7 @@ reprotect_and_return_err:
 
   int add_snap(ImageCtx *ictx, const char *snap_name)
   {
+    assert(ictx->owner_lock.is_locked());
     uint64_t snap_id;
 
     int r = ictx->md_ctx.selfmanaged_snap_create(&snap_id);
@@ -1908,13 +1913,18 @@ reprotect_and_return_err:
       r = cls_client::old_snapshot_add(&ictx->md_ctx, ictx->header_oid,
                                       snap_id, snap_name);
     } else {
-      r = cls_client::snapshot_add(&ictx->md_ctx, ictx->header_oid,
-                                  snap_id, snap_name);
+      librados::ObjectWriteOperation op;
+      if (ictx->image_watcher->is_lock_supported()) {
+       ictx->image_watcher->assert_header_locked(&op);
+      }
+      cls_client::snapshot_add(&op, snap_id, snap_name);
+      r = ictx->md_ctx.operate(ictx->header_oid, &op);
     }
 
     if (r < 0) {
       lderr(ictx->cct) << "adding snapshot to header failed: "
                       << cpp_strerror(r) << dendl;
+      ictx->data_ctx.selfmanaged_snap_remove(snap_id);
       return r;
     }
 
@@ -2656,7 +2666,12 @@ reprotect_and_return_err:
       }
 
       // remove parent from this (base) image
-      r = cls_client::remove_parent(&m_ictx->md_ctx, m_ictx->header_oid);
+      librados::ObjectWriteOperation op;
+      if (m_ictx->image_watcher->is_lock_supported()) {
+       m_ictx->image_watcher->assert_header_locked(&op);
+      }
+      cls_client::remove_parent(&op);
+      r = m_ictx->md_ctx.operate(m_ictx->header_oid, &op);
       if (r < 0) {
         lderr(cct) << "error removing parent" << dendl;
         return;