vitalif/vitastor
5
6
Fork 12
Code Issues 4 Pull Requests 6 Actions Releases 111 Wiki Activity

Fix misleading COPY_BUF_JOURNAL flag on hole slices #78

Merged
vitalif merged 2 commits from changwei.ye/vitastor:fix-misleading-flag into master 2026-08-31 14:22:39 +03:00
Conversation 0 Commits 2 Files Changed 3 +4 -4
changwei.ye commented 2026-08-28 06:33:33 +03:00
Contributor
Copy Link

Description

Fix misleading COPY_BUF_JOURNAL flag on hole slices

During compaction (`fill_partial_checksum_blocks`), when a partially overwritten
checksum block is encountered, the old data needs to be read from the base layer
(`clean_wr`). A coalesced slice is inserted into `read_vec` to logically reference
this base block data.

Previously, this coalesced slice was marked with `COPY_BUF_JOURNAL`. This is a
misuse of the flag because the underlying data originates from the data area (a
BIG_WRITE block), not the journal area. It should conceptually use `COPY_BUF_DATA`.

Although this misuse did not cause any functional bugs—because the coalesced slice
also carries the `COPY_BUF_COALESCED` flag, which perfectly short-circuits the I/O
submission in `read_buffered` and avoids erroneously reading from `journal_fd`—it
creates semantic confusion for future maintainers. `COPY_BUF_JOURNAL` is consistently
used elsewhere to denote small writes (journal data).

This commit changes the flag to `COPY_BUF_DATA` to ensure semantic correctness and
code self-consistency.

Fix compilation errors regarding type conversions and pointer arithmetic

- src/blockstore/v1/init.cpp: Add explicit uint32_t cast to fix narrowing conversion error when initializing dirty_entry state.

- src/client/msgr_receive.cpp: Cast void pointer (iov_base) to char* before performing pointer arithmetic.

compile under clang 18.1.3
‘‘’
[build] /home/yecw/ws/src/vitastor/src/client/msgr_receive.cpp:530:43: error: arithmetic on a pointer to void
[build] 530 | cl->recv_list[i].iov_base += result;
[build] | ~~~~~~~~~~~~~~~~~~~~~~~~~ ^

