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

ceph-disk: make journal setup conditional on osd backend #4238

Merged
12 commits merged into from Apr 6, 2015
Merged

ceph-disk: make journal setup conditional on osd backend #4238

12 commits merged into from Apr 6, 2015

Conversation

liewegas
Copy link
Member

No description provided.

Signed-off-by: Sage Weil <sage@redhat.com>
Completes OSD side of #9580

Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
Copy link
Member Author

@dachary can you take a look at this when you have time?

FWIW we can test these scenarios explicitly by doing 'osd_objectstore = keyvaluestore' and 'enable experimental unrecoverable data corrupting features = keyvaluestore' in ceph.conf. the goal is that for these backends (and memstore, and eventually newstore) ceph-disk will Do The Right Thing and not create any journal

@loic-bot
Copy link

SUCCESS: the output of run-make-check.sh on centos-7 for 4ab60ec is http://paste2.org/bE7z8hnx

:octocat: Sent from GH.

@@ -1487,6 +1507,9 @@ def main_prepare(args):
if stat.S_ISBLK(dmode):
verify_not_in_use(args.data, True)

if args.journal and not allows_journal:
raise Error('journal specified by not allowed by osd backend')
Copy link
Contributor

Choose a reason for hiding this comment

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

s/by/but

- If the journal is required, require it.
- If the journal is not allowed, do not allow one to be specified
- If the journal is not wanted, to not set one up by default when none is
provided.

See #9580

Signed-off-by: Sage Weil <sage@redhat.com>
def check_journal_reqs(args):
_, allows_journal = command([
'ceph-osd', '--check-allows-journal',
'-i', '0',
Copy link
Contributor

Choose a reason for hiding this comment

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

Should all these be hardcoded to 0 ?

I see elsewhere in the script there are calls to ceph-osd that say this argument is ignored...

Copy link
Member Author

@liewegas liewegas Apr 1, 2015 via email

Choose a reason for hiding this comment

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

@ghost
Copy link

ghost commented Apr 1, 2015

I'll add a test to src/test/ceph-disk.sh to verify it works as expected. Since needs_journal is for future use only, do you think it would be better to use needs_journal in ceph-disk or just remove it (and add it when it's needed) ?

@ghost
Copy link

ghost commented Apr 1, 2015

It's probably harmless but it would be less confusing if ceph-osd --mkfs was called without the --osd-journal argument when relevant ( https://ceph.com/git/?p=ceph.git;a=blob;f=src/ceph-disk#l1722 )

@liewegas
Copy link
Member Author

liewegas commented Apr 5, 2015

Dropping the --osd-journal is kind of tedious because we don't have the journal arg/flag in the last few caller frames. :(

@loic-bot
Copy link

loic-bot commented Apr 5, 2015

SUCCESS: the output of run-make-check.sh on centos-7 for 250e4e9 is http://paste2.org/aUJOAdzN

:octocat: Sent from GH.

ldachary and others added 9 commits April 6, 2015 01:58
Signed-off-by: Loic Dachary <ldachary@redhat.com>
The tests explicitly return on error when relevant. Add two error cases:

* detect when the allocation of a loop device fails.
* in the outer loop, return immediately if one of the test fails

Signed-off-by: Loic Dachary <ldachary@redhat.com>
Address all possible failure cases, when ceph-disk.sh completes or when
it starts with leftover from a previous interrupted run. It is assumed
that ceph-disk.sh will crash at any point.

* umount all mount points that belong to ceph-disk.sh (check the
  absolute path of the directory)
* dmsetup remove all device mapper nodes found to hold a loop device
  that ceph-disks.sh no longer uses
* losetup --detach all loop devices that ceph-disks.sh no longer uses

Signed-off-by: Loic Dachary <ldachary@redhat.com>
Move test_activate_dev to test_setup_dev_and_run and make it
run the function given in argument. test_activate_dev calls
test_setup_dev_and_run and no longer needs to implement device
allocation or destruction.

Signed-off-by: Loic Dachary <ldachary@redhat.com>
The activate_dmcrypt_plain_dev_body and activate_dmcrypt_dev_body
functions are almost identical, merge them and differentiate with an
argument.

Signed-off-by: Loic Dachary <ldachary@redhat.com>
Instead of duplicating the device construction / destruction logic for
dmcrypt tests, use test_setup_dev_and_run to do it. It is now able to
recover from devmapper leftover which may occur when a cryptsetup test
fails.

Signed-off-by: Loic Dachary <ldachary@redhat.com>
Add the test_pool_read_write function to share the rados put / get test
that demonstrate the osd that has been created can actually be used. Use
it from the both the regular device and dmcrypt tests.

Signed-off-by: Loic Dachary <ldachary@redhat.com>
Add a test for the activation of the memstore objectstore and verify
that it works without specifying a journal.

Signed-off-by: Loic Dachary <ldachary@redhat.com>
tests for ceph-disk: make journal setup conditional on osd backend
@liewegas liewegas changed the title DNM: ceph-disk: make journal setup conditional on osd backend ceph-disk: make journal setup conditional on osd backend Apr 6, 2015
@loic-bot
Copy link

loic-bot commented Apr 6, 2015

SUCCESS: the output of run-make-check.sh on centos-7 for 29755b3 is http://paste2.org/J37PPdwW

:octocat: Sent from GH.

ghost pushed a commit that referenced this pull request Apr 6, 2015
ceph-disk: make journal setup conditional on osd backend

Reviewed-by: Travis Rhoden <trhoden@redhat.com>
Reviewed-by: Loic Dachary <ldachary@redhat.com>
@ghost ghost merged commit c789407 into master Apr 6, 2015
@shun-s
Copy link
Contributor

shun-s commented Aug 31, 2016

@dachary , i think this pr is very useful in practise, but i doubt wether it will be backported to hammer?
thanks

@liewegas liewegas deleted the wip-9580 branch November 23, 2016 20:11
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants