From: Jason Dillaman Date: Wed, 14 Jan 2015 16:49:13 +0000 (-0500) Subject: librbd: assert header lock ownership for maint operations X-Git-Tag: v0.93~193^2~2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=1b6467b79f5c557e3b51eec040aa7af4728640ed;p=ceph.git 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 --- diff --git a/src/cls/rbd/cls_rbd_client.cc b/src/cls/rbd/cls_rbd_client.cc index 8bbb1ccc2c42..66dc03b3fdfe 100644 --- 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 10073e578c48..19f0b2dabce2 100644 --- 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& 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 b02c6828364d..9f95918ce278 100644 --- 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 1cf4dee7c08a..b30ab5ffcf7f 100644 --- 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 e089cfa16471..5087a92ed7f4 100644 --- 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;