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.

Lustre-change: https://review.whamcloud.com/32641

Signed-off-by: John L. Hammond <john.hammond@intel.com>
Change-Id: Ic76ce49db6e87d5294e18546d5b75a12793aa99c
Reviewed-on: https://review.whamcloud.com/32661
Reviewed-by: Jinshan Xiong <jinshan.xiong@gmail.com>
Tested-by: Jenkins
Reviewed-by: John L. Hammond <jhammond@whamcloud.com>

diff --git a/lustre/llite/file.c b/lustre/llite/file.c
index 2d062d5..1fefb8f 100644 (file)
--- a/lustre/llite/file.c
+++ b/lustre/llite/file.c
@@ -1292,7 +1292,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 7d284f9..5226692 100644 (file)
--- 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;
@@ -220,6 +222,8 @@ int main(int argc, char **argv)
        struct timespec          ts;
        struct lov_user_md_v3    lum;
        __u64                    dv;
+       long long rc = 0;
+       long long last_rc;
 
         if (argc < 3) {
                 fprintf(stderr, usage, argv[0]);
@@ -235,6 +239,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) {
@@ -256,7 +265,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);
@@ -291,6 +301,7 @@ int main(int argc, char **argv)
                                 perror("create stripe file");
                                 exit(save_errno);
                         }
+                       rc = fd;
                         break;
                 case 'd':
                         if (mkdir(fname, 0755) == -1) {
@@ -306,6 +317,7 @@ int main(int argc, char **argv)
                                 perror("open(O_DIRECTORY)");
                                 exit(save_errno);
                         }
+                       rc = fd;
                         break;
                case 'e':
                        commands++;
@@ -337,7 +349,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");
@@ -356,7 +368,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");
@@ -379,7 +391,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));
@@ -410,6 +422,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)
@@ -497,6 +510,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);
@@ -510,7 +524,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)
@@ -537,14 +555,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':
@@ -606,6 +624,7 @@ int main(int argc, char **argv)
                                perror("llapi_create_volatile");
                                exit(fd);
                        }
+                       rc = fd;
                        break;
                case 'w':
                        len = atoi(commands+1);
@@ -634,7 +653,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;
                        }
@@ -647,7 +666,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);
@@ -666,14 +685,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 4c3050f..11b4ab2 100755 (executable)
--- a/lustre/tests/sanity.sh
+++ b/lustre/tests/sanity.sh
@@ -834,6 +834,19 @@ test_23b() { # bug 18988
 }
 run_test 23b "O_APPEND check =========================="
 
+# 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'