Skip to contentSkip to conversation

Refactoring pvmove with generic APIs (rev. 3)

PATCH New Jun'ichi Nomura 1 participant 11 messages Dec 13, 2007

lore
Share & tools
lore
#0 lore raw

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
#1 lore raw

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>
#2 lore raw

PATCH 2/9 Add add_mirrors_to_segments()

In reply to Jun'ichi Nomura

Add 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>
#3 lore raw

PATCH 3/9 Add split_parent_segments_for_layer()

In reply to Jun'ichi Nomura

Add 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>
#4 lore raw

PATCH 4/9 Change pvmove to use the generalized APIs

In reply to Jun'ichi Nomura

This 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>
#5 lore raw

PATCH 5/9 Remove unused mirrored_pv/mirrored_pe params from internal code

In reply to Jun'ichi Nomura

Since 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>
#6 lore raw

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>
#7 lore raw

PATCH 7/9 Add remove_mirrors_from_segments() and remove_layer_from_segments()

In reply to Jun'ichi Nomura

Add 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>
#8 lore raw

PATCH 8/9 Change pvmove to use the generalized mirror-removal APIs

In reply to Jun'ichi Nomura

remove_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>
#9 lore raw

PATCH 9/9 Remove unused remove_pvmove_mirrors()

In reply to Jun'ichi Nomura

remove_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>
#10 lore raw

PATCH 1/1 pvmove test cases for 'make check'

In reply to Jun'ichi Nomura

Attached 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>