Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ESP flagging, name partitions in 300_format_usb_disk.sh #1292

Closed
wants to merge 1 commit into from

Conversation

ProBackup-nl
Copy link
Contributor

(1) Change EFI partition from primary to ESP,
(2) name partitions,
(3) Set boot_flag to "esp" for EFI systemd-boot
(4) Make log message more explicit

(1) Change EFI partition from primary to ESP,
(2) name partitions,
(3) Set boot_flag to "esp" for EFI systemd-boot
(4) Make log message more explicit
@ProBackup-nl ProBackup-nl changed the title Improvements to 300_format_usb_disk.sh for EFI ESP partition flagging, name partitions in 300_format_usb_disk.sh Apr 14, 2017
@ProBackup-nl ProBackup-nl changed the title ESP partition flagging, name partitions in 300_format_usb_disk.sh ESP flagging, name partitions in 300_format_usb_disk.sh Apr 14, 2017
Copy link
Member

@gdha gdha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ProBackup-nl Need to verify this before I can approve it

@ProBackup-nl
Copy link
Contributor Author

I find it quite handy when parted /dev/sdb print also prints a name describing the purpose of the partition, like:

Number  Start   End     Size    File system  Name      Flags
 1      4194kB  214MB   210MB   fat16        EFIBOOT   boot, esp
 2      214MB   15.6GB  15.4GB  ext4         REARDATA

@gdha gdha self-assigned this Apr 19, 2017
@gdha gdha requested review from jsmeix and gozora April 19, 2017 11:51
@gdha gdha added the enhancement Adaptions and new features label Apr 19, 2017
@gdha gdha added this to the ReaR v2.1 milestone Apr 19, 2017
@jsmeix
Copy link
Member

jsmeix commented Apr 19, 2017

Regarding "modern" flags like 'esp' or 'legacy_boot':

Older 'parted' on older Linux distributions do not support them, cf.
#1308 (comment)

The current code errors out when parted fails to set a flag.
I don't know how the various parted versions behave when
they should set a flag that is not supported.
I guess parted returns then a non-zero exit code so that
"rear format" errors out.

Accordingly I think there could be code that retries
those parted calls with a traditional flag as fallback
something (untested proposal) like

 if ! parted -s $RAW_USB_DEVICE set 1 $boot_flag on >&2 ; then
    # try a traditional flag as fallback
    if ! parted -s $RAW_USB_DEVICE set 1 boot on >&2 ; then
        Error "Could not make first partition bootable on '$RAW_USB_DEVICE'"
    fi
fi

Copy link
Member

@jsmeix jsmeix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it is currently not sufficiently
fail-safe when an older parted is used
that does not support those settings.

Copy link
Member

@gdha gdha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Older versions require a primary partition; I'm afraid ESP is a rather new addition.

@ProBackup-nl
Copy link
Contributor Author

@gdha ESP is in parted since release 3.2 (2014-07-28).

@@ -77,14 +80,19 @@ case "$USB_DEVICE_PARTED_LABEL" in
boot_flag="boot"
;;
"gpt")
boot_flag="legacy_boot"
# For modern UEFI/GPT booting set ESP boot_flag
if [[ -d /sys/firmware/efi/efivars && -f /boot/EFI/systemd/systemd-bootx64.efi || -f /usr/lib/systemd/boot/efi/systemd-bootx64.efi ]]; then
Copy link
Member

@gdha gdha May 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ProBackup-nl this directory /boot/EFI/systemd/ is not a standard yet (this path for most common Linux versions)

@gdha
Copy link
Member

gdha commented May 20, 2017

@ProBackup-nl RHEL 6.3 still has parted 2.1, and lots of RHEL 7.2 still have version 3.1. I don't mind to have it on-board, but then at least it may not break parted < version 3.2.
Will you re-work the pull request, or would like to drop it?

@gdha gdha removed this from the ReaR v2.1 milestone Jun 9, 2017
@ProBackup-nl
Copy link
Contributor Author

@gdha Please drop this request. After I have seen how easily new non-optional dependencies (like bc) are introduced in this project, I have lost my appetite.

@schlomo
Copy link
Member

schlomo commented Jun 13, 2017

@ProBackup-nl I am sorry for your frustration. I think that this topic is something that should be part of a future ReaR major release that does away with all the pre-systemd compatibility requirements.

@ProBackup-nl
Copy link
Contributor Author

@schlomo I think that it is not wise to introduce a new (hard/non-optional) dependency for a calculation problem that exists in one old version of a distribution.
In my opinion the code should check whether a (calculation) issue exists, and only in that case the code should require (bc) dependency to handle the exception (incorrect large integer calculations by bash).

@schlomo
Copy link
Member

schlomo commented Jun 14, 2017

@ProBackup-nl good point and I agree. I actually take it to support my thesis that we should cut our obligations to the past and start working on a new ReaR version that is geared towards recent systems (recent probably being somewhat relative). I have a strong feeling that this will simplify a lot of things within ReaR.

@ProBackup-nl
Copy link
Contributor Author

@schlomo By the way, I am not frustrated, more disappointed. Please do post me a note as soon as the bc dependency is optional. I can even see a need to support relatively older systems.

@schlomo
Copy link
Member

schlomo commented Jun 15, 2017

I'll try to remember. BTW, I recently read the Archlinux FAQ and liked this part:

Several distributions, such as Debian, have different versions of shared libraries packaged as different packages: libfoo1, libfoo2, libfoo3 and so on. In this way it is possible to have applications compiled against different versions of libfoo installed on the same system.

In case of a distribution like Arch, only the latest stable versions of packages are officially supported. By dropping support for outdated software, package maintainers are able to spend more time ensuring that the newest versions work as expected. As soon as a new version of a shared library becomes available from upstream, it is added to the repositories and affected packages are rebuilt to use the new version.

It kind of makes me wish we could split ReaR into two parts: One that supports only recent distros and another one for the old stuff. I know that many if not most of our users rely on ReaR supporting their old stuff, this is just me as a developer speaking.

@ProBackup-nl
Copy link
Contributor Author

@schlomo The newest versions of packages is one of the main reasons to use Arch Linux in the first place. Once too many I stumbled upon ancient packages in the major distributions, where I needed a newer version to get things done. And there wasn't a quick solution to install that newer package version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants