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

jewel: ceph-disk: don't activate suppressed journal devices #16703

Merged
merged 1 commit into from Sep 7, 2017

Conversation

Projects
None yet
5 participants
@ddiss
Contributor

ddiss commented Jul 31, 2017

ceph-disk: don't activate suppressed journal devices
Multipath device activation currently requires that devices representing
individual paths making up a multipath device are suppressed, to avoid
errors such as:
  ceph_disk.main.Error: Error: /dev/sda1 is not a multipath block device

Activation of suppressed OSD devices is currently handled by checks in
main_activate() and main_activate_space(). However, suppressed journal
and block devices are not detected.

(Along with udev change) Fixes: http://tracker.ceph.com/issues/19489

Signed-off-by: David Disseldorp <ddiss@suse.de>
(cherry picked from commit c729627)

@tchaikov tchaikov added this to the jewel milestone Jul 31, 2017

@smithfarm smithfarm changed the title from ceph-disk: don't activate suppressed journal devices to jewel: ceph-disk: don't activate suppressed journal devices Jul 31, 2017

@liu-chunmei

similar code is already exist in master branch, but not in the same place, several lines below, please check again.

@ddiss

This comment has been minimized.

Show comment
Hide comment
@ddiss

ddiss Aug 8, 2017

Contributor

@liu-chunmei the is_suppressed(path) line lower down checks for OSD device suppression, this change adds a check for journal device suppression.
Are you asking me to change something else here? I don't quite understand your comment.

Contributor

ddiss commented Aug 8, 2017

@liu-chunmei the is_suppressed(path) line lower down checks for OSD device suppression, this change adds a check for journal device suppression.
Are you asking me to change something else here? I don't quite understand your comment.

@liu-chunmei

This comment has been minimized.

Show comment
Hide comment
@liu-chunmei

liu-chunmei Aug 8, 2017

Contributor

@ddiss seems code is already merged in master branch, please take a look on the latest version.

Contributor

liu-chunmei commented Aug 8, 2017

@ddiss seems code is already merged in master branch, please take a look on the latest version.

@ddiss

This comment has been minimized.

Show comment
Hide comment
@ddiss

ddiss Aug 8, 2017

Contributor

Yes, this change is already in the master branch. This pull request is a backport for Jewel.

Contributor

ddiss commented Aug 8, 2017

Yes, this change is already in the master branch. This pull request is a backport for Jewel.

@ddiss

This comment has been minimized.

Show comment
Hide comment
@ddiss

ddiss Aug 16, 2017

Contributor

Ping, any updates here?

Contributor

ddiss commented Aug 16, 2017

Ping, any updates here?

@@ -3789,6 +3789,10 @@ def main_activate_space(name, args):
if not os.path.exists(args.dev):
raise Error('%s does not exist' % args.dev)
if is_suppressed(args.dev):

This comment has been minimized.

@amitkumar50

amitkumar50 Aug 31, 2017

Contributor

I am sorry. If its already in master on latest clone. this should be Mergerd/closed.

@amitkumar50

amitkumar50 Aug 31, 2017

Contributor

I am sorry. If its already in master on latest clone. this should be Mergerd/closed.

@smithfarm

This comment has been minimized.

Show comment
Hide comment
@smithfarm

smithfarm Sep 7, 2017

Contributor

This passed a ceph-disk suite at http://tracker.ceph.com/issues/20613#note-55

Contributor

smithfarm commented Sep 7, 2017

This passed a ceph-disk suite at http://tracker.ceph.com/issues/20613#note-55

@smithfarm smithfarm merged commit 3aede76 into ceph:jewel Sep 7, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment