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

build: remove ceph-disk-udev entirely #15259

Merged
merged 1 commit into from May 30, 2017

Conversation

Projects
None yet
4 participants
@zealoussnow
Contributor

zealoussnow commented May 24, 2017

Signed-off-by: Leo Zhang nguzcf@gmail.com

@zealoussnow

This comment has been minimized.

Show comment
Hide comment
@zealoussnow

zealoussnow May 24, 2017

Contributor

ceph-disk-udev is not used now.

Contributor

zealoussnow commented May 24, 2017

ceph-disk-udev is not used now.

@tchaikov

This comment has been minimized.

Show comment
Hide comment
@tchaikov

tchaikov May 24, 2017

Contributor

@zealoussnow could you explain the rationale behind this change in the commit message?

Contributor

tchaikov commented May 24, 2017

@zealoussnow could you explain the rationale behind this change in the commit message?

@tchaikov tchaikov added the build/ops label May 24, 2017

@zealoussnow

This comment has been minimized.

Show comment
Hide comment
@zealoussnow

zealoussnow May 24, 2017

Contributor

@tchaikov When I read the udev rules, I found udev will generate some symbol link in /dev/disk/by-parttypeuuid and use ceph-disk mount the osd data partition while system reboot. But don't use ceph-disk-udev, when I remove it, udev works fine as normal. So I committed this PR.

Contributor

zealoussnow commented May 24, 2017

@tchaikov When I read the udev rules, I found udev will generate some symbol link in /dev/disk/by-parttypeuuid and use ceph-disk mount the osd data partition while system reboot. But don't use ceph-disk-udev, when I remove it, udev works fine as normal. So I committed this PR.

@tchaikov

This comment has been minimized.

Show comment
Hide comment
@tchaikov

tchaikov May 24, 2017

Contributor

@zealoussnow this rule is supposed to along with ceph-disk. have you tested ceph-disk as well? or, i'd recommend you read through ceph-disk code to understand the parttypeuuid is used by it.

Contributor

tchaikov commented May 24, 2017

@zealoussnow this rule is supposed to along with ceph-disk. have you tested ceph-disk as well? or, i'd recommend you read through ceph-disk code to understand the parttypeuuid is used by it.

@zealoussnow

This comment has been minimized.

Show comment
Hide comment
@zealoussnow

zealoussnow May 24, 2017

Contributor

@tchaikov I tested ceph-disk right now, it can work without ceph-disk-udev. And I found the parttypeuuid in /dev/disk is generated by rule 60-ceph-by-parttypeuuid.rules.

Contributor

zealoussnow commented May 24, 2017

@tchaikov I tested ceph-disk right now, it can work without ceph-disk-udev. And I found the parttypeuuid in /dev/disk is generated by rule 60-ceph-by-parttypeuuid.rules.

@tchaikov

This comment has been minimized.

Show comment
Hide comment
@tchaikov

tchaikov May 24, 2017

Contributor

@zealoussnow thanks for testing it. i don't want to block you, but as you may have already figured out that i am not familiar with this area =(. probably we need to wait @dachary for more insights.

Contributor

tchaikov commented May 24, 2017

@zealoussnow thanks for testing it. i don't want to block you, but as you may have already figured out that i am not familiar with this area =(. probably we need to wait @dachary for more insights.

@tchaikov tchaikov requested a review from May 24, 2017

@tchaikov tchaikov added the cleanup label May 24, 2017

@zealoussnow

This comment has been minimized.

Show comment
Hide comment
@zealoussnow

zealoussnow May 24, 2017

Contributor

@tchaikov OK, thank you.

Contributor

zealoussnow commented May 24, 2017

@tchaikov OK, thank you.

@ghost

Good catch ! ceph-disk-udev is indeed obsolete. Would you mind removing it entirely ? And all references to it as well ? git grep ceph-disk-udev will show you where they are.

@zealoussnow

This comment has been minimized.

Show comment
Hide comment
@zealoussnow

zealoussnow May 24, 2017

Contributor

@dachary Thank you, I finished it now.

Contributor

zealoussnow commented May 24, 2017

@dachary Thank you, I finished it now.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost May 24, 2017

Also please remove ceph-disk-udev itself and squash the two commits into a single one. Thanks !

ghost commented May 24, 2017

Also please remove ceph-disk-udev itself and squash the two commits into a single one. Thanks !

@@ -944,7 +944,6 @@ install(PROGRAMS
install(PROGRAMS
${CMAKE_SOURCE_DIR}/src/ceph-create-keys
# ${CMAKE_SOURCE_DIR}/src/ceph-disk
${CMAKE_SOURCE_DIR}/src/ceph-disk-udev

This comment has been minimized.

@tchaikov

tchaikov May 24, 2017

Contributor

can we just remove src/ceph-disk-udev from our source tree, as it is not used anymore?

@tchaikov

tchaikov May 24, 2017

Contributor

can we just remove src/ceph-disk-udev from our source tree, as it is not used anymore?

build: remove ceph-disk-udev entirely
Signed-off-by: Leo Zhang <nguzcf@gmail.com>

@zealoussnow zealoussnow changed the title from build: remove ceph-disk-udev from spec to build: remove ceph-disk-udev entirely May 24, 2017

@zealoussnow

This comment has been minimized.

Show comment
Hide comment
@zealoussnow

zealoussnow May 24, 2017

Contributor

@dachary @tchaikov All is OK.

Contributor

zealoussnow commented May 24, 2017

@dachary @tchaikov All is OK.

@ghost

ghost approved these changes May 24, 2017

@yuriw yuriw merged commit 805acce into ceph:master May 30, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@zealoussnow zealoussnow deleted the zealoussnow:wip-0524 branch May 30, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment