#2183 Issue closed: RFC: Should function apply_layout_mappings() also migrate in comments?

Labels: enhancement, fixed / solved / done

jsmeix opened issue at 2019-07-15 12:59:

Current ReaR master code:

On original system /dev/vda and /dev/vdb exist
and both have partitions but /dev/vda is not used and
only /dev/vdb was used (i.e. has mounted filesystems).

Therefore disklayout.conf has entries for /dev/vda and /dev/vdb
but the entries for /dev/vda are commented out like

# Disk /dev/vda
# Format: disk <devname> <size(bytes)> <partition label type>
#disk /dev/vda 53687091200 gpt
# Partitions on /dev/vda
# Format: part <device> <partition size(bytes)> <partition start(bytes)> <partition type|name> <flags> /dev/<partition>
#part /dev/vda 8388608 1048576 rear-noname prep /dev/vda1
#part /dev/vda 536870912 9437184 rear-noname bios_grub /dev/vda2
#part /dev/vda 1073741824 546308096 rear-noname none /dev/vda3
#part /dev/vda 16106127360 1620049920 rear-noname none /dev/vda4
#part /dev/vda 35960897024 17726177280 rear-noname none /dev/vda5
# Disk /dev/vdb
# Format: disk <devname> <size(bytes)> <partition label type>
disk /dev/vdb 53687091200 gpt
# Partitions on /dev/vdb
# Format: part <device> <partition size(bytes)> <partition start(bytes)> <partition type|name> <flags> /dev/<partition>
part /dev/vdb 8388608 1048576 rear-noname prep /dev/vdb1
part /dev/vdb 536870912 9437184 rear-noname bios_grub /dev/vdb2
part /dev/vdb 1073741824 546308096 rear-noname none /dev/vdb3
part /dev/vdb 16106127360 1620049920 rear-noname none /dev/vdb4
part /dev/vdb 35960897024 17726177280 rear-noname none /dev/vdb5
# Filesystems (only ext2,ext3,ext4,vfat,xfs,reiserfs,btrfs are supported).
# Format: fs <device> <mountpoint> <fstype> [uuid=<uuid>] [label=<label>] [<attributes>]
fs /dev/vdb5 / btrfs uuid=ca5d7bc6-1280-4cf9-9327-a6778f409f61 label='SLES' options=rw,relatime,space_cache,subvolid=268,subvol=/@/.snapshots/1/snapshot

On replacement hardware only /dev/vda exists and should be used.

In MIGRATION_MODE disk mapping from /dev/vdb to /dev/vda fails
because there is no counterpart mapping for the old /dev/vda
so that after the failed mapping attempt disklayout.conf looks like this:

# Disk _REAR1_
# Format: disk <devname> <size(bytes)> <partition label type>
#disk _REAR1_ 53687091200 gpt
# Partitions on _REAR1_
# Format: part <device> <partition size(bytes)> <partition start(bytes)> <partition type|name> <flags> /dev/<partition>
#part _REAR1_ 8388608 1048576 rear-noname prep _REAR1_1
#part _REAR1_ 536870912 9437184 rear-noname bios_grub _REAR1_2
#part _REAR1_ 1073741824 546308096 rear-noname none _REAR1_3
#part _REAR1_ 16106127360 1620049920 rear-noname none _REAR1_4
#part _REAR1_ 35960897024 17726177280 rear-noname none _REAR1_5
# Disk /dev/vda
# Format: disk <devname> <size(bytes)> <partition label type>
disk /dev/vda 53687091200 gpt
# Partitions on /dev/vda
# Format: part <device> <partition size(bytes)> <partition start(bytes)> <partition type|name> <flags> /dev/<partition>
part /dev/vda 8388608 1048576 rear-noname prep /dev/vda1
part /dev/vda 536870912 9437184 rear-noname bios_grub /dev/vda2
part /dev/vda 1073741824 546308096 rear-noname none /dev/vda3
part /dev/vda 16106127360 1620049920 rear-noname none /dev/vda4
part /dev/vda 35960897024 17726177280 rear-noname none /dev/vda5
# Filesystems (only ext2,ext3,ext4,vfat,xfs,reiserfs,btrfs are supported).
# Format: fs <device> <mountpoint> <fstype> [uuid=<uuid>] [label=<label>] [<attributes>]
fs /dev/vda5 / btrfs uuid=ca5d7bc6-1280-4cf9-9327-a6778f409f61 label='SLES' options=rw,relatime,space_cache,subvolid=268,subvol=/@/.snapshots/1/snapshot

