Provide final power to the user for mkfs.xfs options if needed (issue 1998) #2005

jsmeix · 2018-12-14T14:53:37Z

Type: Enhancement
Impact: High
Usually this is not needed but if the worst comes to the worst
an easy way to specify mkfs.xfs options it is likely highly appreciated.
Reference to related issue (URL):
SLE15 on ppcl: mkfs.xfs fails if sunit > 0 but swidth = 0 #1998
How was this pull request tested?
Currently it is not at all tested by me.
Currently it is meant so that @gozora can have a first look, cf.
SLE15 on ppcl: mkfs.xfs fails if sunit > 0 but swidth = 0 #1998 (comment)
Brief description of the changes in this pull request:
Via the new optional global MKFS_XFS_OPTIONS config variable
the user can specify mkfs.xfs options for all mkfs.xfs calls
and/or via optional device specific config variables like
MKFS_XFS_OPTIONS_SDA2
the user can specify mkfs.xfs options only for the mkfs.xfs call
for that dvice (e.g. for /dev/sda2 via MKFS_XFS_OPTIONS_SDA2)
where device specific config variables take precedence over
a global MKFS_XFS_OPTIONS config variable which
takes precedence over the not changed default behaviour
where device specific options for the mkfs.xfs calls are set via
xfs_parse $LAYOUT_XFS_OPT_DIR/$xfs_device_basename.xfs

… 1998)

gozora

I just finished test of your code on my Fedora 28 and all looks OK!

With explicit MKFS_XFS_OPTIONS_FEDORAROOT='-d agcount=4 -s size=512 -i attr=2 -i projid32bit=1 -m crc=1 -m finobt=1 -b size=4096 -i maxpct=25 -d sunit=0 -d swidth=0 -l version=2 -l lazy-count=1 -n size=4096 -n version=2 -r ex tsize=4096 -i size=1024' set in local.conf:

++ local mkfs_xfs_device_options_variable_name=MKFS_XFS_OPTIONS_FEDORAROOT
++ test '-d agcount=4 -s size=512 -i attr=2 -i projid32bit=1 -m crc=1 -m finobt=1 -b size=4096 -i maxpct=25 -d sunit=0 -d swidth=0 -l version=2 -l lazy-count=1 -n size=4096 -n version=2 -r extsize=4096 -i size=1024'
++ Log 'Using device specific options for mkfs.xfs as specified in MKFS_XFS_OPTIONS_FEDORAROOT'
+++ date '+%Y-%m-%d %H:%M:%S.%N '
++ local 'timestamp=2018-12-16 16:57:34.958652720 '
++ test 1 -gt 0
++ echo '2018-12-16 16:57:34.958652720 Using device specific options for mkfs.xfs as specified in MKFS_XFS_OPTIONS_FEDORAROOT'
2018-12-16 16:57:34.958652720 Using device specific options for mkfs.xfs as specified in MKFS_XFS_OPTIONS_FEDORAROOT
++ xfs_opts='-d agcount=4 -s size=512 -i attr=2 -i projid32bit=1 -m crc=1 -m finobt=1 -b size=4096 -i maxpct=25 -d sunit=0 -d swidth=0 -l version=2 -l lazy-count=1 -n size=4096 -n version=2 -r extsize=4096 -i size=1024'
++ test '-d agcount=4 -s size=512 -i attr=2 -i projid32bit=1 -m crc=1 -m finobt=1 -b size=4096 -i maxpct=25 -d sunit=0 -d swidth=0 -l version=2 -l lazy-count=1 -n size=4096 -n version=2 -r extsize=4096 -i size=1024'
++ Log 'Using mkfs.xfs options: -d agcount=4 -s size=512 -i attr=2 -i projid32bit=1 -m crc=1 -m finobt=1 -b size=4096 -i maxpct=25 -d sunit=0 -d swidth=0 -l version=2 -l lazy-count=1 -n size=4096 -n version=2 -r extsize=4096 -i size=1024'
+++ date '+%Y-%m-%d %H:%M:%S.%N '
++ local 'timestamp=2018-12-16 16:57:34.959643701 '
++ test 1 -gt 0
++ echo '2018-12-16 16:57:34.959643701 Using mkfs.xfs options: -d agcount=4 -s size=512 -i attr=2 -i projid32bit=1 -m crc=1 -m finobt=1 -b size=4096 -i maxpct=25 -d sunit=0 -d swidth=0 -l version=2 -l lazy-count=1 -n size=4096 -n version=2 -r extsize=4096 -i size=1024'
2018-12-16 16:57:34.959643701 Using mkfs.xfs options: -d agcount=4 -s size=512 -i attr=2 -i projid32bit=1 -m crc=1 -m finobt=1 -b size=4096 -i maxpct=25 -d sunit=0 -d swidth=0 -l version=2 -l lazy-count=1 -n size=4096 -n version=2 -r extsize=4096 -i size=1024
++ '[' -n 715609a3-9210-4f4f-b913-deb0cc024dc5 ']'
++ echo 'if ! mkfs.xfs -f -m uuid=715609a3-9210-4f4f-b913-deb0cc024dc5 -d agcount=4 -s size=512 -i attr=2 -i projid32bit=1 -m crc=1 -m finobt=1 -b size=4096 -i maxpct=25 -d sunit=0 -d swidth=0 -l version=2 -l lazy-count=1 -n size=4096 -n version=2 -r extsize=4096 -i size=1024 /dev/mapper/fedora-root >&2; then'
++ echo '    mkfs.xfs -f -d agcount=4 -s size=512 -i attr=2 -i projid32bit=1 -m crc=1 -m finobt=1 -b size=4096 -i maxpct=25 -d sunit=0 -d swidth=0 -l version=2 -l lazy-count=1 -n size=4096 -n version=2 -r extsize=4096 -i size=1024 /dev/mapper/fedora-root >&2'
++ echo '    xfs_admin -U 715609a3-9210-4f4f-b913-deb0cc024dc5 /dev/mapper/fedora-root >&2'
++ echo '    xfs_repair /dev/mapper/fedora-root'
++ echo fi
++ '[' -n '' ']'
++ Log 'End of generating code to create XFS on /dev/mapper/fedora-root'

