Repository navigation
Conversation
Offline tools such as ddb inspect the meta and WAL blobs of pools which may be damaged. The meta and WAL headers are checksum-protected, so a random corruption is rejected by bio_mc_open(), but a header with a valid checksum and an inconsistent content (torn software write, format bug, flawed repair) is only caught by the assertions of the engine's open path: a WAL blob ID left unset in the meta header or a RDB meta header referencing a data blob abort the process. Add bio_mc_hdr_verify(), a read-only entry point which opens the meta blob I/O context of a pool target, loads the meta header with the existing loader (magic, version and checksum checks), verifies its content, then opens the WAL blob it points to and loads the WAL header the same way. The meta header checks are the invariants bio_mc_open() relies on: a WAL blob ID is set, a RDB references no data blob, and the geometry handed to vos_pool_open() is non-zero (block size, header blocks, capacity). Each failed check is logged with the field and its values and reported as -DER_DF_INVAL; the loader errors are passed through (-DER_UNINIT, -DER_DF_INCOMPT, -DER_CSUM). The verifier never opens a session, replays the WAL or writes to the blobs. The failure sites are traced at the DB_MGMT level, like the rest of the open path. bio_mc_open(), meta_open() and wal_open() are not modified. The checks share the existing static load_meta_header() and load_wal_header() through two internal wrappers, meta_hdr_check() and wal_hdr_check(), declared in bio_wal.h. The content checks of the WAL header are added by a follow-up patch. Tests (bio_ut, VOS_md_on_ssd utest suite): stage checksum-valid forged meta headers on the meta blob of an open pool, one per invariant (missing WAL blob ID, zero block size, header blocks and capacity, data blob of a RDB), and check bio_mc_hdr_verify() returns -DER_DF_INVAL; also cover the bad magic, version and checksum loader errors through the verifier, a healthy pool before and after use, an unknown pool and the RDB/pool flag mismatch (-DER_NONEXIST from SMD). The forging tests end by restoring the original header and checking that it is accepted again and that the session reopens on it. The header tests get per-test setup and teardown functions which create and destroy their pool (a RDB pool for the flag test), so that a failed assertion does not leak it; the test arguments carry the bio_mc_flags of the pool, which the RDB fixture sets and the pool helpers read. Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
knard38
added
CR
|
Ticket title is 'ddb: implement wal_open and wal_close commands' |
|
Test stage Functional on EL 9 completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-19260/1/execution/node/1470/log |
|
Test stage Functional Hardware Medium Provider MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-19260/1/execution/node/2021/log |
| wal_hdr_check(struct bio_meta_context *mc) | ||
| { | ||
| return load_wal_header(mc); | ||
| } |
There was a problem hiding this comment.
The BIO MC open interface has already verified header, what are these functions introduced for? They'll be used by DDB? Them they should be part of DDB implementation.
There was a problem hiding this comment.
From my understanding, bio_mc_open() does verify the headers, but only their integrity (magic, version, checksum), and the new code doesn't duplicate that: meta_hdr_check() and wal_hdr_check() reuse the same load_meta_header() and load_wal_header(). What they add is what the open path doesn't do and what ddb needs on a pool that may be damaged:
- Open aborts on a header with a valid checksum and an inconsistent content. Its content checks are
D_ASSERTs: WAL blob ID set (bio_context.c:961), no data blob for a RDB (bio_context.c:973), checkpoint ID <= committed ID (bio_wal.c:2073), zero block count only with a zero ID (bio_wal.c:167). Fine for the engine, not for a recovery tool which has to report the error and carry on. I ranbio_mc_open()on checksum-valid forged headers (the unit tests of this PR and of the follow-up PR which checks the WAL header content): each of these four asserts ends with SIGABRT, wherebio_mc_hdr_verify()returns-DER_DF_INVALand logs the field. - Open checks only a small part of it. Here, a zero
mh_blk_bytes,mh_hdr_blksormh_tot_blksis accepted bybio_mc_open()and handed as is to VOS bybio_meta_get_attr(). The follow-up PRs (content of the WAL header, identity of the headers) widen the gap: counting their tests too, of the 34 inconsistent but checksum-valid headers forged by the unit tests,bio_mc_open()rejects 1, aborts on 5 and accepts 28, 8 of which abort at the nextbio_wal_query()(avoidfunction). A WAL block size of 0 makesbio_wal_replay()divide by zero. - Open is not read-only.
meta_open()andwal_open()rewrite a v1 header in place (bio_wal.c:2158,bio_wal.c:2046), I checked that the blob goes from version 1 to 2.bio_mc_hdr_verify()leaves it untouched.
On the location: the loaders, the checksum helpers and the opening of the meta/WAL blob contexts (load_*_header(), meta_csum_*(), __bio_ioctxt_open()) are static in BIO, and the content checks mirror the engine's WAL ID arithmetic and asserts (wal_next_id(), wal_used_blks()), so I kept them next to what they mirror. In ddb they would duplicate all of it and not follow a change of the engine. It is not specific to the WAL headers either: DAOS-19782 tracks the same need for VOS (a damaged evtree checksum configuration aborts the fetch on a D_ASSERT), which I plan to solve the same way.
The only public function here is bio_mc_hdr_verify(). In this PR wal_hdr_check() is just the loader, the follow-up PR which verifies the content of the WAL header extends it. ddb's dw_open(), in the ddb follow-up, calls the verifier before bio_mc_open().
I left bio_mc_open(), meta_open() and wal_open() untouched on purpose: turning their asserts into errors would change the failure mode of the engine, and the same asserts are also reached at run time (wal_used_blks() from bio_wal_reserve()).
| {"header verify on inconsistent meta header", wal_ut_bad_meta_hdr, wal_ut_hdr_pool_setup, | ||
| wal_ut_hdr_pool_teardown}, | ||
| {"header verify on rdb flag mismatch", wal_ut_rdb_flag_mismatch, wal_ut_hdr_rdb_pool_setup, | ||
| wal_ut_hdr_pool_teardown}, |
There was a problem hiding this comment.
WAL header is already verified on open, why do we need to introduce these unit tests?
There was a problem hiding this comment.
These tests cover the new bio_mc_hdr_verify(), they don't re-test the open path. Nothing on master forges or checks a header (wal_ut.c and vts_wal.c only use consistent ones), and the interesting cases can't be tested through bio_mc_open(): a header with a valid checksum and an inconsistent content makes it assert, which is why the verifier exists (see my answer on bio_wal.c).
What the tests check:
- The integrity errors of the meta header loader (magic, version, checksum) are returned unchanged through the verifier.
- One forged header per invariant, with a valid checksum, gives
-DER_DF_INVAL: no WAL blob ID, zero block size, zero header blocks, zero capacity, and a data blob on a RDB (with a RDB pool). - Consistent headers are accepted (fresh pool with the session opened or closed, used pool) and an unknown pool gives
-DER_NONEXIST. Each forging test ends by restoring the original header and checking that it is accepted again and that the session reopens on it, so the verifier is shown to accept what the engine opens.
About the WAL header: in this PR it is only loaded with the loader of the open path, so the tests only check that consistent WAL headers are accepted. The forged WAL header cases come with its content checks in the follow-up PR.
If the verifier were moved to ddb, its tests would move with it, so this is the same question as in the other thread.
| } | ||
|
|
||
| /* Geometry handed to vos_pool_open() by bio_meta_get_attr(); the format never writes 0 */ | ||
| if (hdr->mh_blk_bytes == 0 || hdr->mh_hdr_blks == 0 || hdr->mh_tot_blks == 0) { |
There was a problem hiding this comment.
Only zero is invalid size? Are there some initial value? such as: meta_hdr->mh_blk_bytes = META_BLK_SZ. Then if possible, we can exactly check whether they are the expected values or not.
There was a problem hiding this comment.
- Make more restrictive constants check
Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
The geometry check of bio_mc_hdr_verify() only rejected a zero block size, header size and capacity. A header with a valid checksum and any other value is accepted by bio_mc_open() and handed as is to VOS by bio_meta_get_attr(). VOS locates its meta data in the blob at mh_hdr_blks * mh_blk_bytes, so a block size of 512 or 8192 bytes, or two header blocks, moves it. Check the block size and the header size against the constants the format has always written (META_BLK_SZ and META_HDR_BLKS). The capacity follows the size of the pool and has no expected value: check that it is set and that it fits in the meta blob next to the header. It is compared in blocks, since the product with the block size can wrap (a capacity of 1 << 60 blocks gives a capacity of 0 in bio_meta_get_attr()). Each failed check is logged with the values and reported as -DER_DF_INVAL. Tests (bio_ut, VOS_md_on_ssd utest suite): forge checksum-valid meta headers with a block size of 512 and 8192 bytes, with two header blocks, with a capacity which fills the blob exactly (accepted), with one block more and with 1 << 60 blocks (rejected). Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
The doc comment of bio_mc_hdr_verify() listed the checks the function performs. The list repeats the code and the return codes documented below it, and has to be edited each time a check changes. The checks are described where they are implemented, in the comment of meta_hdr_verify(). Keep what a caller needs to know: what the function verifies, that it is read-only (unlike bio_mc_open(), which upgrades a version 1 meta or WAL header when it opens the blobs) and what it is for. Comment only, no functional change. Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@hpe.com>
…/daos-19577/patch-001
|
Test stage Functional Hardware Medium MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-19260/2/execution/node/1836/log |
…/daos-19577/patch-001
|
Test stage Functional Hardware Medium Provider MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-19260/3/execution/node/1864/log |
|
Test stage Functional Hardware Medium MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-19260/3/execution/node/1807/log |
Summary
Offline tools such as ddb inspect the meta and WAL blobs of pools which may be damaged. The headers are checksum-protected, but a header with a valid checksum and an inconsistent content (torn write, format bug, flawed repair) is only caught by assertions of the engine's open path — a WAL blob ID left unset, or a RDB meta header referencing a data blob — which abort the process. A recovery tool has to report the error and carry on.
What this PR adds
bio_mc_hdr_verify(xs_ctxt, pool_id, flags)(bio.h,bio_context.c): opens the meta and WAL blob I/O contexts of a pool target, loads both headers with the existing loaders (magic, version, checksum) and verifies the content of the meta header. It never opens a session, replays the WAL or writes to the blobs (in particular, no v1→v2 header upgrade), and returns 0 when no meta device is configured (PMEM).flagsonly selects the SMD table (BIO_MC_FL_RDB).meta_hdr_verify()inbio_wal.c): a WAL blob ID is set, a RDB references no data blob (the two conditionsbio_mc_open()asserts on), and the geometry is non-zero.D_ERRORand returned as-DER_DF_INVAL; loader errors (-DER_UNINIT,-DER_DF_INCOMPT,-DER_CSUM) and the SMD lookup error (-DER_NONEXIST) are passed through. The failure sites are also traced atDB_MGMTwith the pool UUID.bio_mc_open(),meta_open()andwal_open()are not modified, so the engine keeps its core dump at the point of detection, while recovery tools get their own tolerant, read-only path. The WAL header content checks follow in part 2.Tests
bio_ut(utest suiteVOS_md_on_ssd, AIO bdev):Validation
-Werrorbuild andbio_ut(11/11) in MD-on-SSD mode;git-clang-format,codespellandgit diff --checkclean. NoFeatures:pragma on purpose: no functional test exercises the verifier.Steps for the author:
After all prior steps are complete: