From f435606a16fdbed56f9724cf3901f4b7a73c8ca5 Mon Sep 17 00:00:00 2001 From: Casey Bodley Date: Wed, 14 Sep 2011 11:49:34 -0400 Subject: [PATCH] check write verifiers on COMMIT we were previously only verifying that the server didn't reboot between WRITEs. COMMIT returns a verifier that needs to be checked as well Signed-off-by: Casey Bodley --- daemon/nfs41_ops.c | 4 +++- daemon/nfs41_ops.h | 11 +++-------- daemon/nfs41_types.h | 6 ++++++ daemon/nfs41_xdr.c | 2 +- daemon/pnfs_io.c | 23 ++++++++++++++++------- daemon/readwrite.c | 7 ++++++- daemon/util.c | 11 +++++++++++ daemon/util.h | 4 +++- 8 files changed, 49 insertions(+), 19 deletions(-) diff --git a/daemon/nfs41_ops.c b/daemon/nfs41_ops.c index 9d4e624..f7101e0 100644 --- a/daemon/nfs41_ops.c +++ b/daemon/nfs41_ops.c @@ -840,7 +840,8 @@ int nfs41_commit( IN nfs41_path_fh *file, IN uint64_t offset, IN uint32_t count, - IN bool_t do_getattr) + IN bool_t do_getattr, + OUT nfs41_write_verf *verf) { int status; nfs41_compound compound; @@ -872,6 +873,7 @@ int nfs41_commit( compound_add_op(&compound, OP_COMMIT, &commit_args, &commit_res); commit_args.offset = offset; commit_args.count = count; + commit_res.verf = verf; /* send a GETATTR request to update the attribute cache, * but not if we're talking to a data server! */ diff --git a/daemon/nfs41_ops.h b/daemon/nfs41_ops.h index 88146e2..4af5027 100644 --- a/daemon/nfs41_ops.h +++ b/daemon/nfs41_ops.h @@ -335,7 +335,7 @@ typedef struct __nfs41_commit_args { typedef struct __nfs41_commit_res { uint32_t status; - unsigned char writeverf[NFS4_VERIFIER_SIZE]; + nfs41_write_verf *verf; } nfs41_commit_res; @@ -825,12 +825,6 @@ enum stable_how4 { FILE_SYNC4 = 2 }; -typedef struct __nfs41_write_verf { - unsigned char verf[NFS4_VERIFIER_SIZE]; - unsigned char expected[NFS4_VERIFIER_SIZE]; - enum stable_how4 committed; -} nfs41_write_verf; - typedef struct __nfs41_write_args { stateid_arg *stateid; /* -> nfs41_op_open_res_ok.stateid */ uint64_t offset; @@ -1079,7 +1073,8 @@ int nfs41_commit( IN nfs41_path_fh *file, IN uint64_t offset, IN uint32_t count, - IN bool_t do_getattr); + IN bool_t do_getattr, + OUT nfs41_write_verf *verf); int nfs41_lock( IN nfs41_session *session, diff --git a/daemon/nfs41_types.h b/daemon/nfs41_types.h index 93108dd..ae9867c 100644 --- a/daemon/nfs41_types.h +++ b/daemon/nfs41_types.h @@ -77,6 +77,12 @@ typedef struct __nfs41_readdir_cookie { unsigned char verf[NFS4_VERIFIER_SIZE]; } nfs41_readdir_cookie; +typedef struct __nfs41_write_verf { + unsigned char verf[NFS4_VERIFIER_SIZE]; + unsigned char expected[NFS4_VERIFIER_SIZE]; + enum stable_how4 committed; +} nfs41_write_verf; + typedef struct __netaddr4 { char netid[NFS41_NETWORK_ID_LEN+1]; char uaddr[NFS41_UNIVERSAL_ADDR_LEN+1]; diff --git a/daemon/nfs41_xdr.c b/daemon/nfs41_xdr.c index 2f6fff8..eecf1e9 100644 --- a/daemon/nfs41_xdr.c +++ b/daemon/nfs41_xdr.c @@ -1302,7 +1302,7 @@ static bool_t decode_op_commit( return FALSE; if (res->status == NFS4_OK) - return xdr_opaque(xdr, (char *)res->writeverf, NFS4_VERIFIER_SIZE); + return xdr_opaque(xdr, (char *)res->verf->verf, NFS4_VERIFIER_SIZE); return TRUE; } diff --git a/daemon/pnfs_io.c b/daemon/pnfs_io.c index 8c18690..b83df3c 100644 --- a/daemon/pnfs_io.c +++ b/daemon/pnfs_io.c @@ -421,14 +421,20 @@ retry_write: dprintf(1, "sending COMMIT to data server for offset=%lld len=%lld\n", commit_min, commit_max - commit_min); nfsstat = nfs41_commit(client->session, commit_file, - commit_min, (uint32_t)(commit_max - commit_min), 0); + commit_min, (uint32_t)(commit_max - commit_min), 0, &verf); - /* on successful commit, leave pnfs_status unchanged; if the layout - * was recalled, we still want to return the error */ - if (nfsstat == NFS4_OK) - thread->stable = DATA_SYNC4; - else + if (nfsstat) status = map_ds_error(nfsstat, pattern->state); + else if (!verify_commit(&verf)) { + /* resend the writes unless the layout was recalled */ + if (status != PNFSERR_LAYOUT_RECALLED) + goto retry_write; + status = PNFSERR_IO; + } else { + /* on successful commit, leave pnfs_status unchanged; if the + * layout was recalled, we still want to return the error */ + thread->stable = DATA_SYNC4; + } out: dprintf(IOLVL, "<-- file_layout_write_thread(%u) returning %s\n", thread->id, pnfs_error_string(status)); @@ -550,10 +556,13 @@ enum pnfs_status pnfs_write( goto out_free_pattern; if (stable == UNSTABLE4) { + nfs41_write_verf ignored; + /* not all data was committed, so commit to metadata server */ dprintf(1, "sending COMMIT to meta server for offset=%lld len=%lld\n", offset, *len_out); - nfsstat = nfs41_commit(state->session, &state->file, offset, *len_out, 1); + nfsstat = nfs41_commit(state->session, &state->file, + offset, *len_out, 1, &ignored); if (nfsstat) { dprintf(IOLVL, "nfs41_commit() failed with %s\n", nfs_error_string(nfsstat)); diff --git a/daemon/readwrite.c b/daemon/readwrite.c index 8cd3077..b81aca0 100644 --- a/daemon/readwrite.c +++ b/daemon/readwrite.c @@ -217,7 +217,12 @@ retry_write: } if (committed == UNSTABLE4) { dprintf(1, "sending COMMIT for offset=%d and len=%d\n", args->offset, len); - status = nfs41_commit(session, file, args->offset, len, 1); + status = nfs41_commit(session, file, args->offset, len, 1, &verf); + if (status) + goto out; + + if (!verify_commit(&verf)) + goto retry_write; } out: args->out_len = len; diff --git a/daemon/util.c b/daemon/util.c index 3d8f86d..dc63e1a 100644 --- a/daemon/util.c +++ b/daemon/util.c @@ -127,6 +127,17 @@ bool_t verify_write( return 0; } +bool_t verify_commit( + IN nfs41_write_verf *verf) +{ + if (memcmp(verf->expected, verf->verf, NFS4_VERIFIER_SIZE) == 0) { + dprintf(3, "verify_commit: verifier matches expected\n"); + return 1; + } + dprintf(2, "verify_commit: verifier changed; writes have been lost!\n"); + return 0; +} + ULONG nfs_file_info_to_attributes( IN const nfs41_file_info *info) { diff --git a/daemon/util.h b/daemon/util.h index cf8f1dd..89b62a4 100644 --- a/daemon/util.h +++ b/daemon/util.h @@ -50,8 +50,10 @@ uint32_t max_write_size( IN const nfs41_fh *fh); bool_t verify_write( - IN struct __nfs41_write_verf *verf, + IN nfs41_write_verf *verf, IN OUT enum stable_how4 *stable); +bool_t verify_commit( + IN nfs41_write_verf *verf); /* bitmap4 */ static __inline bool_t bitmap_isset(