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

Fix: Added logic to mount Linux boot disk with LVM and to mount NTFS disks automatically #40

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

vinay-vgs
Copy link
Collaborator

..

@soulless-viewer
Copy link
Collaborator

Hello @vinay-vgs,

Could you pls adjust you PR to adhere the standards described at CONTRIBUTING?

@vinay-vgs vinay-vgs changed the title Added logic to mount Linux boot disk with LVM and to mount NTFS disks automatically Fix: Added logic to mount Linux boot disk with LVM and to mount NTFS disks automatically Mar 20, 2024
Copy link
Collaborator

@halleysouza halleysouza left a comment

Choose a reason for hiding this comment

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

Hey @vinay-vgs thanks for your PR. I am adding some notes, let me know wdyt.

fs=$(lsblk -rf /dev/disk/by-id/google-${disk} | egrep -i 'ext[3-4]|xfs|ntfs'| awk '{print $2}')
case $fs in
ntfs)
apt-get install -y ntfs-3g
Copy link
Collaborator

Choose a reason for hiding this comment

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

I dont think we can assume the VM will have access to the internet. Better to have validation of internet access or we'll risk to delay the boot for too long

disk_map=$(lsblk -rf /dev/disk/by-id/google-${disk}| egrep -i LVM2_member| awk '{print $1}')
if [[ -n ${disk_map} ]];
then
apt-get install -y lvm2
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here

if [[ -n ${disk_map} ]];
then
apt-get install -y lvm2
pvscan; vgscan
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can replace the lines 26, 28 and 29 with only "pvscan -ay" and save some bytes

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.

None yet

3 participants