hammer: librbd: request exclusive lock if current owner cannot execute op #12018
Conversation
|
@dillaman I have temporary added a hack to test this feature (will be removed after the review). Also, I am not very happy with my complete_request function: accessing AioCompletion members looks too hackish. May be you see a better option? |
| @@ -896,6 +896,8 @@ bool ImageWatcher::handle_payload(const FlattenPayload &payload, | |||
|
|
|||
| bool ImageWatcher::handle_payload(const ResizePayload &payload, | |||
| C_NotifyAck *ack_ctx) { | |||
| return ImageWatcher::handle_payload(UnknownPayload(), ack_ctx); | |||
There was a problem hiding this comment.
Yes, just for testing: make remote return -EOPNOTSUPP. It is not going to be committed.
| @@ -113,6 +121,18 @@ int invoke_async_request(ImageCtx *ictx, const std::string& request_type, | |||
| } | |||
|
|
|||
| r = remote_request(); | |||
| if (r == -EOPNOTSUPP && request_lock) { | |||
| AioCompletion *comp = aio_create_completion(); | |||
| ictx->image_watcher->request_lock(boost::bind(&complete_request, _1), | |||
There was a problem hiding this comment.
Perhaps just use:
AioCompletion *comp = aio_create_completion();
comp->add_request();
comp->finish_adding_requests(ictx->cct);
ictx->image_watcher->request_lock(boost::bind(&AioCompletion::complete_request, _1, ictx->cct, 0));
ictx->owner_lock.put_read();
r = comp->wait_for_complete();
...
There was a problem hiding this comment.
@dillaman It looks like it works! Thanks.
There was a problem hiding this comment.
Except one issue -- noise in the log:
2016-11-16 15:27:20.087375 7f02db7fe700 -1 librbd::AioCompletion: completed invalid aio_type: 4
@dillaman May I update AioCompletion::complete not to log error for AIO_TYPE_NONE?
There was a problem hiding this comment.
... or just add the new AIO type for request lock.
There was a problem hiding this comment.
@dillaman Is it worth adding a new type on hammer branch? Also, additionally I would need to call comp->init_time(...) to set the type. If you think it is ok, I can do this way though.
There was a problem hiding this comment.
OK, in that case, it's not worth it. I'd say demote the error message to an ldout(cct, 0)
dillaman
changed the title
This is a direct commit to hammer due to librbd code has evolved significantly in the master. Fixes: http://tracker.ceph.com/issues/17068 Signed-off-by: Mykola Golub <mgolub@mirantis.com>
trociny
force-pushed
the
wip-17068-hammer
branch
from
5455017
to
d89b1f8
Compare
trociny
changed the title
|
@dillaman Updated. |
|
lgtm |
…current owner cannot execute op Reviewed-by: Nathan Cutler <ncutler@suse.com>
…current owner cannot execute op Reviewed-by: Nathan Cutler <ncutler@suse.com>
…current owner cannot execute op Reviewed-by: Nathan Cutler <ncutler@suse.com>
|
@trociny @dillaman This PR was included in this rbd suite: http://tracker.ceph.com/issues/17151#note-12 That run has two issues: the first (OSD suicide timeout) is not reproducible and looks like environmental noise. Then there is a reproducible qemu-iotests failure which matches http://tracker.ceph.com/issues/10773 Can you take a look? If you think the qemu-iotests failure is unrelated, I guess this PR could be merged? |
…current owner cannot execute op Reviewed-by: Nathan Cutler <ncutler@suse.com>
|
@smithfarm lgtm |
resend writes after pool loses full flag Signed-off-by: xinxin shu <xinxin.shu@intel.com> (cherry picked from commit dbcf2e4) Conflicts: src/osdc/Objecter.cc src/osdc/Objecter.h
resend writes after pool loses full flag Signed-off-by: xinxin shu <xinxin.shu@intel.com> (cherry picked from commit dbcf2e4) Conflicts: src/osdc/Objecter.cc src/osdc/Objecter.h
http://tracker.ceph.com/issues/17068