From 2e2e3d3de6da230c1988c8bfbfeda61cbf9d6a9c Mon Sep 17 00:00:00 2001 From: Cedric Koch-Hofer Date: Fri, 9 Jan 2026 15:20:38 +0000 Subject: [PATCH 1/4] DAOS-17321 ddb: Add checksum dump function to ddb VOS API Add dv_dump_csum() to the ddb VOS API to expose checksum metadata stored in a VOS tree. The function fetches checksum info using VOS_OF_FETCH_CSUM and delivers it via a callback. Two akey types are handled: - Single-value: always fetched at DAOS_EPOCH_MAX; callback receives rel=NULL and the dcs_ci_list. - Array: fetched at the caller-specified epoch; callback receives both the daos_recx_ep_list and the dcs_ci_list. Passing dump_cb=NULL is a valid no-op. A new dt_csum_ctx test fixture is introduced with two objects (one with and one without checksums) and three test cases covering error paths, single-value csum retrieval, and array csum retrieval, including callback error propagation. Features: recovery Signed-off-by: Cedric Koch-Hofer --- src/utils/ddb/ddb_vos.c | 106 +++++++++++ src/utils/ddb/ddb_vos.h | 30 +++ src/utils/ddb/tests/ddb_test_driver.c | 259 +++++++++++++++++++++++++- src/utils/ddb/tests/ddb_test_driver.h | 68 ++++--- src/utils/ddb/tests/ddb_vos_tests.c | 226 ++++++++++++++++++++++ 5 files changed, 659 insertions(+), 30 deletions(-) diff --git a/src/utils/ddb/ddb_vos.c b/src/utils/ddb/ddb_vos.c index 15a5e9c62ee..8bf8802c014 100644 --- a/src/utils/ddb/ddb_vos.c +++ b/src/utils/ddb/ddb_vos.c @@ -1093,6 +1093,112 @@ dv_dump_value(daos_handle_t poh, struct dv_tree_path *path, dv_dump_value_cb dum return rc; } +static int +dump_csum_sv(daos_handle_t coh, daos_key_t *dkey, daos_unit_oid_t *oid, daos_iod_t *iod, + dv_dump_csum_cb dump_cb, void *cb_arg) +{ + daos_handle_t ioh; + struct dcs_ci_list *cil; + int rc; + + /* Single-value has one record per epoch; always fetch the latest. */ + rc = vos_fetch_begin(coh, *oid, DAOS_EPOCH_MAX, dkey, 1, iod, VOS_OF_FETCH_CSUM, NULL, &ioh, + NULL); + if (rc) { + D_ERROR("vos_fetch_begin for csum dump of " DF_UOID " failed: " DF_RC "\n", + DP_UOID(*oid), DP_RC(rc)); + goto out; + } + + cil = vos_ioh2ci(ioh); + + rc = dump_cb(cb_arg, NULL, cil); + if (!SUCCESS(rc)) + D_ERROR("Csum dump callback for " DF_UOID " failed: " DF_RC "\n", DP_UOID(*oid), + DP_RC(rc)); + + rc = vos_fetch_end(ioh, NULL, rc); + if (rc != 0) + D_ERROR("vos_fetch_end for csum dump of " DF_UOID " failed: " DF_RC "\n", + DP_UOID(*oid), DP_RC(rc)); + +out: + return rc; +} + +static int +dump_csum_recx(daos_handle_t coh, daos_key_t *dkey, daos_unit_oid_t *oid, daos_iod_t *iod, + daos_epoch_t epoch, dv_dump_csum_cb dump_cb, void *cb_arg) +{ + daos_handle_t ioh; + struct dcs_ci_list *cil; + struct daos_recx_ep_list *rel; + int rc; + + rc = vos_fetch_begin(coh, *oid, epoch, dkey, 1, iod, VOS_OF_FETCH_CSUM, NULL, &ioh, NULL); + if (rc) { + D_ERROR("vos_fetch_begin for csum dump of " DF_UOID " failed: " DF_RC "\n", + DP_UOID(*oid), DP_RC(rc)); + goto out; + } + + cil = vos_ioh2ci(ioh); + rel = vos_ioh2recx_list(ioh); + + rc = dump_cb(cb_arg, rel, cil); + if (!SUCCESS(rc)) + D_ERROR("Csum dump callback for " DF_UOID " failed: " DF_RC "\n", DP_UOID(*oid), + DP_RC(rc)); + + /* rel ownership is transferred by vos_ioh2recx_list(); free before vos_fetch_end. */ + daos_recx_ep_list_free(rel, iod->iod_nr); + rc = vos_fetch_end(ioh, NULL, rc); + if (rc != 0) + D_ERROR("vos_fetch_end for csum dump of " DF_UOID " failed: " DF_RC "\n", + DP_UOID(*oid), DP_RC(rc)); + +out: + return rc; +} + +int +dv_dump_csum(daos_handle_t poh, struct dv_tree_path *path, daos_epoch_t epoch, + dv_dump_csum_cb dump_cb, void *cb_arg) +{ + daos_handle_t coh; + daos_iod_t iod = {0}; + int rc = 0; + + /* No-op when no callback is provided; the caller controls whether to consume results. */ + if (dump_cb == NULL) + goto out; + + rc = vos_cont_open(poh, path->vtp_cont, &coh); + if (!SUCCESS(rc)) { + D_ERROR("Opening container for csum dump of " DF_UOID " failed: " DF_RC "\n", + DP_UOID(path->vtp_oid), DP_RC(rc)); + goto out; + } + + iod.iod_name = path->vtp_akey; + iod.iod_recxs = &path->vtp_recx; + iod.iod_nr = 1; + iod.iod_size = 0; + if (path->vtp_is_recx) { + iod.iod_type = DAOS_IOD_ARRAY; + rc = dump_csum_recx(coh, &path->vtp_dkey, &path->vtp_oid, &iod, epoch, dump_cb, + cb_arg); + } else { + iod.iod_type = DAOS_IOD_SINGLE; + rc = dump_csum_sv(coh, &path->vtp_dkey, &path->vtp_oid, &iod, dump_cb, cb_arg); + } + + vos_cont_close(coh); + +out: + return rc; +} + static void ilog_entry_status(enum ilog_status status, char *status_str, uint32_t status_str_len) { diff --git a/src/utils/ddb/ddb_vos.h b/src/utils/ddb/ddb_vos.h index 09e51382924..ba939e64f17 100644 --- a/src/utils/ddb/ddb_vos.h +++ b/src/utils/ddb/ddb_vos.h @@ -155,6 +155,36 @@ typedef int (*dv_dump_value_cb)(void *cb_arg, d_iov_t *value); int dv_dump_value(daos_handle_t poh, struct dv_tree_path *path, dv_dump_value_cb dump_cb, void *cb_arg); +/** + * Callback invoked by dv_dump_csum() with the fetched checksum information. + * + * @param cb_arg User-provided argument passed through from dv_dump_csum(). + * @param rel Recx/epoch list describing the stored extents. NULL for single-value akeys; + * non-NULL for array akeys. The caller must not free this pointer. + * @param cil Checksum info list. Valid only for the duration of the callback. + * @return 0 on success; a negative error code is propagated back to the caller of + * dv_dump_csum(). + */ +typedef int (*dv_dump_csum_cb)(void *cb_arg, struct daos_recx_ep_list *rel, + struct dcs_ci_list *cil); + +/** + * Fetch and dump the checksum information for the akey identified by \a path. + * + * @param poh Open pool handle. + * @param path VOS tree path identifying the container, object, dkey, and akey. + * For array akeys, path->vtp_recx selects the extent to inspect. + * @param epoch Epoch for the fetch. Only used for array akeys; single-value akeys + * always fetch the latest epoch. + * @param dump_cb Callback invoked with the result. If NULL, the function returns 0 + * without opening the container or calling VOS. + * @param cb_arg Opaque argument forwarded to \a dump_cb. + * @return 0 on success, or a negative error code. + */ +int +dv_dump_csum(daos_handle_t poh, struct dv_tree_path *path, daos_epoch_t epoch, + dv_dump_csum_cb dump_cb, void *cb_arg); + struct ddb_ilog_entry { uint32_t die_idx; int32_t die_status; diff --git a/src/utils/ddb/tests/ddb_test_driver.c b/src/utils/ddb/tests/ddb_test_driver.c index bbaa3ae8c5f..5f27c476152 100644 --- a/src/utils/ddb/tests/ddb_test_driver.c +++ b/src/utils/ddb/tests/ddb_test_driver.c @@ -58,14 +58,14 @@ char *g_akeys_str[] = { char *g_invalid_key_str = "invalid key"; -daos_unit_oid_t g_oids[10]; -uuid_t g_uuids[10]; -daos_key_t g_dkeys[10]; -daos_key_t g_akeys[10]; -daos_recx_t g_recxs[10]; -daos_key_t g_invalid_key; -daos_recx_t g_invalid_recx = {.rx_nr = 9999, .rx_idx = 9999}; - +daos_unit_oid_t g_oids[10]; +uuid_t g_uuids[10]; +daos_key_t g_dkeys[10]; +daos_key_t g_akeys[10]; +daos_recx_t g_recxs[10]; +daos_key_t g_invalid_key; +daos_recx_t g_invalid_recx = {.rx_nr = 9999, .rx_idx = 9999}; +const char *g_csum_uuid_str = "12345678-1234-1234-1234-123456789101"; daos_unit_oid_t dvt_gen_uoid(uint32_t i) @@ -624,6 +624,249 @@ char_in_tests(char a, char *str, uint32_t str_len) return false; } +static int +csum_test_sv_setup(struct dt_vos_pool_ctx *tctx, daos_handle_t coh, d_sg_list_t *sgl) +{ + struct dt_csum_ctx *csum_ctx; + daos_iod_t iod; + char *buf; + int rc; + + csum_ctx = tctx->dvt_extra; + + D_ALLOC(buf, csum_ctx->dct_sv_size); + if (buf == NULL) { + rc = -DER_NOMEM; + goto out; + } + + d_iov_set(&iod.iod_name, g_akeys_str[0], strlen(g_akeys_str[0])); + iod.iod_nr = 1; + iod.iod_type = DAOS_IOD_SINGLE; + iod.iod_size = csum_ctx->dct_sv_size; + iod.iod_recxs = NULL; + + /* g_oids[0]: single value written without checksum */ + memset(buf, 'a', csum_ctx->dct_sv_size); + d_iov_set(sgl->sg_iovs, &buf[0], csum_ctx->dct_sv_size); + rc = vos_obj_update(coh, g_oids[0], 1, 0, 0, &g_dkeys[0], 1, &iod, NULL, sgl); + if (rc != 0) + goto out_buf; + + /* g_oids[1]: single value written with checksum */ + memset(buf, 'b', csum_ctx->dct_sv_size); + d_iov_set(sgl->sg_iovs, &buf[0], csum_ctx->dct_sv_size); + rc = daos_csummer_calc_iods(csum_ctx->dct_csummer, sgl, &iod, NULL, 1, false, NULL, 0, + &csum_ctx->dct_sv_ic); + if (rc != 0) + goto out_buf; + rc = + vos_obj_update(coh, g_oids[1], 1, 0, 0, &g_dkeys[0], 1, &iod, csum_ctx->dct_sv_ic, sgl); + if (rc != 0) + daos_csummer_free_ic(csum_ctx->dct_csummer, &csum_ctx->dct_sv_ic); + +out_buf: + D_FREE(buf); +out: + return rc; +} + +static int +csum_test_recx_setup(struct dt_vos_pool_ctx *tctx, daos_handle_t coh, d_sg_list_t *sgl) +{ + struct dt_csum_ctx *csum_ctx; + daos_iod_t iod; + char *buf; + char filler; + daos_recx_t recx; + int recx_idx; + daos_epoch_t epoch; + int rc; + + csum_ctx = tctx->dvt_extra; + + D_ALLOC(buf, csum_ctx->dct_recx_size); + if (buf == NULL) { + rc = -DER_NOMEM; + goto out; + } + + d_iov_set(&iod.iod_name, g_akeys_str[1], strlen(g_akeys_str[1])); + iod.iod_nr = 1; + iod.iod_type = DAOS_IOD_ARRAY; + iod.iod_size = 1; + + recx.rx_nr = csum_ctx->dct_recx_size; + + /* + * Two recxs are written per object, with overlapping extents: + * recx 0: rx_idx=0, rx_nr=dct_recx_size (epoch 1) + * recx 1: rx_idx=dct_recx_size/2, rx_nr=dct_recx_size (epoch 2) + * The overlap means that a fetch covering [0, dct_recx_size) intersects recx 1 only + * partially, but VOS_OF_FETCH_CSUM records the full stored extent of recx 1. + */ + + /* g_oids[0]: recxs written without checksum */ + epoch = 1; + filler = 'c'; + for (recx_idx = 0; recx_idx < DVT_FAKE_RECX_COUNT; recx_idx++) { + recx.rx_idx = recx_idx * csum_ctx->dct_recx_size / 2; + iod.iod_recxs = &recx; + + memset(buf, filler, csum_ctx->dct_recx_size); + d_iov_set(sgl->sg_iovs, &buf[0], csum_ctx->dct_recx_size); + + rc = vos_obj_update(coh, g_oids[0], epoch, 0, 0, &g_dkeys[0], 1, &iod, NULL, sgl); + if (rc != 0) + goto out_buf; + + epoch++; + filler++; + } + + /* g_oids[1]: recxs written with checksum */ + epoch = 1; + for (recx_idx = 0; recx_idx < DVT_FAKE_RECX_COUNT; recx_idx++) { + recx.rx_idx = recx_idx * csum_ctx->dct_recx_size / 2; + iod.iod_recxs = &recx; + + memset(buf, filler, csum_ctx->dct_recx_size); + d_iov_set(sgl->sg_iovs, &buf[0], csum_ctx->dct_recx_size); + + rc = daos_csummer_calc_iods(csum_ctx->dct_csummer, sgl, &iod, NULL, 1, false, NULL, + 0, &csum_ctx->dct_recx_ics[recx_idx]); + if (rc) + goto out_ics; + rc = vos_obj_update(coh, g_oids[1], epoch, 0, 0, &g_dkeys[0], 1, &iod, + csum_ctx->dct_recx_ics[recx_idx], sgl); + if (rc != 0) + goto out_ics; + + epoch++; + filler++; + } + +out_ics: + if (rc != 0) { + for (recx_idx = 0; recx_idx < DVT_FAKE_RECX_COUNT; recx_idx++) { + if (csum_ctx->dct_recx_ics[recx_idx] == NULL) + break; + daos_csummer_free_ic(csum_ctx->dct_csummer, + &csum_ctx->dct_recx_ics[recx_idx]); + } + } +out_buf: + D_FREE(buf); +out: + return rc; +} + +int +ddb_test_csum_setup(void **state) +{ + struct dt_vos_pool_ctx *tctx; + struct dt_csum_ctx *csum_ctx; + daos_handle_t poh; + daos_handle_t coh; + d_sg_list_t sgl; + int rc; + + tctx = *state; + tctx->dvt_extra = NULL; + + D_ALLOC_PTR(csum_ctx); + if (csum_ctx == NULL) { + rc = -DER_NOMEM; + goto out; + } + csum_ctx->dct_sv_size = DVT_FAKE_SV_SIZE; + csum_ctx->dct_recx_size = DVT_FAKE_RECX_SIZE; + csum_ctx->dct_chunk_size = DVT_FAKE_CHUNK_SIZE; + csum_ctx->dct_csum_type = DVT_FAKE_CSUM_TYPE; + csum_ctx->dct_sv_ic = NULL; + memset(&csum_ctx->dct_recx_ics[0], 0, sizeof(csum_ctx->dct_recx_ics)); + rc = daos_csummer_init_with_type(&csum_ctx->dct_csummer, csum_ctx->dct_csum_type, + csum_ctx->dct_chunk_size, 0); + if (rc != 0) + goto out_csum_ctx; + tctx->dvt_extra = csum_ctx; + + rc = vos_pool_open(tctx->dvt_pmem_file, tctx->dvt_pool_uuid, 0, &poh); + if (rc != 0) + goto out_csummer; + + rc = uuid_parse(g_csum_uuid_str, csum_ctx->dct_cont_uuid); + if (rc != 0) + goto out_pool; + rc = vos_cont_create(poh, csum_ctx->dct_cont_uuid); + if (rc != 0) + goto out_pool; + rc = vos_cont_open(poh, csum_ctx->dct_cont_uuid, &coh); + if (rc != 0) + goto out_cont_destroy; + + rc = d_sgl_init(&sgl, 1); + if (rc != 0) + goto out_cont_close; + + rc = csum_test_sv_setup(tctx, coh, &sgl); + if (rc != 0) + goto out_sgl; + rc = csum_test_recx_setup(tctx, coh, &sgl); + +out_sgl: + d_sgl_fini(&sgl, false); +out_cont_close: + vos_cont_close(coh); +out_cont_destroy: + if (rc != 0) + vos_cont_destroy(poh, csum_ctx->dct_cont_uuid); +out_pool: + vos_pool_close(poh); +out_csummer: + if (rc != 0) + daos_csummer_destroy(&csum_ctx->dct_csummer); +out_csum_ctx: + if (rc != 0) { + D_FREE(csum_ctx); + tctx->dvt_extra = NULL; + } +out: + return rc; +} + +int +ddb_test_csum_teardown(void **state) +{ + struct dt_vos_pool_ctx *tctx; + struct dt_csum_ctx *csum_ctx; + daos_handle_t poh; + int ci_idx; + int rc = 0; + + tctx = *state; + csum_ctx = tctx->dvt_extra; + if (csum_ctx == NULL) + goto out; + + rc = vos_pool_open(tctx->dvt_pmem_file, tctx->dvt_pool_uuid, 0, &poh); + if (rc != 0) + goto out; + + daos_csummer_free_ic(csum_ctx->dct_csummer, &csum_ctx->dct_sv_ic); + for (ci_idx = 0; ci_idx < DVT_FAKE_RECX_COUNT; ci_idx++) + daos_csummer_free_ic(csum_ctx->dct_csummer, &csum_ctx->dct_recx_ics[ci_idx]); + daos_csummer_destroy(&csum_ctx->dct_csummer); + + vos_cont_destroy(poh, csum_ctx->dct_cont_uuid); + + D_FREE(csum_ctx); + + vos_pool_close(poh); +out: + return rc; +} + /* * ----------------------------------------------- * Execute diff --git a/src/utils/ddb/tests/ddb_test_driver.h b/src/utils/ddb/tests/ddb_test_driver.h index 833303ec16d..6fc4018d664 100644 --- a/src/utils/ddb/tests/ddb_test_driver.h +++ b/src/utils/ddb/tests/ddb_test_driver.h @@ -18,30 +18,49 @@ #include #include -extern bool g_verbose; -extern const char *g_uuids_str[10]; -extern const char *g_invalid_uuid_str; -extern uuid_t g_uuids[10]; -extern daos_unit_oid_t g_oids[10]; -extern daos_unit_oid_t g_invalid_oid; -extern char *g_dkeys_str[10]; -extern char *g_akeys_str[10]; -extern daos_key_t g_dkeys[10]; -extern daos_key_t g_akeys[10]; -extern daos_key_t g_invalid_key; -extern daos_recx_t g_recxs[10]; -extern daos_recx_t g_invalid_recx; +extern bool g_verbose; +extern const char *g_uuids_str[10]; +extern const char *g_invalid_uuid_str; +extern uuid_t g_uuids[10]; +extern daos_unit_oid_t g_oids[10]; +extern daos_unit_oid_t g_invalid_oid; +extern char *g_dkeys_str[10]; +extern char *g_akeys_str[10]; +extern daos_key_t g_dkeys[10]; +extern daos_key_t g_akeys[10]; +extern daos_key_t g_invalid_key; +extern daos_recx_t g_recxs[10]; +extern daos_recx_t g_invalid_recx; +extern const char *g_csum_uuid_str; struct dt_vos_pool_ctx { - daos_handle_t dvt_poh; - uuid_t dvt_pool_uuid; - int dvt_fd; - char dvt_pmem_file[128]; - uint32_t dvt_cont_count; - uint32_t dvt_obj_count; - uint32_t dvt_dkey_count; - uint32_t dvt_akey_count; - bool dvt_special_pool_destroy; + daos_handle_t dvt_poh; + uuid_t dvt_pool_uuid; + int dvt_fd; + char dvt_pmem_file[128]; + uint32_t dvt_cont_count; + uint32_t dvt_obj_count; + uint32_t dvt_dkey_count; + uint32_t dvt_akey_count; + bool dvt_special_pool_destroy; + void *dvt_extra; +}; + +#define DVT_FAKE_RECX_COUNT (2) +#define DVT_FAKE_SV_SIZE (1u << 10) +#define DVT_FAKE_RECX_SIZE (1u << 13) +#define DVT_FAKE_CHUNK_SIZE (1u << 12) +#define DVT_FAKE_CSUM_TYPE (HASH_TYPE_CRC64) + +struct dt_csum_ctx { + uuid_t dct_cont_uuid; + size_t dct_sv_size; + size_t dct_recx_size; + enum DAOS_HASH_TYPE dct_csum_type; + size_t dct_chunk_size; + struct daos_csummer *dct_csummer; + struct dcs_iod_csums *dct_sv_ic; + struct dcs_iod_csums *dct_recx_ics[DVT_FAKE_RECX_COUNT]; }; daos_unit_oid_t dvt_gen_uoid(uint32_t i); @@ -58,6 +77,11 @@ void dvt_iov_alloc_str(d_iov_t *iov, const char *str); int ddb_test_setup_vos(void **state); int ddb_teardown_vos(void **state); +int +ddb_test_csum_setup(void **state); +int + ddb_test_csum_teardown(void **state); + int ddb_parse_tests_run(void); int ddb_vos_tests_run(void); int diff --git a/src/utils/ddb/tests/ddb_vos_tests.c b/src/utils/ddb/tests/ddb_vos_tests.c index 52fabaa572b..5235f491ffb 100644 --- a/src/utils/ddb/tests/ddb_vos_tests.c +++ b/src/utils/ddb/tests/ddb_vos_tests.c @@ -1104,6 +1104,24 @@ dv_test_teardown(void **state) return 0; } +static int +dv_test_csum_setup(void **state) +{ + assert_success(dv_test_setup(state)); + assert_success(ddb_test_csum_setup(state)); + + return 0; +} + +static int +dv_test_csum_teardown(void **state) +{ + assert_success(ddb_test_csum_teardown(state)); + assert_success(dv_test_teardown(state)); + + return 0; +} + static void pool_flags_tests(void **state) { @@ -1209,11 +1227,216 @@ read_only_vs_write_mode_test(void **state) SHA256_DIGEST_LEN); } +/* Callback that returns *(int *)cb_args, or 0 if cb_args is NULL. */ +static int +csum_cb_return_rc(void *cb_args, struct daos_recx_ep_list *rel, struct dcs_ci_list *cil) +{ + return (cb_args != NULL) ? (*(int *)cb_args) : (0); +} + +static void +dump_csum_error_tests(void **state) +{ + struct dt_vos_pool_ctx *tctx = *state; + struct dt_csum_ctx *csum_ctx = tctx->dvt_extra; + struct dv_tree_path path = {0}; + int rc; + + uuid_copy(path.vtp_cont, csum_ctx->dct_cont_uuid); + path.vtp_dkey = g_dkeys[0]; + path.vtp_akey = g_akeys[0]; /* single value type */ + path.vtp_is_recx = false; + + /* invalid poh: error comes from vos_cont_open, not the callback (which would return 0) */ + rc = dv_dump_csum(DAOS_HDL_INVAL, &path, DAOS_EPOCH_MAX, csum_cb_return_rc, NULL); + assert_rc_equal(-DER_INVAL, rc); +} + +static int +check_csum_sv_cb_001(void *cb_args, struct daos_recx_ep_list *rel, struct dcs_ci_list *cil) +{ + assert_null(cb_args); + assert_null(rel); + assert_non_null(cil); + + assert_int_equal(cil->dcl_csum_infos_nr, 0); + + return 0; +} + +static int +check_csum_sv_cb_002(void *cb_args, struct daos_recx_ep_list *rel, struct dcs_ci_list *cil) +{ + struct dt_csum_ctx *csum_ctx; + struct dcs_csum_info *ci; + + assert_non_null(cb_args); + assert_null(rel); + assert_non_null(cil); + + csum_ctx = cb_args; + assert_int_equal(csum_ctx->dct_sv_size, DVT_FAKE_SV_SIZE); + assert_int_equal(csum_ctx->dct_chunk_size, DVT_FAKE_CHUNK_SIZE); + assert_int_equal(csum_ctx->dct_csum_type, DVT_FAKE_CSUM_TYPE); + + assert_int_equal(cil->dcl_csum_infos_nr, 1); + + ci = dcs_csum_info_get(cil, 0); + assert_true(ci_is_valid(ci)); + assert_int_equal(ci->cs_nr, 1); + assert_true(daos_csummer_compare_csum_info(csum_ctx->dct_csummer, ci, + csum_ctx->dct_sv_ic->ic_data)); + + return 0; +} + +static void +dump_csum_sv_tests(void **state) +{ + struct dt_vos_pool_ctx *tctx = *state; + struct dt_csum_ctx *csum_ctx = tctx->dvt_extra; + struct dv_tree_path path = {0}; + int rc; + + uuid_copy(path.vtp_cont, csum_ctx->dct_cont_uuid); + path.vtp_dkey = g_dkeys[0]; + path.vtp_akey = g_akeys[0]; /* single value type */ + path.vtp_is_recx = false; + + /* no csum info */ + path.vtp_oid = g_oids[0]; + rc = dv_dump_csum(tctx->dvt_poh, &path, DAOS_EPOCH_MAX, check_csum_sv_cb_001, NULL); + assert_success(rc); + + /* with csum info */ + path.vtp_oid = g_oids[1]; + rc = dv_dump_csum(tctx->dvt_poh, &path, DAOS_EPOCH_MAX, check_csum_sv_cb_002, csum_ctx); + assert_success(rc); + + /* with csum info, without callback */ + path.vtp_oid = g_oids[1]; + rc = dv_dump_csum(tctx->dvt_poh, &path, DAOS_EPOCH_MAX, NULL, csum_ctx); + assert_success(rc); + + /* callback failure is propagated */ + path.vtp_oid = g_oids[1]; + rc = dv_dump_csum(tctx->dvt_poh, &path, DAOS_EPOCH_MAX, csum_cb_return_rc, + &(int){-DER_INVAL}); + assert_rc_equal(-DER_INVAL, rc); +} + +static int +check_csum_recx_cb_001(void *cb_args, struct daos_recx_ep_list *rel, struct dcs_ci_list *cil) +{ + assert_null(cb_args); + assert_non_null(rel); + assert_non_null(cil); + + assert_int_equal(rel->re_nr, DVT_FAKE_RECX_COUNT); + assert_int_equal(rel->re_items[0].re_recx.rx_idx, 0); + assert_int_equal(rel->re_items[0].re_recx.rx_nr, DVT_FAKE_RECX_SIZE); + assert_int_equal(rel->re_items[0].re_ep, 1); + assert_int_equal(rel->re_items[1].re_recx.rx_idx, DVT_FAKE_RECX_SIZE / 2); + /* + * VOS_OF_FETCH_CSUM records the full stored extent, not the IOD intersection: + * recx 1 starts at rx_idx=DVT_FAKE_RECX_SIZE/2 but its rx_nr is DVT_FAKE_RECX_SIZE. + */ + assert_int_equal(rel->re_items[1].re_recx.rx_nr, DVT_FAKE_RECX_SIZE); + assert_int_equal(rel->re_items[1].re_ep, 2); + + /* No csum was stored for g_oids[0], so the checksum info list is empty. */ + assert_int_equal(cil->dcl_csum_infos_nr, 0); + + return 0; +} + +static int +check_csum_recx_cb_002(void *cb_args, struct daos_recx_ep_list *rel, struct dcs_ci_list *cil) +{ + struct dt_csum_ctx *csum_ctx; + struct dcs_csum_info *ci; + + assert_non_null(cb_args); + assert_non_null(rel); + assert_non_null(cil); + + csum_ctx = cb_args; + assert_int_equal(csum_ctx->dct_recx_size, DVT_FAKE_RECX_SIZE); + assert_int_equal(csum_ctx->dct_chunk_size, DVT_FAKE_CHUNK_SIZE); + assert_int_equal(csum_ctx->dct_csum_type, DVT_FAKE_CSUM_TYPE); + + assert_int_equal(rel->re_nr, DVT_FAKE_RECX_COUNT); + assert_int_equal(rel->re_items[0].re_recx.rx_idx, 0); + assert_int_equal(rel->re_items[0].re_recx.rx_nr, DVT_FAKE_RECX_SIZE); + assert_int_equal(rel->re_items[0].re_ep, 1); + assert_int_equal(rel->re_items[1].re_recx.rx_idx, DVT_FAKE_RECX_SIZE / 2); + assert_int_equal(rel->re_items[1].re_recx.rx_nr, DVT_FAKE_RECX_SIZE); + assert_int_equal(rel->re_items[1].re_ep, 2); + + assert_non_null(cil); + assert_int_equal(cil->dcl_csum_infos_nr, DVT_FAKE_RECX_COUNT); + + ci = dcs_csum_info_get(cil, 0); + assert_true(ci_is_valid(ci)); + assert_int_equal(ci->cs_nr, 2); + assert_true(daos_csummer_compare_csum_info(csum_ctx->dct_csummer, ci, + csum_ctx->dct_recx_ics[0]->ic_data)); + + ci = dcs_csum_info_get(cil, 1); + assert_true(ci_is_valid(ci)); + assert_int_equal(ci->cs_nr, 2); + assert_true(daos_csummer_compare_csum_info(csum_ctx->dct_csummer, ci, + csum_ctx->dct_recx_ics[1]->ic_data)); + + return 0; +} + +static void +dump_csum_recx_tests(void **state) +{ + struct dt_vos_pool_ctx *tctx = *state; + struct dt_csum_ctx *csum_ctx = tctx->dvt_extra; + struct dv_tree_path path = {0}; + int rc; + + uuid_copy(path.vtp_cont, csum_ctx->dct_cont_uuid); + path.vtp_dkey = g_dkeys[0]; + path.vtp_akey = g_akeys[1]; /* array value type */ + path.vtp_is_recx = true; + path.vtp_recx.rx_idx = 0; + path.vtp_recx.rx_nr = csum_ctx->dct_recx_size; + + /* no csum info */ + path.vtp_oid = g_oids[0]; + rc = dv_dump_csum(tctx->dvt_poh, &path, DAOS_EPOCH_MAX, check_csum_recx_cb_001, NULL); + assert_success(rc); + + /* with csum info */ + path.vtp_oid = g_oids[1]; + rc = dv_dump_csum(tctx->dvt_poh, &path, DAOS_EPOCH_MAX, check_csum_recx_cb_002, csum_ctx); + assert_success(rc); + + /* with csum info, without callback */ + path.vtp_oid = g_oids[1]; + rc = dv_dump_csum(tctx->dvt_poh, &path, DAOS_EPOCH_MAX, NULL, csum_ctx); + assert_success(rc); + + /* callback failure is propagated */ + path.vtp_oid = g_oids[1]; + rc = dv_dump_csum(tctx->dvt_poh, &path, DAOS_EPOCH_MAX, csum_cb_return_rc, + &(int){-DER_INVAL}); + assert_rc_equal(-DER_INVAL, rc); +} + /* * All these tests use the same VOS tree that is created at suit_setup. Therefore, tests * that modify the state of the tree (delete, add, etc) should be run after all others. */ #define TEST(x) { #x, x, dv_test_setup, dv_test_teardown } + +/* Checksum tests need special setup/teardown */ +#define TEST_CSUM(test) {#test, test, dv_test_csum_setup, dv_test_csum_teardown} + const struct CMUnitTest dv_test_cases[] = { {"open_pool", open_pool_test, NULL, NULL}, /* don't want this test to run with setup */ TEST(list_items_test), @@ -1240,6 +1463,9 @@ const struct CMUnitTest dv_test_cases[] = { {"pool_flag_update", pool_flags_tests, NULL, NULL}, {"read_only_vs_write_mode", read_only_vs_write_mode_test, NULL, NULL}, /* don't want this test to run with setup */ + TEST_CSUM(dump_csum_error_tests), + TEST_CSUM(dump_csum_sv_tests), + TEST_CSUM(dump_csum_recx_tests), }; int From 338c5e689a19cf5b52389092f94287f1256d6e05 Mon Sep 17 00:00:00 2001 From: Cedric Koch-Hofer Date: Wed, 10 Jun 2026 06:02:01 +0000 Subject: [PATCH 2/4] DAOS-17321 ddb: Thread epoch through dump_csum_sv for snapshot access The original implementation of dump_csum_sv() hardcoded DAOS_EPOCH_MAX, making it impossible to retrieve the checksum of a single-value record as it existed at a snapshot epoch. Add an epoch parameter to dump_csum_sv() and pass the caller-provided epoch through from dv_dump_csum(), making SV and array akeys consistent. Update the @param epoch doc comment accordingly. Extend the test fixture (dt_csum_ctx) to store DVT_FAKE_SV_COUNT successive SV writes in dct_sv_ics[], mirroring the dct_recx_ics[] pattern. Add two new sub-cases in dump_csum_sv_tests: - fetching at epoch 1 returns the epoch-1 checksum (dct_sv_ics[0]) - fetching at DAOS_EPOCH_MAX returns the latest checksum (dct_sv_ics[1]) Reviewed-at: https://github.com/daos-stack/daos/pull/18444#discussion_r3374156791 Features: recovery Signed-off-by: Cedric Koch-Hofer --- src/utils/ddb/ddb_vos.c | 9 +++-- src/utils/ddb/ddb_vos.h | 5 +-- src/utils/ddb/tests/ddb_test_driver.c | 51 ++++++++++++++++++++------- src/utils/ddb/tests/ddb_test_driver.h | 3 +- src/utils/ddb/tests/ddb_vos_tests.c | 33 +++++++++++++++-- 5 files changed, 77 insertions(+), 24 deletions(-) diff --git a/src/utils/ddb/ddb_vos.c b/src/utils/ddb/ddb_vos.c index 8bf8802c014..451e9cf5196 100644 --- a/src/utils/ddb/ddb_vos.c +++ b/src/utils/ddb/ddb_vos.c @@ -1095,15 +1095,13 @@ dv_dump_value(daos_handle_t poh, struct dv_tree_path *path, dv_dump_value_cb dum static int dump_csum_sv(daos_handle_t coh, daos_key_t *dkey, daos_unit_oid_t *oid, daos_iod_t *iod, - dv_dump_csum_cb dump_cb, void *cb_arg) + daos_epoch_t epoch, dv_dump_csum_cb dump_cb, void *cb_arg) { daos_handle_t ioh; struct dcs_ci_list *cil; int rc; - /* Single-value has one record per epoch; always fetch the latest. */ - rc = vos_fetch_begin(coh, *oid, DAOS_EPOCH_MAX, dkey, 1, iod, VOS_OF_FETCH_CSUM, NULL, &ioh, - NULL); + rc = vos_fetch_begin(coh, *oid, epoch, dkey, 1, iod, VOS_OF_FETCH_CSUM, NULL, &ioh, NULL); if (rc) { D_ERROR("vos_fetch_begin for csum dump of " DF_UOID " failed: " DF_RC "\n", DP_UOID(*oid), DP_RC(rc)); @@ -1190,7 +1188,8 @@ dv_dump_csum(daos_handle_t poh, struct dv_tree_path *path, daos_epoch_t epoch, cb_arg); } else { iod.iod_type = DAOS_IOD_SINGLE; - rc = dump_csum_sv(coh, &path->vtp_dkey, &path->vtp_oid, &iod, dump_cb, cb_arg); + rc = dump_csum_sv(coh, &path->vtp_dkey, &path->vtp_oid, &iod, epoch, dump_cb, + cb_arg); } vos_cont_close(coh); diff --git a/src/utils/ddb/ddb_vos.h b/src/utils/ddb/ddb_vos.h index ba939e64f17..7ade0fa462b 100644 --- a/src/utils/ddb/ddb_vos.h +++ b/src/utils/ddb/ddb_vos.h @@ -174,8 +174,9 @@ typedef int (*dv_dump_csum_cb)(void *cb_arg, struct daos_recx_ep_list *rel, * @param poh Open pool handle. * @param path VOS tree path identifying the container, object, dkey, and akey. * For array akeys, path->vtp_recx selects the extent to inspect. - * @param epoch Epoch for the fetch. Only used for array akeys; single-value akeys - * always fetch the latest epoch. + * @param epoch Epoch for the fetch. For single-value akeys, controls which version is + * returned — pass DAOS_EPOCH_MAX to get the latest, or a snapshot epoch to + * access an earlier version. For array akeys, selects the visible extent set. * @param dump_cb Callback invoked with the result. If NULL, the function returns 0 * without opening the container or calling VOS. * @param cb_arg Opaque argument forwarded to \a dump_cb. diff --git a/src/utils/ddb/tests/ddb_test_driver.c b/src/utils/ddb/tests/ddb_test_driver.c index 5f27c476152..339897a5521 100644 --- a/src/utils/ddb/tests/ddb_test_driver.c +++ b/src/utils/ddb/tests/ddb_test_driver.c @@ -630,6 +630,9 @@ csum_test_sv_setup(struct dt_vos_pool_ctx *tctx, daos_handle_t coh, d_sg_list_t struct dt_csum_ctx *csum_ctx; daos_iod_t iod; char *buf; + char filler; + int sv_idx; + daos_epoch_t epoch; int rc; csum_ctx = tctx->dvt_extra; @@ -653,18 +656,39 @@ csum_test_sv_setup(struct dt_vos_pool_ctx *tctx, daos_handle_t coh, d_sg_list_t if (rc != 0) goto out_buf; - /* g_oids[1]: single value written with checksum */ - memset(buf, 'b', csum_ctx->dct_sv_size); - d_iov_set(sgl->sg_iovs, &buf[0], csum_ctx->dct_sv_size); - rc = daos_csummer_calc_iods(csum_ctx->dct_csummer, sgl, &iod, NULL, 1, false, NULL, 0, - &csum_ctx->dct_sv_ic); - if (rc != 0) - goto out_buf; - rc = - vos_obj_update(coh, g_oids[1], 1, 0, 0, &g_dkeys[0], 1, &iod, csum_ctx->dct_sv_ic, sgl); - if (rc != 0) - daos_csummer_free_ic(csum_ctx->dct_csummer, &csum_ctx->dct_sv_ic); + /* + * g_oids[1]: DVT_FAKE_SV_COUNT successive overwrites with distinct content and checksum. + * sv_idx 0: epoch 1, filler 'b' + * sv_idx 1: epoch 2, filler 'c' + * Fetching at epoch N returns dct_sv_ics[N-1]; DAOS_EPOCH_MAX returns the last entry. + */ + epoch = 1; + filler = 'b'; + for (sv_idx = 0; sv_idx < DVT_FAKE_SV_COUNT; sv_idx++) { + memset(buf, filler, csum_ctx->dct_sv_size); + d_iov_set(sgl->sg_iovs, &buf[0], csum_ctx->dct_sv_size); + rc = daos_csummer_calc_iods(csum_ctx->dct_csummer, sgl, &iod, NULL, 1, false, NULL, + 0, &csum_ctx->dct_sv_ics[sv_idx]); + if (rc != 0) + goto out_ics; + rc = vos_obj_update(coh, g_oids[1], epoch, 0, 0, &g_dkeys[0], 1, &iod, + csum_ctx->dct_sv_ics[sv_idx], sgl); + if (rc != 0) + goto out_ics; + + epoch++; + filler++; + } + +out_ics: + if (rc != 0) { + for (sv_idx = 0; sv_idx < DVT_FAKE_SV_COUNT; sv_idx++) { + if (csum_ctx->dct_sv_ics[sv_idx] == NULL) + break; + daos_csummer_free_ic(csum_ctx->dct_csummer, &csum_ctx->dct_sv_ics[sv_idx]); + } + } out_buf: D_FREE(buf); out: @@ -783,7 +807,7 @@ ddb_test_csum_setup(void **state) csum_ctx->dct_recx_size = DVT_FAKE_RECX_SIZE; csum_ctx->dct_chunk_size = DVT_FAKE_CHUNK_SIZE; csum_ctx->dct_csum_type = DVT_FAKE_CSUM_TYPE; - csum_ctx->dct_sv_ic = NULL; + memset(&csum_ctx->dct_sv_ics[0], 0, sizeof(csum_ctx->dct_sv_ics)); memset(&csum_ctx->dct_recx_ics[0], 0, sizeof(csum_ctx->dct_recx_ics)); rc = daos_csummer_init_with_type(&csum_ctx->dct_csummer, csum_ctx->dct_csum_type, csum_ctx->dct_chunk_size, 0); @@ -853,7 +877,8 @@ ddb_test_csum_teardown(void **state) if (rc != 0) goto out; - daos_csummer_free_ic(csum_ctx->dct_csummer, &csum_ctx->dct_sv_ic); + for (ci_idx = 0; ci_idx < DVT_FAKE_SV_COUNT; ci_idx++) + daos_csummer_free_ic(csum_ctx->dct_csummer, &csum_ctx->dct_sv_ics[ci_idx]); for (ci_idx = 0; ci_idx < DVT_FAKE_RECX_COUNT; ci_idx++) daos_csummer_free_ic(csum_ctx->dct_csummer, &csum_ctx->dct_recx_ics[ci_idx]); daos_csummer_destroy(&csum_ctx->dct_csummer); diff --git a/src/utils/ddb/tests/ddb_test_driver.h b/src/utils/ddb/tests/ddb_test_driver.h index 6fc4018d664..0dde187e030 100644 --- a/src/utils/ddb/tests/ddb_test_driver.h +++ b/src/utils/ddb/tests/ddb_test_driver.h @@ -46,6 +46,7 @@ struct dt_vos_pool_ctx { void *dvt_extra; }; +#define DVT_FAKE_SV_COUNT (2) #define DVT_FAKE_RECX_COUNT (2) #define DVT_FAKE_SV_SIZE (1u << 10) #define DVT_FAKE_RECX_SIZE (1u << 13) @@ -59,7 +60,7 @@ struct dt_csum_ctx { enum DAOS_HASH_TYPE dct_csum_type; size_t dct_chunk_size; struct daos_csummer *dct_csummer; - struct dcs_iod_csums *dct_sv_ic; + struct dcs_iod_csums *dct_sv_ics[DVT_FAKE_SV_COUNT]; struct dcs_iod_csums *dct_recx_ics[DVT_FAKE_RECX_COUNT]; }; diff --git a/src/utils/ddb/tests/ddb_vos_tests.c b/src/utils/ddb/tests/ddb_vos_tests.c index 5235f491ffb..d8e668788f7 100644 --- a/src/utils/ddb/tests/ddb_vos_tests.c +++ b/src/utils/ddb/tests/ddb_vos_tests.c @@ -1285,7 +1285,29 @@ check_csum_sv_cb_002(void *cb_args, struct daos_recx_ep_list *rel, struct dcs_ci assert_true(ci_is_valid(ci)); assert_int_equal(ci->cs_nr, 1); assert_true(daos_csummer_compare_csum_info(csum_ctx->dct_csummer, ci, - csum_ctx->dct_sv_ic->ic_data)); + csum_ctx->dct_sv_ics[0]->ic_data)); + + return 0; +} + +static int +check_csum_sv_cb_003(void *cb_args, struct daos_recx_ep_list *rel, struct dcs_ci_list *cil) +{ + struct dt_csum_ctx *csum_ctx; + struct dcs_csum_info *ci; + + assert_non_null(cb_args); + assert_null(rel); + assert_non_null(cil); + + csum_ctx = cb_args; + assert_int_equal(cil->dcl_csum_infos_nr, 1); + + ci = dcs_csum_info_get(cil, 0); + assert_true(ci_is_valid(ci)); + assert_int_equal(ci->cs_nr, 1); + assert_true(daos_csummer_compare_csum_info(csum_ctx->dct_csummer, ci, + csum_ctx->dct_sv_ics[1]->ic_data)); return 0; } @@ -1308,9 +1330,14 @@ dump_csum_sv_tests(void **state) rc = dv_dump_csum(tctx->dvt_poh, &path, DAOS_EPOCH_MAX, check_csum_sv_cb_001, NULL); assert_success(rc); - /* with csum info */ + /* with csum info, epoch 1 returns the epoch-1 checksum */ + path.vtp_oid = g_oids[1]; + rc = dv_dump_csum(tctx->dvt_poh, &path, 1, check_csum_sv_cb_002, csum_ctx); + assert_success(rc); + + /* with csum info, EPOCH_MAX returns the epoch-2 (latest) checksum */ path.vtp_oid = g_oids[1]; - rc = dv_dump_csum(tctx->dvt_poh, &path, DAOS_EPOCH_MAX, check_csum_sv_cb_002, csum_ctx); + rc = dv_dump_csum(tctx->dvt_poh, &path, DAOS_EPOCH_MAX, check_csum_sv_cb_003, csum_ctx); assert_success(rc); /* with csum info, without callback */ From 89aef6b1ecb0163485bf373741948a2ef24ba7a6 Mon Sep 17 00:00:00 2001 From: Cedric Koch-Hofer Date: Wed, 10 Jun 2026 06:31:35 +0000 Subject: [PATCH 3/4] DAOS-17321 ddb: Avoid double error logging on callback failure in csum dump MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When dump_csum_sv() or dump_csum_recx() invoke the caller's callback and it returns an error, the same rc was previously logged twice: once as a callback error (D_ERROR) and again as a vos_fetch_end error (D_ERROR), because vos_fetch_end propagates the err argument it receives. Introduce a separate cb_rc variable to track the callback return code. Downgrade the callback failure log from D_ERROR to D_DEBUG — the error is propagated to the caller who is responsible for handling it. Only log vos_fetch_end at D_ERROR when it returns a genuinely new error (rc != cb_rc). Add a guard at the out label to ensure cb_rc is always returned to the caller even if vos_fetch_end silently returns 0. Reviewed-at: https://github.com/daos-stack/daos/pull/18444#discussion_r3374158202 Features: recovery Signed-off-by: Cedric Koch-Hofer --- src/utils/ddb/ddb_vos.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/src/utils/ddb/ddb_vos.c b/src/utils/ddb/ddb_vos.c index 451e9cf5196..718e2f3e6c0 100644 --- a/src/utils/ddb/ddb_vos.c +++ b/src/utils/ddb/ddb_vos.c @@ -1099,10 +1099,11 @@ dump_csum_sv(daos_handle_t coh, daos_key_t *dkey, daos_unit_oid_t *oid, daos_iod { daos_handle_t ioh; struct dcs_ci_list *cil; + int cb_rc = 0; int rc; rc = vos_fetch_begin(coh, *oid, epoch, dkey, 1, iod, VOS_OF_FETCH_CSUM, NULL, &ioh, NULL); - if (rc) { + if (!SUCCESS(rc)) { D_ERROR("vos_fetch_begin for csum dump of " DF_UOID " failed: " DF_RC "\n", DP_UOID(*oid), DP_RC(rc)); goto out; @@ -1110,17 +1111,19 @@ dump_csum_sv(daos_handle_t coh, daos_key_t *dkey, daos_unit_oid_t *oid, daos_iod cil = vos_ioh2ci(ioh); - rc = dump_cb(cb_arg, NULL, cil); - if (!SUCCESS(rc)) - D_ERROR("Csum dump callback for " DF_UOID " failed: " DF_RC "\n", DP_UOID(*oid), - DP_RC(rc)); + cb_rc = dump_cb(cb_arg, NULL, cil); + if (!SUCCESS(cb_rc)) + D_DEBUG(DB_IO, "Csum dump callback for " DF_UOID " returned: " DF_RC "\n", + DP_UOID(*oid), DP_RC(cb_rc)); - rc = vos_fetch_end(ioh, NULL, rc); - if (rc != 0) + rc = vos_fetch_end(ioh, NULL, cb_rc); + if (!SUCCESS(rc) && rc != cb_rc) D_ERROR("vos_fetch_end for csum dump of " DF_UOID " failed: " DF_RC "\n", DP_UOID(*oid), DP_RC(rc)); out: + if (!SUCCESS(cb_rc)) + rc = cb_rc; return rc; } @@ -1131,10 +1134,11 @@ dump_csum_recx(daos_handle_t coh, daos_key_t *dkey, daos_unit_oid_t *oid, daos_i daos_handle_t ioh; struct dcs_ci_list *cil; struct daos_recx_ep_list *rel; + int cb_rc = 0; int rc; rc = vos_fetch_begin(coh, *oid, epoch, dkey, 1, iod, VOS_OF_FETCH_CSUM, NULL, &ioh, NULL); - if (rc) { + if (!SUCCESS(rc)) { D_ERROR("vos_fetch_begin for csum dump of " DF_UOID " failed: " DF_RC "\n", DP_UOID(*oid), DP_RC(rc)); goto out; @@ -1143,19 +1147,21 @@ dump_csum_recx(daos_handle_t coh, daos_key_t *dkey, daos_unit_oid_t *oid, daos_i cil = vos_ioh2ci(ioh); rel = vos_ioh2recx_list(ioh); - rc = dump_cb(cb_arg, rel, cil); - if (!SUCCESS(rc)) - D_ERROR("Csum dump callback for " DF_UOID " failed: " DF_RC "\n", DP_UOID(*oid), - DP_RC(rc)); + cb_rc = dump_cb(cb_arg, rel, cil); + if (!SUCCESS(cb_rc)) + D_DEBUG(DB_IO, "Csum dump callback for " DF_UOID " returned: " DF_RC "\n", + DP_UOID(*oid), DP_RC(cb_rc)); /* rel ownership is transferred by vos_ioh2recx_list(); free before vos_fetch_end. */ daos_recx_ep_list_free(rel, iod->iod_nr); - rc = vos_fetch_end(ioh, NULL, rc); - if (rc != 0) + rc = vos_fetch_end(ioh, NULL, cb_rc); + if (!SUCCESS(rc) && rc != cb_rc) D_ERROR("vos_fetch_end for csum dump of " DF_UOID " failed: " DF_RC "\n", DP_UOID(*oid), DP_RC(rc)); out: + if (!SUCCESS(cb_rc)) + rc = cb_rc; return rc; } From 3a85ff94b1e6c2da38f10577425640b640e5e64b Mon Sep 17 00:00:00 2001 From: Cedric Koch-Hofer Date: Fri, 12 Jun 2026 15:05:11 +0000 Subject: [PATCH 4/4] DAOS-17321 ddb: Use caller-provided pool handle in csum test setup ddb_test_csum_setup() and ddb_test_csum_teardown() each opened and closed their own VOS pool handle, causing a redundant vos_pool_open / vos_pool_close round-trip when called from dv_test_csum_setup / dv_test_csum_teardown, which already hold an open handle in tctx->dvt_poh via dv_test_setup(). Replace the internal pool open/close with direct use of tctx->dvt_poh, and document the precondition in comments on both functions and their declarations. The pool handle lifecycle remains the caller's responsibility, keeping the functions self-contained and reusable from any test file that provides an open pool handle. Features: recovery Signed-off-by: Cedric Koch-Hofer --- src/utils/ddb/tests/ddb_test_driver.c | 45 ++++++++++++--------------- src/utils/ddb/tests/ddb_test_driver.h | 5 ++- src/utils/ddb/tests/ddb_vos_tests.c | 2 +- 3 files changed, 25 insertions(+), 27 deletions(-) diff --git a/src/utils/ddb/tests/ddb_test_driver.c b/src/utils/ddb/tests/ddb_test_driver.c index 339897a5521..61ab707f306 100644 --- a/src/utils/ddb/tests/ddb_test_driver.c +++ b/src/utils/ddb/tests/ddb_test_driver.c @@ -785,12 +785,17 @@ csum_test_recx_setup(struct dt_vos_pool_ctx *tctx, daos_handle_t coh, d_sg_list_ return rc; } +/* + * Populate the VOS pool with test data for checksum tests. + * + * Requires tctx->dvt_poh to be a valid open pool handle. The caller is responsible for opening and + * closing that handle. + */ int ddb_test_csum_setup(void **state) { struct dt_vos_pool_ctx *tctx; struct dt_csum_ctx *csum_ctx; - daos_handle_t poh; daos_handle_t coh; d_sg_list_t sgl; int rc; @@ -815,17 +820,13 @@ ddb_test_csum_setup(void **state) goto out_csum_ctx; tctx->dvt_extra = csum_ctx; - rc = vos_pool_open(tctx->dvt_pmem_file, tctx->dvt_pool_uuid, 0, &poh); - if (rc != 0) - goto out_csummer; - rc = uuid_parse(g_csum_uuid_str, csum_ctx->dct_cont_uuid); if (rc != 0) - goto out_pool; - rc = vos_cont_create(poh, csum_ctx->dct_cont_uuid); + goto out_csummer; + rc = vos_cont_create(tctx->dvt_poh, csum_ctx->dct_cont_uuid); if (rc != 0) - goto out_pool; - rc = vos_cont_open(poh, csum_ctx->dct_cont_uuid, &coh); + goto out_csummer; + rc = vos_cont_open(tctx->dvt_poh, csum_ctx->dct_cont_uuid, &coh); if (rc != 0) goto out_cont_destroy; @@ -844,9 +845,7 @@ ddb_test_csum_setup(void **state) vos_cont_close(coh); out_cont_destroy: if (rc != 0) - vos_cont_destroy(poh, csum_ctx->dct_cont_uuid); -out_pool: - vos_pool_close(poh); + vos_cont_destroy(tctx->dvt_poh, csum_ctx->dct_cont_uuid); out_csummer: if (rc != 0) daos_csummer_destroy(&csum_ctx->dct_csummer); @@ -859,23 +858,23 @@ ddb_test_csum_setup(void **state) return rc; } -int +/* + * Clean up the VOS pool state created by ddb_test_csum_setup(). + * + * Requires tctx->dvt_poh to be the same valid open pool handle that was provided to + * ddb_test_csum_setup(). The caller retains ownership of the handle. + */ +void ddb_test_csum_teardown(void **state) { struct dt_vos_pool_ctx *tctx; struct dt_csum_ctx *csum_ctx; - daos_handle_t poh; int ci_idx; - int rc = 0; tctx = *state; csum_ctx = tctx->dvt_extra; if (csum_ctx == NULL) - goto out; - - rc = vos_pool_open(tctx->dvt_pmem_file, tctx->dvt_pool_uuid, 0, &poh); - if (rc != 0) - goto out; + return; for (ci_idx = 0; ci_idx < DVT_FAKE_SV_COUNT; ci_idx++) daos_csummer_free_ic(csum_ctx->dct_csummer, &csum_ctx->dct_sv_ics[ci_idx]); @@ -883,13 +882,9 @@ ddb_test_csum_teardown(void **state) daos_csummer_free_ic(csum_ctx->dct_csummer, &csum_ctx->dct_recx_ics[ci_idx]); daos_csummer_destroy(&csum_ctx->dct_csummer); - vos_cont_destroy(poh, csum_ctx->dct_cont_uuid); + vos_cont_destroy(tctx->dvt_poh, csum_ctx->dct_cont_uuid); D_FREE(csum_ctx); - - vos_pool_close(poh); -out: - return rc; } /* diff --git a/src/utils/ddb/tests/ddb_test_driver.h b/src/utils/ddb/tests/ddb_test_driver.h index 0dde187e030..13457c9a9f1 100644 --- a/src/utils/ddb/tests/ddb_test_driver.h +++ b/src/utils/ddb/tests/ddb_test_driver.h @@ -78,9 +78,12 @@ void dvt_iov_alloc_str(d_iov_t *iov, const char *str); int ddb_test_setup_vos(void **state); int ddb_teardown_vos(void **state); +/* Requires tctx->dvt_poh to be a valid open pool handle (caller-owned). */ int ddb_test_csum_setup(void **state); -int +/* Requires tctx->dvt_poh to be the same valid open pool handle as provided to + * ddb_test_csum_setup(). */ +void ddb_test_csum_teardown(void **state); int ddb_parse_tests_run(void); diff --git a/src/utils/ddb/tests/ddb_vos_tests.c b/src/utils/ddb/tests/ddb_vos_tests.c index d8e668788f7..63dc15dd323 100644 --- a/src/utils/ddb/tests/ddb_vos_tests.c +++ b/src/utils/ddb/tests/ddb_vos_tests.c @@ -1116,7 +1116,7 @@ dv_test_csum_setup(void **state) static int dv_test_csum_teardown(void **state) { - assert_success(ddb_test_csum_teardown(state)); + ddb_test_csum_teardown(state); assert_success(dv_test_teardown(state)); return 0;