From b2ccf11d3a108141f113f3f5640d459205f208f2 Mon Sep 17 00:00:00 2001 From: Dan Mick Date: Fri, 30 Nov 2012 18:11:09 -0800 Subject: [PATCH] librbd: handle parent change while async I/Os are in flight During a test_librbd_fsx run including flatten, ImageCtx->parent was being dereferenced while null. Between the time the parent overlap is calculated and the time the guard+write completes with ENOENT and submits the copyup+write, the parent image could have changed (by resize) or been made irrelevant (by child flatten) such that the parent overlap is now incorrect. Handle "no parent" by just sending the copyup+write; the copyup part will be a no-op. Move to WRITE_FLAT state in this case because there's no more child to deal with. Handle "overlap changed" by recalculating overlap before reading parent data; if none is left, don't read, but rather just clear m_object_image_extents, in which case the copyup will again be a no-op because it will be of zero length. However we still have a parent, so stay in WRITE_COPYUP state and come back through as usual. Signed-off-by: Dan Mick Fixes: #3524 (cherry picked from commit 41e16a3b40efb80a5ed7a5587438569ca86c85a3) --- src/librbd/AioRequest.cc | 42 +++++++++++++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/src/librbd/AioRequest.cc b/src/librbd/AioRequest.cc index 63424e397fc..b9a76e48e0a 100644 --- a/src/librbd/AioRequest.cc +++ b/src/librbd/AioRequest.cc @@ -157,15 +157,47 @@ namespace librbd { ldout(m_ictx->cct, 20) << "WRITE_CHECK_GUARD" << dendl; if (r == -ENOENT) { + Mutex::Locker l(m_ictx->snap_lock); Mutex::Locker l2(m_ictx->parent_lock); - // copyup the entire object up to the overlap point - ldout(m_ictx->cct, 20) << "reading from parent " << m_object_image_extents << dendl; - assert(m_object_image_extents.size()); + /* + * Parent may have disappeared; if so, recover by using + * send_copyup() to send the original write req (the copyup + * operation itself will be a no-op, since someone must have + * populated the child object while we weren't looking). + * Move to WRITE_FLAT state as we'll be done with the + * operation once the null copyup completes. + */ - m_state = LIBRBD_AIO_WRITE_COPYUP; - read_from_parent(m_object_image_extents); + if (m_ictx->parent == NULL) { + ldout(m_ictx->cct, 20) << "parent is gone; do null copyup " << dendl; + m_state = LIBRBD_AIO_WRITE_FLAT; + send_copyup(); + finished = false; + break; + } + + // If parent still exists, overlap might also have changed. + uint64_t newlen = m_ictx->prune_parent_extents( + m_object_image_extents, m_ictx->parent_md.overlap); + + // copyup the entire object up to the overlap point, if any + if (newlen != 0) { + ldout(m_ictx->cct, 20) << "should_complete(" << this << ") overlap " + << m_ictx->parent_md.overlap << " newlen " + << newlen << " image_extents" + << m_object_image_extents << dendl; + + m_state = LIBRBD_AIO_WRITE_COPYUP; + read_from_parent(m_object_image_extents); + } else { + ldout(m_ictx->cct, 20) << "should_complete(" << this + << "): parent overlap now 0" << dendl; + m_object_image_extents.clear(); + m_state = LIBRBD_AIO_WRITE_FLAT; + send_copyup(); + } finished = false; break; }