Cleanup partial failure of activation

Hi,

If activate_lv() fails, callers won't call deactivate_lv(). However, activate_lv() may succeeded activation partially.

For example, in this case: (with the latest 2.02.29-cvs)

  # dmsetup create --notable vg-lv0_mimage_0
  # lvcreate -l1 -m1 -nlv0 vg

lvcreate fails. "lv0", "lv0_mimage_0", "lv0_mimage_1" and "lv0_mlog" are once created but removed from the VG metadata. That's ok. However, maps of vg-lv0 and vg-lv0_mlog remain in kernel.

Attached patch deactivates a partially activated LV so that callers of activate_lv() don't need to care about partial failure.

Thanks,

Jun'ichi Nomura, NEC Corporation of America
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cleanup-partial-activation-failure.patch
Type: text/x-patch
Size: 506 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20071121/5f942175/attachment.bin>

But could the deactivation call deactivate something that was active *before* it was called? Are you sure this will always accurately return things to the state they were in prior to the failed call, and not deactivate something that was previously active?

Alasdair

agk at redhat.com

Currently, mirror, snapshot and pvmove are only stacking-type LVs. They don't allow partial activation.
# Is this assumption correct?

If the above is correct, the tree is either activated or deactivated as a whole. So, if activation failed, deactivating the LV would return it to the state before activation is called.

And if, in future, partial activation is allowed, I think the current activation implementation can't handle this case anyway:
1. activate partial tree
2. activate other tree on top of the partial tree
..
3. deactivate the other tree
<but don't want to deactivate the partial tree>

I'm not whether there actually is such a use case.

Thanks,

Jun'ichi Nomura, NEC Corporation of America

Have all the code paths been audited?

The general "self-correcting" philosophy throughout the activation code is to ensure the LV is in a known final state when they return success, and to cope with a broad range of initial states.

This patch would represent a change in the semantics of the functions, reducing the range of initial states handled sensibly.

Look at the lv_resume code path, for example. Wouldn't it be more sensible to revert to the previous table on failure than to attempt to remove the device?

I don't believe this problem can be resolved with such a tiny patch.

There are two approaches:
1) have proper reversion code inside the guts of the library activation code
2) let the caller, which possesses knowledge about the *sequence* of steps
making up the operation, handle reversion sensibly

I expect a mixture of the two - mostly (2) in the places we need it now (quick and easy and a few cases already done), but eventually more use of (1) which is technically a better solution (but harder).

Alasdair

agk at redhat.com

Thanks for the explanation.
I see the point.

Thanks,

Jun'ichi Nomura, NEC Corporation of America