#810 Issue closed: mkrescue fails with UEFI and TSM

Labels: enhancement, bug, cleanup, fixed / solved / done

pavoldomin opened issue at 2016-03-30 21:14:

  • rear version (/usr/sbin/rear -V): Most likely any (tested with 1.17.2)
  • OS version (cat /etc/rear/os.conf or lsb_release -a): (very likely any), tested with SLES11 SP3
  • rear configuration files (cat /etc/rear/site.conf or cat /etc/rear/local.conf):
BACKUP=TSM
COPY_AS_IS_TSM=( /etc/adsm/TSM.PWD /opt/tivoli/tsm/client /usr/local/ibm/gsk8* )
COPY_AS_IS_EXCLUDE_TSM=( )
PROGS_TSM=( dsmc tput )
TSM_RESULT_FILE_PATH=/opt/tivoli/tsm/rear
TSM_DSMC_RESTORE_OPTIONS=( )
TSM_RESTORE_PIT_DATE=
TSM_RESTORE_PIT_TIME=
TSM_RESULT_SAVE=y

the rest is the usual stuff

  • Brief description of the issue
    rear mkrescue (or mkbackup) fails with the message like
    2016-03-29 17:14:15.078510507 ERROR: Could not copy initrd to UEFI
  • Work-around, if any
    I have modified rear code: extended UEFI image size by experimentally determined value for this particular setup. I have not tested an actual recovery though (but do not see reason why it would not work).

I will post the patch I used immediately, for your consideration.

gozora commented at 2016-04-03 15:15:

I just created pull request #813.
@pavoldomin can you please test, if it works as expected with TSM?

Thanks!

jsmeix commented at 2016-04-05 12:45:

FYI my tests with unchanged rear 1.18 on an UEFI system:

"rear -d -D mkrescue" fails with 'Could not copy initrd to UEFI'
because of that error in the log file:

cp: error writing '/tmp/rear.kAFGCscGy27EXW4/tmp/mnt/EFI/BOOT/initrd.cgz': No space left on device

The 128 kKiB (128000 KiB) rear 1.18 default is already
too small for me (I do not use thrid-party backup software).

I simply changed
/usr/share/rear/output/ISO/Linux-i386/20_mount_efibootimg.sh
directly to

dd if=/dev/zero of=$TMP_DIR/efiboot.img count=5 bs=32M

i.e. with 5 x 32MiB = 160MiB it is sufficient for me.

This results in "rear -d -D mkrescue" output:
Wrote ISO image: /var/lib/rear/output/rear-caps.iso (447M)

An UEFI bootable ISO image is much bigger (almost 450MiB)
than a traditional BIOS bootable ISO image which is less than
120 MiB for me.

gozora commented at 2016-04-05 13:00:

As the things become a bit unclear recently, did you use https://github.com/gozora/rear for your tests or some different commit.

jsmeix commented at 2016-04-05 13:37:

@gozora

Regarding that the virtual image must be aligned to 32MB
I suggest a (hopefully) cleaner and more fail-safe
and (hopefully) more obvious way how to do it
in your new 70_create_efibootimg.sh as follows
(excerpt):

# function to calculate exact size of EFI virtual image (efiboot.img)
function efiboot_img_size {
    # Get size of directory holding EFI virtual image content.
    # The virtual image must be aligned to 32MiB blocks
    # therefore the size of directory is measured in 32MiB blocks.
    efi_img_dir=$1
    # Specify a minimum EFI virtual image size measured in 32MiB blocks:
    case "$( basename $UEFI_BOOTLOADER )" in
        (shim.efi|elilo.efi)
            # minimum EFI virtual image size for shim and elilo
            # default: 128MiB = 4 * 32MiB blocks
            efi_img_min_sz=4
        ;;
        (*)
            # minimum EFI virtual image size for grub
            # default: 32MiB = 1 * 32MiB block
            efi_img_min_sz=1
        ;;
    esac
    # Fallback output of the minimum EFI virtual image size measured in 32MiB blocks:
    test $efi_img_dir || echo $efi_img_min_sz
    # The du output is stored in an artificial bash array
    # so that $efi_img_sz can be simply used to get the first word
    # which is the efi_img_sz value measured in 32MiB blocks:
    efi_img_sz=( $( du --block-size=32M --summarize $efi_img_dir ) )
    # Fallback output of the minimum EFI virtual image size measured in 32MiB blocks:
    test $efi_img_sz || echo $efi_img_min_sz
    test $efi_img_sz -ge 1 || echo $efi_img_min_sz
    # Output at least the minimum EFI virtual image size measured in 32MiB blocks:
    if test $efi_img_sz -lt $efi_img_min_sz ; then
        echo $efi_img_min_sz
    else
        echo $efi_img_sz
    fi
}
# prepare EFI virtual image
dd if=/dev/zero of=$TMP_DIR/efiboot.img count=$( efiboot_img_size $TMP_DIR/mnt ) bs=32M

jsmeix commented at 2016-04-05 13:40:

I did not you use https://github.com/gozora/rear for my tests above.
For my tests above I used the released rear 1.18 as is.
I liked to show that for my system 128 MiB is too small.
Perhaps the default minimal efi image size should be bigger?

But now I used your new 70_create_efibootimg.sh
with the changes in my other comment directly above
and that "just works" for me.

jsmeix commented at 2016-04-05 13:44:

FYI excerpts from my succesful "rear -d -D mkrescue" log
with your new 70_create_efibootimg.sh with the changes
in my other comment above:

+ source /usr/share/rear/output/ISO/Linux-i386/70_create_efibootimg.sh
++ is_true 1
++ case "$1" in
++ return 0
+++ efiboot_img_size /tmp/rear.XO6CsKzXPsuGy5c/tmp/mnt
+++ efi_img_dir=/tmp/rear.XO6CsKzXPsuGy5c/tmp/mnt
+++ case "$( basename $UEFI_BOOTLOADER )" in
++++ basename /boot/efi/EFI/opensuse/shim.efi
+++ efi_img_min_sz=4
+++ test /tmp/rear.XO6CsKzXPsuGy5c/tmp/mnt
+++ efi_img_sz=($( du --block-size=32M --summarize $efi_img_dir ))
++++ du --block-size=32M --summarize /tmp/rear.XO6CsKzXPsuGy5c/tmp/mnt
+++ test 5
+++ test 5 -ge 1
+++ test 5 -lt 4
+++ echo 5
++ dd if=/dev/zero of=/tmp/rear.XO6CsKzXPsuGy5c/tmp/efiboot.img count=5 bs=32M
5+0 records in
5+0 records out
167772160 bytes (168 MB) copied, 0.143509 s, 1.2 GB/s

jsmeix commented at 2016-04-05 13:53:

FYI why "du --block-size=32M -s" should work as expected
(at least on my system):

33554433 = ( 32 x 1024 x 1024 ) + 1

# dd if=/dev/zero of=/tmp/foo count=1 bs=33554433
1+0 records in
1+0 records out
33554433 bytes (34 MB) copied, 0.0492154 s, 682 MB/s
# ls -lh /tmp/foo
-rw-r--r-- 1 root root 33M Apr  5 15:51 /tmp/foo
# du --block-size=32M -s /tmp/foo
2       /tmp/foo

jsmeix commented at 2016-04-06 07:17:

How embarrassing - my code in https://github.com/rear/rear/issues/810#issuecomment-205808349 has bugs:

All fallback outputs must of course also
return from the efiboot_img_size function, i.e.
instead of things like

test condition-to-proceed || echo fallback-value

it must be

test condition-to-proceed || { echo fallback-value ; return ; }

Furthermore the

    test $efi_img_sz || ...

line is unnecessary
because the subsequent test

    test $efi_img_sz -ge 1 || ...

alone is sufficient.

gozora commented at 2016-04-06 07:20:

@jsmeix Can you send me corrected function by mail?
I'll correct it ...

jsmeix commented at 2016-04-06 07:32:

@gozora

I will send you the corrected function.
I am sorry that my bad code caused trouble for you.
Yesterday I changed too much at once
and worse I did it too fast and worst I did not test
if my error handling code actually works :-(

Do you like to keep the current default 128MiB
minimum EFI virtual image size for shim and elilo
which is too small for me (i.e. it seems 128MiB
is too small for my "usual" SLES12 UEFI system)
or
should I also specify 5 x 32MiB = 160 MiB for the
minimum EFI virtual image size for shim and elilo?

gozora commented at 2016-04-06 07:52:

Not a big deal, I could discover it as well, but like I mentioned before, your style of code is a bit hard to read for me I preffer if [[ ]]; then else ... rather than test

For the default size, I guess that it doesn't matter what is the default size, the function should automatically calculate size needed for EFI virtual image, or am I wrong?

jsmeix commented at 2016-04-06 08:24:

Regarding what you called "default size":

Actually it is not a default size but a minimal size
(and its variable is named "..._min_sz" - cf. below)
so that a too big minimal size matters because
the image could never be smaller than that.
A too big minimal size wastes space for the user, cf.
https://github.com/rear/rear/pull/813#issuecomment-205191789

Regarding my coding syntax style:

Feel free to change my "test ..." into "if [[ ... ]] ; then ...".
For me both are equally well understandable.
It is just different syntax for the same meaning.
I do not care much about coding syntax style.

But I do care very much about coding semantics style:

Where I am particularly picky in semantics style
is naming of any "thingies" (variables, functions, files, ...).

Frankly I insist on using descriptive meaningful names.

In particular I insist on using differentiating qualifiers/adjectives/...
in names for example

  • "efi_img_dir" instead of meaningless "$1"
  • "efi_img_sz" instead of meaningless "size"

I insist on using different names for different things
so that others can easily "grep" over the whole code
for such names and get a correct overview what
actually belongs to that particular "thingy".

For example I never use "i" as a variable name.
Just try to "grep" for it over the whole code
to find where a bug is that is related to "i" ;-)

gozora commented at 2016-04-06 08:38:

Yes, sorry it is definitely not default size, my fault. It is clear to me what efi_img_min_sz actually does.
What I don't understand is why you want to increase it to 160MiB when function it self can determine what the right size is? I mean when 128MiB are not enough for your SLES12, this function should return 160MiB, right?

gozora commented at 2016-04-06 08:44:

And for the coding style, we don't need to change anything, I told it is hard for me not impossible ;-).
For the descriptive and meaningful names I can only agree! (however I sometimes tend to forget to keep this rule)

jsmeix commented at 2016-04-06 08:50:

You are absolutely right.

But then I don't understand why there is a minimal size at all
(i.e. I do not understand your https://github.com/rear/rear/pull/813#issuecomment-205200296).

Shouldn't the "du" command determine what the right size is
in all cases i.e. for shim and elilo and also for grub?

If the "du" command determines the right size in any case
we could simply rely on it and remove all code that deals
with the minimal size and its usage as fallback output value.

In case of errors with the "du" command we could then
simply abort with "Error error-message" because I cannot
imagine why the "du" command may fail in practice.

gozora commented at 2016-04-06 08:57:

hmmm, actually you are right!
All what we need is:

  1. determine size of staging directory ($TMP_DIR/mnt/)
  2. and align it to 32 MiB
  3. return the size to dd

Is this what you meant?

jsmeix commented at 2016-04-06 09:10:

Yes, and here my current stripped down 70_create_efibootimg.sh
(that still works for me with same results as before):

# 70_create_efibootimg.sh
is_true $USING_UEFI_BOOTLOADER || return    # empty or 0 means NO UEFI
# Calculate exact size of EFI virtual image (efiboot.img):
# Get size of directory holding EFI virtual image content.
# The virtual image must be aligned to 32MiB blocks
# therefore the size of directory is measured in 32MiB blocks.
# The du output is stored in an artificial bash array
# so that $efi_img_sz can be simply used to get the first word
# which is the disk usage of the directory measured in 32MiB blocks:
efi_img_sz=( $( du --block-size=32M --summarize $TMP_DIR/mnt ) )
StopIfError "Failed to determine disk usage of EFI virtual image content directory."
# prepare EFI virtual image aligned to 32MiB blocks:
dd if=/dev/zero of=$TMP_DIR/efiboot.img count=$efi_img_sz bs=32M
mkfs.vfat $v -F 16 $TMP_DIR/efiboot.img >&2
mkdir -p $v $TMP_DIR/efi_virt >&2
mount $v -o loop -t vfat -o fat=16 $TMP_DIR/efiboot.img $TMP_DIR/efi_virt >&2
# copy files from staging directory
cp $v -r $TMP_DIR/mnt/. $TMP_DIR/efi_virt
umount $v $TMP_DIR/efiboot.img >&2
mv $v -f $TMP_DIR/efiboot.img $TMP_DIR/isofs/boot/efiboot.img >&2
StopIfError "Could not move efiboot.img file"

This time I even tested that the StopIfError
actually works if the du command fails ;-)

And no longer any "test" usage ;-)

gozora commented at 2016-04-06 09:17:

This is what call code optimization!
Will update pull request!

jsmeix commented at 2016-04-06 11:09:

This issue should be fixed with https://github.com/rear/rear/pull/816

jsmeix commented at 2016-04-08 07:46:

@pavoldomin
I would greatly appreciate it if you could test
whether or not the current rear master
with full automated UEFI image size calculation
(implemented in the new 70_create_efibootimg.sh that replaced
the whole stuff before, cf. https://github.com/rear/rear/pull/816/files)
"just works" for you.

pavoldomin commented at 2016-04-27 06:11:

Found in trash again. I've tested it already, works as charm.

jsmeix commented at 2016-04-27 07:01:

@pavoldomin
many thanks for your feedback!

I was already a bit wondering why you didn't reply,
thinking about things like you are perhaps on vacation
but actually it was some proprietary non-free software
that made its own false decisions (https://github.com/rear/rear/issues/821#issuecomment-214761409)
about your incoming mail - presumably with the very best
of intentions to protect you from possibly unpleasant stuff ;-)


[Export of Github issue for rear/rear.]