From 960c7cd809472ab0e3ef836a6cbd61a9a80229ce Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Fri, 16 Jun 2017 12:45:46 -0400 Subject: [PATCH] librbd: clean up remove request state machine Signed-off-by: Jason Dillaman --- src/librbd/image/RemoveRequest.cc | 281 +++++++++++++++--------------- src/librbd/image/RemoveRequest.h | 38 ++-- 2 files changed, 160 insertions(+), 159 deletions(-) diff --git a/src/librbd/image/RemoveRequest.cc b/src/librbd/image/RemoveRequest.cc index 885e280a8ae0d..48da5221e8845 100644 --- a/src/librbd/image/RemoveRequest.cc +++ b/src/librbd/image/RemoveRequest.cc @@ -17,7 +17,7 @@ #define dout_subsys ceph_subsys_rbd #undef dout_prefix #define dout_prefix *_dout << "librbd::image::RemoveRequest: " << this << " " \ - << __func__ << " " + << __func__ << ": " namespace librbd { namespace image { @@ -62,20 +62,21 @@ void RemoveRequest::open_image() { } template -Context *RemoveRequest::handle_open_image(int *result) { - ldout(m_cct, 20) << ": r=" << *result << dendl; +void RemoveRequest::handle_open_image(int r) { + ldout(m_cct, 20) << "r=" << r << dendl; - if (*result < 0) { + if (r < 0) { m_image_ctx->destroy(); m_image_ctx = nullptr; - if (*result != -ENOENT) { - lderr(m_cct) << "error opening image: " << cpp_strerror(*result) << dendl; - return m_on_finish; + if (r != -ENOENT) { + lderr(m_cct) << "error opening image: " << cpp_strerror(r) << dendl; + finish(r); + return; } remove_image(); - return nullptr; + return; } m_image_id = m_image_ctx->id; @@ -85,7 +86,6 @@ Context *RemoveRequest::handle_open_image(int *result) { m_unknown_format = false; check_exclusive_lock(); - return nullptr; } template @@ -118,36 +118,34 @@ void RemoveRequest::acquire_exclusive_lock() { } template -Context *RemoveRequest::handle_exclusive_lock_force(int *result) { - ldout(m_cct, 20) << ": r=" << *result << dendl; +void RemoveRequest::handle_exclusive_lock_force(int r) { + ldout(m_cct, 20) << "r=" << r << dendl; delete m_exclusive_lock; m_exclusive_lock = nullptr; - if (*result < 0) { + if (r < 0) { lderr(m_cct) << "error shutting down exclusive lock: " - << cpp_strerror(*result) << dendl; - send_close_image(*result); - return nullptr; + << cpp_strerror(r) << dendl; + send_close_image(r); + return; } assert(m_image_ctx->exclusive_lock == nullptr); validate_image_removal(); - return nullptr; } template -Context *RemoveRequest::handle_exclusive_lock(int *result) { - ldout(m_cct, 20) << ": r=" << *result << dendl; +void RemoveRequest::handle_exclusive_lock(int r) { + ldout(m_cct, 20) << "r=" << r << dendl; - if ((*result < 0) || !m_image_ctx->exclusive_lock->is_lock_owner()) { + if (r < 0 || !m_image_ctx->exclusive_lock->is_lock_owner()) { lderr(m_cct) << "cannot obtain exclusive lock - not removing" << dendl; send_close_image(-EBUSY); - return nullptr; + return; } validate_image_removal(); - return nullptr; } template @@ -229,14 +227,14 @@ void RemoveRequest::filter_out_mirror_watchers() { } template -Context *RemoveRequest::handle_check_image_watchers(int *result) { - ldout(m_cct, 20) << ": r=" << *result << dendl; +void RemoveRequest::handle_check_image_watchers(int r) { + ldout(m_cct, 20) << "r=" << r << dendl; - if (*result < 0) { - lderr(m_cct) << "error listing image watchers: " << cpp_strerror(*result) + if (r < 0) { + lderr(m_cct) << "error listing image watchers: " << cpp_strerror(r) << dendl; - send_close_image(*result); - return nullptr; + send_close_image(r); + return; } // If an image is being bootstrapped by rbd-mirror, it implies @@ -247,11 +245,10 @@ Context *RemoveRequest::handle_check_image_watchers(int *result) { if (m_watchers.size() > 1) { lderr(m_cct) << "image has watchers - not removing" << dendl; send_close_image(-EBUSY); - return nullptr; + return; } check_group(); - return nullptr; } template @@ -272,29 +269,28 @@ void RemoveRequest::check_group() { } template -Context *RemoveRequest::handle_check_group(int *result) { - ldout(m_cct, 20) << ": r=" << *result << dendl; +void RemoveRequest::handle_check_group(int r) { + ldout(m_cct, 20) << "r=" << r << dendl; cls::rbd::GroupSpec s; - if (*result == 0) { + if (r == 0) { bufferlist::iterator it = m_out_bl.begin(); - *result = librbd::cls_client::image_get_group_finish(&it, &s); + r = librbd::cls_client::image_get_group_finish(&it, &s); } - if (*result < 0 && *result != -EOPNOTSUPP) { + if (r < 0 && r != -EOPNOTSUPP) { lderr(m_cct) << "error fetching group for image: " - << cpp_strerror(*result) << dendl; - send_close_image(*result); - return nullptr; + << cpp_strerror(r) << dendl; + send_close_image(r); + return; } if (s.is_valid()) { lderr(m_cct) << "image is in a group - not removing" << dendl; send_close_image(-EMLINK); - return nullptr; + return; } trim_image(); - return nullptr; } template @@ -313,21 +309,20 @@ void RemoveRequest::trim_image() { } template -Context *RemoveRequest::handle_trim_image(int *result) { - ldout(m_cct, 20) << ": r=" << *result << dendl; +void RemoveRequest::handle_trim_image(int r) { + ldout(m_cct, 20) << "r=" << r << dendl; - if (*result < 0) { + if (r < 0) { lderr(m_cct) << "warning: failed to remove some object(s): " - << cpp_strerror(*result) << dendl; + << cpp_strerror(r) << dendl; } if (m_old_format) { - send_close_image(*result); - return nullptr; + send_close_image(r); + return; } remove_child(); - return nullptr; } template @@ -350,21 +345,20 @@ void RemoveRequest::remove_child() { } template -Context *RemoveRequest::handle_remove_child(int *result) { - ldout(m_cct, 20) << ": r=" << *result << dendl; +void RemoveRequest::handle_remove_child(int r) { + ldout(m_cct, 20) << "r=" << r << dendl; - if (*result == -ENOENT) { - *result = 0; - } else if (*result < 0) { + if (r == -ENOENT) { + r = 0; + } else if (r < 0) { lderr(m_cct) << "error removing child from children list: " - << cpp_strerror(*result) << dendl; - send_close_image(*result); - return nullptr; + << cpp_strerror(r) << dendl; + send_close_image(r); + return; } send_disable_mirror(); - return nullptr; } template @@ -381,18 +375,17 @@ void RemoveRequest::send_disable_mirror() { } template -Context *RemoveRequest::handle_disable_mirror(int *result) { - ldout(m_cct, 20) << ": r=" << *result << dendl; +void RemoveRequest::handle_disable_mirror(int r) { + ldout(m_cct, 20) << "r=" << r << dendl; - if (*result == -EOPNOTSUPP) { - *result = 0; - } else if (*result < 0) { + if (r == -EOPNOTSUPP) { + r = 0; + } else if (r < 0) { lderr(m_cct) << "error disabling image mirroring: " - << cpp_strerror(*result) << dendl; + << cpp_strerror(r) << dendl; } - send_close_image(*result); - return nullptr; + send_close_image(r); } template @@ -408,23 +401,23 @@ void RemoveRequest::send_close_image(int r) { } template -Context *RemoveRequest::handle_send_close_image(int *result) { - ldout(m_cct, 20) << ": r=" << *result << dendl; +void RemoveRequest::handle_send_close_image(int r) { + ldout(m_cct, 20) << "r=" << r << dendl; - if (*result < 0) { + if (r < 0) { lderr(m_cct) << "error encountered while closing image: " - << cpp_strerror(*result) << dendl; + << cpp_strerror(r) << dendl; } m_image_ctx->destroy(); m_image_ctx = nullptr; if (m_ret_val < 0) { - *result = m_ret_val; - return m_on_finish; + r = m_ret_val; + finish(r); + return; } remove_header(); - return nullptr; } template @@ -440,16 +433,15 @@ void RemoveRequest::remove_header() { } template -Context *RemoveRequest::handle_remove_header(int *result) { - ldout(m_cct, 20) << ": r=" << *result << dendl; +void RemoveRequest::handle_remove_header(int r) { + ldout(m_cct, 20) << "r=" << r << dendl; - if ((*result < 0) && (*result != -ENOENT)) { - lderr(m_cct) << "error removing header: " << cpp_strerror(*result) << dendl; - m_ret_val = *result; + if (r < 0 && r != -ENOENT) { + lderr(m_cct) << "error removing header: " << cpp_strerror(r) << dendl; + m_ret_val = r; } remove_image(); - return nullptr; } template @@ -469,16 +461,16 @@ void RemoveRequest::remove_header_v2() { } template -Context *RemoveRequest::handle_remove_header_v2(int *result) { - ldout(m_cct, 20) << ": r=" << *result << dendl; +void RemoveRequest::handle_remove_header_v2(int r) { + ldout(m_cct, 20) << "r=" << r << dendl; - if ((*result < 0) && (*result != -ENOENT)) { - lderr(m_cct) << "error removing header: " << cpp_strerror(*result) << dendl; - return m_on_finish; + if (r < 0 && r != -ENOENT) { + lderr(m_cct) << "error removing header: " << cpp_strerror(r) << dendl; + finish(r); + return; } send_journal_remove(); - return nullptr; } template @@ -495,19 +487,19 @@ void RemoveRequest::send_journal_remove() { } template -Context *RemoveRequest::handle_journal_remove(int *result) { - ldout(m_cct, 20) << ": r=" << *result << dendl; +void RemoveRequest::handle_journal_remove(int r) { + ldout(m_cct, 20) << "r=" << r << dendl; - if (*result < 0 && *result != -ENOENT) { - lderr(m_cct) << "failed to remove image journal: " << cpp_strerror(*result) + if (r < 0 && r != -ENOENT) { + lderr(m_cct) << "failed to remove image journal: " << cpp_strerror(r) << dendl; - return m_on_finish; + finish(r); + return; } else { - *result = 0; + r = 0; } send_object_map_remove(); - return nullptr; } template @@ -526,19 +518,19 @@ void RemoveRequest::send_object_map_remove() { } template -Context *RemoveRequest::handle_object_map_remove(int *result) { - ldout(m_cct, 20) << ": r=" << *result << dendl; +void RemoveRequest::handle_object_map_remove(int r) { + ldout(m_cct, 20) << "r=" << r << dendl; - if (*result < 0 && *result != -ENOENT) { - lderr(m_cct) << "failed to remove image journal: " << cpp_strerror(*result) + if (r < 0 && r != -ENOENT) { + lderr(m_cct) << "failed to remove image journal: " << cpp_strerror(r) << dendl; - return m_on_finish; + finish(r); + return; } else { - *result = 0; + r = 0; } mirror_image_remove(); - return nullptr; } template @@ -557,27 +549,24 @@ void RemoveRequest::mirror_image_remove() { } template -Context *RemoveRequest::handle_mirror_image_remove(int *result) { - ldout(m_cct, 20) << ": r=" << *result << dendl; +void RemoveRequest::handle_mirror_image_remove(int r) { + ldout(m_cct, 20) << "r=" << r << dendl; - if (*result < 0 && *result != -ENOENT && *result != -EOPNOTSUPP) { + if (r < 0 && r != -ENOENT && r != -EOPNOTSUPP) { lderr(m_cct) << "failed to remove mirror image state: " - << cpp_strerror(*result) << dendl; - return m_on_finish; - } else { - *result = 0; + << cpp_strerror(r) << dendl; + finish(r); + return; } if (m_from_trash_remove) { // both the id object and the directory entry have been removed in // a previous call to trash_move. - - *result = 0; - return m_on_finish; + finish(0); + return; } remove_id_object(); - return nullptr; } template @@ -605,11 +594,11 @@ void RemoveRequest::remove_v1_image() { template void RemoveRequest::handle_remove_v1_image(int r) { - ldout(m_cct, 20) << ": r=" << r << dendl; + ldout(m_cct, 20) << "r=" << r << dendl; m_old_format = (r == 0); - if ((r == 0) || ((r < 0) && !m_unknown_format)) { - if ((r < 0) && (r != -ENOENT)) { + if (r == 0 || (r < 0 && !m_unknown_format)) { + if (r < 0 && r != -ENOENT) { lderr(m_cct) << "error removing image from v1 directory: " << cpp_strerror(r) << dendl; } @@ -657,25 +646,26 @@ void RemoveRequest::dir_get_image_id() { } template -Context *RemoveRequest::handle_dir_get_image_id(int *result) { - ldout(m_cct, 20) << ": r=" << *result << dendl; +void RemoveRequest::handle_dir_get_image_id(int r) { + ldout(m_cct, 20) << "r=" << r << dendl; - if ((*result < 0) && (*result != -ENOENT)) { - lderr(m_cct) << "error fetching image id: " << cpp_strerror(*result) + if (r < 0 && r != -ENOENT) { + lderr(m_cct) << "error fetching image id: " << cpp_strerror(r) << dendl; - return m_on_finish; + finish(r); + return; } - if (!*result) { + if (r == 0) { bufferlist::iterator iter = m_out_bl.begin(); - *result = librbd::cls_client::dir_get_id_finish(&iter, &m_image_id); - if (*result < 0) { - return m_on_finish; + r = librbd::cls_client::dir_get_id_finish(&iter, &m_image_id); + if (r < 0) { + finish(r); + return; } } remove_header_v2(); - return nullptr; } template @@ -695,25 +685,26 @@ void RemoveRequest::dir_get_image_name() { } template -Context *RemoveRequest::handle_dir_get_image_name(int *result) { - ldout(m_cct, 20) << ": r=" << *result << dendl; +void RemoveRequest::handle_dir_get_image_name(int r) { + ldout(m_cct, 20) << "r=" << r << dendl; - if ((*result < 0) && (*result != -ENOENT)) { - lderr(m_cct) << "error fetching image name: " << cpp_strerror(*result) + if (r < 0 && r != -ENOENT) { + lderr(m_cct) << "error fetching image name: " << cpp_strerror(r) << dendl; - return m_on_finish; + finish(r); + return; } - if (!*result) { + if (r == 0) { bufferlist::iterator iter = m_out_bl.begin(); - *result = librbd::cls_client::dir_get_name_finish(&iter, &m_image_name); - if (*result < 0) { - return m_on_finish; + r = librbd::cls_client::dir_get_name_finish(&iter, &m_image_name); + if (r < 0) { + finish(r); + return; } } remove_header_v2(); - return nullptr; } template @@ -729,17 +720,17 @@ void RemoveRequest::remove_id_object() { } template -Context *RemoveRequest::handle_remove_id_object(int *result) { - ldout(m_cct, 20) << ": r=" << *result << dendl; +void RemoveRequest::handle_remove_id_object(int r) { + ldout(m_cct, 20) << "r=" << r << dendl; - if ((*result < 0) && (*result != -ENOENT)) { - lderr(m_cct) << "error removing id object: " << cpp_strerror(*result) + if (r < 0 && r != -ENOENT) { + lderr(m_cct) << "error removing id object: " << cpp_strerror(r) << dendl; - return m_on_finish; + finish(r); + return; } dir_remove_image(); - return nullptr; } template @@ -758,15 +749,23 @@ void RemoveRequest::dir_remove_image() { } template -Context *RemoveRequest::handle_dir_remove_image(int *result) { - ldout(m_cct, 20) << ":r =" << *result << dendl; +void RemoveRequest::handle_dir_remove_image(int r) { + ldout(m_cct, 20) << "r=" << r << dendl; - if ((*result < 0) && (*result != -ENOENT)) { + if (r < 0 && r != -ENOENT) { lderr(m_cct) << "error removing image from v2 directory: " - << cpp_strerror(*result) << dendl; + << cpp_strerror(r) << dendl; } - return m_on_finish; + finish(r); +} + +template +void RemoveRequest::finish(int r) { + ldout(m_cct, 20) << "r=" << r << dendl; + + m_on_finish->complete(r); + delete this; } } // namespace image diff --git a/src/librbd/image/RemoveRequest.h b/src/librbd/image/RemoveRequest.h index 2a1ec3b603378..24d94d86b9c8f 100644 --- a/src/librbd/image/RemoveRequest.h +++ b/src/librbd/image/RemoveRequest.h @@ -125,50 +125,50 @@ private: std::list m_watchers; void open_image(); - Context *handle_open_image(int *result); + void handle_open_image(int r); void send_journal_remove(); - Context* handle_journal_remove(int *result); + void handle_journal_remove(int r); void send_object_map_remove(); - Context* handle_object_map_remove(int *result); + void handle_object_map_remove(int r); void mirror_image_remove(); - Context* handle_mirror_image_remove(int *result); + void handle_mirror_image_remove(int r); void check_exclusive_lock(); void acquire_exclusive_lock(); - Context *handle_exclusive_lock(int *result); - Context *handle_exclusive_lock_force(int *result); + void handle_exclusive_lock(int r); + void handle_exclusive_lock_force(int r); void validate_image_removal(); void check_image_snaps(); void filter_out_mirror_watchers(); void check_image_watchers(); - Context *handle_check_image_watchers(int *result); + void handle_check_image_watchers(int r); void check_group(); - Context *handle_check_group(int *result); + void handle_check_group(int r); void trim_image(); - Context *handle_trim_image(int *result); + void handle_trim_image(int r); void remove_child(); - Context *handle_remove_child(int *result); + void handle_remove_child(int r); void send_disable_mirror(); - Context *handle_disable_mirror(int *result); + void handle_disable_mirror(int r); void send_close_image(int r); - Context *handle_send_close_image(int *result); + void handle_send_close_image(int r); void remove_header(); - Context *handle_remove_header(int *result); + void handle_remove_header(int r); void remove_header_v2(); - Context *handle_remove_header_v2(int *result); + void handle_remove_header_v2(int r); void remove_image(); @@ -178,16 +178,18 @@ private: void remove_v2_image(); void dir_get_image_id(); - Context *handle_dir_get_image_id(int *result); + void handle_dir_get_image_id(int r); void dir_get_image_name(); - Context *handle_dir_get_image_name(int *result); + void handle_dir_get_image_name(int r); void remove_id_object(); - Context *handle_remove_id_object(int *result); + void handle_remove_id_object(int r); void dir_remove_image(); - Context *handle_dir_remove_image(int *result); + void handle_dir_remove_image(int r); + + void finish(int r); }; } // namespace image -- 2.39.5