#890 Issue `closed`: Use of invalid option "-U" for mkfs during recover on RHEL 5¶

Labels: bug, fixed / solved / done

EQXTFL opened issue at 2016-06-22 14:53:¶

rear version (/usr/sbin/rear -V):
1.18
OS version (cat /etc/rear/os.conf or lsb_release -a):
RHEL 5.10
rear configuration files (cat /etc/rear/site.conf or cat /etc/rear/local.conf):
ISO/NFS
Brief description of the issue
During FS creation in a recover, mkfs is called as follows:
mkfs -t ext3 -b 1024 -i 8128 -U UUID /dev/sda2

The version of mkfs available does not support the -U option.
Removing the option allows the recover process to complete.

jsmeix commented at 2016-06-22 15:03:¶

I think you mean that code in
usr/share/rear/layout/prepare/GNU/Linux/13_include_filesystem_code.sh
(excerpts)

    case "$fstype" in
        (ext*)
    ....
            # Actually create the filesystem with initially correct UUID
            # (Addresses Fedora/systemd problem, see issue 851)
            if [ -n "$uuid" ] ; then
                echo "mkfs -t ${fstype}${blocksize}${fragmentsize}${bytes_per_inode} -U $uuid $device >&2" >> "$LAYOUT_CODE"
            else
                echo "mkfs -t ${fstype}${blocksize}${fragmentsize}${bytes_per_inode}  $device >&2" >> "$LAYOUT_CODE"
            fi

Accordingly it seems on RHEL 5.10 the filesystem has a UUID
but on RHEL 5.10 the mkfs command does not support to set it?

Because the code was intentionally implemented
this way by @exedor see https://github.com/rear/rear/issues/851
we cannot simply remove setting the UUID
without causing regressions elewhere.

jsmeix commented at 2016-06-22 15:07:¶

@gdha
I also assign you because you had benn involved in https://github.com/rear/rear/issues/851

By the way:
This time I am so happy to find useful comments
in the code that show why it is implemented as is.
Such explanatory comments help so much
to get issues fixed properly
cf. https://github.com/rear/rear/wiki/Coding-Style

EQXTFL commented at 2016-06-22 18:53:¶

@jsmeix
I agree, we should make sure to use -U whenever possible. Maybe we can check the mkfs options available in addition to -n "$uuid". You know, something like $? on mkfs.${fstype} 2>&1 | grep -q "-U".

exedor commented at 2016-06-22 23:35:¶

Oh man...yeah that's true. Older version of mkfs did not support this. I
can look at an alternative form of the patch which may still address the
issue.

I'd hate to do version checking...that kinda sucks because there are so
many different versions of operating systems.

On 6/22/2016 9:03 AM, Johannes Meixner wrote:

I think you mean that code in
usr/share/rear/layout/prepare/GNU/Linux/13_include_filesystem_code.sh
(excerpts)
 case "$fstype" in
     (ext*)
 ....
         # Actually create the filesystem with initially correct UUID
         # (Addresses Fedora/systemd problem, see issue 851)
         if [ -n "$uuid" ] ; then
             echo "mkfs -t ${fstype}${blocksize}${fragmentsize}${bytes_per_inode} -U $uuid $device >&2" >> "$LAYOUT_CODE"
         else
             echo "mkfs -t ${fstype}${blocksize}${fragmentsize}${bytes_per_inode}  $device >&2" >> "$LAYOUT_CODE"
         fi
Accordingly it seems on RHEL 5.10 the filesystem has a UUID
but on RHEL 5.10 the mkfs command does not support to set it?

Because the code was intentionally implemented
this way by @exedor https://github.com/exedor see #851
https://github.com/rear/rear/issues/851
we cannot simply remove setting the UUID
without causing regressions elewhere.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rear/rear/issues/890#issuecomment-227773038, or
mute the thread
https://github.com/notifications/unsubscribe/AGGIOMmPwXYru_PG1gUdmUv45ewM_3mAks5qOU7YgaJpZM4I73w6.

exedor commented at 2016-06-22 23:54:¶

I was thinking the same thing. I'll put another patch together.

On 6/22/2016 12:54 PM, Mike H wrote:

@jsmeix https://github.com/jsmeix
I agree, we should make sure to use -U whenever possible. Maybe we can
check the mkfs options available in addition to -n "$uuid". You know,
something like $? on mkfs.${fstype} 2>&1 | grep -q "-U".

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rear/rear/issues/890#issuecomment-227842318, or
mute the thread
https://github.com/notifications/unsubscribe/AGGIOGwdaaw93DE75L9NBxmm-EcNcurqks5qOYTKgaJpZM4I73w6.

jsmeix commented at 2016-06-23 09:21:¶

@EQXTFL
I feel a bit unhappy using things like

mkfs.ext2 2>&1 | grep -q -- '-U' && echo 'mkfs.ext2 supports -U' || echo 'mkfs.ext2 does not support -U'

regardless that it seems to work for me on SLES11
and on openSUSE Leap 42.1 (that is like SLES12)
because "man mkfs.ext2" does not tell that
calling mkfs.ext2 without options is the actually
right way to get the help text.

I am a bit astonished about what kind of commands
still exist that have no support for '-h' or '--help'.

But on the other hand I have no better idea right now.

@exedor
I also prefer to avoid checks based
on versions of operating systems, cf.
https://github.com/rear/rear/issues/882#issuecomment-226499432

exedor commented at 2016-06-23 15:14:¶

I hear you....it seems like a kludge. Another option would be to check
for a legit return code, if it fails assume it was because the UUID
option isn't supported, then fall back to the non -U call combined with
the tune2fs call and then check that for failure and treat it the same
way it was before.

On 6/23/2016 3:22 AM, Johannes Meixner wrote:

@EQXTFL https://github.com/EQXTFL
I feel a bit unhappy using things like

mkfs.ext2 2>&1 | grep -q -- '-U' && echo 'mkfs.ext2 supports -U' || echo 'mkfs.ext2 does not support -U'

regardless that it seems to work for me on SLES11
and on openSUSE Leap 42.1 (that is like SLES12)
because "man mkfs.ext2" does not tell that
calling mkfs.ext2 without options is the actually
right way to get the help text.

I am a bit astonished about what kind of commands
still exist that have no support for '-h' or '--help'.

But on the other hand I have no better idea right now.

@exedor https://github.com/exedor
I also prefer to avoid checks based
on versions of operating systems, cf.
#882 (comment)
https://github.com/rear/rear/issues/882#issuecomment-226499432

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rear/rear/issues/890#issuecomment-227995399, or
mute the thread
https://github.com/notifications/unsubscribe/AGGIOJU155L7KE_vWRpE24zkZIKKY5Mvks5qOlA4gaJpZM4I73w6.

jsmeix commented at 2016-06-23 16:30:¶

FYI regarding "kludge":
I added right now a famous
"Dirty hacks welcome"
section to
https://github.com/rear/rear/wiki/Coding-Style

For the fun of it regarding "retry commands":
Cf. https://github.com/rear/rear/issues/791 therein
in particular https://github.com/rear/rear/issues/791#issuecomment-223918906

jsmeix commented at 2016-06-24 07:31:¶

I think I have an idea how to implement it sufficiently well:

I agree with what @exedor wrote above in https://github.com/rear/rear/issues/890#issuecomment-228082714
and in general in https://github.com/rear/rear/issues/851

I tested my revision so that instead of calling tune2fs
after the file system was created, I instead check
at the mkfs call and then use it's -U parameter
so it just uses the correct UUID at filesystem creation
time instead of first creating bogus UUIDs and then
trying to fix them up later. Seemed like a cleaner
approach until it broke older versions.

Because "mkfs -U" works for me at least since SLE11
(I did not test SLE10 and I don't care because that is no
longer supported in Rear-1.18, see doc/rear-release-notes.txt)
I assume "mkfs -U" also works on RHEL6
but only fails on RHEL5.

Therefore I think the right way it to try "mkfs -U"
and only if that fails descend into dirty hacks.

The problem with the implementation in
layout/prepare/GNU/Linux/13_include_filesystem_code.sh
is the indirection that one has to generate code which
makes it look somewhat overcomplicated, somehing like:

    case "$fstype" in
        (ext*)
...
            if test "$uuid" ; then
                echo "# when 'mkfs -U' fails" >> "$LAYOUT_CODE"
                echo "# assume it failed because on older systems" >> "$LAYOUT_CODE"
                echo "# mkfs does not support '-U' (issue 890)" >> "$LAYOUT_CODE"
                echo "# then retry mkfs without '-U'" >> "$LAYOUT_CODE"
                echo "# plus 'tunefs -U' but that may fail" >> "$LAYOUT_CODE"
                echo "# on newer systems (issue 851)" >> "$LAYOUT_CODE"
                echo "if ! mkfs ... -U $uuid ... ; then" >> "$LAYOUT_CODE"
                echo "    mkfs ... " >> "$LAYOUT_CODE"
                echo "    $tunefs -U ... " >> "$LAYOUT_CODE"
                echo "fi" >> "$LAYOUT_CODE"
            else
                echo "mkfs ... " >> "$LAYOUT_CODE"
            fi

jsmeix commented at 2016-06-27 12:25:¶

With https://github.com/rear/rear/pull/894
I implemeted what I wrote above.

I tested it on SLE11 with ext3 and SLE12 with ext4
but on those systems "mkfs -U" works.

I.e. I did not test it on a system where "mkfs -U" fails
in particular I did not test it on RHEL5.

@EQXTFL
please test if current rear master
works for you on RHEL5
and provide feedback here.

jsmeix commented at 2016-07-15 09:55:¶

No feedback.

With the fallback assumtion that "no news is good news"
I close it now.

[Export of Github issue for rear/rear.]

#890 Issue closed: Use of invalid option "-U" for mkfs during recover on RHEL 5¶