added mods for slackware support - mkbackup working with UEFI/USB #1849
Conversation
| @@ -88,7 +89,8 @@ EOF | |||
|
|
|||
| # Create bootloader, this overwrite BOOTX64.efi copied in previous step ... | |||
| # Fail if BOOTX64.efi can't be created | |||
| ${GRUB_MKIMAGE} -o ${EFI_DST}/BOOTX64.efi -p ${EFI_DIR} -O x86_64-efi linux part_gpt ext2 normal gfxterm gfxterm_background gfxterm_menu test all_video loadenv fat | |||
| # Slackware grub doesnt have gfxterm_background or gfxterm_menu... | |||
| ${GRUB_MKIMAGE} -o ${EFI_DST}/BOOTX64.efi -p ${EFI_DIR} -O x86_64-efi linux part_gpt ext2 normal gfxterm gfxmenu test all_video loadenv fat | |||
There was a problem hiding this comment.
I think here a test for Slackware and conditional code execution is needed
like you did it in your usr/share/rear/pack/Linux-i386/300_copy_kernel.sh
# add slackware test on top to prevent error because
# slackware grub doesnt have gfxterm_background or gfxterm_menu...
if [ -f /etc/slackware-version ] ; then
${GRUB_MKIMAGE} -o ${EFI_DST}/BOOTX64.efi -p ${EFI_DIR} -O x86_64-efi linux part_gpt ext2 normal gfxterm gfxmenu test all_video loadenv fat
else
${GRUB_MKIMAGE} -o ${EFI_DST}/BOOTX64.efi -p ${EFI_DIR} -O x86_64-efi linux part_gpt ext2 normal gfxterm gfxterm_background gfxterm_menu test all_video loadenv fat
fi
or perhaps even cleaner by using a meaningful variable for the GRUB modules like
grub_modules="x86_64-efi linux part_gpt ext2 normal gfxterm gfxmenu test all_video loadenv fat"
# slackware grub doesnt have gfxterm_background or gfxterm_menu
test -f /etc/slackware-version || grub_modules="$grub_modules gfxterm_background gfxterm_menu"
# Create bootloader, this overwrite BOOTX64.efi copied in previous step ...
# Fail if BOOTX64.efi can't be created
${GRUB_MKIMAGE} -o ${EFI_DST}/BOOTX64.efi -p ${EFI_DIR} -O $grub_modules
The actual test for Slackware should be done via the usually
used variables like OS_MASTER_VENDOR, see my other comment.
There was a problem hiding this comment.
Changing modules that are currently used and tested was my only concern. If this code can be re-written so it does not interfere with existing distributions, I have not problems left with this PR.
V.
| @@ -10,7 +10,13 @@ | |||
| # Using another kernel is a TODO for now | |||
|
|
|||
| if [ ! -s "$KERNEL_FILE" ]; then | |||
| if [ -r "/boot/vmlinuz-$KERNEL_VERSION" ]; then | |||
| # add slackware test on top to prevent error on get_kernel_version | |||
| if [ -f /etc/slackware-version ]; then | |||
There was a problem hiding this comment.
The test for Slackware should be done via the usually used variables
OS_MASTER_VENDOR OS_VENDOR OS_VENDOR_VERSION and so on,
see the SetOSVendorAndVersion function in usr/share/rear/lib/config-functions.sh
how those variables are set.
Perhaps those variables are already set right for Slackware by that function
perhaps you need to add support to get them set right for Slackware to that function.
To find out to what values those variables are set in your particular case
run rear dump for example in my case on SLES15 I get (excerpts):
# usr/sbin/rear -D dump
...
This is a 'Linux-x86_64' system, compatible with 'Linux-i386'.
System definition:
ARCH = Linux-i386
OS = GNU/Linux
OS_MASTER_VENDOR = SUSE
OS_MASTER_VERSION = 15
OS_MASTER_VENDOR_ARCH = SUSE/i386
OS_MASTER_VENDOR_VERSION = SUSE/15
OS_MASTER_VENDOR_VERSION_ARCH = SUSE/15/i386
OS_VENDOR = SUSE_LINUX
OS_VERSION = 15
OS_VENDOR_ARCH = SUSE_LINUX/i386
OS_VENDOR_VERSION = SUSE_LINUX/15
OS_VENDOR_VERSION_ARCH = SUSE_LINUX/15/i386
For example how those variables are used see the get_part_device_name_format function
in usr/share/rear/lib/layout-functions.sh that contains (excerpt):
case $OS_MASTER_VENDOR in
(SUSE)
...
;;
(Fedora)
...
;;
(Debian)
...
;;
(*)
....
;;
esac
|
@gozora |
|
@schlomo Merging is blocked The target branch requires all commits to be signed. but this is a pull request from an external contributor. I would like to know how we should handle pull requests from external contributors. I would prefer when external contributors are not forced to sign their pull requests. I would prefer when it is sufficient that pull requests from external contributors Is that possible or would that contradict our improved security in |
|
Hi Johannes,
I like your suggestions and will implement the cleaner one. I used the
process of muntzing to reduce the grub modules and would like you to
consider that the eliminated modules were not required and maybe should be
left out? Let me know your preference.
…On Tue, Jul 3, 2018 at 7:21 AM, Johannes Meixner ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In usr/share/rear/output/USB/Linux-i386/100_create_efiboot.sh
<#1849 (comment)>:
> @@ -88,7 +89,8 @@ EOF
# Create bootloader, this overwrite BOOTX64.efi copied in previous step ...
# Fail if BOOTX64.efi can't be created
- ${GRUB_MKIMAGE} -o ${EFI_DST}/BOOTX64.efi -p ${EFI_DIR} -O x86_64-efi linux part_gpt ext2 normal gfxterm gfxterm_background gfxterm_menu test all_video loadenv fat
+ # Slackware grub doesnt have gfxterm_background or gfxterm_menu...
+ ${GRUB_MKIMAGE} -o ${EFI_DST}/BOOTX64.efi -p ${EFI_DIR} -O x86_64-efi linux part_gpt ext2 normal gfxterm gfxmenu test all_video loadenv fat
I think here a test for Slackware and conditional code execution is needed
like you did it in your usr/share/rear/pack/Linux-i386/300_copy_kernel.sh
# add slackware test on top to prevent error because
# slackware grub doesnt have gfxterm_background or gfxterm_menu...
if [ -f /etc/slackware-version ] ; then
${GRUB_MKIMAGE} -o ${EFI_DST}/BOOTX64.efi -p ${EFI_DIR} -O x86_64-efi linux part_gpt ext2 normal gfxterm gfxmenu test all_video loadenv fat
else
${GRUB_MKIMAGE} -o ${EFI_DST}/BOOTX64.efi -p ${EFI_DIR} -O x86_64-efi linux part_gpt ext2 normal gfxterm gfxterm_background gfxterm_menu test all_video loadenv fat
fi
or perhaps even cleaner by using a menaingful variable for the GRUB
modules like
grub_modules="x86_64-efi linux part_gpt ext2 normal gfxterm gfxmenu test all_video loadenv fat"
# slackware grub doesnt have gfxterm_background or gfxterm_menu
test -f /etc/slackware-version || grub_modules="$grub_modules gfxterm_background gfxterm_menu"
# Create bootloader, this overwrite BOOTX64.efi copied in previous step ...
# Fail if BOOTX64.efi can't be created
${GRUB_MKIMAGE} -o ${EFI_DST}/BOOTX64.efi -p ${EFI_DIR} -O $grub_modules
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1849 (review)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/Am1cd7p2DIACS0WfwCnBc0P9FsDGprtlks5uC1OhgaJpZM4U_nDx>
.
|
|
In the 300_copy_kernel.sh I will be happy to use the OS_MASTER_VERSION in
my test. I thought I was following convention by using
"/etc/slackware-version" since the following elif tests all use file
references: /etc/redhat-release, /etc/debian_version,
/etc/arch-release,... Please let me know if you want this test to be the
only one using OS_MASTER_VERSION.
Thanks for considering my pull request.
…On Tue, Jul 3, 2018 at 7:32 AM, Johannes Meixner ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In usr/share/rear/pack/Linux-i386/300_copy_kernel.sh
<#1849 (comment)>:
> @@ -10,7 +10,13 @@
# Using another kernel is a TODO for now
if [ ! -s "$KERNEL_FILE" ]; then
- if [ -r "/boot/vmlinuz-$KERNEL_VERSION" ]; then
+ # add slackware test on top to prevent error on get_kernel_version
+ if [ -f /etc/slackware-version ]; then
The test for Slackware should be done via the usually used variables
OS_MASTER_VENDOR OS_VENDOR OS_VENDOR_VERSION and so on,
see the SetOSVendorAndVersion function in usr/share/rear/lib/config-
functions.sh
how those variables are set.
Perhaps those variables are already set right for Slackware by that
function
perhaps you need to add support to get them set right for Slackware to
that function.
To find out to what values those variables are set in your particular case
run rear dump for example in my case on SLES15 I get (excerpts):
# usr/sbin/rear -D dump
...
This is a 'Linux-x86_64' system, compatible with 'Linux-i386'.
System definition:
ARCH = Linux-i386
OS = GNU/Linux
OS_MASTER_VENDOR = SUSE
OS_MASTER_VERSION = 15
OS_MASTER_VENDOR_ARCH = SUSE/i386
OS_MASTER_VENDOR_VERSION = SUSE/15
OS_MASTER_VENDOR_VERSION_ARCH = SUSE/15/i386
OS_VENDOR = SUSE_LINUX
OS_VERSION = 15
OS_VENDOR_ARCH = SUSE_LINUX/i386
OS_VENDOR_VERSION = SUSE_LINUX/15
OS_VENDOR_VERSION_ARCH = SUSE_LINUX/15/i386
For example how those variables are used see the
get_part_device_name_format function
in usr/share/rear/lib/layout-functions.sh that contains (excerpt):
case $OS_MASTER_VENDOR in
(SUSE)
...
;;
(Fedora)
...
;;
(Debian)
...
;;
(*)
....
;;
esac
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1849 (review)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/Am1cd9FlwUziKaBCOhvGKFiPygOOLpeCks5uC1ZrgaJpZM4U_nDx>
.
|
|
Hello @wdmsde,
I'd prefer not to messing up with existing code and keep modules as they are, at lest for non-Slackware distributions. We did not had issues with non-booting ReaR recovery system because "this and that" for a quite a long time, and I really like it that way ;-). V. |
| @@ -88,7 +89,8 @@ EOF | |||
|
|
|||
| # Create bootloader, this overwrite BOOTX64.efi copied in previous step ... | |||
| # Fail if BOOTX64.efi can't be created | |||
| ${GRUB_MKIMAGE} -o ${EFI_DST}/BOOTX64.efi -p ${EFI_DIR} -O x86_64-efi linux part_gpt ext2 normal gfxterm gfxterm_background gfxterm_menu test all_video loadenv fat | |||
| # Slackware grub doesnt have gfxterm_background or gfxterm_menu... | |||
| ${GRUB_MKIMAGE} -o ${EFI_DST}/BOOTX64.efi -p ${EFI_DIR} -O x86_64-efi linux part_gpt ext2 normal gfxterm gfxmenu test all_video loadenv fat | |||
There was a problem hiding this comment.
Changing modules that are currently used and tested was my only concern. If this code can be re-written so it does not interfere with existing distributions, I have not problems left with this PR.
V.
|
@schlomo @gdha |
|
@wdmsde You are right. Unfortunately sometimes the ReaR code is such a mess |
|
@jsmeix
I upgraded my grub version to 2.02 on Slackware and was able to reduce my
patch size. I submitted another pull request that incorporates your
suggestions.
…On Wed, Jul 4, 2018 at 4:12 AM, Johannes Meixner ***@***.***> wrote:
@wdmsde <https://github.com/wdmsde>
regarding your
#1849 (comment)
<#1849 (comment)>
You are right.
For this pull request just follow the existing convention in the current
code.
Unfortunately sometimes the ReaR code is such a mess
that has grown over the time where things needs to be cleaned up.
We try to do as much as we can as time permits but as long as things work
there is no pressure to clean up code that works - "never change ..." ;-)
Right now I did #1851 <#1851>
to clean up our various 300_copy_kernel.sh scripts as time permits in the
future.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1849 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/Am1cd_hEKQgomurWeMxLNMaGeiJ19r92ks5uDHkLgaJpZM4U_nDx>
.
|
|
Superseded by #1853 |
Pull Request Details:
Type: Enhancement
Impact: Normal
Reference to related issue (URL):
http://lists.relax-and-recover.org/pipermail/rear-users/2018-July/003572.html
How was this pull request tested?
Tested on laptop running Slackware 14.2 with UEFI. Used "rear -v mkbackup" to USB drive. The USB drive booted OK but I did not test recover function yet. The backup tar was available on the drive.
Brief description of the changes in this pull request:
Added code to support Slackware 14.2 distribution. This was a brute force approach and my mktemp changes may break compatibility with other OSs. Requesting feedback to improve quality.