rgw nfs v3 completions #10745
rgw nfs v3 completions #10745
Conversation
Allow passing POSIX open flags as well as api call flags. Needed for NFS3 support. Signed-off-by: Matt Benjamin <mbenjamin@redhat.com>
Allow passing POSIX open flags as well as api call flags. Needed for NFS3 support. Signed-off-by: Matt Benjamin <mbenjamin@redhat.com>
| @@ -220,9 +221,10 @@ int rgw_truncate(struct rgw_fs *rgw_fs, | |||
| */ | |||
| #define RGW_OPEN_FLAG_NONE 0x0000 | |||
| #define RGW_OPEN_FLAG_CREATE 0x0001 | |||
| #define RGW_OPEN_FLAG_V3 0x0002 /* ops have v3 semantics */ | |||
There was a problem hiding this comment.
"V3" is not hugely meaningful to anyone not doing NFS work. Maybe "STATELESS"?
There was a problem hiding this comment.
I used that elsewhere, so could do that.
| if (! (flags & FLAG_LOCKED)) { | ||
| guard.lock(); | ||
| } | ||
|
|
There was a problem hiding this comment.
This is bad style. A unique_lock is the lock and flag all in one, if the function expects to be called with the lock both locked and not locked, it should take a unique_lock by reference.
There was a problem hiding this comment.
I don't really want a unique_lock propagating out to rgw_* callers
|
Looks pretty good so far, apart from the one locking thing. |
Implements a temporal mechanism to enforce write completion for setups which lack open state tracking (e.g., NFS3). Signed-off-by: Matt Benjamin <mbenjamin@redhat.com>
Signed-off-by: Matt Benjamin <mbenjamin@redhat.com>
Signed-off-by: Matt Benjamin <mbenjamin@redhat.com>
Signed-off-by: Matt Benjamin <mbenjamin@redhat.com>
mattbenjamin
force-pushed
the
rgw-nfs-31
branch
from
eaa7b49
to
70dad0f
Compare
|
Removed an empty branch and bogus WS from the main commit. |
|
It appears this doesn't need a rebase, and is isolated. |
| struct rgw_file_handle **fh, uint32_t flags); | ||
| const char *name, struct stat *st, uint32_t mask, | ||
| struct rgw_file_handle **fh, uint32_t posix_flags, | ||
| uint32_t flags); |
There was a problem hiding this comment.
do we need some kind of versioning for changes to this public api?
There was a problem hiding this comment.
Do you think a #define would be sufficient?
This change borrows the major, minor+extra format used by libcephfs. The version numbering is starting at 1,1,0 on the theory that the implicit version at Jewel is 1,0,0. Signed-off-by: Matt Benjamin <mbenjamin@redhat.com>
|
|
||
| int commit(uint64_t offset, uint64_t length, uint32_t flags) { | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
just a stub for later? we might want to return ENOTSUPP for now, instead of returning success while there could still be unstable writes
There was a problem hiding this comment.
I wouldn't. If you give NFS3ERR_NOTSUPP in response to COMMIT you'll make the kernel freak out and probably record an item in non-volatile memory to remind itself not to talk to that server again.
There was a problem hiding this comment.
yeah, atm, I'm just passing this result up through ganesha, which would have the result adam describes; we -could- return ENOTSUP here if we strongly prefer it, but then the rgw fsal would need to translate it to this
This change replaces the existing member-hook typedef as well as the new set-type typedefs, so committed separately. Signed-off-by: Matt Benjamin <mbenjamin@redhat.com>
Add a comment explaining why the method currently returns 0 unconditionally. Signed-off-by: Matt Benjamin <mbenjamin@redhat.com>
No description provided.