The remaining _REAR1_* result that apply_layout_mappings()
returns with non-zero exit code in its

    # Step 3:
    # Verify that there are none of those temporary replacement words from step 1 left in file_to_migrate

which lets layout/prepare/default/320_apply_mappings.sh Error out at

apply_layout_mappings "$LAYOUT_FILE" || Error "Failed to apply disklayout mappings to $LAYOUT_FILE"

One workaround is to manually remove the entries
for /dev/vda that are commented out in disklayout.conf.

Another - probably even easier - and at least cleaner - workaround is
to manually add the missing counterpart mapping to the mapping file
which is initially only

/dev/vdb /dev/vda

With an added dummy counterpart mapping like

/dev/vdb /dev/vda
/dev/vda /dev/vdaold

the mapping works and now it is even documented what the old thingy was
because that results the following disklayout.conf after the mapping:

# Disk /dev/vdaold
# Format: disk <devname> <size(bytes)> <partition label type>
#disk /dev/vdaold 53687091200 gpt
# Partitions on /dev/vdaold
# Format: part <device> <partition size(bytes)> <partition start(bytes)> <partition type|name> <flags> /dev/<partition>
#part /dev/vdaold 8388608 1048576 rear-noname prep /dev/vdaold1
#part /dev/vdaold 536870912 9437184 rear-noname bios_grub /dev/vdaold2
#part /dev/vdaold 1073741824 546308096 rear-noname none /dev/vdaold3
#part /dev/vdaold 16106127360 1620049920 rear-noname none /dev/vdaold4
#part /dev/vdaold 35960897024 17726177280 rear-noname none /dev/vdaold5
# Disk /dev/vda
# Format: disk <devname> <size(bytes)> <partition label type>
disk /dev/vda 53687091200 gpt
# Partitions on /dev/vda
# Format: part <device> <partition size(bytes)> <partition start(bytes)> <partition type|name> <flags> /dev/<partition>
part /dev/vda 8388608 1048576 rear-noname prep /dev/vda1
part /dev/vda 536870912 9437184 rear-noname bios_grub /dev/vda2
part /dev/vda 1073741824 546308096 rear-noname none /dev/vda3
part /dev/vda 16106127360 1620049920 rear-noname none /dev/vda4
part /dev/vda 14485028864 17726177280 rear-noname none /dev/vda5
# Filesystems (only ext2,ext3,ext4,vfat,xfs,reiserfs,btrfs are supported).
# Format: fs <device> <mountpoint> <fstype> [uuid=<uuid>] [label=<label>] [<attributes>]
fs /dev/vda5 / btrfs uuid=ca5d7bc6-1280-4cf9-9327-a6778f409f61 label='SLES' options=rw,relatime,space_cache,subvolid=268,subvol=/@/.snapshots/1/snapshot

The question is if that special case could be autodetected
so that things "just work" in MIGRATION_MODE even
in such special cases.

The danger with any such "nice to have" autodetection
to make things "just work" even in special cases is that some
other subtle real errors might be no longer detected and
ReaR blindly proceeds (as it did too often in the past)
until it miserably fails out of control somewhere later
at an unrelated place with a weird error message, cf.
"Try to care about possible errors" in
https://github.com/rear/rear/wiki/Coding-Style

jsmeix commented at 2019-07-15 13:06:

