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
nixos/modules/virtualization: Options to add an extra disk in virtualbox VM #60246
Conversation
@ambrop72 can you take a look at this? |
}; | ||
type = types.nullOr (types.submodule { | ||
options = { | ||
size = mkOption { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like if the option name expresses the unit when it's not obvious, so personally I'd name this option size_MiB
. Otherwise when one sees configuration setting this it is necessary to consult the docs to know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That battle is already lost unfortunately - there's (baseImage,memory)Size
(both in MiB) too, and I guess, we should stay consistent with these options.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That battle is already lost unfortunately - there's
(baseImage,memory)Size
(both in MiB) too, and I guess, we should stay consistent with these options.
Right then.
eval $(partx $dataDiskImage -o START,SECTORS --nr 1 --pairs) | ||
mkfs.ext4 -F -L ${cfg.extraDisk.label} $dataDiskImage -E offset=$(sectorsToBytes $START) $(sectorsToKilobytes $SECTORS)K | ||
echo "creating extra disk: data-disk.vmdk" | ||
${pkgs.qemu}/bin/qemu-img convert -f raw -O vmdk $dataDiskImage data-disk.vmdk |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be using createrawvmdk
instead like for the main disk?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ambrop72 I have done this change, and rebased the commit..
Thank you for your contributions.
|
1dac88d
to
41e745b
Compare
I wonder if we should do |
autoResize = true; | ||
fsType = "ext4"; | ||
}; | ||
fileSystems = { "/" = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull the "/" = ...
to the next line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
autoResize = true; | ||
fsType = "ext4"; | ||
}; | ||
} // (if cfg.extraDisk == null then { } else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use lib.optionalAttrs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
41e745b
to
a78a3fa
Compare
a78a3fa
to
d127d85
Compare
Motivation for this change
The current VirtualBox VM creation has a limitation/bug due to which we cannot have more than 64gb storage. This is a workaround for that, and can also be useful in other cases.
Ref: #59867
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)