Bug 758833

Summary: snapshot deletion issues when created on lvm1 meta formated volume
Product: Red Hat Enterprise Linux 5 Reporter: Corey Marthaler <cmarthal>
Component: lvm2Assignee: Alasdair Kergon <agk>
Status: CLOSED ERRATA QA Contact: Cluster QE <mspqa-list>
Severity: low Docs Contact:
Priority: low    
Version: 5.8CC: agk, dwysocha, heinzm, jbrassow, lmiksik, mbroz, prajnoha, prockai, thornber, zkabelac
Target Milestone: rcKeywords: Regression, TestBlocker
Target Release: ---   
Hardware: x86_64   
OS: Linux   
Whiteboard:
Fixed In Version: lvm2-2.02.88-6.el5 Doc Type: Bug Fix
Doc Text:
Do not document (regression in internal build).
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-02-21 06:05:55 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Bug Depends On:    
Bug Blocks: 784372    

Description Corey Marthaler 2011-11-30 19:34:03 UTC
Description of problem:
For some reason the snapshot delete in this test case no longer works.

SCENARIO - [create_snap_on_lvm1_meta_format]
Create snapshot on VG created with LVM1 format
Recreating VG and PVs with LVM1 metadata format

hayes-01: pvcreate --metadatatype 1 --setphysicalvolumesize 1T /dev/etherd/e1.1p1 /dev/etherd/e1.1p2
  Writing physical volume data to disk "/dev/etherd/e1.1p1"
  Writing physical volume data to disk "/dev/etherd/e1.1p2"
hayes-01 vgcreate --metadatatype 1 snapper /dev/etherd/e1.1p1 /dev/etherd/e1.1p2

Making origin volume
hayes-01: lvcreate -L 300M snapper -n origin
Creating snap of origin
hayes-01: lvcreate -s /dev/snapper/origin -c 32 -n snap_with_lvm1_meta -L 100M
sleep 20
Removing volume snapper/snap_with_lvm1_meta
hayes-01: lvremove -f /dev/snapper/snap_with_lvm1_meta
  Unable to deactivate open snapper-origin-real (253:4)
  Failed to resume origin.

couldn't remove volume snap_with_lvm1_meta


The second attempt to remove the snapshot works, but the dm device isn't removed.
[root@hayes-01 ~]# lvs -a -o +devices
  LV                  VG       Attr   LSize    Devices               
  origin              snapper  -wn-a- 300.00M  /dev/etherd/e1.1p1(0) 
  snap_with_lvm1_meta snapper  -wn-a- 100.00M  /dev/etherd/e1.1p1(75)

[root@hayes-01 ~]# lvremove -f /dev/snapper/snap_with_lvm1_meta
  Logical volume "snap_with_lvm1_meta" successfully removed

[root@hayes-01 ~]# dmsetup ls
snapper-snap_with_lvm1_meta-cow (253, 5)
snapper-origin  (253, 2)


Version-Release number of selected component (if applicable):
2.6.18-274.el5

lvm2-2.02.88-4.el5    BUILT: Wed Nov 16 09:40:55 CST 2011
lvm2-cluster-2.02.88-4.el5    BUILT: Wed Nov 16 09:46:51 CST 2011
device-mapper-1.02.67-2.el5    BUILT: Mon Oct 17 08:31:56 CDT 2011
device-mapper-event-1.02.67-2.el5    BUILT: Mon Oct 17 08:31:56 CDT 2011
cmirror-1.1.39-10.el5    BUILT: Wed Sep  8 16:32:05 CDT 2010
kmod-cmirror-0.1.22-3.el5    BUILT: Tue Dec 22 13:39:47 CST 2009


How reproducible:
Everytime

Comment 2 Peter Rajnoha 2011-12-01 16:57:13 UTC
This is a regression introduced by changes in snapshot removal sequence (http://www.redhat.com/archives/lvm-devel/2011-July/msg00027.html). Before, we deactivated the snapshot first while having only the origin active and then processing it further. Now, we have the origin as well as the snapshot active...

In more detail:

While trying to suspend the tree (the "vg_write, suspend_lv, vg_commit, resume_lv" sequence in vg_remove_snapshot fn), we're calling lv_from_lvid/vg_read. For lvm2 format, we read old metadata, which is fine. 

However, for lvm1, we're reading a new state we've just written and this results with the origin_count for origin LV to be dropped to zero and the origin is not counted in the deptree to suspend it in the above mentioned suspend call thus causing it to be processed incorrectly and resulting with the exact error reported here.

...need to elabore further how to deal with this.

Comment 6 Milan Broz 2011-12-19 10:09:16 UTC
Peter, how complicated is patch here?

I think lvm1 metadata snapshot functionality is really minor case in RHEL5,
so better move to 5.9?

Comment 7 Peter Rajnoha 2011-12-19 10:34:03 UTC
Thing is that for lvm1 format we would need to read old metadata state when calling that lv_from_lvid/vg_read. The sequence we have in vg_remove_snapshot (and probably anywhere else) is: "vg_write; suspend_lv; vg_commit".

Since there's no "precommitted metadata" in lvm1 (and only vg_write takes a real action), we would need to postpone writing metadata on disk to vg_commit for lvm1 format in this particular case - but such a change could corrupt functionality in other parts of the code, I'm afraid. IOW I'm not quite sure about a right solution now (tweaking that lv_from_lvid/vg_read call just for lvm1 case doesn't seem to be the right one too). Even if changed, this would need a bit of testing first if there are no regressions - I think it's risky at this moment for 5.8, I'd move it to 5.9.

