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

Improve install device selection in installer #182

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

yfyf
Copy link
Collaborator

@yfyf yfyf commented Sep 24, 2024

  • Look for "/iso" mount not only in children of blockdevices, but top-level devices too (e.g. this happens when running qemu with -cdrom playos-installer-<..>.iso)
  • Eliminate devices that are too small or read-only from potential installation targets.

cc @knuton

Checklist

  • Changelog updated
  • Code documented
  • User manual updated

@yfyf
Copy link
Collaborator Author

yfyf commented Sep 24, 2024

P.S. I was "inspired" to do this PR because I was running qemu with -cdrom playos-installer-XX.iso and it would fail to identify the mounted ISO as the boot medium (and hence would not filter it out from potential installation targets).

@knuton knuton self-requested a review September 24, 2024 15:20
Copy link
Member

@knuton knuton left a comment

Choose a reason for hiding this comment

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

Looks good! A few minor suggestions.

installer/install-playos/install-playos.py Outdated Show resolved Hide resolved
controller/Changelog.md Outdated Show resolved Hide resolved
installer/install-playos/install-playos.py Outdated Show resolved Hide resolved
installer/install-playos/install-playos.py Outdated Show resolved Hide resolved
installer/install-playos/install-playos.py Show resolved Hide resolved
@knuton knuton added the changes suggested Asking for changes before next round of reviewing label Sep 24, 2024
- Look for "/iso" mount not only in children of blockdevices, but
  top-level devices too (e.g. this happens when running qemu with
  `-cdrom playos-installer-<..>.iso`)
- Eliminate devices that are too small or read-only from potential
  installation targets.
@yfyf yfyf force-pushed the improve-install-dev-selection branch from f2b1d14 to fc86dec Compare September 26, 2024 07:53
@yfyf
Copy link
Collaborator Author

yfyf commented Sep 26, 2024

Resolved all the comments, tested manually by adding additional disks and checking that it picks the right one based on size:

Found 2 disk devices:
        /dev/vdb (Virtio Block Device - 1 GB)
        /dev/vda (Virtio Block Device - 28 GB)
Could not identify installer medium. Considering all disks as installation targets.
Minimum required size for installation target: 26 GB
Found 1 possible installation targets:
        /dev/vda (Virtio Block Device - 28 GB)

(Note: Installer medium is not identified because I am running via nix runInLinuxVM.)

@knuton
Copy link
Member

knuton commented Sep 26, 2024

tested manually by adding additional disks and checking that it picks the right one based on size

Did the following additional sanity checks on physical machine:

  • The 32 GB installation medium is ignored
  • Additional 8 GB device is ignored
  • 120 GB disk is selected

@knuton knuton removed the changes suggested Asking for changes before next round of reviewing label Sep 26, 2024
Copy link
Member

@knuton knuton left a comment

Choose a reason for hiding this comment

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

Thanks 👌

@knuton knuton merged commit 38e9fa9 into dividat:main Sep 26, 2024
5 checks passed
@yfyf yfyf deleted the improve-install-dev-selection branch September 26, 2024 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants