Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #2187 - disklayout.conf file contains duplicate lines #2194

Merged
merged 1 commit into from Jul 23, 2019

Conversation

rmetrich
Copy link
Contributor

#1356 ## Relax-and-Recover (ReaR) Pull Request Template

Please fill in the following items before submitting a new pull request:

Pull Request Details:

Create the 'lvmvol' lines commented out when multiple segments exist for a given LV.
This is not an issue unless Migration Mode is used.
In such case, using 'lvcreate' commands already does best effort and loses LV information.

Reproducer:

  1. Add 2 disks to an existing system

  2. Create a VG with the disks

    # vgcreate data /dev/sdb /dev/sdc
    
  3. Create 2 LVs with segments

    # lvcreate -n lv1 -L 1G data
    # lvcreate -n lv2 -L 1G data
    # lvextend /dev/data/lv1 -L +1G -i 2
    # lvextend /dev/data/lv2 -L +1G -i 2
    # lvextend /dev/data/lv1 -L +1G -i 1
    # lvextend /dev/data/lv2 -L +1G -i 1
    
  4. Format and mount the LVs

    # mkdir -p /lv1 /lv2
    # mkfs.ext4 /dev/data/lv1
    # mkfs.ext4 /dev/data/lv2
    # mount /dev/data/lv1 /lv1
    # mount /dev/data/lv2 /lv2
    
  5. Create rescue ISO

  6. Check /var/lib/rear/layout/disklayout.conf

    # Format for LVM LVs
    # lvmvol <volume_group> <name> <size(bytes)> <layout> [key:value ...]
    # WARNING: Volume data/lv1 has multiple segments. Restoring it in Migration Mode using 'lvcreate' won't preserve segments and volume properties of the other segments!
    lvmvol /dev/data lv1 3221225472b linear,striped segmentsize:1073741824b stripes:1
    #lvmvol /dev/data lv1 3221225472b linear,striped stripesize:65536b segmentsize:1073741824b stripes:2
    #lvmvol /dev/data lv1 3221225472b linear,striped segmentsize:1073741824b stripes:1
    # WARNING: Volume data/lv2 has multiple segments. Restoring it in Migration Mode using 'lvcreate' won't preserve segments and volume properties of the other segments!
    lvmvol /dev/data lv2 2147483648b linear,striped segmentsize:1073741824b stripes:1
    #lvmvol /dev/data lv2 2147483648b linear,striped stripesize:65536b segmentsize:1073741824b stripes:2
    

With the fix (see above), the other segments of the LV lv1 and lv2 are created commented out, which allows the administrator to recreate them manually if needed.
Note in particular that lvcreate code will create the lv1 and lv2 LVs with only 1 stripe, i.e. the number of stripes specified for the first segment only.

Create the 'lvmvol' lines commented out when multiple segments exist for a given LV.
This is not an issue unless Migration Mode is used. In such case, using 'lvcreate' commands already does best effort and loses LV information.

Signed-off-by: Renaud Métrich <rmetrich@redhat.com>
@gdha gdha added this to the ReaR v2.6 milestone Jul 22, 2019
@gdha gdha added the bug label Jul 22, 2019
Copy link
Member

@gdha gdha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rmetrich Looks good on first sight - will you also back-port this in RedHat's rear-2.4 package?

@rmetrich rmetrich merged commit c688e55 into rear:master Jul 23, 2019
pcahyna added a commit to pcahyna/rear that referenced this pull request Jul 29, 2019
Do not overload the `kval` variable - intended for passing options
to lvcreate - by adding extra keys to it, which are not supported
by lvcreate.
Introduce another variable `infokval` for this purpose and print
those unsupported and purely informational keys only in comments.

kval=""
[ -z "$thinpool" ] || kval="${kval:+$kval }thinpool:$thinpool"
[ $chunksize -eq 0 ] || kval="${kval:+$kval }chunksize:${chunksize}b"
[ $stripesize -eq 0 ] || kval="${kval:+$kval }stripesize:${stripesize}b"
[ $segmentsize -eq $size ] || kval="${kval:+$kval }segmentsize:${segmentsize}b"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rmetrich to me it seems like an abuse of the kval variable to put segmentsize into it, since unlike the others it is not a valid option for lvcreate. I thus submitted #2196 to change this, if you agree.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, what? Abuse?
OK, I recognize I was lazy here :-) Thanks for the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants