Skip to content

Handle disk sector size #621

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.

Sign up for GitHub

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

Jump to bottom
Open
vjr wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Handle disk sector size #621

vjr wants to merge 7 commits into from

Conversation

@vjr
Copy link
Member

@vjr vjr commented Oct 8, 2021 •
edited
Loading

Fixes #620

Requires pop-os/distinst#266

@vjr vjr mentioned this pull request Oct 8, 2021
Open
@vjr vjr requested a review from davidmhewitt October 8, 2021 17:32
@vjr vjr mentioned this pull request Oct 9, 2021
Merged
@vjr
Copy link
Member Author

vjr commented Oct 23, 2021

Fixes #448

@vjr vjr marked this pull request as ready for review October 23, 2021 00:59
@vjr vjr requested a review from a team October 23, 2021 01:00
@vjr vjr linked an issue Oct 23, 2021 that may be closed by this pull request
Installer support for non-512-byte sector sizes #448
Open
@vjr
Copy link
Member Author

vjr commented Oct 23, 2021

Note to self:

Here is the command I used to launch the elementary OS live ISO and test the installer, after creating a raw format qemu disk image with qemu-img create -f raw odin.raw 20G :

qemu-system-x86_64 -machine q35,accel=kvm -m 16384 -smp 8 -cpu host -boot d -cdrom /home/vishal/Downloads/elementaryos-6.0-stable.20211005.iso -drive file=/home/vishal/Documents/QEMU/odin.raw,format=raw,id=odin,if=none -device virtio-scsi,id=scsi -device scsi-hd,bus=scsi.0,drive=odin,logical_block_size=4096,physical_block_size=4096 -bios /usr/share/qemu/OVMF.fd

I just installed the gnome-boxes deb package from the ubuntu archives which got me the required QEMU, KVM and OVMF bits.

After running the installer IIRC I just changed the -boot d option to -boot c to reboot into the installed system to see if it really worked.

davidmhewitt
davidmhewitt previously requested changes Nov 26, 2021
View reviewed changes
Copy link
Member

@davidmhewitt davidmhewitt left a comment

Choose a reason for hiding this comment

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

I've imported a version of distinst with the associated fixes into the elementary PPAs, and the code looks pretty good here, just one small thing to change and then I'll test this out.

I don't like passing this sector_size through all of these layers of construction since it's just a property of the disk, not of every partition and mount, but that isn't a problem with your code, it's jut the original architecture of this, so we can sort that later.

@vjr vjr requested a review from davidmhewitt November 28, 2021 11:32
@vjr
Copy link
Member Author

vjr commented Dec 2, 2021

@davidmhewitt @elementary/desktop-developers I've created PR pop-os/distinst#278 and along with this PR it appears to succeed installation for the 3 test cases I tried in QEMU:

  1. Custom install - disk partitioned with a 500 MB EFI and the rest formated ext4 mounted on / (root)
  2. Erase and install without encryption
  3. Erase and install with encryption

Note that I say "appears to succeed" because the installer succeeds and partitions appear to be created fine, I've not rebooted into the installed image in QEMU to verify it works post install - would appreciate if anyone else would be able to test and comment.

In the mean time I'll try to more fully test in the coming days and report my findings here.

This was referenced Dec 2, 2021
Open
Closed
@danirabbit danirabbit mentioned this pull request Dec 9, 2021
Merged
1 task
@tamara-schmitz tamara-schmitz mentioned this pull request Nov 10, 2025
Open
tamara-schmitz added a commit to tamara-schmitz/elementaryos-installer that referenced this pull request Nov 10, 2025
@tamara-schmitz
check ESP size in bytes not sectors
5f18f74 
Partially a necessary fix for elementary#448 and elementary#644.
Also a continuation of PR elementary#621 which mostly does not apply anymore
four years later except for the ESP check.
@tamara-schmitz tamara-schmitz mentioned this pull request Nov 10, 2025
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@davidmhewitt davidmhewitt Awaiting requested review from davidmhewitt

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Installer shows incorrect partition sizes, cannot install due to perceived ESP partition size Installer support for non-512-byte sector sizes

2 participants

@vjr @davidmhewitt