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

Handle disk sector size #621

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Handle disk sector size #621

wants to merge 7 commits into from

Conversation

vjr
Copy link
Member

@vjr vjr commented Oct 8, 2021

Fixes #620

Requires pop-os/distinst#266

@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
@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.

@danirabbit danirabbit added this to In progress in OS 6.1 via automation Nov 2, 2021
@danirabbit danirabbit removed this from In progress in OS 6.1 Nov 22, 2021
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.

src/Widgets/PartitionBar.vala Outdated Show resolved Hide resolved
@vjr vjr added this to In progress in OS 6.1 Dec 1, 2021
@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.

@danirabbit danirabbit moved this from In progress to Needs Review in OS 6.1 Dec 5, 2021
@danirabbit danirabbit mentioned this pull request Dec 9, 2021
1 task
@danirabbit danirabbit removed this from Needs Review in OS 6.1 Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants