Additional fixes and changes to lvconvert restructure

These patchset includes bugfixes and feature updates to the recently committed lvconvert changes.

Bug fixes
[ 1/11] Correct 'const' for _lv_mimage_in_sync()
[ 2/11] Don't start polldaemon if conversion failed in early stage [ 3/11] Multiple fixes about to-corelog conversion
[ 4/11] Fix possible mirror image corruption
[ 5/11] Don't insert temporary layer for corelog mirror [ 6/11] init_mirror_in_sync(0) for corelog mirror addition [ 7/11] Fix semantic consistency of mirrorlog option

Feature updates
[ 8/11] Add internal 'CONVERTING' flag for LVs under conversion [ 9/11] Add a new attr field char for the CONVERTING LV [10/11] Warn if lvconvert needs activation to complete conversion [11/11] Start lvconvert polldaemon on activation (like pvmove)

Other things I would like to work on next are:
- Generalization of seg->mirrored_seg as upward link from LV to seg(s) which use the LV.
- Review and fix remove_mirror_images() so that 'removable_pvs' filtering works on stacked mirror.
- Review the suspend code path. I think 'noflush' should be avoided for lower-level mirrors. Otherwise the upper-level mirror can't suspend.
- Remaining patches from the last post:
https://www.redhat.com/archives/lvm-devel/2007-December/msg00045.html
[11/13] Add 'lv_mirrors' field to reporting functions [12/13] Remove hardcoded "_mlog" name from deptree construction [13/13] Allow disk logs for temporary resync mirror
- Creating a script for 'make check'

Thanks,

Jun'ichi Nomura, NEC Corporation of America

The patch removes this warning:

report/report.c:315: warning: passing argument 1 of '_lv_mimage_in_sync' discards qualifiers from pointer target type

Thanks,

Jun'ichi Nomura, NEC Corporation of America
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-const-correctness-for-lv_mimage_in_sync.patch
Type: text/x-patch
Size: 597 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20080104/4e2c5353/attachment.bin>

If lvconvert fails in the early stage, we shouldn't invoke polldaemon.

Thanks,

Jun'ichi Nomura, NEC Corporation of America
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-dont-poll-if-failed.patch
Type: text/x-patch
Size: 517 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20080104/2e852642/attachment.bin>

_remove_mirror_images() is the core function used for both mirror image removal and mirror log removal.
However, it has some bugs and to-corelog conversion didn't work at all.

The patches fixes them at once.
Please see the patch header for detailed information about each bug.

Thanks,

Jun'ichi Nomura, NEC Corporation of America
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-corelog-convert.patch
Type: text/x-patch
Size: 4452 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20080104/2f648174/attachment.bin>

collapse_mirrored_lv() is a function to move mimage LVs from temporary sync layer to the original LV after sync completion. However, the code did resume_lv between the removal of mimage LVs and addition of them. As a result, it's possible to make the mimage LVs out-of-sync while mirror log says in-sync.

Thanks,

Jun'ichi Nomura, NEC Corporation of America
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-atomic-remove-and-merge.patch
Type: text/x-patch
Size: 5279 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20080104/e6a4d298/attachment.bin>

lvconvert inserts a temporary layer so that it can sync newly added mirrors while keeping the original mirror in-sync. However, LVM2 manages in-sync status for corelog mirror by the global _mirror_in_sync variable.
So the insertion just duplicates the sync effort.

If the original mirror is corelog, we can just add mimage LV to it and restart sync.

Thanks,

Jun'ichi Nomura, NEC Corporation of America
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-dont-insert-tmplayer-for-corelog-mirror.patch
Type: text/x-patch
Size: 1834 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20080104/ebdb32d8/attachment.bin>

Since corelog mirror is controlled by the global _mirror_in_sync, lv_add_mirrors() should explicitly reset it when adding mirrors with corelog.
Otherwise, it might be set 'in-sync' in some other places and skip sync process.

Thanks,

Jun'ichi Nomura, NEC Corporation of America
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-init-sync-status-for-corelog.patch
Type: text/x-patch
Size: 844 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20080104/c58e3f54/attachment.bin>

'--mirrorlog'/'--corelog' options of lvconvert are used to add/remove disklog of mirrored LV.
However, it had different meanings for "adding mirrors to mirror", i.e. the options specify whether to use disklog for conversion. The patch corrects it.

Jun'ichi Nomura, NEC Corporation of America
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-semantic-consistency-of-mirrorlog-option.patch
Type: text/x-patch
Size: 2103 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20080104/53355b99/attachment.bin>

Add 'CONVERTING' flag for LVs under conversion. Reporting code and daemon restarting code will use it.

Thanks,

Jun'ichi Nomura, NEC Corporation of America
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lvconvert-add-flag.patch
Type: text/x-patch
Size: 2571 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20080104/6377cdc4/attachment.bin>

The patch changes the reporting format to show the conversion structure.

# lvs -o+convert_lv vg
  LV    VG   Attr   LSize Origin Snap%  Move Log Copy%  Convert          
  lvol0 vg   cwi--- 1.00G                               lvol0_mimagetmp_2

'c' in the 'Attr' field means 'LV under conversion'. 'Convert' field points to the temporary LV for the conversion.

Might be better to add a new char for Attr field than using volume-type char.

Thanks,

Jun'ichi Nomura, NEC Corporation of America
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lvconvert-add-attr-char-for-converting.patch
Type: text/x-patch
Size: 3974 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20080104/ed53e075/attachment.bin>

lvconvert requires activation of the LV, when it adds mirrors to already mirrored LV. The patch makes it to print a message when the LV was not activated.
(Alternative may force activation on lvconvert.)

Thanks,

Jun'ichi Nomura, NEC Corporation of America
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lvconvert-check-active.patch
Type: text/x-patch
Size: 974 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20080104/b3f50ed3/attachment.bin>

polldaemon should be invoked on activation
if the LV is under conversion.

Thanks,

Jun'ichi Nomura, NEC Corporation of America
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lvconvert-start-polling-on-activation.patch
Type: text/x-patch
Size: 2390 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20080104/c729f9a8/attachment.bin>

Not sure here - should we perform the activation immediately?

All applied, thanks.

Alasdair

agk at redhat.com

I think it's ok to leave it for the users decision. IMO, if the LV is not active when lvconvert is executed, there is likely a reason the LV is not activated.

It followed the way linear-to-mirror conversion used to work. If linear inactive LV is converted to mirror, syncing doesn't start until it's activated.
(With that regard, maybe I should have let lvconvert take
'--wait' rather than '--background' and return immediately
by default.)

Thanks,

Jun'ichi Nomura, NEC Corporation of America

..

For remove_mirror_images() to properly work with stacked mirror, I'm going to allow "mirror" segment with 1 area.

'removable_pvs' is used for selectively removing mimage LVs. Especially, it's important for mirror reconfiguration after device failure.
However, the current code can't handle the case where "_mimagetmp" LV turns to become single.

I've been thinking about this situation:

1 mirror is added to 2-way mirrored LV

      lv0
        +-- lv0_mimagetmp_2
        |      +-- lv0_mimage_0
        |      +-- lv0_mimage_1
        |      +-- lv0_mlog
        +-- lv0_mimage_2

lv0_mimage_2 is still syncing.
lv0_mlog contains sync status between lv0_mimage_0 and lv0_mimage_1.

If lv0_mimage_1 is removed, what should we do?

What the current code will do is:

      lv0
        +-- lv0_mimagetmp_2
        +-- lv0_mimage_2

Of course, it's not correct.

IMO, first, the lv0_mimage_1 is removed and lv0_mlog is set to mark in-sync,

      lv0
        +-- lv0_mimagetmp_2
        |      +-- lv0_mimage_0
        |      +-- lv0_mlog
        +-- lv0_mimage_2

then, polldaemon converts it after sync completion,

      lv0
        +-- lv0_mimage_0
        +-- lv0_mimage_2
        +-- lv0_mlog

So in the intermediate state, "mirror" segment will have only 1 area. There's already a code related to pvmove that uses "linear" target for "mirror" segment.
It's not enough to convert to "linear" segment because that makes mirror parameters (log LV, region size, etc.) lost.

Suspend is done top-down. So 'noflush' should not be a problem.

Thanks,

Jun'ichi Nomura, NEC Corporation of America