DAOS-19629 vos: Do not trim checksums fetched with VOS_OF_FETCH_CSUM by knard38 · Pull Request #19057 · daos-stack/daos

knard38 · 2026-09-14T08:27:11Z

Description

A regular fetch returns the visible part of each extent: its bio_iov starts at en_sel_ext.ex_lo, so save_csum() drops, with evt_entry_csum_update(), the checksums of the leading chunks hidden by newer extents or lying before the fetched range, to keep the checksum info aligned with the data verified by ds_csum_add2iod().

A csum-only fetch (VOS_OF_FETCH_CSUM) returns no data: akey_fetch_recx() records the whole physical extent (en_ext) in the recx list while save_csum() still trimmed its checksum info to the visible part. The two lists, meant to be read side by side, then disagreed on the number of chunks whenever the leading chunk(s) of an extent were hidden, and the checksums of those chunks could not be retrieved at all.

Keep the checksum info whole in csum-only fetch mode, so that the recx list entry and the checksum info entry of an extent both describe the same physical extent, one checksum per chunk. Document the contract of VOS_OF_FETCH_CSUM and add the VOS401.3 test covering the three layouts that hide the leading chunk(s) of an extent: leading overwrite, fetch range starting inside the extent, and middle overwrite splitting the extent in two visible parts.

Steps for the author:

Commit message follows the guidelines.
Appropriate Features or Test-tag pragmas were used.
Appropriate Functional Test Stages were run.
At least two positive code reviews including at least one code owner from each category referenced in the PR.
Testing is complete. If necessary, forced-landing label added and a reason added in a comment.

After all prior steps are complete:

Gatekeeper requested (daos-gatekeeper added as a reviewer).

A regular fetch returns the visible part of each extent: its bio_iov starts at en_sel_ext.ex_lo, so save_csum() drops, with evt_entry_csum_update(), the checksums of the leading chunks hidden by newer extents or lying before the fetched range, to keep the checksum info aligned with the data verified by ds_csum_add2iod(). A csum-only fetch (VOS_OF_FETCH_CSUM) returns no data: akey_fetch_recx() records the whole physical extent (en_ext) in the recx list while save_csum() still trimmed its checksum info to the visible part. The two lists, meant to be read side by side, then disagreed on the number of chunks whenever the leading chunk(s) of an extent were hidden, and the checksums of those chunks could not be retrieved at all. Keep the checksum info whole in csum-only fetch mode, so that the recx list entry and the checksum info entry of an extent both describe the same physical extent, one checksum per chunk. Document the contract of VOS_OF_FETCH_CSUM and add the VOS401.3 test covering the three layouts that hide the leading chunk(s) of an extent: leading overwrite, fetch range starting inside the extent, and middle overwrite splitting the extent in two visible parts. Features: checksum Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>

github-actions · 2026-09-14T08:27:29Z

Ticket title is 'vos: VOS_OF_FETCH_CSUM reports a trimmed checksum for a partially visible extent'
Status is 'In Review'
Labels: 'ddb'
https://daosio.atlassian.net/browse/DAOS-19629

daosbuild3 · 2026-09-14T16:33:33Z

Test stage Functional Hardware Medium MD on SSD completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-19057/1/testReport/

Nasf-Fan

Generally, the patch looks fine for me. Just wondering whether the new test case will be run during CI test for the PR by default? I did not find related logs in the CI result.

Nasf-Fan · 2026-09-15T06:40:57Z

+
+	arg = *state;
+
+	vts_key_gen(&dkey_name[0], arg->dkey_size, true, arg);


D_ASSERT(UPDATE_DKEY_SIZE >= arg->dkey_size);

Fixed with commit 9608ec6

knard38 · 2026-09-15T08:48:21Z

Generally, the patch looks fine for me. Just wondering whether the new test case will be run during CI test for the PR by default? I did not find related logs in the CI result.

@Nasf-Fan , from my understanding all the unit tests are running by default: the recovery tag is specific for the functional test.
You could find the new unit test results in https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos/job/PR-19057/1/testReport/UTEST_VOS/IO_oclass/

…/daos-19629/patch-001

Replace the equality checks added to io_csum_fetch_recx_partial() with bound checks (arg->dkey_size <= UPDATE_DKEY_SIZE, arg->akey_size <= UPDATE_AKEY_SIZE). The test group also runs under the DAOS_OT_DKEY_UINT64/DAOS_OT_AKEY_UINT64 otype variants, where dkey_size/akey_size are set to sizeof(uint64_t) instead of UPDATE_DKEY_SIZE/UPDATE_AKEY_SIZE, so the equality asserts would abort in those configurations. Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>

Nasf-Fan · 2026-09-16T00:56:35Z

https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos/job/PR-19057/1/testReport/UTEST_VOS/IO_oclass/

Thanks! It seems we did not collect logs for successful test cases.

