Skip to contentSkip to conversation

lvrename: add mirrored LV support

PATCH In review Jun'ichi Nomura 2 participants 3 messages Jul 27, 2007 – Aug 1, 2007

lore
Share & tools
lore

Jun'ichi Nomura

author
#0 lore raw

PATCH 1/2 lvrename: add mirrored LV support

Hi,

Currently, renaming is not supported for mirrored LVs:

# lvrename testvg/l testvg/newlv
  Mirrored LV, "l" cannot be renamed: Function not implemented

This patch enables mirrored LV renaming.

Mirrored LV contains "sub LV"s.
e.g. "lv0" contains "lv0_mimage_0", .., "lv0_mimage_n" and "lv0_mlog". "_mimage" and "_mlog" are reserved words, so normal LVs never has these in their names.
To rename mirrored LV, we have to rename those sub LVs accordingly.

The patch is tested on CVS head (pre 2.02.28) for the following cases:
- normal LVs (linear, striped, snapshot) for regression check
- mirrored LVs (-m2, -m1, --corelog for both active and inactive)

Example:

[Before renaming]

# lvs -a -o name
  LV
  l
  [l_mimage_0]
  [l_mimage_1]
  [l_mlog]
# dmsetup ls
testvg-l_mlog   (253, 9)
testvg-l_mimage_0       (253, 10)
testvg-l_mimage_1       (253, 11)
testvg-l        (253, 12)
# lvrename testvg/l testvg/newlv
  Renamed "l" to "newlv" in volume group "testvg"

[After renaming]

# lvs -a -o name
  LV
  newlv
  [newlv_mimage_0]
  [newlv_mimage_1]
  [newlv_mlog]
# dmsetup ls
testvg-newlv_mlog       (253, 9)
testvg-newlv_mimage_0   (253, 10)
testvg-newlv_mimage_1   (253, 11)
testvg-newlv    (253, 12)

Thanks,

Signature · 8 lines
Jun'ichi Nomura, NEC Corporation of America
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lvm2-lvrename-support-mirror.patch
Type: text/x-patch
Size: 4504 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20070727/8b1d2243/attachment.bin>
#1 lore raw

Review

Quoted 8 lines · Jun'ichi Nomura
(quoted diff) +0 -8
-	if ((lv->status & MIRRORED) ||
-	    (lv->status & MIRROR_LOG) ||
-	    (lv->status & MIRROR_IMAGE)) {
-		log_error("Mirrored LV, \"%s\" cannot be renamed: %s",
-			  lv->name, strerror(ENOSYS));
-		return 0;
-	}
-

I don't think you want to remove this wholesale. Testing reveals that you are allowed to change a sub-lv name by itself, which is not something we want, I don't think. Instead, leave the checks in for MIRROR_LOG and MIRROR_IMAGE.

brassow

author
#2 lore raw

Hi Jon,

Thanks for testing and reviewing.

Quoted 9 lines · Jonathan Brassow

Review

Quoted 8 lines
(quoted diff) +0 -8
-    if ((lv->status & MIRRORED) ||
-        (lv->status & MIRROR_LOG) ||
-        (lv->status & MIRROR_IMAGE)) {
-        log_error("Mirrored LV, \"%s\" cannot be renamed: %s",
-              lv->name, strerror(ENOSYS));
-        return 0;
-    }
-
Quoted 4 lines

I don't think you want to remove this wholesale. Testing reveals that you are allowed to change a sub-lv name by itself, which is not something we want, I don't think. Instead, leave the checks in for MIRROR_LOG and MIRROR_IMAGE.

You are right.
I didn't intend to remove the limitation.

I fixed it and modified 2 other parts:
- Removed "Failed on <LV name>" messages from _for_each_sublv(), as they have no useful information.
Callback function is responsible to put necessary info about failure. Added a comment about it.
- Changed _rename_sub_lv() to return success if it can't determine a new name for the sub LV.
e.g. if "lv1" is a sub LV of "lv0", renaming "lv0" will
success without changing "lv1".
This situation won't happen with current LVM2 code. But if such LVs exist, I think expected behaviour is leaving them unchanged and renaming others as far as possible.

Revised patch is attached.
(and fixed subject to "2/2" as it should have been)

Thanks,

Signature · 8 lines
Jun'ichi Nomura, NEC Corporation of America
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lvm2-lvrename-support-mirror.patch
Type: text/x-patch
Size: 4821 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20070801/45b86bed/attachment.bin>