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

libcephfs: add unmount function in cephfs.pyx #10774

Merged
merged 2 commits into from Sep 6, 2016

Conversation

renhwztetecs
Copy link
Contributor

we need unmount function when change the mount point

Signed-off-by: huanwen ren ren.huanwen@zte.com.cn

@xiexingguo xiexingguo added feature cephfs Ceph File System labels Aug 18, 2016
@xiexingguo
Copy link
Member

The failure is true:

Error compiling Cython file:
------------------------------------------------------------
...
            raise make_ex(ret, "error calling ceph_mount")
        self.state = "mounted"

    def unmount(self):
        if self.state == "mounted":
        with nogil:
       ^
------------------------------------------------------------

cephfs.pyx:485:8: Expected an increase in indentation level
Compiling cephfs.pyx because it changed.
Cythonizing cephfs.pyx
Traceback (most recent call last):
  File "/home/jenkins-build/build/workspace/ceph-pull-requests/src/pybind/cephfs/setup.py", line 189, in <module>
    os.path.join(os.path.dirname(__file__), "..", "rados")
  File "/usr/lib/python2.7/dist-packages/Cython/Build/Dependencies.py", line 798, in cythonize
    cythonize_one(*args[1:])
  File "/usr/lib/python2.7/dist-packages/Cython/Build/Dependencies.py", line 915, in cythonize_one
    raise CompileError(None, pyx_file)
Cython.Compiler.Errors.CompileError: cephfs.pyx
make[3]: *** [src/pybind/cephfs/CMakeFiles/cython_cephfs] Error 1
make[2]: *** [src/pybind/cephfs/CMakeFiles/cython_cephfs.dir/all] Error 2

def release_module():
global cephfs
cephfs = libcephfs.LibCephFS(conffile='')
cephfs.unmount()
Copy link
Contributor

Choose a reason for hiding this comment

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

Surely you don't mean to construct cephfs and then call unmount on it immediately?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah,I think I understand the mistake before

@jcsp
Copy link
Contributor

jcsp commented Aug 29, 2016

The "add unmount test" commit doesn't really seem to add a test, it just calls unmount at the end of existing test, right? Please could you add a test that mounts, does something, unmounts, then mounts again (using the same LibCephFS instance) and shows that the LibCephFS instance still works properly after the second mount.

@renhwztetecs
Copy link
Contributor Author

renhwztetecs commented Aug 30, 2016

add test_mount_unmount() test

we need unmount function when change the mount point

Signed-off-by: huanwen ren <ren.huanwen@zte.com.cn>
cephfs.unmount()
cephfs.mount()
test_open()

Copy link
Contributor

@ajarr ajarr Sep 2, 2016

Choose a reason for hiding this comment

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

Please remove the extra blank lines at the end.

@ajarr
Copy link
Contributor

ajarr commented Sep 2, 2016

@renhwztetecs , besides the comments looks OK.

@renhwztetecs
Copy link
Contributor Author

@aakso
all done,thanks

test_directory()
cephfs.unmount()
cephfs.mount()
test_open()
Copy link
Contributor

@ajarr ajarr Sep 6, 2016

Choose a reason for hiding this comment

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

@renhwztetecs, the line still has a trailing whitespace.

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 change it.

@ajarr
Copy link
Contributor

ajarr commented Sep 6, 2016

Jenkins has failed due to,

Could NOT find Python3Interp: Found unsuitable version "2.7.5", but
required is at least "3" (found /usr/bin/python)
Call Stack (most recent call first):
/usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:313 (_FPHSA_FAILURE_MESSAGE)
cmake/modules/FindPython3Interp.cmake:146 (FIND_PACKAGE_HANDLE_STANDARD_ARGS)
src/CMakeLists.txt:203 (find_package)

as already noted in http://www.spinics.net/lists/ceph-devel/msg32397.html

I tested the PR locally. The included test ran successfully.
http://pastebin.com/t1tNubvW

@jcsp, the test looks OK to you?

Signed-off-by: huanwen ren <ren.huanwen@zte.com.cn>
@ajarr
Copy link
Contributor

ajarr commented Sep 6, 2016

LGTM

@jcsp jcsp merged commit 9fda6d1 into ceph:master Sep 6, 2016
@renhwztetecs renhwztetecs deleted the renhw-wip-cephfs-python-umount branch September 6, 2016 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cephfs Ceph File System feature
Projects
None yet
4 participants