Refactoring pvmove with generic APIs

Hi,

This patchset is a follow up to the previous post:
https://www.redhat.com/archives/lvm-devel/2007-November/msg00034.html

This version introduces 3 APIs and let pvmove use them:
- 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.
- convert_segments_mirrored()
Convert each segment in the LV to mirrored.
- split_parent_segments()
For a given layer LV, walk through LVs in the VG and split any LV segment whose underlying layer segment is splitted.

I'm trying to add functionality for lvconvert, i.e. mirroring as a whole LV, not segment by segment.
However, it involves moving some codes from toollib to lib and I think further discussion may be necessary.
And anyway they will be an addition to this patchset.

So I'm posting this set first as they are stable and easy to review.

[1/6] add insert_layer_for_segments_on_pv()
[2/6] add convert_segments_mirrored()
[3/6] add split_parent_segments()
[4/6] change pvmove to use the generalized APIs
[5/6] remove unused mirrored_pv/mirrored_pe params from internal code [6/6] remove unused can_split param from allocate_extents() [Appendix] pvmove test script for 'make check'

Thanks,

Jun'ichi Nomura, NEC Corporation of America

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.

Thanks,

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: 6599 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20071206/77d8cdb6/attachment.bin>

Add convert_segments_mirrored().

The function converts each segment in the LV to mirrored.

This is a generalized version of add_pvmove_destinations() in the previous patchset:
https://www.redhat.com/archives/lvm-devel/2007-November/msg00036.html

Thanks,

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: 5926 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20071206/bcf07cf0/attachment.bin>

Add split_parent_segments().

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.

This is a generalized version of _split_source_lv_segment() in the previous post:
https://www.redhat.com/archives/lvm-devel/2007-November/msg00035.html

Thanks,

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: 2778 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20071206/b4dae9e4/attachment.bin>

This patch changes pvmove to use the generic APIs added in the previous patches.

Thanks,

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: 10183 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20071206/eebad08b/attachment.bin>

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,

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/20071206/2abae80e/attachment.bin>

'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,

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: 3628 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20071206/17554318/attachment.bin>

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.

# ./t-pvmove-basic.sh 
*   ok 1: set up temp file and loopback device
*   ok 2: create pv1
*   ok 3: create pv2
*   ok 4: create pv3
*   ok 5: create pv4
*   ok 6: create pv5
*   ok 7: Run this: pvcreate  
*   ok 8: set up a VG
*   ok 9: check environment setup/cleanup
*   ok 10: only specified LV is moved: from pv2 to pv5 only for lv1
*   ok 11: the 1st seg of 3-segs LV is moved: from pv1 of lv1 to pv4
*   ok 12: the 2nd seg of 3-segs LV is moved: from pv2 of lv1 to pv4
*   ok 13: the 3rd seg of 3-segs LV is moved: from pv3 of lv1 to pv4
*   ok 14: 1 out of 3 LVs is moved: from pv4 to pv5
*   ok 15: 2 out of 3 LVs are moved: from pv3 to pv5
*   ok 16: 3 out of 3 LVs are moved: from pv2 to pv5
*   ok 17: move the 1st stripe: from pv2 of lv2 to pv1
*   ok 18: move the 2nd stripe: from pv3 of lv2 to pv1
*   ok 19: move the 3rd stripe: from pv4 of lv2 to pv1
*   ok 20: match to the start of segment:from pv2:0-0 to pv5
*   ok 21: match to the middle of segment: from pv2:1-1 to pv5
*   ok 22: match to the end of segment: from pv2:2-2 to pv5
*   ok 23: no destination split: from pv2:0-2 to pv5
*   ok 24: destination split into 2: from pv2:0-2 to pv5:5-5 and pv4:5-6
*   ok 25: destination split into 3: from pv2:0-2 to {pv3,4,5}:5-5
*   ok 26: alloc normal on same PV for source and destination: from pv3:0-2 to pv3:5-7
*   ok 27: (cleanup previous test)
*   ok 28: alloc anywhere on same PV for source and destination: from pv3:0-2 to pv3:5-7
*   ok 29: alloc anywhere but better area available: from pv3:0-2 to pv3:5-7 or pv5:5-6,pv4:5-5
*   ok 30: alloc contiguous but area not available: from pv2:0-2 to pv5:5-5 and pv4:5-6
*   ok 31: (cleanup previous test)
*   ok 32: alloc contiguous and contiguous area available: from pv2:0-2 to pv5:0-0,pv5:3-5 and pv4:5-6
*   ok 33: multiple source LVs: from pv3 to pv5
*   ok 34: move inactive LV: from pv2 to pv5
*   ok 35: no PEs to move: from pv3 to pv1
*   ok 36: (cleanup previous test)
*   ok 37: no space available: from pv2:0-0 to pv1:0-0
*   ok 38: (cleanup previous test)
*   ok 39: same source and destination: from pv1 to pv1
*   ok 40: (cleanup previous test)
*   ok 41: pvmove abort
* passed all 41 test(s)

Thanks,

Jun'ichi Nomura, NEC Corporation of America
-------------- next part --------------
A non-text attachment was scrubbed...
Name: t-pvmove-basic.sh
Type: application/x-shellscript
Size: 12446 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20071206/c22128ad/attachment.bin>

Nick,

it would be better to make this more generic.

Ie. convert_segments(struct lvm_conversion_handler *handler, ...) rather than having a per target conversion function.

Heinz

Regards,
Heinz    -- The LVM Guy --

*** Software bugs are stupid.
    Nevertheless it needs not so stupid people to solve them ***

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

Heinz Mauelshagen                                 Red Hat GmbH
Consulting Development Engineer                   Am Sonnenhang 11
Storage Development                               56242 Marienrachdorf
                                                  Germany
Mauelshagen at RedHat.com                            PHONE +49  171 7803392
                                                  FAX   +49 2626 924446
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

Hi Heinz,

Thanks for the comment.

Yes, generic interface is nice.
But as for conversion, I think starting from per-target conversion functions is better.

We may have a generic library interface like this:

int lv_convert(struct logical_volume *lv,
struct segment_type *segtype,
struct conversion_param *conv)

The conversion parameters are very specific to segment types. So callers have to set up the conversion_param based on the segtype.

Parameter are perhaps passed as an array of name-value pairs. I.e.

  struct conversion_param {
     int n_params;
     struct {
       const char *name;
       const char *value;
     } *params;

}

conv = { 4,

           { { "mirrors", "3" }, { "region_size", "64" },
             { "log_count", "0" }, { "type", "segment-by-segment" } }
         }

And the segment type handlers interpret it.
I think it's good for external applications using liblvm2 but too much for LVM2 internal use.

What do you think?

Anyway, IMO, it's easy to add a generic wrapper once we make up underlying conversion functions for each segment types. So I think we should focus first on building up working conversion functions.

This pvmove patchset provides segment-by-segment conversion for mirror.
Next step would be whole-lv conversion for mirror. Then we can add a single wrapper for to-mirror conversion.

Thanks,

Jun'ichi Nomura, NEC Corporation of America