Refactoring pvmove with generic APIs (rev. 3)
cover Refactoring pvmove with generic APIs (rev. 3)
Hi,
This patchset is a follow up to the previous posts:
https://www.redhat.com/archives/lvm-devel/2007-November/msg00034.html
https://www.redhat.com/archives/lvm-devel/2007-December/msg00010.html
I found that I didn't generalize the removal-side of pvmove code. This version adds generic APIs for removing mirrors and layers for segment-by-segment mirroring.
So we have:
- insert_layer_for_segments_on_pv()
Insert a layer (a linear lv segment) below each lv segment of the LV, only when the segment is on a specified PV.
- remove_layer_for_segments_all()
Remove the layer from all LVs
- split_parent_segments_for_layer()
For a given layer LV, walk through LVs in the VG and split any LV segment whose underlying layer segment is splitted.
- add_mirrors_to_segments()
- remove_mirrors_from_segments()
Add mirrors to each segment of the LV
I'll follow up this patchset with generic APIs for lvconvert-type mirroring, i.e. mirroring as a whole LV.
The 7th to 9th patches are newly added.
Other patches are slightly modified to cope with the name changes and also for better comments, error messages and generalization. It might be better to move new ones to earlier part and change pvmove at once. But I kept the ordering for ease of the reviews. Changes are mentioned in the header of each patch.
[1/9] add insert_layer_for_segments_on_pv()
[2/9] add add_mirrors_to_segments()
[3/9] add split_parent_segments_for_layer()
[4/9] change pvmove to use the generalized APIs
[5/9] remove unused mirrored_pv/mirrored_pe params from internal code [6/9] remove unused can_split param from allocate_extents() [7/9] add remove_mirrors_from_segments() and remove_layer_from_segments() [8/9] change pvmove to use the generalized removal APIs
[9/9] remove unused remove_pvmove_mirrors() function [Appendix] pvmove test script for 'make check'
Thanks,
Signature · 1 line
Jun'ichi Nomura, NEC Corporation of America
PATCH 1/9 Add insert_layer_for_segments_on_pv()
Add insert_layer_for_segments_on_pv().
The function can be used to insert a layer below the LV, only for the segments on a specified PV.
Part of the code is derived from pvmove.
Changes since rev2:
* Use move_lv_segment_area() for layer insertion
* Add error message for insert_layer_for_segments_on_pv()
* Use PRIu32 instead of "%u"
* Replace 'stack; return 0;' with 'return_0;'
* Changed internal function names to more specific ones
- _extend_layer_lv() -> _extend_layer_lv_for_segment()
- _seg_pv_match() -> _match_seg_area_to_pe_range()
- _align_segment_boundary() -> _align_segment_boundary_to_pe_ranges()
Thanks,
Signature · 8 lines
Jun'ichi Nomura, NEC Corporation of America -------------- next part -------------- A non-text attachment was scrubbed... Name: lvm2-insert-layer-for-segments.patch Type: text/x-patch Size: 7033 bytes Desc: not available URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20071213/2f17f147/attachment.bin>
PATCH 2/9 Add add_mirrors_to_segments()
In reply to Jun'ichi NomuraAdd add_mirrors_to_segments().
The function adds mirrors to each segment in the LV.
Changes since rev2:
* Use seg_is_{striped,mirrored} for segtype check
* Changed the API name:
- convert_segments_mirrored() -> add_mirrors_to_segments()
* Add error message to add_mirrors_to_segments()
* Removed pvmove-specific comment from add_mirrors_to_segments()
Thanks,
Signature · 8 lines
Jun'ichi Nomura, NEC Corporation of America -------------- next part -------------- A non-text attachment was scrubbed... Name: lvm2-convert-segments-mirrored.patch Type: text/x-patch Size: 6013 bytes Desc: not available URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20071213/1f38115f/attachment.bin>
PATCH 3/9 Add split_parent_segments_for_layer()
In reply to Jun'ichi NomuraAdd split_parent_segments_for_layer().
The function walks through LVs in the VG.
For each lv segment, check if it's on multiple layer segments. If so, split the lv segment so that only 1 layer segment is under the lv segment.
For example, a segment on pvmove layer is a source segment of the move. If there are multiple underlying pvmove segments, it means the destination is splitted and the source segment should be splitted when the move completes.
The function is a helper for cases like this.
Changes since rev2:
* Changed function names to more specific ones
- split_parent_segments() -> split_parent_segments_for_layer()
- _split_parent_segment() -> _split_parent_area()
* Added cmd_context argument to split_parent_segments_for_layer.
* Use build_parallel_areas() to find the lowest-level segment boundaries for layer LV.
* Introduced _find_seg_pvs_by_le().
* Add incompatible boundary check in _split_parent_area() (e.g. the layer segments are mistakenly merged.)
Thanks,
Signature · 8 lines
Jun'ichi Nomura, NEC Corporation of America -------------- next part -------------- A non-text attachment was scrubbed... Name: lvm2-split-parent-segments.patch Type: text/x-patch Size: 4427 bytes Desc: not available URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20071213/a88849a6/attachment.bin>
PATCH 4/9 Change pvmove to use the generalized APIs
In reply to Jun'ichi NomuraThis patch changes pvmove to use the generic APIs added in the previous patches for "pvmove" layer insertion and mirror creation.
Changes since rev2:
* Use the renamed APIs
- add_mirrors_to_segments()
- split_parent_segments_for_layer()
Thanks,
Signature · 8 lines
Jun'ichi Nomura, NEC Corporation of America -------------- next part -------------- A non-text attachment was scrubbed... Name: lvm2-pvmove-use-generalized-conversion.patch Type: text/x-patch Size: 10360 bytes Desc: not available URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20071213/770fbb72/attachment.bin>
PATCH 5/9 Remove unused mirrored_pv/mirrored_pe params from internal code
In reply to Jun'ichi NomuraSince pvmove now uses generalized APIs, mirrored_pv and mirrored_pe, special params to pass the source PV segment to allocator, are no longer used.
The patch removes them from allocation internal codes. I left lv_extend() untouched since it was not changed by the previous attempt.
Of course, it's ok to remove them from lv_extend(), too.
Thanks,
Signature · 8 lines
Jun'ichi Nomura, NEC Corporation of America -------------- next part -------------- A non-text attachment was scrubbed... Name: lvm2-remove-unused-mirror_pe-args.patch Type: text/x-patch Size: 4595 bytes Desc: not available URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20071213/8e85c862/attachment.bin>
PATCH 6/9 Remove unused can_split param from allocate_extents()
In reply to Jun'ichi Nomura'can_split' was a parameter to specify that the allocation should not be splitted even if the policy is not ALLOC_CONTIGUOUS. It was there for pvmove, which was not able to handle splitted allocation.
However, since pvmove now can handle splitted destinations, we can remove the param from allocate_extents().
Thanks,
Signature · 8 lines
Jun'ichi Nomura, NEC Corporation of America -------------- next part -------------- A non-text attachment was scrubbed... Name: lvm2-remove-unused-can_split-args.patch Type: text/x-patch Size: 3593 bytes Desc: not available URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20071213/ac87e4f1/attachment.bin>
PATCH 7/9 Add remove_mirrors_from_segments() and remove_layer_from_segments()
In reply to Jun'ichi NomuraAdd generic interfaces for removing mirrors from segment-by-segment mirror and removing layers for segments.
(Generalizing pvmove layer removal)
- remove_layers_for_segments()
Remove layer below the segments of the LV.
- remove_layers_for_segments_all()
Remove the layer from all LVs in the VG.
- remove_mirrors_from_segments()
Thanks,
Signature · 8 lines
Jun'ichi Nomura, NEC Corporation of America -------------- next part -------------- A non-text attachment was scrubbed... Name: lvm2-remove-layer-from-segments.patch Type: text/x-patch Size: 6961 bytes Desc: not available URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20071213/058defb8/attachment.bin>
PATCH 8/9 Change pvmove to use the generalized mirror-removal APIs
In reply to Jun'ichi Nomuraremove_pvmove_mirrors() is replaced by generic interfaces.
# The 'FIXME' comment was copied from the original code,
# not newly added.
Thanks,
Signature · 8 lines
Jun'ichi Nomura, NEC Corporation of America -------------- next part -------------- A non-text attachment was scrubbed... Name: lvm2-pvmove-use-generalized-mirror-removal.patch Type: text/x-patch Size: 1036 bytes Desc: not available URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20071213/9a42601f/attachment.bin>
PATCH 9/9 Remove unused remove_pvmove_mirrors()
In reply to Jun'ichi Nomuraremove_pvmove_mirrors() is no longer used so removed.
Thanks,
Signature · 8 lines
Jun'ichi Nomura, NEC Corporation of America -------------- next part -------------- A non-text attachment was scrubbed... Name: lvm2-remove-unused-remove_pvmove_mirrors.patch Type: text/x-patch Size: 3194 bytes Desc: not available URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20071213/1024eecb/attachment.bin>
PATCH 1/1 pvmove test cases for 'make check'
In reply to Jun'ichi NomuraAttached is a test script for basic pvmove operations. More tests are added since the previous version. I hope it gets included in the test directory for future regression testings.
To run this, put it in LVM2/test directory, then
# cd test; ./t-pvmove-basic.sh With the posted patch set, all tests succeeds. Without the patches, tests for splitted destinations will fail.
Thanks,
Signature · 8 lines
Jun'ichi Nomura, NEC Corporation of America -------------- next part -------------- A non-text attachment was scrubbed... Name: lvm2-add-pvmove-test.patch Type: text/x-patch Size: 13799 bytes Desc: not available URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20071213/dd739db4/attachment.bin>