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

fs/ceph-fuse: normalize file open flags on the wire #14822

Merged
merged 1 commit into from May 3, 2017

Conversation

Projects
None yet
4 participants
@jan--f
Member

jan--f commented Apr 27, 2017

The file open flags (O_foo) are platform specific and should never go
out to an interface that is not local to the system.
Otherwise this leads to bogus flags getting transmitted on ppc64.

Signed-off-by: Jan Fajerski jfajerski@suse.com

#define CEPH_O_CLOEXEC 02000000
#define CEPH_O_SYNC 04000000
#define CEPH_O_PATH 010000000
#define CEPH___O_TMPFILE 020000000

This comment has been minimized.

@ukernel

ukernel Apr 27, 2017

Member

flags such as O_CLOEXEC control client's behaviour. It does not make sense to send them to mds. please only define RDONLY, WRONLY, RDWR, CREAT, EXCL, TRUNC, DIRECTORY, NOFOLLOW

@batrick

This comment has been minimized.

Member

batrick commented Apr 27, 2017

What about the MDS side? e.g.:

bool need_auth = !file_mode_is_readonly(cmode) || (flags & O_TRUNC);

Should use CEPH_O_TRUNC?

@jan--f

This comment has been minimized.

Member

jan--f commented Apr 27, 2017

@batrick The MDS should probably only compare the the CEPH_O_* flags. File interactions from the MDS side probably never go into a network message right?

I was planing on tackling this as a separate PR.

@batrick

This comment has been minimized.

Member

batrick commented Apr 27, 2017

File interactions from the MDS side probably never go into a network message right?

What do you mean?

I was planing on tackling this as a separate PR.

Since they are related, I would prefer to see the MDS changes in the same PR.

@jan--f

This comment has been minimized.

Member

jan--f commented Apr 27, 2017

File interactions from the MDS side probably never go into a network message right?

Sorry that was a bit hasty. My assumption is that flags from a client message will never be used for MDS-local file io. So there is no need to pay attention to when the normalized CEPH_O_* flags are used vs the O_* flags of the MDS' architecture. We can simply use CEPH_O_* across the board and don't do any conversion from CEPH_O_* to the whatever the architecture defines as O_*.

Just trying to make sure my assumption is correct.

@batrick

This comment has been minimized.

Member

batrick commented Apr 27, 2017

Sorry that was a bit hasty. My assumption is that flags from a client message will never be used for MDS-local file io.

For RADOS, I believe that's true. (Of course, the MDS doesn't do any local file IO for client req)

So there is no need to pay attention to when the normalized CEPH_O_* flags are used vs the O_* flags of the MDS' architecture. We can simply use CEPH_O_* across the board and don't do any conversion from CEPH_O_* to the whatever the architecture defines as O_*.

That would be correct.

@jan--f

This comment has been minimized.

Member

jan--f commented Apr 28, 2017

@batrick Care to have another look?

@batrick

Just one small holdout: https://github.com/ceph/ceph/pull/14822/files#diff-eecf840e09d7481f7b8f415789852c7dR2949

and the same in openc. Now that I see where ceph_flags_to_mode is defined in common/ceph_fs.cc and include/ceph_fs.h, it would be good to move your Client ceph_flags_sys2wire into ceph_fs.cc, merge ceph_oflags.h into ceph_fs.h, and adjust the ceph_flags_to_mode function to use your new CEPH_O defines.

@jan--f

This comment has been minimized.

Member

jan--f commented Apr 28, 2017

Right that sounds reasonable. It'll require some changes in Client.cc though, since ceph_flags_to_mode is used there too.

fs: normalize file open flags internally used by cephfs
The file open flags (O_foo) are platform specific. Normalize these flags
before they are send to the MDS. For processing of client messages the
MDS should only compare to these normalized flags.
Otherwise this can lead to bogus flags getting transmitted on ppc64.

Signed-off-by: Jan Fajerski <jfajerski@suse.com>
@batrick

This comment has been minimized.

Member

batrick commented Apr 28, 2017

Reviewed-by: Patrick Donnelly <pdonnell@redhat.com>

@ukernel ukernel merged commit d0b3d4a into ceph:master May 3, 2017

2 of 3 checks passed

default Build triggered. sha1 is merged.
Details
Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
@batrick

This comment has been minimized.

Member

batrick commented May 9, 2017

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