uild] /home/yecw/ws/src/vitastor/src/blockstore/v1/init.cpp:1009:34: error: non-constant-expression cannot be narrowed from type 'int' to 'uint32_t' (aka 'unsigned int') in initializer list [-Wc++11-narrowing]
[build] 1009 | .state = (BS_ST_SMALL_WRITE | BS_ST_SYNCED | (je->type == JE_SMALL_WRITE_INSTANT ? BS_ST_INSTANT : 0)),
[build] | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[build] /home/yecw/ws/src/vitastor/src/blockstore/v1/init.cpp:1009:34: note: insert an explicit cast to silence this issue
[build] 1009 | .state = (BS_ST_SMALL_WRITE | BS_ST_SYNCED | (je->type == JE_SMALL_WRITE_INSTANT ? BS_ST_INSTANT : 0)),
[build] | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[build] | static_cast<uint32_t>( )
[build] /home/yecw/ws/src/vitastor/src/blockstore/v1/init.cpp:1093:34: error: non-constant-expression cannot be narrowed from type 'int' to 'uint32_t' (aka 'unsigned int') in initializer list [-Wc++11-narrowing]
[build] 1093 | .state = (BS_ST_BIG_WRITE | BS_ST_SYNCED | (je->type == JE_BIG_WRITE_INSTANT ? BS_ST_INSTANT : 0)),
[build] | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[build] /home/yecw/ws/src/vitastor/src/blockstore/v1/init.cpp:1093:34: note: insert an explicit cast to silence this issue
[build] 1093 | .state = (BS_ST_BIG_WRITE | BS_ST_SYNCED | (je->type == JE_BIG_WRITE_INSTANT ? BS_ST_INSTANT : 0)),
[build] | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[build] | static_cast<uint32_t>(
“

Contributor Name

Changwei Ye

CLA

### Description Fix misleading COPY_BUF_JOURNAL flag on hole slices During compaction (`fill_partial_checksum_blocks`), when a partially overwritten checksum block is encountered, the old data needs to be read from the base layer (`clean_wr`). A coalesced slice is inserted into `read_vec` to logically reference this base block data. Previously, this coalesced slice was marked with `COPY_BUF_JOURNAL`. This is a misuse of the flag because the underlying data originates from the data area (a BIG_WRITE block), not the journal area. It should conceptually use `COPY_BUF_DATA`. Although this misuse did not cause any functional bugs—because the coalesced slice also carries the `COPY_BUF_COALESCED` flag, which perfectly short-circuits the I/O submission in `read_buffered` and avoids erroneously reading from `journal_fd`—it creates semantic confusion for future maintainers. `COPY_BUF_JOURNAL` is consistently used elsewhere to denote small writes (journal data). This commit changes the flag to `COPY_BUF_DATA` to ensure semantic correctness and code self-consistency. Fix compilation errors regarding type conversions and pointer arithmetic - src/blockstore/v1/init.cpp: Add explicit uint32_t cast to fix narrowing conversion error when initializing dirty_entry state. - src/client/msgr_receive.cpp: Cast void pointer (iov_base) to char* before performing pointer arithmetic. compile under clang 18.1.3 ‘‘’ [build] /home/yecw/ws/src/vitastor/src/client/msgr_receive.cpp:530:43: error: arithmetic on a pointer to void [build] 530 | cl->recv_list[i].iov_base += result; [build] | ~~~~~~~~~~~~~~~~~~~~~~~~~ ^ uild] /home/yecw/ws/src/vitastor/src/blockstore/v1/init.cpp:1009:34: error: non-constant-expression cannot be narrowed from type 'int' to 'uint32_t' (aka 'unsigned int') in initializer list [-Wc++11-narrowing] [build] 1009 | .state = (BS_ST_SMALL_WRITE | BS_ST_SYNCED | (je->type == JE_SMALL_WRITE_INSTANT ? BS_ST_INSTANT : 0)), [build] | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [build] /home/yecw/ws/src/vitastor/src/blockstore/v1/init.cpp:1009:34: note: insert an explicit cast to silence this issue [build] 1009 | .state = (BS_ST_SMALL_WRITE | BS_ST_SYNCED | (je->type == JE_SMALL_WRITE_INSTANT ? BS_ST_INSTANT : 0)), [build] | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [build] | static_cast<uint32_t>( ) [build] /home/yecw/ws/src/vitastor/src/blockstore/v1/init.cpp:1093:34: error: non-constant-expression cannot be narrowed from type 'int' to 'uint32_t' (aka 'unsigned int') in initializer list [-Wc++11-narrowing] [build] 1093 | .state = (BS_ST_BIG_WRITE | BS_ST_SYNCED | (je->type == JE_BIG_WRITE_INSTANT ? BS_ST_INSTANT : 0)), [build] | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [build] /home/yecw/ws/src/vitastor/src/blockstore/v1/init.cpp:1093:34: note: insert an explicit cast to silence this issue [build] 1093 | .state = (BS_ST_BIG_WRITE | BS_ST_SYNCED | (je->type == JE_BIG_WRITE_INSTANT ? BS_ST_INSTANT : 0)), [build] | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [build] | static_cast<uint32_t>( “ ### Contributor Name Changwei Ye ### CLA - [x] I accept Vitastor CLA agreement: https://git.yourcmc.ru/vitalif/vitastor/src/branch/master/CLA-en.md
changwei.ye added 1 commit 2026-08-28 06:33:33 +03:00
Fix misleading COPY_BUF_JOURNAL flag on hole slices 99ea4c6acb 
During compaction (`fill_partial_checksum_blocks`), when a partially overwritten
checksum block is encountered, the old data needs to be read from the base layer
(`clean_wr`). A coalesced slice is inserted into `read_vec` to logically reference
this base block data.

Previously, this coalesced slice was marked with `COPY_BUF_JOURNAL`. This is a
misuse of the flag because the underlying data originates from the data area (a
BIG_WRITE block), not the journal area. It should conceptually use `COPY_BUF_DATA`.

Although this misuse did not cause any functional bugs—because the coalesced slice
also carries the `COPY_BUF_COALESCED` flag, which perfectly short-circuits the I/O
submission in `read_buffered` and avoids erroneously reading from `journal_fd`—it
creates semantic confusion for future maintainers. `COPY_BUF_JOURNAL` is consistently
used elsewhere to denote small writes (journal data).

This commit changes the flag to `COPY_BUF_DATA` to ensure semantic correctness and
code self-consistency.
changwei.ye force-pushed fix-misleading-flag from 99ea4c6acb to 980096e0c2 2026-08-28 12:21:39 +03:00 Compare
changwei.ye force-pushed fix-misleading-flag from 980096e0c2 to 31dfebbbe8 2026-08-31 10:53:56 +03:00 Compare
changwei.ye changed title from Fix misleading COPY_BUF_JOURNAL flag on hole slices to WIP: Fix misleading COPY_BUF_JOURNAL flag on hole slices 2026-08-31 12:47:57 +03:00
changwei.ye force-pushed fix-misleading-flag from 31dfebbbe8 to 10a6782bdf 2026-08-31 13:44:51 +03:00 Compare
vitalif changed title from WIP: Fix misleading COPY_BUF_JOURNAL flag on hole slices to Fix misleading COPY_BUF_JOURNAL flag on hole slices 2026-08-31 14:22:19 +03:00
vitalif merged commit 10a6782bdf into master 2026-08-31 14:22:39 +03:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Assignees
Clear assignees
vitalif
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: vitalif/vitastor#78
No description provided.
Delete Branch "changwei.ye/vitastor:fix-misleading-flag"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?