…/daos-19629/patch-001 Features: checksum

daosbuild3 · 2026-09-17T00:16:50Z

Test stage Functional Hardware Medium MD on SSD completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-19057/3/testReport/

knard38 · 2026-09-17T06:33:04Z

The last CI Build #3 failure is unrelated to this PR. It failed with a functional test of the pool-service/OSA code which have no relation with this PR which only touches VOS checksum handling for VOS_OF_FETCH_CSUM fetches.

After some investigation, this failure should be related to the ticket DAOS-19589 which seems to not have fully fix the regression introduced by the the ticket DAOS-19087. More details could be found in the JIRA comment of the ticket DAOS-19589.

knard38 · 2026-09-17T06:37:16Z

According to my investigation of the last CI failure (see #19057 (comment)), could you lend this PR with the following message:

title: DAOS-19629 vos: Do not trim VOS_OF_FETCH_CSUM checksums
body:

A csum-only fetch (VOS_OF_FETCH_CSUM) returns no data: the recx list
always reports the whole physical extent of each visible entry, but
save_csum() still trimmed the paired checksum info to the visible
part, the way a regular data fetch does to stay aligned with its
bio_iov. The two lists then disagreed on the number of chunks
whenever a leading part of an extent was hidden by a newer overwrite
or by the start of the fetch range, and the checksums of those
hidden chunks could not be retrieved at all.

Keep the checksum info whole for csum-only fetches, so that a recx
list entry and its checksum info always describe the same physical
extent, one checksum per chunk. Document the VOS_OF_FETCH_CSUM
contract in vos_types.h and add vos_tests case VOS401.3, covering
the three layouts that hide a leading part of an extent: an
overwrite before the fetch, a fetch range starting inside the
extent, and an overwrite splitting the extent into two visible
parts.

knard38 self-assigned this Sep 14, 2026

knard38 added the CR Catastrophic Recovery Feature label Sep 14, 2026

knard38 added the release-3.0.0 Targeted for release 3.0.0 label Sep 14, 2026

knard38 marked this pull request as ready for review September 14, 2026 08:29

knard38 requested review from a team as code owners September 14, 2026 08:29

knard38 requested review from Nasf-Fan, NiuYawei and janekmi September 14, 2026 08:29

Nasf-Fan reviewed Sep 15, 2026

View reviewed changes

kanard38 and others added 2 commits September 15, 2026 09:14

Merge remote-tracking branch 'origin/master' into ckochhof/fix/master…

439b790

…/daos-19629/patch-001

knard38 requested a review from Nasf-Fan September 15, 2026 11:43

Nasf-Fan approved these changes Sep 16, 2026

View reviewed changes

Merge remote-tracking branch 'origin/master' into ckochhof/fix/master…

4c46a22

…/daos-19629/patch-001 Features: checksum

NiuYawei approved these changes Sep 16, 2026

View reviewed changes

knard38 requested a review from a team September 17, 2026 06:33

Navigation Menu

Uh oh!

DAOS-19629 vos: Do not trim checksums fetched with VOS_OF_FETCH_CSUM - #19057

DAOS-19629 vos: Do not trim checksums fetched with VOS_OF_FETCH_CSUM#19057
knard38 wants to merge 4 commits into
masterfrom
ckochhof/fix/master/daos-19629/patch-001

knard38 commented Sep 14, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 14, 2026 •

edited

Loading

Uh oh!

daosbuild3 commented Sep 14, 2026

Uh oh!

Nasf-Fan left a comment

Uh oh!

Nasf-Fan Sep 15, 2026

Uh oh!

knard38 Sep 15, 2026

Uh oh!

knard38 commented Sep 15, 2026

Uh oh!

Nasf-Fan commented Sep 16, 2026

Uh oh!

daosbuild3 commented Sep 17, 2026

Uh oh!

knard38 commented Sep 17, 2026

Uh oh!

knard38 commented Sep 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants


		arg = *state;

		vts_key_gen(&dkey_name[0], arg->dkey_size, true, arg);

Uh oh!

Conversation

knard38 commented Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Steps for the author:

After all prior steps are complete:

Uh oh!

github-actions Bot commented Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daosbuild3 commented Sep 14, 2026

Uh oh!

Nasf-Fan left a comment

Choose a reason for hiding this comment

Uh oh!

Nasf-Fan Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

knard38 Sep 15, 2026

Choose a reason for hiding this comment

Uh oh!

knard38 commented Sep 15, 2026

Uh oh!

Nasf-Fan commented Sep 16, 2026

Uh oh!

daosbuild3 commented Sep 17, 2026

Uh oh!

knard38 commented Sep 17, 2026

Uh oh!

knard38 commented Sep 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

knard38 commented Sep 14, 2026 •

edited

Loading

github-actions Bot commented Sep 14, 2026 •

edited

Loading