diff --git a/src/blockstore/blockstore_read.cpp b/src/blockstore/blockstore_read.cpp index b3782dde..4a571f00 100644 --- a/src/blockstore/blockstore_read.cpp +++ b/src/blockstore/blockstore_read.cpp @@ -75,10 +75,11 @@ int blockstore_impl_t::dequeue_read(blockstore_op_t *op) return false; } } - if (need_skip && wr->type() == BS_HEAP_SMALL_WRITE && + if (need_skip && (wr->type() == BS_HEAP_SMALL_WRITE || wr->type() == BS_HEAP_INTENT_WRITE) && wr->small().offset < blk_end && wr->small().offset+wr->small().len > blk_start) { // Small write may mutate big write checksums during flush + // Intent write directly mutates physical data in-place skip_csum = COPY_BUF_SKIP_CSUM; } return true; diff --git a/src/test/test_blockstore.cpp b/src/test/test_blockstore.cpp index ba734dde..1dec1286 100644 --- a/src/test/test_blockstore.cpp +++ b/src/test/test_blockstore.cpp @@ -1091,6 +1091,63 @@ static void test_list_limit() free(op.buf); } +static void test_intent_write_corruption() +{ + printf("\n-- test_intent_write_corruption\n"); + + bs_test_t test; + test.default_cfg(); + test.config["csum_block_size"] = "16384"; + test.config["bitmap_granularity"] = "4096"; + test.config["perfect_csum_update"] = "false"; + test.init(); + + // 1. Write BIG_WRITE (0-48K) + blockstore_op_t op; + op.opcode = BS_OP_WRITE_STABLE; + op.oid = { .inode = 1, .stripe = 0 }; + op.version = 1; + op.offset = 0; + op.len = 49152; // 48K + op.buf = (uint8_t*)memalign_or_die(MEM_ALIGNMENT, op.len); + memset(op.buf, 0xaa, op.len); + test.exec_op(&op); + assert(op.retval == op.len); + + + // 2. Write unaligned INTENT_WRITE B (8K-12K) + free(op.buf); + op.opcode = BS_OP_WRITE_STABLE; + op.oid = { .inode = 1, .stripe = 0 }; + op.version = 2; + op.offset = 8192; // 8K + op.len = 4096; // 4K + op.buf = (uint8_t*)memalign_or_die(MEM_ALIGNMENT, op.len); + memset(op.buf, 0xbb, op.len); + test.exec_op(&op); + assert(op.retval == op.len); + + // 3. Read 12-40K + // This will hit the hole logic where B provided 8-12K, but we don't even request it. + // The request is for 12-40K. + // 12K is not aligned to 16K, so A must pad by reading 0-16K. + // However, 8-12K of the physical block has been overwritten by B. + // A will try to verify the 0-16K block with its old checksum and fail. + free(op.buf); + op.opcode = BS_OP_READ; + op.oid = { .inode = 1, .stripe = 0 }; + op.version = UINT64_MAX; // latest version + op.offset = 12288; // 12K + op.len = 28672; // 28K (so end = 40K) + op.buf = (uint8_t*)memalign_or_die(MEM_ALIGNMENT, op.len); + test.exec_op(&op); + + // Assert that the read succeeds without checksum corruption errors + assert(op.retval == op.len); + + free(op.buf); +} + // FIXME Add a simple intent_write / big_intent test int main(int narg, char *args[]) @@ -1100,6 +1157,7 @@ int main(int narg, char *args[]) test_fsync(true); test_fsync_meta_before_complete(); test_intent_over_unstable(); + test_intent_write_corruption(); test_padded_csum_intent(false); test_padded_csum_intent(true); test_padded_csum_sparse_leading_hole();