From f8df907e8aaeaee559f913a68a345011a7aa1f23 Mon Sep 17 00:00:00 2001 From: "John L. Hammond" Date: Wed, 6 Jun 2018 08:14:50 -0500 Subject: [PATCH] LU-11069 llite: correct file position after appending writes In ll_file_io_generic() use the position returned in the kiocb to set the returned file position. This ensures that the file position is set correctly after an appending write. Add sanity test_23d() to check that calling lseek() for the current offset returns the correct value in this situation. Signed-off-by: John L. Hammond Change-Id: Ic76ce49db6e87d5294e18546d5b75a12793aa99c Reviewed-on: https://review.whamcloud.com/32641 Tested-by: Jenkins Reviewed-by: Jinshan Xiong Reviewed-by: Dmitry Eremin Tested-by: Maloo Reviewed-by: Oleg Drokin --- lustre/llite/file.c | 8 +++++- lustre/tests/multiop.c | 77 +++++++++++++++++++++++++++++++++++++------------- lustre/tests/sanity.sh | 13 +++++++++ 3 files changed, 78 insertions(+), 20 deletions(-) diff --git a/lustre/llite/file.c b/lustre/llite/file.c index 4877b80..cdda11e 100644 --- a/lustre/llite/file.c +++ b/lustre/llite/file.c @@ -1405,7 +1405,13 @@ restart: if (args->via_io_subtype == IO_NORMAL) { iov_iter_advance(args->u.normal.via_iter, io->ci_nob); - pos += io->ci_nob; + + /* CLIO is too complicated. See LU-11069. */ + if (cl_io_is_append(io)) + pos = io->u.ci_rw.rw_iocb.ki_pos; + else + pos += io->ci_nob; + args->u.normal.via_iocb->ki_pos = pos; #ifdef HAVE_KIOCB_KI_LEFT args->u.normal.via_iocb->ki_left = count; diff --git a/lustre/tests/multiop.c b/lustre/tests/multiop.c index 32dcb93..9cb65d0 100644 --- a/lustre/tests/multiop.c +++ b/lustre/tests/multiop.c @@ -89,6 +89,7 @@ char usage[] = " N rename filename to path\n" " o open(O_RDONLY)\n" " O open(O_CREAT|O_RDWR)\n" +" p print return value of last command\n" " r[num] read [optional length]\n" " R reference entire mmap-ed region\n" " s stat\n" @@ -104,7 +105,8 @@ char usage[] = " W write entire mmap-ed region\n" " y fsync\n" " Y fdatasync\n" -" z[num] seek [optional position, default 0]\n" +" z[num] lseek(SEEK_SET) [optional offset, default 0]\n" +" Z[num] lseek(SEEK_CUR) [optional offset, default 0]\n" " _ wait for signal\n"; void usr1_handler(int unused) @@ -211,7 +213,7 @@ int main(int argc, char **argv) struct statfs stfs; size_t mmap_len = 0, i; unsigned char *mmap_ptr = NULL, junk = 0; - int rc, len, fd = -1; + int len, fd = -1; int flags; int save_errno; int verbose = 0; @@ -219,6 +221,8 @@ int main(int argc, char **argv) struct lu_fid fid; struct timespec ts; struct lov_user_md_v3 lum; + long long rc = 0; + long long last_rc; if (argc < 3) { fprintf(stderr, usage, argv[0]); @@ -234,6 +238,11 @@ int main(int argc, char **argv) fname = argv[1]; for (commands = argv[2]; *commands; commands++) { + /* XXX Most commands return 0 or we exit so we only + * update rc where really needed. */ + last_rc = rc; + rc = 0; + switch (*commands) { case '_': if (verbose) { @@ -255,7 +264,8 @@ int main(int argc, char **argv) } break; case 'a': - if (fgetxattr(fd, XATTR, NULL, 0) == -1) { + rc = fgetxattr(fd, XATTR, NULL, 0); + if (rc < 0) { save_errno = errno; perror("fgetxattr"); exit(save_errno); @@ -290,6 +300,7 @@ int main(int argc, char **argv) perror("create stripe file"); exit(save_errno); } + rc = fd; break; case 'd': if (mkdir(fname, 0755) == -1) { @@ -305,6 +316,7 @@ int main(int argc, char **argv) perror("open(O_DIRECTORY)"); exit(save_errno); } + rc = fd; break; case 'e': commands++; @@ -334,7 +346,7 @@ int main(int argc, char **argv) str = "read"; else if (rc == LL_LEASE_WRLCK) str = "write"; - fprintf(stdout, "%s lease(%d) released.\n", + fprintf(stdout, "%s lease(%lld) released.\n", str, rc); } else if (rc == 0) { fprintf(stdout, "lease already broken.\n"); @@ -353,7 +365,7 @@ int main(int argc, char **argv) str = "read"; else if (rc == LL_LEASE_WRLCK) str = "write"; - fprintf(stdout, "%s lease(%d) has applied.\n", + fprintf(stdout, "%s lease(%lld) has applied.\n", str, rc); if (*commands == '-') errx(-1, "expect lease to not exist"); @@ -376,7 +388,7 @@ int main(int argc, char **argv) rc = llapi_fd2fid(fd, &fid); if (rc != 0) fprintf(stderr, - "llapi_path/fd2fid() on %d, rc=%d\n", + "llapi_path/fd2fid() on %d, rc=%lld\n", fd, rc); else printf(DFID"\n", PFID(&fid)); @@ -407,6 +419,7 @@ int main(int argc, char **argv) perror("create stripe file"); exit(save_errno); } + rc = fd; break; case 'j': if (flock(fd, LOCK_EX) == -1) @@ -494,6 +507,7 @@ int main(int argc, char **argv) perror("open(O_RDWR|O_CREAT)"); exit(save_errno); } + rc = fd; break; case 'o': len = get_flags(commands+1, &flags); @@ -507,7 +521,11 @@ int main(int argc, char **argv) perror("open"); exit(save_errno); } + rc = fd; break; + case 'p': + printf("%lld\n", last_rc); + break; case 'r': len = atoi(commands+1); if (len <= 0) @@ -534,14 +552,14 @@ int main(int argc, char **argv) exit(save_errno); } if (rc < len) { - fprintf(stderr, "short read: %u/%u\n", + fprintf(stderr, "short read: %lld/%u\n", rc, len); if (rc == 0) exit(ENODATA); } len -= rc; if (verbose >= 2) - printf("%.*s\n", rc, buf_align); + printf("%.*s\n", (int)rc, buf_align); } break; case 'R': @@ -603,6 +621,7 @@ int main(int argc, char **argv) perror("llapi_create_volatile"); exit(fd); } + rc = fd; break; case 'w': len = atoi(commands+1); @@ -631,7 +650,7 @@ int main(int argc, char **argv) exit(save_errno); } if (rc < len) - fprintf(stderr, "short write: %u/%u\n", + fprintf(stderr, "short write: %lld/%u\n", rc, len); len -= rc; } @@ -646,7 +665,7 @@ int main(int argc, char **argv) rc = llapi_get_data_version(fd, &dv, 0); if (rc) { fprintf(stderr, "cannot get file data version" - " %d\n", rc); + " %lld\n", rc); exit(-rc); } printf("dataversion is %ju\n", (uintmax_t)dv); @@ -658,7 +677,7 @@ int main(int argc, char **argv) rc = llapi_get_ost_layout_version(fd, &layout_version); if (rc) { fprintf(stderr, "cannot get ost layout version" - " %d\n", rc); + " %lld\n", rc); exit(-rc); } printf("ostlayoutversion: %u\n", layout_version); @@ -678,14 +697,34 @@ int main(int argc, char **argv) exit(save_errno); } break; - case 'z': - len = atoi(commands+1); - if (lseek(fd, len, SEEK_SET) == -1) { - save_errno = errno; - perror("lseek"); - exit(save_errno); - } - break; + case 'z': { + off_t off; + + len = atoi(commands + 1); + off = lseek(fd, len, SEEK_SET); + if (off == (off_t)-1) { + save_errno = errno; + perror("lseek"); + exit(save_errno); + } + + rc = off; + break; + } + case 'Z': { + off_t off; + + len = atoi(commands + 1); + off = lseek(fd, len, SEEK_CUR); + if (off == (off_t)-1) { + save_errno = errno; + perror("lseek"); + exit(save_errno); + } + + rc = off; + break; + } case '-': case '0': case '1': diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh index deaeaa4..22ae34d 100755 --- a/lustre/tests/sanity.sh +++ b/lustre/tests/sanity.sh @@ -901,6 +901,19 @@ test_23c() { } run_test 23c "O_APPEND size checks for tiny writes" +# LU-11069 file offset is correct after appending writes +test_23d() { + local file=$DIR/$tfile + local offset + + echo CentaurHauls > $file + offset=$($MULTIOP $file oO_WRONLY:O_APPEND:w13Zp) + if ((offset != 26)); then + error "wrong offset, expected 26, got '$offset'" + fi +} +run_test 23d "file offset is correct after appending writes" + # rename sanity test_24a() { echo '-- same directory rename' -- 1.8.3.1