Bug 761267

Summary: lvconvert --merge does not check if snapshot is invalid
Product: Red Hat Enterprise Linux 6 Reporter: John Ruemker <jruemker>
Component: lvm2Assignee: Mike Snitzer <msnitzer>
Status: CLOSED ERRATA QA Contact: Cluster QE <mspqa-list>
Severity: medium Docs Contact:
Priority: medium    
Version: 6.1CC: agk, cmarthal, dustymabe, dwysocha, ealcaniz, gbarros, heinzm, jbrassow, mbroz, prajnoha, prockai, shyu, thornber, zkabelac
Target Milestone: rc   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: lvm2-2.02.95-1.el6 Doc Type: Bug Fix
Doc Text:
Disallow an invalid snapshot to be merged. Allow the removal of an invalid snapshot that was: - to be merged on next activation. - invalidated while merging (user will be prompted for confirmation)
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-06-20 15:00:37 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
Fail if snapshot is invalid none

Description John Ruemker 2011-12-07 22:33:58 UTC
Description of problem: lvconvert --merge does not check if the snapshot in question is invalid before proceeding.  It starts the operation, and then fails part-way through, leaving the logical volume in a weird state which is difficult to get out of.  

  # lvconvert --merge test/lv1-snap
    /dev/test/lv1-snap: read failed after 0 of 4096 at 943652864: Input/output error
    /dev/test/lv1-snap: read failed after 0 of 4096 at 943710208: Input/output error
    /dev/test/lv1-snap: read failed after 0 of 4096 at 0: Input/output error
    /dev/test/lv1-snap: read failed after 0 of 4096 at 4096: Input/output error
    Merging of volume lv1-snap started.
    lv1: Merging snapshot invalidated. Aborting merge.

After this failure, the logical volume is no longer listed in lvs, but the extents are still in use and its still in the metadata.  This prevents you from creating another snapshot:

  # lvcreate -l 3 -n lv1-snap2 -s test/lv1
    Snapshots of an origin that has a merging snapshot is not supported

You can't remove the snapshot:

  # lvremove test/lv1-snap
    Can't remove merging snapshot logical volume "lv1-snap"

And rebooting doesn't help.  The only way out of this seems to be to remove the origin and start over.  

Version-Release number of selected component (if applicable): lvm2-2.02.83-3.el6


How reproducible: Always


Steps to Reproduce:
1. Create a snapshot logical volume
2. Invalidate the snapshot by writing enough data to the origin. 
3. Attempt to merge the snapshot with 'lvconvert --merge <vg/lv>'
  
Actual results: Logical volume is not listed and can't be removed, but still takes up extents.  Snapshots can't be created off origin.  


Expected results: Merge fails, invalid snapshot is still listed but inactive and can be removed.  Origin is not considered to still be merging.


Additional info: This same problem was discussed a few months ago here:

    http://www.redhat.com/archives/linux-lvm/2011-July/msg00047.html

Comment 1 John Ruemker 2011-12-07 22:36:34 UTC
Created attachment 542237 [details]
Fail if snapshot is invalid

Code to check for invalid snapshot based on lv_attr_dup functionality, as mentioned in thread in previous comment. 

Using this patch:

[root@jrummy6-64 ~]# lvs test
  /dev/test/lv1-snap: read failed after 0 of 4096 at 943652864: Input/output error
  /dev/test/lv1-snap: read failed after 0 of 4096 at 943710208: Input/output error
  /dev/test/lv1-snap: read failed after 0 of 4096 at 0: Input/output error
  /dev/test/lv1-snap: read failed after 0 of 4096 at 4096: Input/output error
  LV       VG   Attr   LSize   Origin Snap%  Move Log Copy%  Convert
  lv1      test owi-ao 900.00m
  lv1-snap test Swi-I-  12.00m lv1    100.00
[root@jrummy6-64 ~]# lvconvert --merge test/lv1-snap
  /dev/test/lv1-snap: read failed after 0 of 4096 at 943652864: Input/output error
  /dev/test/lv1-snap: read failed after 0 of 4096 at 943710208: Input/output error
  /dev/test/lv1-snap: read failed after 0 of 4096 at 0: Input/output error
  /dev/test/lv1-snap: read failed after 0 of 4096 at 4096: Input/output error
  Unable to merge invalid snapshot "lv1-snap"
[root@jrummy6-64 ~]# lvs
  /dev/test/lv1-snap: read failed after 0 of 4096 at 943652864: Input/output error
  /dev/test/lv1-snap: read failed after 0 of 4096 at 943710208: Input/output error
  /dev/test/lv1-snap: read failed after 0 of 4096 at 0: Input/output error
  /dev/test/lv1-snap: read failed after 0 of 4096 at 4096: Input/output error
  LV       VG        Attr   LSize   Origin Snap%  Move Log Copy%  Convert
  lv1      test      owi-ao 900.00m
  lv1-snap test      Swi-I-  12.00m lv1    100.00
  lv_root  vg_jrummy -wi-ao  13.54g
  lv_swap  vg_jrummy -wi-ao 992.00m
[root@jrummy6-64 ~]# lvremove test/lv1-snap
  /dev/test/lv1-snap: read failed after 0 of 4096 at 943652864: Input/output error
  /dev/test/lv1-snap: read failed after 0 of 4096 at 943710208: Input/output error
  /dev/test/lv1-snap: read failed after 0 of 4096 at 0: Input/output error
  /dev/test/lv1-snap: read failed after 0 of 4096 at 4096: Input/output error
Do you really want to remove active logical volume lv1-snap? [y/n]: y
  Logical volume "lv1-snap" successfully removed
[root@jrummy6-64 ~]# lvs test
  LV   VG   Attr   LSize   Origin Snap%  Move Log Copy%  Convert
  lv1  test -wi-ao 900.00m

Comment 2 Alasdair Kergon 2011-12-08 12:59:28 UTC
What if we applied that patch, the snapshot is not invalid so the new test passes OK, then immediately afterwards the snapshot happens to become invalid?

Is there a better way to do this that avoids that 'race'?

Comment 3 John Ruemker 2011-12-08 14:51:45 UTC
Since _poll_merge_progress should detect the snapshot being invalidated after we start, could we just have it do a proper cleanup at that point?

static progress_t _poll_merge_progress(struct cmd_context *cmd,
                                       struct logical_volume *lv,
                                       const char *name __attribute__((unused)),
                                       struct daemon_parms *parms)
{
        percent_t percent = PERCENT_0;

        if (!lv_snapshot_percent(lv, &percent)) {
                log_error("%s: Failed query for merging percentage. Aborting merge.", lv->name);
                return PROGRESS_CHECK_FAILED;
        } else if (percent == PERCENT_INVALID) {
                log_error("%s: Merging snapshot invalidated. Aborting merge.", lv->name);
                /* Cleanup here */
                return PROGRESS_CHECK_FAILED;
        }


-John

Comment 6 Mike Snitzer 2012-01-13 03:35:18 UTC
The proposed patch helps avoid the problem in what I'd imagine would be the most common case for non-root LV merges.

However, it is still possible to initiate a deferred merge (merge occurs on next activation) that will ultimately fail and need cleanup.

We still need the ability to directly remove a merging snapshot that is invalid:

# lvcreate -L 5G -n lv vg
  Logical volume "lv" created
# mkfs.ext4 /dev/vg/lv 
...
# mount /dev/vg/lv /mnt/test
# lvcreate -s -L 100M -n lv_snap vg/lv
  Logical volume "lv_snap" created
# lvconvert --merge vg/lv_snap
  Can't merge over open origin volume
  Merging of snapshot lv_snap will start next activation.
# dd if=/dev/zero of=/mnt/test/test bs=1024k count=100
(dmesg:
device-mapper: snapshots: Invalidating snapshot: Unable to allocate exception.)
# umount /dev/vg/lv
# vgchange -an vg
  0 logical volume(s) in volume group "vg" now active
# vgchange -ay vg
  1 logical volume(s) in volume group "vg" now active
(dmesg:
device-mapper: snapshots: Snapshot is marked invalid.
device-mapper: snapshots: Snapshot is invalid: can't merge)

# dmsetup table vg-lv
0 10485760 snapshot-merge 253:0 253:2 P 8
# dmsetup status vg-lv
0 10485760 snapshot-merge Invalid

# lvs
  LV   VG   Attr   LSize Origin Snap%  Move Log Copy%  Convert
  lv   vg   Owi-a- 5.00g                                      
# lvs -a
  LV        VG   Attr   LSize   Origin Snap%  Move Log Copy%  Convert
  lv        vg   Owi-a-   5.00g                                      
  [lv_snap] vg   Swi-a- 100.00m lv     100.00                        

NOTE: the -a argument to lvs is always needed to see the hidden merging snapshot (whether it is invalid or not).

# lvremove vg/lv_snap
  Can't remove merging snapshot logical volume "lv_snap"

NOTE: this works to remove the merging snapshot, but is completely hackish:
# lvremove vg/lv
  Logical volume "lv_snap" successfully removed
Do you really want to remove active logical volume lv? [y/n]: n
  Logical volume lv not removed

# lvs
  LV   VG   Attr   LSize Origin Snap%  Move Log Copy%  Convert
  lv   vg   -wi-a- 5.00g
# dmsetup status vg-lv
0 10485760 linear

Comment 9 Mike Snitzer 2012-01-23 14:42:15 UTC
5 patches snapshot merge fixes were committed to upstream lvm2.  These fixes will be released in upcoming lvm2 2.02.89 release.

Comment 11 Mike Snitzer 2012-04-25 15:45:38 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:
Disallow an invalid snapshot to be merged.
Allow the removal of an invalid snapshot that was:
- to be merged on next activation.
- invalidated while merging (user will be prompted for confirmation)

Comment 12 Corey Marthaler 2012-05-07 18:40:15 UTC
Fix verified in the latest rpms.

2.6.32-269.el6.x86_64
lvm2-2.02.95-7.el6    BUILT: Wed May  2 05:14:03 CDT 2012
lvm2-libs-2.02.95-7.el6    BUILT: Wed May  2 05:14:03 CDT 2012
lvm2-cluster-2.02.95-7.el6    BUILT: Wed May  2 05:14:03 CDT 2012
udev-147-2.41.el6    BUILT: Thu Mar  1 13:01:08 CST 2012
device-mapper-1.02.74-7.el6    BUILT: Wed May  2 05:14:03 CDT 2012
device-mapper-libs-1.02.74-7.el6    BUILT: Wed May  2 05:14:03 CDT 2012
device-mapper-event-1.02.74-7.el6    BUILT: Wed May  2 05:14:03 CDT 2012
device-mapper-event-libs-1.02.74-7.el6    BUILT: Wed May  2 05:14:03 CDT 2012
cmirror-2.02.95-7.el6    BUILT: Wed May  2 05:14:03 CDT 2012


[root@taft-01 ~]# lvs -a -o +devices
  LV      VG   Attr     LSize   Origin Data%  Devices
  origin  taft owi-a-s- 100.00m               /dev/sdb1(0)
  snap    taft swi-a-s-  12.00m origin   0.00 /dev/sdb1(25)

[root@taft-01 ~]# dd if=/dev/zero of=/dev/taft/origin bs=1M count=13
13+0 records in
13+0 records out
13631488 bytes (14 MB) copied, 0.311749 s, 43.7 MB/s
[root@taft-01 ~]# lvs -a -o +devices
  /dev/taft/snap: read failed after 0 of 4096 at 104792064: Input/output error
  [...]
  LV      VG   Attr     LSize   Origin Data%  Devices
  origin  taft owi-a-s- 100.00m               /dev/sdb1(0)
  snap    taft Swi-I-s-  12.00m origin 100.00 /dev/sdb1(25)

[root@taft-01 ~]# lvconvert --merge taft/snap
  /dev/taft/snap: read failed after 0 of 4096 at 104792064: Input/output error
  [...]
  Unable to merge invalidated snapshot LV "snap"

[root@taft-01 ~]# lvcreate -s taft/origin -n snap2 -L 10M
  /dev/taft/snap: read failed after 0 of 4096 at 104792064: Input/output error
  [...]
  Rounding up size to full physical extent 12.00 MiB
  Logical volume "snap2" created

[root@taft-01 ~]# lvs -a -o +devices
  /dev/taft/snap: read failed after 0 of 4096 at 104792064: Input/output error
  [...]
  LV      VG   Attr     LSize   Origin Data%  Devices
  origin  taft owi-a-s- 100.00m               /dev/sdb1(0)
  snap    taft Swi-I-s-  12.00m origin 100.00 /dev/sdb1(25)
  snap2   taft swi-a-s-  12.00m origin   0.00 /dev/sdb1(28)

[root@taft-01 ~]# lvremove taft/snap
  /dev/taft/snap: read failed after 0 of 4096 at 104792064: Input/output error
  [...]
Do you really want to remove active logical volume snap? [y/n]: y
  Logical volume "snap" successfully removed
[root@taft-01 ~]# lvs -a -o +devices
  LV      VG   Attr     LSize   Origin Data%  Devices
  origin  taft owi-a-s- 100.00m               /dev/sdb1(0)
  snap2   taft swi-a-s-  12.00m origin   0.00 /dev/sdb1(28)

Comment 14 errata-xmlrpc 2012-06-20 15:00:37 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-0962.html