From 689927b0e5d078fc16778ef4a144def7226f8a4c Mon Sep 17 00:00:00 2001 From: dparmar18 Date: Fri, 8 Apr 2022 14:28:48 +0530 Subject: [PATCH] cephfs-shell: get cmd must get both path and should validate them Description: - While using `get` command, `local_path` parameter is optional. Changing it to mandatory. - Rationale: Till now, there used to be a default path of `local_path` as `default='.'` but wasn't mentioned anywhere. It led to confusion. On top of it, considering get command to be a ssh inspired utlity, or any other CLI tool that copies file between filesystems, source and destination path are always mandatory. Therefore in order to simulate this behavior in cephfs-shell`s command(s), my opinion is to make get command accept both the paths. - Added checks to make sure: 1) File does exist at `remote_path` 2) File with the same name doesn't exist in `local_path` 3) Removed code that would run through the directory and if it finds nothing in `root_src_dir`, then it will try to do: `os.makedirs(root_dst_dir + b'/' + root_src_dir)`, but it will never be empty as 1) takes care of it. Fixes: https://tracker.ceph.com/issues/55216 Signed-off-by: dparmar18 --- src/tools/cephfs/cephfs-shell | 40 ++++++++++++++--------------------- 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/src/tools/cephfs/cephfs-shell b/src/tools/cephfs/cephfs-shell index 8ab719422db..cc1c6be25d3 100755 --- a/src/tools/cephfs/cephfs-shell +++ b/src/tools/cephfs/cephfs-shell @@ -716,12 +716,11 @@ class CephFSShell(Cmd): return self.complete_filenames(text, line, begidx, endidx) get_parser = argparse.ArgumentParser( - description='Copy a file from Ceph File System from Local Directory.') + description='Copy a file from Ceph File System to Local Directory.') get_parser.add_argument('remote_path', type=str, action=path_to_bytes, help='Path of the file in the remote system') get_parser.add_argument('local_path', type=str, action=path_to_bytes, - help='Path of the file in the local system', - nargs='?', default='.') + help='Path of the file in the local system') get_parser.add_argument('-f', '--force', action='store_true', help='Overwrites the destination if it already exists.') @@ -730,6 +729,19 @@ class CephFSShell(Cmd): """ Copy a file/directory from CephFS to given path. """ + if not is_file_exists(args.remote_path) and not \ + is_dir_exists(args.remote_path): + set_exit_code_msg(errno.ENOENT, "error: no file/directory" + " found at specified remote " + "path") + return + if (os.path.isfile(args.local_path) or os.path.isdir( + args.local_path)) and not args.force: + set_exit_code_msg(msg=f"error: file/directory " + f"{args.local_path.decode('utf-8')}" + f" already exists, use --force to " + f"overwrite") + return root_src_dir = args.remote_path root_dst_dir = args.local_path fname = root_src_dir.rsplit(b'/', 1) @@ -748,17 +760,6 @@ class CephFSShell(Cmd): copy_to_local(root_src_dir, root_dst_dir) else: files = list(reversed(sorted(dirwalk(root_src_dir)))) - if len(files) == 0: - try: - os.makedirs(root_dst_dir + b'/' + root_src_dir) - except OSError as e: - if args.force: - pass - else: - set_exit_code_msg(e.errno, f"{root_src_dir.decode('utf-8')}: " - "already exists! use --force to overwrite") - return - for file_ in files: dst_dirpath, dst_file = file_.rsplit(b'/', 1) if dst_dirpath in files: @@ -771,16 +772,7 @@ class CephFSShell(Cmd): except OSError: pass else: - if not args.force: - try: - os.stat(dst_path) - set_exit_code_msg(errno.EEXIST, f"{file_.decode('utf-8')}: " - "file already exists! use --force to override") - return - except OSError: - copy_to_local(file_, dst_path) - else: - copy_to_local(file_, dst_path) + copy_to_local(file_, dst_path) return 0 -- 2.39.5