remove 'struct'

@rpavani1998

Thanks! It is Resolved! @ajarr

Where do you read from the local file? 'open' just returns the file object, right?
And I think you'd want open in binary mode.

You'd want to read in byte chunks (size 4MB? not sure about the size) from local file and write that
byte chunk into the CephFS file. Finally close the fd of the local file and the CephFS file.

Note that you can't read the entire contents of a very large file at once. What if the file size is much bigger than the machine's memory?

I think 'get' should also be able to write to stdout

See do_get() in https://github.com/ceph/ceph/blob/master/src/tools/rados/rados.cc

You should also be read from stdin and write to CephFS.

See do_put() in https://github.com/ceph/ceph/blob/master/src/tools/rados/rados.cc

Need changes similar to the one requested for copy_from_local

Also where do you close the filehandle that you opened?

you don't want to hard code the ceph.conf path

ls [option] [path...]

cat filename...

opendir -> cd

we can remove this

make sure to add any new commands to the docs

sure, I will update it.

Add support for variable number of arguments, e.g. mkdir dir1 dir2.

The parser should be instantiated only once since it is the same for every call to mkdir. You can make this a class variable like:

    mkdir_parser = None
    def do_mkdir(self,arglist):
        if mkdir_parse is None:
            self.parser = argparse.ArgumentParser(description='Create Directory.')
            ...

This is fine but you should provide a way to set the umask(3) of the shell session. Unfortunately we haven't written a binding yet for setting the umask in libcephfs.c. I have created an issue:

http://tracker.ceph.com/issues/24430

I suggest spending a little time familiarizing yourself with the umask command. We can discuss how to implement it tomorrow.

I am actually struck I am unable to figure out where to add the default directory and file permissions? Is this the correct location?

No, that would be in your shell.

Is this correct?

Yes, that's a suitable default.

This should be a #define DEFAULT_UMASK. The variable will be stored in struct ceph_mount_info.

Okay.

please remove this change

This should be a #define DEFAULT_UMASK. The variable will be stored in struct ceph_mount_info.

so this will be this->mask = mask;.

You will need to add a callback function to struct ceph_mount_info. We can cover that today in our meeting.

This should be cephfs.stat? I think you're leaking a file descriptor returned from get_file.

Also, fd is not the right name for the variable. Instead, use buf or info.

Line is too long.

Line break is incorrect. Don't know how Python code is handled in this subproject, but in the Ceph Dashboard we have a linter that makes sure the Python code is structured correct.

You can sinply use File {}: {} here.

No need to use ``{x}```if the format parameters are ordered correct.

Missing blank between rsplit function args.

Add a line break after """ and start the method description in a new line.

Missing blank after a comma.

s/file_/file/

Please start method description in a new line. Check the whole code for that.

In my opinion all arguments should be defined somewhere at the top of the file and not distributed accross the whole file.

Is close(fd) sufficient here? Do you also need to fsync?

Why would it be necessary to fsync?

Suppose we open the file and get Fb caps. We write a bunch of data to the cache and then ^c out of the shell program. That data may never be written back. It's also possible in the same situation that there could be a writeback error out of the cache, and that may not (or may?) be reported at close. Either way, we're not checking the error on close here so I'm not sure we'd see it and deal with that properly if it does happen.

To be clear, I think we ought to explicitly fsync here before closing the file in most cases. If that's not what we want then we may need to wire up a "fsync" (and/or syncfs) operation into this tool as well.

Also, we should probably consider doing this for metadata operations too. chmod/chown type operations, truncates, time changes, etc. All of that stuff can be cached locally by the client if it has the right caps, and I think we want this tool to flush these changes to the cluster before returning from each command, for the most part.

Looking over the code in more detail, I think close basically does flush dirty data and release caps back to the server, so as long as we know that we only have the file open once within the process, we should be ok without fsyncing. Note that this differs from normal POSIX semantics where that is not at all guaranteed.

Suppose we open the file and get Fb caps. We write a bunch of data to the cache and then ^c out of the shell program.

I think we can catch SIGINT and shut the mount down properly. I don't like the idea of adding the latency of fsync to every file put (especially for recursive put).

Looking over the code in more detail, I think close basically does flush dirty data and release caps back to the server, so as long as we know that we only have the file open once within the process, we should be ok without fsyncing. Note that this differs from normal POSIX semantics where that is not at all guaranteed.

I'm not happy to learn that ceph_close performs an fsync :(

Well, you could just fsync at the end of a recursive put so that might not be quite so bad.

Note too that cached metadata ops could also be a thing here too if you have the right caps. So stuff like a truncate may not always get issued back to the server.

That said, suppose you get something like a SIGKILL or SIGSEGV. You might think that the ops you did first were persisted on storage, but that's not necessarily going to be the case, depending on the activity.

I'm fine with allowing these ops to be done in the client-side cache, but if you're going to do that, then you probably ought to wire up fsync and/or syncfs into this tool to give the caller some way to forcibly flush all pending deltas out. Then be sure to document that anyone using it should issue those calls to ensure that things are persisted correctly.

Adding a sync call is reasonable.