Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

librbd: Asynchronous (v2) image creation #9585

Merged
merged 15 commits into from Aug 10, 2016
Merged

Conversation

vshankar
Copy link
Contributor

@vshankar vshankar commented Jun 8, 2016

No description provided.

@vshankar
Copy link
Contributor Author

vshankar commented Jun 8, 2016

@dillaman I'm investigating the failure, but the patch should mostly be reviewable.

@@ -0,0 +1,633 @@
// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: move to "operation" subdirectory / namespace

@@ -149,6 +149,18 @@ namespace librbd {
parent);
}

void create_image(librados::ObjectWriteOperation *op, uint64_t size, uint8_t order,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd leave all the sync versions that are still needed by librbd -- just have the sync version call the async version to setup the object operation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sync versions do use the async helpers which got introduced in the prior commit. This commit changes the sync version to use async helpers.


delete m_journaler;

if ((m_r_saved == 0) || (*result < 0)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dillaman Here, the cleanup cuts short if an error in encountered when shutting down journaler. Would it makes sense to continue deleting journal objects in this case?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vshankar Yeah, I can't think of a reason why not to continue on.

@vshankar
Copy link
Contributor Author

@ceph-jenkins retest this please

@vshankar
Copy link
Contributor Author

Change in this patchset:

  • Renamed journal::Create/RemoveJournalRequest to journal::Create/RemoveReequest
  • Continue removing jounal on shutdown failure

using klass = RemoveRequest<I>;
Context *ctx = create_context_callback<klass, &klass::handle_init_journaler>(this);

m_journaler->init(ctx);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check for exist before initializing the journal.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of relied on JournalMetadata::init which already calls aio_watch* on the (header) object which should return -ENOENT. Shouldn't that suffice?

[*] https://github.com/ceph/ceph/blob/master/src/journal/JournalMetadata.cc#L449

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-ENOENT can also be returned if the client isn't registered. If you stat it first and get -ENOENT, you can silently exit the state machine. Otherwise, you can continue to init and remove.

@vshankar
Copy link
Contributor Author

vshankar commented Aug 4, 2016

@ceph-jenkins retest this please

@dillaman
Copy link

dillaman commented Aug 5, 2016

@vshankar Passed a QA test suite run -- ready to merge if you tell me you are finished.

Introduce static member function in ImageCtx to fetch (and
initialize) singleton thread pool.

Signed-off-by: Venky Shankar <vshankar@redhat.com>
Signed-off-by: Venky Shankar <vshankar@redhat.com>
Asynchronous removal of journal objects for a given rbd image.

Signed-off-by: Venky Shankar <vshankar@redhat.com>
Signed-off-by: Venky Shankar <vshankar@redhat.com>
Signed-off-by: Venky Shankar <vshankar@redhat.com>
Signed-off-by: Venky Shankar <vshankar@redhat.com>
Introduce asynchronous journal object creation. This is
a prerequisite for asynchronous creation of rbd image
journals.

Signed-off-by: Venky Shankar <vshankar@redhat.com>
Signed-off-by: Venky Shankar <vshankar@redhat.com>
Signed-off-by: Venky Shankar <vshankar@redhat.com>
Signed-off-by: Venky Shankar <vshankar@redhat.com>
... plus changes in rbd class library to use helpers methods.

Signed-off-by: Venky Shankar <vshankar@redhat.com>
create_v2() uses util::generate_image_id() therefore not
requiring 'bid' to be passed in as parameter. This makes
bid generation in create() unnecessary -- hence move this
call to create_v1().

Signed-off-by: Venky Shankar <vshankar@redhat.com>
Use the newly instroduced asynchronous image creation state
machine (CreateRequest) to create mirrored images.

Signed-off-by: Venky Shankar <vshankar@redhat.com>
Signed-off-by: Venky Shankar <vshankar@redhat.com>
Synchronous callers now call the asynchronous version wrapped
around C_SaferCond. Also take care of mocked methods.

Fixes: http://tracker.ceph.com/issues/15321
Signed-off-by: Venky Shankar <vshankar@redhat.com>
@vshankar
Copy link
Contributor Author

vshankar commented Aug 7, 2016

@dillaman Resolved a conflict with the master branch due to commit 4de7c8d ("librbd: prevent creation of v2 image ids that are too large"). I moved the required change to commit 1c82132 ("librbd: helper to generate unique image id). Rest should be good.

@dillaman dillaman merged commit 02d91db into ceph:master Aug 10, 2016
@vshankar vshankar deleted the wip-15321 branch August 10, 2016 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants