[VDO-5752] Merge Feature/add-vdoformat-into-kernel into main branch#428
[VDO-5752] Merge Feature/add-vdoformat-into-kernel into main branch#428bjohnsto merged 23 commits intodm-vdo:mainfrom
Conversation
There was a problem hiding this comment.
Just a couple minor comments on commit messages. The one for patch 8 () is more important since it'll be going upstream. The cover letter draft does mention a reason for that patch, but having it in the commit message for that patch would be good.
The cover letter draft similarly is pretty good. I'd make some minor adjustments but there's nothing critical:
In the first paragraph, I'd swap the sentences. Start with what the series does ("This series moves the formatting capability...", then move on to why. ("Currently, ...")
The second paragraph, detailing how it works, looks good.
The patch-by -patch breakdown seems a little verbose, but fine. There's a missing backtick in the patch 9 paragraph, but the real cover letter will need to be all text (no markup) for e-mail, so that'll all have to get stripped out anyway.
In general, though, I'd say that if there is something interesting to say about a specific patch, the information should be in the commit message for that patch rather than (or possibly in addition to) in the cover letter. Anyone reading the cover letter should also have the entire series available to them, but the patches will get committed and the cover letter won't. So a later reader will have to depend on the commit messages. To the extent that this per-patch information is useful, could you try to make sure it's reflected in the commit messages themselves, too?
|
Oh, commit messages can also be patched up/massaged when we submit this upstream, but I would prefer to do it sooner if possible. |
5f60be5 to
62cfcfb
Compare
lorelei-sakai
left a comment
There was a problem hiding this comment.
Two minor but possibly worrisome message edits.
Not even sure these are worth fussing over.
Add vdo_initialize_component_states() to populate the super block, computing the space required for the main VDO components on disk. Those include the slab depot, block map, and recovery journal. Signed-off-by: Bruce Johnston <bjohnsto@redhat.com>
Update the user mode code to call the new super block initialization code in encodings.[ch] and remove the original user mode formatting functions. Update vdoConfig.c to use new location of the constant RECOVERY_JOURNAL_STARTING_SEQUENCE_NUMBER now that it has been moved into constants.h. This fixes compiling user mode code due to the change in the previous commit. signed-off-by: Bruce Johnston <bjohnsto@redhat.com>
Extend the dm table line with three new optional parameters: indexMemory (UDS index memory size), indexSparse (dense vs sparse index), and slabSize (blocks per allocation slab). These values are parsed, validated, and stored in the device configuration for use during formatting. Rework the slab size constants from the single MAX_VDO_SLAB_BITS into explicit MIN_VDO_SLAB_BLOCKS, MAX_VDO_SLAB_BLOCKS, and DEFAULT_VDO_SLAB_BLOCKS values. Bump the target version from 9.1.0 to 9.2.0 to reflect this table line change. Signed-off-by: Bruce Johnston <bjohnsto@redhat.com>
Update the user mode code to use new respelled constant but also make sure vdoformat tool still uses slab bits. Signed-off-by: Bruce Johnston <bjohnsto@redhat.com>
Add a validation check that the logical size passed via the table line does not exceed MAXIMUM_VDO_LOGICAL_BLOCKS. Signed-off-by: Bruce Johnston <bjohnsto@redhat.com>
Add a new perl test which is a copy of VDOFormat.pm that has been modified to work with in kernel formatting. A separate test makes sense since some of the error messages are different between vdoformat and the kernel and some parameters behave differently when passed into the kernel. Signed-off-by: Bruce Johnston <bjohnsto@redhat.com>
Add vdo_encode_volume_geometry() to write the geometry block into a buffer so that it can be written to disk. The corresponding decode path already exists. Signed-off-by: Bruce Johnston <bjohnsto@redhat.com>
Update user mode code to use the new volume geometry encoding function. Signed-off-by: Bruce Johnston <bjohnsto@redhat.com>
Introduce a vdo_geometry_block structure, containing a vio and buffer, mirroring the existing vdo_super_block structure. Both are now initialized at VDO startup and freed at shutdown, establishing the infrastructure needed to read and write the geometry block using the same mechanisms as the super block. Refactor read_geometry_block() to use the new structure. Signed-off-by: Bruce Johnston <bjohnsto@redhat.com>
Add vdo_submit_metadata_vio_wait(), a synchronous I/O submission helper that blocks until completion. This is needed for I/O during early initialization before work queues are available. Refactor read_geometry_block() to use it. Signed-off-by: Bruce Johnston <bjohnsto@redhat.com>
Add the core formatting logic. The initialization path is updated to read the geometry block (block 0 on the storage device). If the block is entirely zeroed, the device is treated as unformatted and vdo_format() is called. Otherwise, the existing geometry is parsed and the VDO is loaded as before. The vdo_format() function initializes the volume geometry and super block, and marks the VDO as needing it's layout saved to disk. Signed-off-by: Bruce Johnston <bjohnsto@redhat.com>
Add user mode version of uuid_gen function. Signed-off-by: Bruce Johnston <bjohnsto@redhat.com>
Add vdo_save_super_block() and vdo_save_geometry_block() to perform asynchronous writes of the super block and geometry block respectively. Add vdo_clear_layout() to zero the UDS index's first block, the block map partition, and the recovery journal partition. These operations are driven by new phases in the pre-load state machine (PRE_LOAD_PHASE_FORMAT_*), ensuring that disk writes happen during pre-resume rather than during dmsetup create. Signed-off-by: Bruce Johnston <bjohnsto@redhat.com>
Add user mode version of blkdev_issue_zeroout function. Since the function is being put in the uds code, we also need to move the definition of gfp_t to uds code. Signed-off-by: Bruce Johnston <bjohnsto@redhat.com>
Add support for in kernel formatting to our unit tests and add a new unit test to test such formatting. One subtest in FormatVDOInKernel_t1.c mimics FormatVDO_t1.c and one mimics FormatVDO_t3.c. There is no version for FormatVDO_t2.c because it makes no sense to have 0 as the logical size passed into the dmsetup code since dmsetup blocks such a thing with an error. Signed-off-by: Bruce Johnston <bjohnsto@redhat.com>
Add additional sub tests now that formatting actually works. Signed-off-by: Bruce Johnston <bjohnsto@redhat.com>
Turn off the new optional formatting parameters from showing up in device table, so the parameter count will work. Signed-off-by: Bruce Johnston <bjohnsto@redhat.com>
Update the code in the corruptGeometry test to write random data rather than all zeroes. An empty geometry block is now the indicator for when to format using the kernel code. Signed-off-by: Bruce Johnston <bjohnsto@redhat.com>
Add a suite of tests that are broad enough to sufficiently test formatting in the kernel. This includes changes that create the actual suite and a new build type to run it with the appropriate kernel formatting flag set. Signed-off-by: Bruce Johnston <bjohnsto@redhat.com>
Add device-mapper target version checking function and code to call it to decide when to add format in kernel parameters to the vdo table line. The current version is 9.1 with the feature branch having the version bumped to 9.2 The kernel code doesn't do a very good job parsing optional parameters that it doesn't know about, so the new formatting parameters were casuing failures when passed to old VDO modules that didn't have the formatting code in it. Signed-off-by: Bruce Johnston <bjohnsto@redhat.com>
Draft cover letter for upstream submission. Skips test and user mode changes
Subject: [PATCH 0/9] dm vdo: add in-kernel VDO formatting support
This series moves the formatting capability of the userspace tool
vdoformatinto the kernel dm-vdo target itself, allowing a VDO volume to be created and formatted in a singledmsetup createinvocation. Currently, creating a new VDO volume requires a two-step process: first running thevdoformattool to write the on-disk metadata structures, and then loading the dm target viadmsetup create.The kernel determines whether a device needs formatting by reading the geometry block (block 0 on the storage device) during initialization. If the block is entirely zeroed, the device is treated as unformatted and the target formats it. Otherwise, the existing VDO is loaded as before. The actual writes of the formatted metadata to disk are deferred to the dm target's pre-resume operation rather than being performed during
dmsetup create.The series is organized as follows:
Patches 1-2 add initialization functions for the two primary on-disk metadata structures.
vdo_initialize_volume_geometry()populates the geometry block, computing the index size from the UDS configuration to determine region boundaries.vdo_initialize_component_states()populates the super block, including location information for the slab depot, block map, and recovery journal. Theuds_compute_index_size()function is added to calculate the space required for the uds indexer on disk.Patch 3 extends the dm table line with three new optional parameters:
indexMemory(UDS index memory size),indexSparse(dense vs sparse index), andslabSize(blocks per allocation slab). These values are parsed, validated, and stored in the device configuration for use during formatting. The slab size constants are reworked from the singleMAX_VDO_SLAB_BITSinto explicitMIN_VDO_SLAB_BLOCKS,MAX_VDO_SLAB_BLOCKS, andDEFAULT_VDO_SLAB_BLOCKSvalues. The target version is bumped from 9.1.0 to 9.2.0 to reflect this table line change.Patch 4 adds an upfront validation check that the logical size passed via the table line does not exceed
MAXIMUM_VDO_LOGICAL_BLOCKS.Patch 5 adds a
vdo_encode_volume_geometry()function that writes the magic number, header, geometry fields, and CRC32 checksum into a buffer so that it can be written to disk.Patch 6 introduces a
vdo_geometry_blockstructure containing a vio and buffer, mirroring the existingvdo_super_blockstructure. Both are now initialized at VDO startup and freed at shutdown, establishing the infrastructure needed to read and write the geometry block using the same mechanisms as the super block.Patch 7 adds
vdo_submit_metadata_vio_wait(), a synchronous I/O submission helper that blocks until completion. This is needed for I/O during early initialization before work queues are available. The existingread_geometry_block()is refactored to use it.Patch 8 adds the core formatting logic. The
vdo_format()function generates a UUID and nonce, initializes the volume geometry and component states, and marks the VDO as needing formatting. The initialization path is updated to read the geometry block and branch: if all zeros, format; otherwise, parse the existing geometry and load.Patch 9 completes the feature by writing the formatted metadata to disk. New
vdo_save_super_block()andvdo_save_geometry_block()functions perform asynchronous writes of the super block and geometry block respectively. Avdo_clear_layout()function zeroes the UDS index's first block, the block map partition, and the recovery journal partition. These operations are driven by new phases in the pre-load state machine (PRE_LOAD_PHASE_FORMAT_*), ensuring that disk writes happen during pre-resume rather than duringdmsetup create.Diffstat across the series:
12 files changed, 645 insertions(+), 133 deletions(-)
constants.h | 17 +-
dm-vdo-target.c | 139 ++++++++++++++-
encodings.c | 229 +++++++++++++++++++++++----
encodings.h | 18 ++-
index-layout.c | 6 +-
indexer.h | 2 -
io-submitter.c | 31 ++++
io-submitter.h | 4 +
status-codes.c | 2 +
types.h | 3 +
vdo.c | 308 +++++++++++++++++++++++++++----------
vdo.h | 19 +++
In addition, the remaining commits provide testing (user mode and functional) of the new feature, as well as vdoformat refactoring to use the formatting functions that now exist in encodings.[ch].