Without explicit MKFS_XFS_OPTIONS set, defaults were used:

2018-12-16 17:02:27.230560095 Using device specific options for mkfs.xfs from /var/lib/rear/layout/xfs/fedora-root.xfs
+++ xfs_parse /var/lib/rear/layout/xfs/fedora-root.xfs
+++ xfs_opts=()
+++ local xfs_opt_file=/var/lib/rear/layout/xfs/fedora-root.xfs
+++ '[' '!' -r /var/lib/rear/layout/xfs/fedora-root.xfs ']'
++++ cat /var/lib/rear/layout/xfs/fedora-root.xfs
+++ infile='meta-data=/dev/mapper/fedora-root isize=512    agcount=4, agsize=406016 blks
         =                       sectsz=512   attr=2, projid32bit=1
         =                       crc=1        finobt=1 spinodes=0 rmapbt=0
         =                       reflink=0
data     =                       bsize=4096   blocks=1624064, imaxpct=25
         =                       sunit=0      swidth=0 blks
naming   =version 2              bsize=4096   ascii-ci=0 ftype=1
log      =internal               bsize=4096   blocks=2560, version=2
         =                       sectsz=512   sunit=0 blks, lazy-count=1
realtime =none                   extsz=4096   blocks=0, rtextents=0'
...

++ Log 'Using mkfs.xfs options: -i size=512 -d agcount=4 -s size=512 -i attr=2 -i projid32bit=1 -m crc=1 -m finobt=1 -b size=4096 -i maxpct=25 -d sunit=0 -d swidth=0 -l version=2 -l lazy-count=1 -n size=4096 -n version=2 -r extsize=4096 '
+++ date '+%Y-%m-%d %H:%M:%S.%N '
++ local 'timestamp=2018-12-16 17:02:27.337180331 '
++ test 1 -gt 0
++ echo '2018-12-16 17:02:27.337180331 Using mkfs.xfs options: -i size=512 -d agcount=4 -s size=512 -i attr=2 -i projid32bit=1 -m crc=1 -m finobt=1 -b size=4096 -i maxpct=25 -d sunit=0 -d swidth=0 -l version=2 -l lazy-count=1 -n size=4096 -n version=2 -r extsize=4096 '
2018-12-16 17:02:27.337180331 Using mkfs.xfs options: -i size=512 -d agcount=4 -s size=512 -i attr=2 -i projid32bit=1 -m crc=1 -m finobt=1 -b size=4096 -i maxpct=25 -d sunit=0 -d swidth=0 -l version=2 -l lazy-count=1 -n size=4096 -n version=2 -r extsize=4096 
++ '[' -n 715609a3-9210-4f4f-b913-deb0cc024dc5 ']'
++ echo 'if ! mkfs.xfs -f -m uuid=715609a3-9210-4f4f-b913-deb0cc024dc5 -i size=512 -d agcount=4 -s size=512 -i attr=2 -i projid32bit=1 -m crc=1 -m finobt=1 -b size=4096 -i maxpct=25 -d sunit=0 -d swidth=0 -l version=2 -l lazy-count=1 -n size=4096 -n version=2 -r extsize=4096  /dev/mapper/fedora-root >&2; then'
++ echo '    mkfs.xfs -f -i size=512 -d agcount=4 -s size=512 -i attr=2 -i projid32bit=1 -m crc=1 -m finobt=1 -b size=4096 -i maxpct=25 -d sunit=0 -d swidth=0 -l version=2 -l lazy-count=1 -n size=4096 -n version=2 -r extsize=4096  /dev/mapper/fedora-root >&2'
++ echo '    xfs_admin -U 715609a3-9210-4f4f-b913-deb0cc024dc5 /dev/mapper/fedora-root >&2'
++ echo '    xfs_repair /dev/mapper/fedora-root'
++ echo fi
++ '[' -n '' ']'
++ Log 'End of generating code to create XFS on /dev/mapper/fedora-root'

Should anybody wonder about FEDORAROOT it is LVM logical volume root inside volume group fedora (/dev/mapper/fedora-root)

V.

schlomo · 2018-12-16T17:16:31Z

Great, I am wondering about the following question: Why not feed the content of the variables into the $LAYOUT_XFS_OPT_DIR/$xfs_device_basename.xfs files and skip the whole logic of sourcing the XFS options from both files and config vars? Won't that make it simpler?

gozora · 2018-12-16T17:31:26Z

@schlomo
Syntax of $LAYOUT_XFS_OPT_DIR/$xfs_device_basename.xfs and MKFS_XFS_OPTIONS is not compatible.
$LAYOUT_XFS_OPT_DIR/$xfs_device_basename.xfs hold plain output of xfs_info which looks something like:

meta-data=/dev/sda               isize=512    agcount=66, agsize=268435392 blks
         =                       sectsz=4096  attr=2, projid32bit=1
         =                       crc=1        finobt=0 spinodes=0
data     =                       bsize=4096   blocks=17580367872, imaxpct=1
         =                       sunit=64     swidth=576 blks
naming   =version 2              bsize=4096   ascii-ci=0 ftype=1
log      =internal               bsize=4096   blocks=521728, version=2
         =                       sectsz=4096  sunit=1 blks, lazy-count=1
realtime =none                   extsz=4096   blocks=0, rtextents=0

and need additional parsing by ReaR xfs_parse function.

Where MKFS_XFS_OPTIONS accepts plain options for mkfs.xfs command like -i size=512 -d agcount=4 -s size=512 -i attr=2 -i projid32bit=1 -m crc=1 -m finobt=1 -b size=4096 -i maxpct=25 -d sunit=0 -d swidth=0 -l version=2 -l lazy-count=1 -n size=4096 -n version=2 -r extsize=4096

V.

schlomo

If this code helps the original bug reporter then I am all in favor, otherwise this seems like adding a lot of complexity to workaround a bug or changed behavior in mkfs.xfs.

How would a regular user know with which values to fill this override variable? When he is in the ReaR rescue media? And already totally frustrated?

Maybe this is something that we really should tackle at the mkrescue side, e.g. trying to create an XFS filesystem in a (sparse) file with the given options and removing this file. If there is a mismatch between mkfs.xfs and the options dump then it should break during mkrescue and not later.

The more I think about this topic the more I come to believe that add some magic and very long variable to local.conf and try rear recover again is not really user friendly. Part of the problem is that the user is forced to supply all the mkfs.xfs options where I doubt that many users will be able to do that.

schlomo · 2018-12-16T17:41:42Z

usr/share/rear/layout/prepare/GNU/Linux/130_include_filesystem_code.sh

+            # Set which options to use for mkfs.xfs:
+            if test "${!mkfs_xfs_device_options_variable_name:-}" ; then
+                # When the user has specified device specific options for mkfs.xfs e.g. in MKFS_XFS_OPTIONS_SDA2 use that:
+                Log "Using device specific options for mkfs.xfs as specified in $mkfs_xfs_device_options_variable_name"


IMHO the log output should be more specific and explain that it is overriding the detected settings, e.g. Overriding mkfs.xfs options from $mkfs_xfs_device_options_variable_name variable or something like this.

Overriding only happens when there is a file /var/lib/rear/layout/xfs/$device.xfs
so that I could enhance the Log messaging and Log different messages
depending on whether or not /var/lib/rear/layout/xfs/$device.xfs exists.

schlomo · 2018-12-16T17:42:52Z

usr/share/rear/layout/prepare/GNU/Linux/130_include_filesystem_code.sh

+            else
+                if test "$MKFS_XFS_OPTIONS" ; then
+                    # When the user has specified global options for mkfs.xfs in MKFS_XFS_OPTIONS use that:
+                    Log "Using global options for mkfs.xfs as specified in MKFS_XFS_OPTIONS"


Same here, explicitly explain the overriding like this Overriding mkfs.xfs options from MKFS_XFS_OPTIONS variable

Overriding only happens when there is a file /var/lib/rear/layout/xfs/$device.xfs
so that I could enhance the Log messaging and Log different messages
depending on whether or not /var/lib/rear/layout/xfs/$device.xfs exists.

schlomo · 2018-12-16T17:43:31Z

usr/share/rear/layout/prepare/GNU/Linux/130_include_filesystem_code.sh

+                    # The function xfs_parse in lib/filesystems-functions.sh falls back to mkfs.xfs defaults
+                    # when there is no file $LAYOUT_XFS_OPT_DIR/$xfs_device_basename.xfs where the
+                    # XFS filesystem options of that particular device on the original system were saved:
+                    Log "Using device specific options for mkfs.xfs from $LAYOUT_XFS_OPT_DIR/$xfs_device_basename.xfs"


And here I would emphasize that these are the detected values, e.g. Using the automatically detected device specific ...

Detected values exist only if /var/lib/rear/layout/xfs/$device.xfs exists
so that I could enhance the Log messaging and Log different messages
depending on whether or not /var/lib/rear/layout/xfs/$device.xfs exists.

schlomo · 2018-12-16T17:45:01Z

usr/share/rear/layout/prepare/GNU/Linux/130_include_filesystem_code.sh

+                fi
+            fi
+            # In case of fallback to mkfs.xfs defaults xfs_opts is empty:
+            test "$xfs_opts" && Log "Using mkfs.xfs options: $xfs_opts" || Log "Using mkfs.xfs defaults"


In which case would we be using the mkfs.xfs defaults? If we recreate an filesystem then there should be also the option file. Maybe if somebody hand-crafted a layout configuration from scratch that could happen... ?

In case of MIGRATION_MODE there could be no
/var/lib/rear/layout/xfs/$original_device.xfs
for example /var/lib/rear/layout/xfs/sdb2.xfs
may exist but there is no /dev/sdb on the replacement hardware.

As far as I understand the code in
layout/prepare/GNU/Linux/130_include_filesystem_code.sh
the variable $device means the target device,
for example when sdb on the original system
is now sdc on the replacement hardware the code in
layout/prepare/GNU/Linux/130_include_filesystem_code.sh
would look for /var/lib/rear/layout/xfs/sdc2.xfs which
does not exist because as far as I see there is not yet
an automated rename of that file in MIGRATION_MODE
which looks like missing functionality (probably even a bug)
in ReaR's XFS related code because such things could be
expected to work in MIGRATION_MODE - but fixing that
would be a different separated issue.

jsmeix · 2018-12-17T10:41:45Z

@schlomo
in general regarding your above questions:
Have MIGRATION_MODE in mind.

When XFS filesystems get recreated on same hardware
things "just work" (except the original system is already broken).

But when things do not "just work"
(e.g. because of different replacement hardware
or because the original system is already broken
as in #1998)
then we are in MIGRATION_MODE.

My point is that MIGRATION_MODE can be expected by the user
(when he recreates on different replacement hardware)
but MIGRATION_MODE can also happen all of a sudden
when things do not "just work" for whatever reason.

When we are in MIGRATION_MODE and "rear recover" fails
then ReaR should provide means so that the user can
relatively easily recover by some manual intervention.

Currently such manual intervention is often adapting diskrestore.sh
which is not really user-friendly (I consider code hacking within
the recovery system in general not really user-friendly) and
it becomes annoying and error-prone when the user has to receate/migrate
many systems because he would have to manually adapt diskrestore.sh
again and again for each system.

Config variables like MKFS_XFS_OPTIONS and MKFS_XFS_OPTIONS_$device
are meant to make the user's life a bit easier in such cases because with
such config variables he could do things like

# export MKFS_XFS_OPTIONS="generic options"
# export MKFS_XFS_OPTIONS_SDA2="all options for sda2"
# export MKFS_XFS_OPTIONS_SDB3="$MKFS_XFS_OPTIONS additional sdb3 options"
# rear -D recover

jsmeix · 2018-12-17T11:04:03Z

Regarding it should break during 'mkrescue' and not later in
#2005 (review)

YES!
I do fully agree.

But current ReaR is far away from that behaviour, cf.
https://github.com/rear/rear/wiki/Developers-Guide

Current ReaR requires to test "rear recover" on already
available replacement hardware and see how things work,
cf. the section "Inappropriate expectations" in
https://en.opensuse.org/SDB:Disaster_Recovery

But sometimes (perhaps even often in practice) "rear recover"
was not sufficiently tested (or the last test was too long ago),
cf. continuous validation in
https://en.opensuse.org/SDB:Disaster_Recovery
so that "rear recover" unexpectedly fails when it is a real disaster recovery
( according to Murphy's Law it only fails when it is a real disaster recovery ;-)
and then the poor user is inside the recovery system where he must
get things back to work...

…ssue 1998)

jsmeix · 2018-12-17T16:53:24Z

I have tested it and for me with a simple XFS it works o.k.

Now it uses LogPrint to show exceptional info on the user's terminal
and Log when things happen as usual: "no news is good news".

jsmeix · 2018-12-18T10:16:44Z

If there are no severe objections I would like to merge it today afternoon.
Of course I could further enhance it later if needed.

schlomo · 2018-12-18T10:30:33Z

👍

Provide final power to the user for mkfs.xfs options if needed (issue…

66d992d

… 1998)

jsmeix added the enhancement Adaptions and new features label Dec 14, 2018

jsmeix added this to the ReaR v2.5 milestone Dec 14, 2018

jsmeix self-assigned this Dec 14, 2018

jsmeix requested a review from gozora December 14, 2018 14:53

jsmeix mentioned this pull request Dec 14, 2018

SLE15 on ppcl: mkfs.xfs fails if sunit > 0 but swidth = 0 #1998

Closed

gozora approved these changes Dec 16, 2018

View reviewed changes

schlomo reviewed Dec 16, 2018

View reviewed changes

jsmeix added 2 commits December 17, 2018 16:14

Better Log and LogPrint messages (issue 1998)

7891502

Document MKFS_XFS_OPTIONS and MKFS_XFS_OPTIONS_... in default.conf (i…

b8b7989

…ssue 1998)

jsmeix merged commit f35340f into rear:master Dec 18, 2018

jsmeix deleted the XFS_options_final_power_to_the_user_issue_1998 branch December 18, 2018 13:51

jsmeix added the fixed / solved / done label Dec 18, 2018

jsmeix mentioned this pull request Dec 21, 2018

Provide final power to the user for mkfs options if needed #2010

Closed

jsmeix mentioned this pull request Sep 3, 2019

Thin pool recreation logic / use of vgcfgrestore is broken #2222

Closed

jsmeix mentioned this pull request Feb 18, 2020

Mapping disks doesn't map corresponding XFS information file #2333

Closed

jsmeix mentioned this pull request Mar 22, 2022

XFS: diskrestore.sh failed at 'mount: /mnt/local: ...' because of wrong mount options #2777

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Provide final power to the user for mkfs.xfs options if needed (issue 1998) #2005

Provide final power to the user for mkfs.xfs options if needed (issue 1998) #2005

jsmeix commented Dec 14, 2018

gozora left a comment

schlomo commented Dec 16, 2018

gozora commented Dec 16, 2018

schlomo left a comment

schlomo Dec 16, 2018

jsmeix Dec 17, 2018

schlomo Dec 16, 2018

jsmeix Dec 17, 2018

schlomo Dec 16, 2018

jsmeix Dec 17, 2018

schlomo Dec 16, 2018

jsmeix Dec 17, 2018

jsmeix commented Dec 17, 2018

jsmeix commented Dec 17, 2018 •

edited

jsmeix commented Dec 17, 2018 •

edited

jsmeix commented Dec 18, 2018

schlomo commented Dec 18, 2018

Provide final power to the user for mkfs.xfs options if needed (issue 1998) #2005

Provide final power to the user for mkfs.xfs options if needed (issue 1998) #2005

Conversation

jsmeix commented Dec 14, 2018

gozora left a comment

Choose a reason for hiding this comment

schlomo commented Dec 16, 2018

gozora commented Dec 16, 2018

schlomo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsmeix commented Dec 17, 2018

jsmeix commented Dec 17, 2018 • edited

jsmeix commented Dec 17, 2018 • edited

jsmeix commented Dec 18, 2018

schlomo commented Dec 18, 2018

jsmeix commented Dec 17, 2018 •

edited

jsmeix commented Dec 17, 2018 •

edited