My immediate idea is to enhance Step 3
in function apply_layout_mappings() so that it only looks for
leftover replacement strings _REAR*_ in non-comment lines like

        # Only treat leftover temporary replacement words
        # as an error if they are in a non-comment line
        # cf. https://github.com/rear/rear/issues/2183
        if grep -v '^[[:space:]]*#' "$file_to_migrate" | grep -q "$replacement" ; then
            apply_layout_mappings_succeeded="no"

jsmeix commented at 2019-07-15 13:34:

The other idea to add an additional check that each mapping
from 'foo' to 'bar' must have a counterpart mapping for 'bar'
e.g. from 'bar' to 'bar.old' if the old 'bar' is no longer needed
does not work well.

Reason:

If there is only a mapping from 'foo' to 'bar'
(e.g. as above there is only a mapping from /dev/vdb to /dev/vda)
the user would be enforced to manually add a dummy counterpart mapping
for 'bar' (/dev/vda) in his mapping file (usually /var/lib/rear/layout/disk_mappings)
even if there is nowhere 'bar' (/dev/vda) in his original config files
where the single mapping from 'foo' to 'bar' would "just work"
without a need for a counterpart mapping for 'bar',
cf. the workarounds in the initial comment above
https://github.com/rear/rear/issues/2183#issue-468113259

jsmeix commented at 2019-07-18 12:20:

With the fix in my
https://github.com/rear/rear/issues/2183#issuecomment-511394129
things "just work" for me.

With the unchanged autogenerated mapping file that contains only

/dev/vdb /dev/vda

the mapping works and results the following disklayout.conf after the mapping:

# Disk _REAR1_
# Format: disk <devname> <size(bytes)> <partition label type>
#disk _REAR1_ 53687091200 gpt
# Partitions on _REAR1_
# Format: part <device> <partition size(bytes)> <partition start(bytes)> <partition type|name> <flags> /dev/<partition>
#part _REAR1_ 8388608 1048576 rear-noname prep _REAR1_1
#part _REAR1_ 536870912 9437184 rear-noname bios_grub _REAR1_2
#part _REAR1_ 1073741824 546308096 rear-noname none _REAR1_3
#part _REAR1_ 16106127360 1620049920 rear-noname none _REAR1_4
#part _REAR1_ 35960897024 17726177280 rear-noname none _REAR1_5
# Disk /dev/vda
# Format: disk <devname> <size(bytes)> <partition label type>
disk /dev/vda 53687091200 gpt
# Partitions on /dev/vda
# Format: part <device> <partition size(bytes)> <partition start(bytes)> <partition type|name> <flags> /dev/<partition>
part /dev/vda 8388608 1048576 rear-noname prep /dev/vda1
part /dev/vda 536870912 9437184 rear-noname bios_grub /dev/vda2
part /dev/vda 1073741824 546308096 rear-noname none /dev/vda3
part /dev/vda 16106127360 1620049920 rear-noname none /dev/vda4
part /dev/vda 14485028864 17726177280 rear-noname none /dev/vda5
# Filesystems (only ext2,ext3,ext4,vfat,xfs,reiserfs,btrfs are supported).
# Format: fs <device> <mountpoint> <fstype> [uuid=<uuid>] [label=<label>] [<attributes>]
fs /dev/vda5 / btrfs uuid=ca5d7bc6-1280-4cf9-9327-a6778f409f61 label='SLES' options=rw,relatime,space_cache,subvolid=268,subvol=/@/.snapshots/1/snapshot

i.e. there is a leftover replacement string _REAR1_ in comment lines.
I think this is an acceptable result for an automated mapping.

jsmeix commented at 2019-07-18 14:10:

With https://github.com/rear/rear/pull/2189 merged
this issue should be fixed.

jsmeix commented at 2019-07-18 14:12:

So the answer to the question

Should function apply_layout_mappings() also migrate in comments?

is:
Yes, the apply_layout_mappings() function does also migrate in comments
but it is no error when there are leftover replacement strings in comments.


[Export of Github issue for rear/rear.]