Comment 8 Milan Broz 2011-12-19 11:42:47 UTC
ok, let's move this for consideration in 5.9.

(lvm1 metadata format is very rarely used, moreover there is in-place convert, so id problem appears, there is a workaround - convert to lvm2 format)

Comment 10 Alasdair Kergon 2011-12-19 13:06:59 UTC
So is this currently broken upstream?

Comment 11 Peter Rajnoha 2011-12-19 13:36:27 UTC
(In reply to comment #10)
> So is this currently broken upstream?

Unfortunately, yes.

Comment 12 Alasdair Kergon 2011-12-20 00:11:11 UTC
I've reinstated (most of) the old code path upstream, then, but only for lvm1 metadata, and added a 'this is deprecated' message.

I anticipate that we will drop support for this configuration in RHEL7. The deprecation warning will be picked up by 6.3 too.

Comment 13 Alasdair Kergon 2011-12-20 00:19:55 UTC
But this bugzilla must still be addressed for 5.8 - we should not release the package in this state.

Options are to apply the upstream patch that effectively reverts part of a 5.8 change, or to add a patch that stops supporting lvm1 snapshots completely.

If anyone actually is still using lvm1 snapshots, that'll mean a new patch to handle that situation and a release note telling them NOT to apply the upgrade until they have run 'lvconvert' etc.  Detection would mean either failing to perform the activation completely, or else ignoring snapshots and not activating them (but still activating the origin).

I favour reverting the change, rather than stopping supporting something that worked up to now and having to sort out and write new tests for enforcement rules.

Comment 15 Milan Broz 2011-12-20 08:21:06 UTC
ok, patch is now available and is quite simple, let's fix this in 5.8

Comment 17 Alasdair Kergon 2012-01-17 18:27:44 UTC
upstream revert commit was http://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=14a165bf3fa8e38827d683d16348716816d8a13d

Comment 18 Milan Broz 2012-01-18 09:19:53 UTC
Fixed in lvm2-2.02.88-6.el5.

Comment 21 Milan Broz 2012-01-18 09:36:24 UTC
    Technical note added. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    New Contents:
Do not document (regression in internal build).

Comment 22 Corey Marthaler 2012-01-18 21:30:25 UTC
Fix verified in the latest rpms.


2.6.18-301.el5
lvm2-2.02.88-6.el5    BUILT: Wed Jan 18 03:34:29 CST 2012
lvm2-cluster-2.02.88-6.el5    BUILT: Wed Jan 18 03:33:26 CST 2012
device-mapper-1.02.67-2.el5    BUILT: Mon Oct 17 08:31:56 CDT 2011
device-mapper-event-1.02.67-2.el5    BUILT: Mon Oct 17 08:31:56 CDT 2011
cmirror-1.1.39-14.el5    BUILT: Wed Nov  2 17:25:33 CDT 2011
kmod-cmirror-0.1.22-3.el5    BUILT: Tue Dec 22 13:39:47 CST 2009



SCENARIO - [create_snap_on_lvm1_meta_format]
Create snapshot on VG created with LVM1 format
Recreating VG and PVs with LVM1 metadata format
  WARNING: /dev/sdb1: Overriding real size. You could lose data.
  Writing physical volume data to disk "/dev/sdb1"
  WARNING: /dev/sdc1: Overriding real size. You could lose data.
  Writing physical volume data to disk "/dev/sdc1"
Making origin volume
Creating snap of origin
Removing volume snapper/snap_with_lvm1_meta
Removing origin snapper/origin
Restoring VG and PVs to LVM2 metadata format
  Writing physical volume data to disk "/dev/sdb1"
  Writing physical volume data to disk "/dev/sdc1"

Comment 23 errata-xmlrpc 2012-02-21 06:05:55 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHBA-2012-0161.html