Allowing multipath devices to be candidate disk during mapping. #1314

schabrolles · 2017-04-17T14:28:09Z

During migration, ReaR recovery propose candidate disk if they have the same (or bigger) size than the original.
The problem is that size of mulitpath device is currently not stored in layout file:

This patch propose :

To store multipath device size in $LAYOUT_FILE
Add line beginning by multipath to be part of the comparison process when searching for device with the same size or finding unmapped candidate.

- Need to store multipath device size in LAYOUT_FILE - Add line beginning by multipath to be part of the comparison process when searching for device with the same size or finding unmapped candidate.

gdha · 2017-04-18T12:59:24Z

@schabrolles Is this part 1 of a pull request? Or am I missing the point here?

schabrolles · 2017-04-18T13:29:40Z

@gdha You are right in a way. (#1309 #1315)
This pull request treat what we have to modify to have a multipathed disk proposed as a replacement disk during migration.

(before this one) You will need (Multipathing when BOOT_OVER_SAN=y #1309) to activate multipath if you migrate from non-multipathed to multipathed with BOOT_OVER_SAN=y
(after in the process) You will nedd (Multipath device mapping #1315) to properly map the old <=> new device with good multipath partition naming.

jsmeix · 2017-04-19T10:29:10Z

@schabrolles
in layout/save/GNU/Linux/280_multipath_layout.sh
you inserted output of $dm_size in the 'multipath' lines
in $DISKLAYOUT_FILE via

echo "multipath /dev/mapper/$dm_name $dm_size ${slaves%,}" >> $DISKLAYOUT_FILE

but I miss adapted code where those 'multipath' lines are input
because now ${slaves%,} has become the third parameter
(before it was the second parameter).

As far as I see
layout/prepare/GNU/Linux/210_load_multipath.sh
does not need adaptions because it only
uses the first parameter /dev/mapper/$dm_name

As far as I see also
layout/save/default/335_remove_excluded_multipath_vgs.sh
does not need adaptions because it also only
uses the first parameter /dev/mapper/$dm_name

Now I wonder where ${slaves%,} is used at all?

If ${slaves%,} is nowhere used we should remove it.

jsmeix · 2017-04-19T10:39:19Z

I think I found where ${slaves%,} is used: In
layout/save/default/320_autoexclude.sh
(which is run after layout/save/GNU/Linux/280_multipath_layout.sh)
as

while read multipath device slaves junk ; do
...
done < <(grep ^multipath $LAYOUT_FILE)

where now "read multipath device slaves junk"
must become "read multipath device dm_size slaves junk"

Furthermore with positional parameters
one must ensure no positional parameter is empty
so that in layout/save/GNU/Linux/280_multipath_layout.sh
plain

dm_size=$(cat /sys/block/$name/size)

is not fail-safe and must be at least

dm_size=$(cat /sys/block/$name/size)
test "$dm_size" || Error "Failed to get /sys/block/$name/size"

so that it errors out early during "rear mkbackup/mkrescue"
instead of blindly proceed and let it fail arbitrarily later
because "read multipath device dm_size slaves junk"
reads wrong parameters when dm_size is empty.

jsmeix · 2017-04-19T10:43:54Z

Same in
layout/prepare/default/520_exclude_components.sh

while read multipath device slaves junk ; do
...
done < <(grep ^multipath "$LAYOUT_FILE")

must now be

while read multipath device dm_size slaves junk ; do

jsmeix · 2017-04-19T10:45:41Z

FYI:
See #718 regarding
DISKLAYOUT_FILE versus LAYOUT_FILE inconsistency.

jsmeix

Because dm_size must not be empty
error ouf it it is empty.
Check layout/save/default/320_autoexclude.sh
and layout/prepare/default/520_exclude_components.sh
whether or not
"read multipath device slaves junk"
therein needs to be adapted as
"read multipath device dm_size slaves junk"

schabrolles · 2017-04-19T11:08:17Z

@jsmeix Good catch... I will do this.

…fig. (see rear#1314)

…gnore)

schabrolles · 2017-04-19T16:02:03Z

@jsmeix Do you want me to change LAYOUT_FILE to DISKLAYOUT_FILE in the following files ?

layout/save/default/320_autoexclude.sh
layout/prepare/default/520_exclude_components.sh
layout/prepare/default/250_compare_disks.sh
layout/prepare/default/300_map_disks.sh
layout/save/GNU/Linux/280_multipath_layout.sh

schabrolles · 2017-04-19T16:05:52Z

@jsmeix it seems that the latest changes in layout/save/default/320_autoexclude.sh
and layout/prepare/default/520_exclude_components.sh brake the recovery process when recover from (multipath) => (non-mulitpath). In the other way, it is working.
I need more time to understand what is happening here.

- Due to changes in commit (dd65bd0) dm_size is now the field 2 slave disks move to field 3

schabrolles · 2017-04-19T19:57:06Z

@jsmeix, I think I found it. (9359a1a)

generate_layout_dependencies needed to be updated for multipath after the addition of dm_size parameter at field 2. (cf commit dd65bd0).
This change shifts the slave disk at field 3.

I'm gonna test this tomorrow with Sles11sp4, Sles12, RHEL 6 and RHEL 7.
single path => multipath AND multipath => single path

jsmeix · 2017-04-20T10:41:14Z

@schabrolles
many thanks for your careful testing.
That positional parameters in LAYOUT_FILE / DISKLAYOUT_FILE
make things fragile and caused several weird looking issues
in the past where things fell apart at totally unexpected places :-(

schabrolles · 2017-04-20T11:21:51Z

@jsmeix if I understand well, we should use $DISKLAYOUT_FILE each time we want to use /var/lib/rear/layout/disklayout.conf ... So if you want, I can start to make the changes in the files used in this pull request.

What do you think ?

jsmeix · 2017-04-20T13:04:43Z

@schabrolles
I would very much appreciate it when one same name
is used for one same thing.

To fix the DISKLAYOUT_FILE versus LAYOUT_FILE
inconsistency please do it in a separated pull request
for #718
after the current pending pull requests were merged.

I fear there is perhaps some obscure reason why
different variable names are used?

I would prefer to keep separated issues separated (KSIS;-)
and not mix them up in one pull request (cf. RFC 1925 item 5).

jsmeix

@schabrolles
I will merge it when your tests
with Sles11sp4, Sles12, RHEL 6
and RHEL 7 single path => multipath
AND multipath => single path are
succesful.

- mpathconf is present in Fedora based distribution. It helps to make the first configuration of multipath. It can be used to properly setup the multipath configuration during recovery when you migration from non-multipath system to multipath. (see rear#1311, rear#1314)

schabrolles · 2017-04-21T12:08:07Z

@jsmeix I've finished my test and the migration works for all of the tests.

I only have one issue regarding xfs with rhel7 when moving from multipath to not multipath .... but it is not related to this pull request. I will investigate and open another pull request/issue if needed.

Allowing multipath devices to be candidate disks during mapping.

dd65bd0

- Need to store multipath device size in LAYOUT_FILE - Add line beginning by multipath to be part of the comparison process when searching for device with the same size or finding unmapped candidate.

gdha self-assigned this Apr 18, 2017

gdha added the enhancement Adaptions and new features label Apr 18, 2017

jsmeix self-requested a review April 19, 2017 10:47

jsmeix requested changes Apr 19, 2017

View reviewed changes

schabrolles added 3 commits April 19, 2017 13:41

Add a test to avoid empty $dm_size during saving multipath layout con…

8e5725b

…fig. (see rear#1314)

Need to add dm_size field when parsing LAYOUT_FILE (rear#1314)

67b7f17

Remove .remote-sync.json (files was sended by mistake, not in my giti…

91cc934

…gnore)

Move multipath disks to field 3

9359a1a

- Due to changes in commit (dd65bd0) dm_size is now the field 2 slave disks move to field 3

jsmeix approved these changes Apr 20, 2017

View reviewed changes

jsmeix added this to the ReaR v2.1 milestone Apr 20, 2017

schabrolles mentioned this pull request Apr 21, 2017

Use mpathconf to setup multipath in recovery (if available) #1324

Merged

jsmeix merged commit 5117e5c into rear:master Apr 24, 2017

jsmeix added the fixed / solved / done label Apr 24, 2017

schabrolles deleted the multipath_candidate branch May 4, 2017 14:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allowing multipath devices to be candidate disk during mapping. #1314

Allowing multipath devices to be candidate disk during mapping. #1314

schabrolles commented Apr 17, 2017

gdha commented Apr 18, 2017

schabrolles commented Apr 18, 2017 •

edited

jsmeix commented Apr 19, 2017

jsmeix commented Apr 19, 2017

jsmeix commented Apr 19, 2017

jsmeix commented Apr 19, 2017

jsmeix left a comment

schabrolles commented Apr 19, 2017

schabrolles commented Apr 19, 2017 •

edited

schabrolles commented Apr 19, 2017

schabrolles commented Apr 19, 2017 •

edited

jsmeix commented Apr 20, 2017

schabrolles commented Apr 20, 2017

jsmeix commented Apr 20, 2017

jsmeix left a comment

schabrolles commented Apr 21, 2017

Allowing multipath devices to be candidate disk during mapping. #1314

Allowing multipath devices to be candidate disk during mapping. #1314

Conversation

schabrolles commented Apr 17, 2017

gdha commented Apr 18, 2017

schabrolles commented Apr 18, 2017 • edited

jsmeix commented Apr 19, 2017

jsmeix commented Apr 19, 2017

jsmeix commented Apr 19, 2017

jsmeix commented Apr 19, 2017

jsmeix left a comment

Choose a reason for hiding this comment

schabrolles commented Apr 19, 2017

schabrolles commented Apr 19, 2017 • edited

schabrolles commented Apr 19, 2017

schabrolles commented Apr 19, 2017 • edited

jsmeix commented Apr 20, 2017

schabrolles commented Apr 20, 2017

jsmeix commented Apr 20, 2017

jsmeix left a comment

Choose a reason for hiding this comment

schabrolles commented Apr 21, 2017

schabrolles commented Apr 18, 2017 •

edited

schabrolles commented Apr 19, 2017 •

edited

schabrolles commented Apr 19, 2017 •

edited