From 35c13ff782517b3b96c1593dd3d26c22e5c618b1 Mon Sep 17 00:00:00 2001 From: Mohamad Chaarawi Date: Mon, 22 Jan 2024 10:34:04 -0600 Subject: [PATCH] DAOS-15049 dfs: fix bug in setattr when mtime and size are being set (#13640) - in dfs_osetattr(), if user sets both mtime and size, the hlc for the mtime is not being update on storage. To fix that, use the hlc reported from the array_stat and update that when updating the mtime in the entry. Signed-off-by: Mohamad Chaarawi --- src/client/dfs/dfs.c | 56 +++++++++++++++++++-------------- src/tests/suite/dfs_unit_test.c | 39 ++++++++++++++++++----- 2 files changed, 64 insertions(+), 31 deletions(-) diff --git a/src/client/dfs/dfs.c b/src/client/dfs/dfs.c index 267cf133caf..b882e4f8e12 100644 --- a/src/client/dfs/dfs.c +++ b/src/client/dfs/dfs.c @@ -5365,11 +5365,12 @@ dfs_osetattr(dfs_t *dfs, dfs_obj_t *obj, struct stat *stbuf, int flags) bool set_size = false; bool set_mtime = false; bool set_ctime = false; - int i = 0; - size_t len; - int rc; + int i = 0, hlc_recx_idx = 0; + size_t len; uint64_t obj_hlc = 0; struct stat rstat = {}; + daos_array_stbuf_t array_stbuf = {0}; + int rc; if (dfs == NULL || !dfs->mounted) return EINVAL; @@ -5466,6 +5467,10 @@ dfs_osetattr(dfs_t *dfs, dfs_obj_t *obj, struct stat *stbuf, int flags) d_iov_set(&sg_iovs[i], &obj_hlc, sizeof(uint64_t)); recxs[i].rx_idx = HLC_IDX; recxs[i].rx_nr = sizeof(uint64_t); + if (flags & DFS_SET_ATTR_SIZE) { + /** we need to update this again after the set size */ + hlc_recx_idx = i; + } i++; set_mtime = true; @@ -5515,38 +5520,41 @@ dfs_osetattr(dfs_t *dfs, dfs_obj_t *obj, struct stat *stbuf, int flags) rstat.st_blocks = (stbuf->st_size + (1 << 9) - 1) >> 9; rstat.st_size = stbuf->st_size; - /* mtime and ctime need to be updated too only if not set earlier */ - if (!set_mtime || !set_ctime) { - daos_array_stbuf_t array_stbuf = {0}; + /** + * if mtime is set, we need to to just update the hlc on the entry. if mtime and/or + * ctime were not set, we need to update the stat buf returned. both cases require + * an array stat for the hlc. + */ + /** TODO - need an array API to just stat the max epoch without size */ + rc = daos_array_stat(obj->oh, th, &array_stbuf, NULL); + if (rc) + D_GOTO(out_obj, rc = daos_der2errno(rc)); - /** TODO - need an array API to just stat the max epoch without size */ - rc = daos_array_stat(obj->oh, th, &array_stbuf, NULL); - if (rc) + if (!set_mtime) { + rc = d_hlc2timespec(array_stbuf.st_max_epoch, &rstat.st_mtim); + if (rc) { + D_ERROR("d_hlc2timespec() failed " DF_RC "\n", DP_RC(rc)); D_GOTO(out_obj, rc = daos_der2errno(rc)); - - if (!set_mtime) { - rc = d_hlc2timespec(array_stbuf.st_max_epoch, &rstat.st_mtim); - if (rc) { - D_ERROR("d_hlc2timespec() failed "DF_RC"\n", DP_RC(rc)); - D_GOTO(out_obj, rc = daos_der2errno(rc)); - } } + } else { + D_ASSERT(hlc_recx_idx > 0); + D_ASSERT(recxs[hlc_recx_idx].rx_idx == HLC_IDX); + d_iov_set(&sg_iovs[hlc_recx_idx], &array_stbuf.st_max_epoch, + sizeof(uint64_t)); + } - if (!set_ctime) { - rc = d_hlc2timespec(array_stbuf.st_max_epoch, &rstat.st_ctim); - if (rc) { - D_ERROR("d_hlc2timespec() failed "DF_RC"\n", DP_RC(rc)); - D_GOTO(out_obj, rc = daos_der2errno(rc)); - } + if (!set_ctime) { + rc = d_hlc2timespec(array_stbuf.st_max_epoch, &rstat.st_ctim); + if (rc) { + D_ERROR("d_hlc2timespec() failed " DF_RC "\n", DP_RC(rc)); + D_GOTO(out_obj, rc = daos_der2errno(rc)); } } } iod.iod_nr = i; - if (i == 0) D_GOTO(out_stat, rc = 0); - sgl.sg_nr = i; sgl.sg_nr_out = 0; sgl.sg_iovs = &sg_iovs[0]; diff --git a/src/tests/suite/dfs_unit_test.c b/src/tests/suite/dfs_unit_test.c index 4ae6129c6db..88424dfd1c0 100644 --- a/src/tests/suite/dfs_unit_test.c +++ b/src/tests/suite/dfs_unit_test.c @@ -1454,6 +1454,11 @@ run_time_tests(dfs_obj_t *obj, char *name, int mode) struct timespec prev_ts, first_ts; daos_size_t size; dfs_obj_t *tmp_obj; + struct tm tm = {0}; + time_t ts; + char *p; + struct tm *timeptr; + char time_str[64]; int rc; rc = dfs_stat(dfs_mt, NULL, name, &stbuf); @@ -1541,8 +1546,34 @@ run_time_tests(dfs_obj_t *obj, char *name, int mode) prev_ts.tv_sec = stbuf.st_mtim.tv_sec; prev_ts.tv_nsec = stbuf.st_mtim.tv_nsec; - /** set size on file with dfs_osetattr and stat at same time */ if (S_ISREG(mode)) { + /** set mtime and size at the same time; mtime should be what we set */ + memset(&stbuf, 0, sizeof(stbuf)); + stbuf.st_size = 1000; + p = strptime("2023-12-31", "%Y-%m-%d", &tm); + assert_non_null(p); + ts = mktime(&tm); + stbuf.st_mtim.tv_sec = ts; + stbuf.st_mtim.tv_nsec = 0; + rc = dfs_osetattr(dfs_mt, obj, &stbuf, DFS_SET_ATTR_SIZE | DFS_SET_ATTR_MTIME); + assert_int_equal(rc, 0); + assert_int_equal(stbuf.st_size, 1000); + /** check the mtime was updated with the setattr */ + assert_int_equal(ts, stbuf.st_mtim.tv_sec); + timeptr = localtime(&stbuf.st_mtim.tv_sec); + strftime(time_str, sizeof(time_str), "%Y-%m-%d", timeptr); + print_message("mtime = %s\n", time_str); + assert_true(strncmp("2023", time_str, 4) == 0); + + memset(&stbuf, 0, sizeof(stbuf)); + rc = dfs_ostat(dfs_mt, obj, &stbuf); + assert_int_equal(rc, 0); + assert_int_equal(stbuf.st_size, 1000); + timeptr = localtime(&stbuf.st_mtim.tv_sec); + strftime(time_str, sizeof(time_str), "%Y-%m-%d", timeptr); + assert_int_equal(ts, stbuf.st_mtim.tv_sec); + assert_true(strncmp("2023", time_str, 4) == 0); + memset(&stbuf, 0, sizeof(stbuf)); stbuf.st_size = 1024; rc = dfs_osetattr(dfs_mt, obj, &stbuf, DFS_SET_ATTR_SIZE); @@ -1552,12 +1583,6 @@ run_time_tests(dfs_obj_t *obj, char *name, int mode) assert_true(check_ts(prev_ts, stbuf.st_mtim)); } - struct tm tm = {0}; - time_t ts; - char *p; - struct tm *timeptr; - char time_str[64]; - /** set the mtime to 2020 */ p = strptime("2020-12-31", "%Y-%m-%d", &tm); assert_non_null(p);