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

init-ceph: do umount when the path exists. #6866

Merged
1 commit merged into from Dec 13, 2015

Conversation

xiaoxichen
Copy link
Contributor

If the specified mount point is in use, umount it instead
of skipping mounting the fs.

Signed-off-by: Xiaoxi Chen xiaoxi.chen@intel.com

@xiaoxichen
Copy link
Contributor Author

@dachary would you mind take a look?

@ghost
Copy link

ghost commented Dec 10, 2015

@xiaoxichen this is a feature change and I'm not sure that's the desired behaviour. It would be great if you could include a detailed explanation, with example use cases, that demonstrate why this is always the good thing to do, including when the mount point is completely unrelated to Ceph.

@xiaoxichen
Copy link
Contributor Author

@dachary , the use case is,

I think this is always better because in previous code , if we forgot that we already mount something unrelated to *this ceph osd * to the particular mount point, we will skip the mount and finally get an error complaining superblock not matching OSD ID. umount & remount is better because

  1. If the wrong FS not in use, we can get the right FS we want and make ceph boot smoothly.
  2. If the wrong FS is in use, we will get EBUSY on umount, which seems explain the situation more clearly than superblock mismatch.

Another use case is.
in our own lab and some of the customers environment, we do see the disk lable(/dev/sdX) changed after plug in/plug out disk , or reboot. So we would prefer use by-path to lable a disk.

But actually by-path is a symbol link to /dev/sdX,
root@bigmem:/# ls -l /dev/disk/by-path
total 0
lrwxrwxrwx 1 root root 9 Sep 28 16:07 pci-0000:03:00.0-sas-0x5003048003f3be00-lun-0 -> ../../sda
lrwxrwxrwx 1 root root 10 Sep 16 14:26 pci-0000:03:00.0-sas-0x5003048003f3be00-lun-0-part1 -> ../../sda1
lrwxrwxrwx 1 root root 10 Sep 16 14:26 pci-0000:03:00.0-sas-0x5003048003f3be00-lun-0-part2 -> ../../sda2

so after mounting, we will see

/dev/sda2 146412700 398220 146014480 1% /var/lib/ceph/mnt/osd-device-5-data

Now if we have some failure that make the disk lable changed(external bay power reset, for example), the init script could help us to mount the right disk to right place.

@ghost ghost self-assigned this Dec 11, 2015
@ghost
Copy link

ghost commented Dec 11, 2015

This makes sense to me, could you amend the commit message to include your explanation ? Did you run a teuthology-test to verify it does what is expected ?

@xiaoxichen
Copy link
Contributor Author

@dachary ,commit message updated.

I didn't do a teuthology-test but test it manually.. If that is necessary, could you pls guide me to the test suite? And maybe we need a new testcase that mount an dump fs before upstart, so this part of code can be exercised ?

If the specified mount point is in use, umount it instead
of skipping mounting the fs.

In previous code , if we forgot that we already mount something unrelated to
*this ceph osd * to the particular mount point, we will skip the mount and
finally get an error complaining superblock not matching OSD ID.

umount & remount is better because
1. If the wrong FS not in use, we can get the right FS we want and make ceph boot smoothly.
2. If the wrong FS is in use, we will get EBUSY on umount, which seems explain the situation
more clearly than superblock mismatch.

Signed-off-by: Xiaoxi Chen <xiaoxi.chen@intel.com>
@ghost
Copy link

ghost commented Dec 13, 2015

@xiaoxichen please ignore the bot false negative (see http://tracker.ceph.com/issues/14060 for more details).

ghost pushed a commit that referenced this pull request Dec 13, 2015
init-ceph: do umount when the path exists.

Reviewed-by: Loic Dachary <ldachary@redhat.com>
@ghost ghost merged commit 09d7a60 into ceph:master Dec 13, 2015
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
3 participants