poplar_recovery_builder: Fix cleanup #3
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The rootfs tarball was extracted using sudo. Address a permissions error by using sudo rm. Signed-off-by: Andreas Färber <afaerber@suse.de>
|
Did you find that the top-level mount directory ("binary") was owned by root? I definitely appreciate the patch, but I'm not sure it's needed. Can you |
|
Yes. I observed a real error, something like "rm: No permission to remove binary", when using an openSUSE tarball such as http://download.opensuse.org/ports/aarch64/tumbleweed/images/openSUSE-Tumbleweed-ARM-JeOS.aarch64-rootfs.aarch64-Current.tbz. a) This failed because it expects .tar.gz ( |
|
BTW now that you fixed the documentation to mention USB_BOOT (there was some confusing non-linear |
|
@ldts Another question: Does the second partition strictly need to be type |
|
Similarly, the first partition does not seem to be formatted (l-loader.bin?), so type |
|
On 05/26/2017 02:26 AM, Andreas Färber wrote:
Yes. I observed a real error, something like "rm: No permission to remove binary", when using an openSUSE tarball such as http://download.opensuse.org/ports/aarch64/tumbleweed/images/openSUSE-Tumbleweed-ARM-JeOS.aarch64-rootfs.aarch64-Current.tbz. a) This failed because it expects .tar.gz (|tar -xzf|) instead of our .tbz (|tar -xjf|). The script only checks for a non-zero argument, not for a compatible file type. b) It failed because our tarball directly has |bin/| etc. directories, no intermediate |binary/| subdir. Besides |mount| there are multiple places using |mkdir -p| with |${MOUNT}|. From whatever call site exactly there was a |binary| directory that on openSUSE my user did not have permissions for, and all the extracted files outside |binary| stayed behind. After adding |-C binary| to the |tar| line everything worked, but not before. Both issues would be nice to address please.
You're using the script in way different from what was intended!
But that's good, I've now fixed things so they should work for both
a Linaro or an OpenSUSE root file system. I think it's pretty cool
that it was relatively easy to do that.
I still don't understand why removing the mount point required
root permission. It shouldn't. But I accepted your suggested
change anyway--you're probably right and I'm just missing something.
Thank you very much for your suggestions.
-Alex
|
|
On 05/26/2017 02:44 AM, Andreas Färber wrote:
BTW now that you fixed the documentation to mention USB_BOOT (there was some confusing non-linear |git push -f| apparently...? My pull originally had two commits from Robert prepended, and GitHub defaults to |latest| rather than |master| with no |contributing.md| to explain which one to use), it would be nice to make a binary copy of |mbr.gz| available for download, so that one does not strictly need to run this script. As I found out, it is necessary to re-flash the MBR after flashing |fastboot.bin|. Also I noticed that the recovery image is only 2 GB instead of 8 GB - how do you expect users to expand the third rootfs partition to maximum size?
I don't know whether the two commits from Robert got pulled or
not. I just merged your pull request. (Can you verify for me?)
I have noted that we should get a "CONTRIBUTING.md" file in all our
repositories but I am not going to add them right now. For now:
- The basic intent is for "latest" to be the best branch to use.
In theory that might mean "master" is less stable but in practice
that should never happen.
- The kernel has the most mature branching system, with branches
based on upstream with Poplar-specific commits appended. Other
trees should get more like that at some point, it's just not a
priority right now.
- Right now I'm planning on doing occasional forced updates to branches.
I know this isn't pleasant, and we may change this practice at some
point, but for now I am just not guaranteeing "linear" updates.
- If you or anyone else wants to propose a "CONTRIBUTING.md" file
(or any other changes) I will gladly accept pull requests.
If you flash the front of the disk with "fastboot.bin", yes, you
will have to rewrite the MBR. But I don't intend for people to
do that. In setting up the recovery installer and laying out the
eMMC partitioning, I tried to distinguish between "l-loader.bin"
(which is named "fastboot.bin" on the USB stick) and "loader.bin"
(which is a copy of what goes in the first partition in eMMC).
The second simply omits the first sector from the first; they're
otherwise identical. We have to implement our boot loader the
way we do because of some details of how the boot ROM works. I
was hoping we could just hide those details and talk about the
MBR being separate from the "loader". And have people only
update the "loader partition" without overwriting the MBR.
This script, and the partitioning scheme it creates, is still
subject to change. We're soon going to be setting up Android,
for example, and the partitioning we want for that is very likely
to differ from what's there now. A standalone MBR *is* created
by the script, and it is present on the USB stick. But because
the layout of the eMMC media isn't finalized I don't really want
to supply an MBR when things might change again. (No need to
track versions of these MBRs that way, for example.)
Finally, the script sets the size of the output image at 2GB
simply because that was enough to hold what we now have
filling the disk. It'll need to be larger as we add more
that we want to put on eMMC media (though not necessarily
the full 8GB, since we compress everything). The larger
the image, the longer it takes to copy and install. (The
waiting got very tiresome while developing and testing
this).
-Alex
… |poppy:~ # parted /dev/mmcblk0 GNU Parted 3.2 Using /dev/mmcblk0 Welcome to GNU Parted! Type 'help' to view a list of commands. (parted) p Model: MMC 8WPD3R (sd/mmc) Disk /dev/mmcblk0: 7818MB Sector size (logical/physical): 512B/512B Partition Table: msdos Disk Flags: Number Start End Size Type File system Flags 1 512B 4194kB 4194kB primary type=83 2 4194kB 138MB 134MB primary fat32 boot, lba, type=0c 3 138MB 2147MB 2009MB primary ext4 type=83 |
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <#3 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA-5Y4PkPv5cNBEe0bFw09RtwSUsiLdDks5r9oLSgaJpZM4NnCoK>.
|
|
On 05/26/2017 02:50 AM, Andreas Färber wrote:
@ldts <https://github.com/ldts> Another question: Does the second
partition strictly need to be type |0x0c|, or could I change it to
EFI |0xef| just as well when using as |/boot/efi|? Who exactly
accesses the |loader.bin| file on it?
Other than the Linux FS partitions, the types are probably wrong.
For EFI, that was actually one of our intentions with that boot
partition; you're foreshadowing that.
The boot ROM is what accesses the loader partition. It has to
exist at that location on the boot media, and it's the offset
512 bytes), not the partition location that matters.
A copy of the loader file is placed in the boot partition just
for reference. It isn't necessary. But if one wanted to fix
a corrupted loader partition (conceivably) then that file could
be used to overwrite what was in partition 1. It's basically
reinforcing the notion I mentioned in my previous message, that
the loader is something that's based on "l-loader" but which
avoids touching the first sector on the disk.
-Alex
… — You are receiving this because you commented. Reply to this email
directly, view it on GitHub
<#3 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA-5YylTez4HEwedfyMnbYUxK9vf7f-3ks5r9oRWgaJpZM4NnCoK>.
|
|
On 05/26/2017 02:54 AM, Andreas Färber wrote:
Similarly, the first partition does not seem to be formatted
(l-loader.bin?), so type |0x83| does not seem correct.
The "parted" command I used for partitioning doesn't easily
support setting the range of partition types that "fdisk"
does. Partition 1 should really just be "raw data" but
I really didn't have a good value to use for that (and as
I mentioned, the boot ROM doesn't care). It might be good
to use better-defined values for the benefit of other tools
I suppose.
-Alex
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The rootfs tarball was extracted using sudo.
Address a permissions error by using sudo rm.
Signed-off-by: Andreas Färber afaerber@suse.de