-
Notifications
You must be signed in to change notification settings - Fork 16
Implement and easy mode for running VM images #82
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
base: main
Are you sure you want to change the base?
Conversation
Add "disk_image" as an optional positional argument. If present, this is attached to the VM as a virtio-blk device. Signed-off-by: Sergio Lopez <slp@redhat.com>
Set up some safe default values for both cpu and memory so they become optional arguments. Signed-off-by: Sergio Lopez <slp@redhat.com>
| pub struct Args { | ||
| /// Number of vCPUs for the VM. | ||
| #[arg(long, short)] | ||
| #[arg(long, short, default_value_t = 2)] |
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.
Is max CPUs worth considering?
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.
In what sense? As in defaulting to max CPUs?
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.
Yeah... I mean for most generic usage all cores is what would be desired I think? Using less cores than available seems like the exception rather than the norm...
|
Love this btw, I don't recall there ever being a VMM that had single arg execution with reasonable defaults, this is refreshing! |
| pub struct Args { | ||
| /// Number of vCPUs for the VM. | ||
| #[arg(long, short)] | ||
| #[arg(long, short, default_value_t = 2)] |
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.
In what sense? As in defaulting to max CPUs?
|
|
||
| /// Amount of RAM available to VM. | ||
| #[arg(long, short)] | ||
| #[arg(long, short, default_value_t = 4096)] |
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.
The entry-level M1 Mac Minis only have 8GB of memory. Do we think 50% of that is going to be ok?
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.
FWIW I wouldn't have it any lower, 4GB is the max it will use right? I mean Chrome on the host is capable of using 8GB but we don't limit it. Plus the M1 Mac Mini is notoriously underspec'd in terms of RAM and like 4 generations old now? M5 is out soon. People can always manually set 2GB, etc. in exceptional cases which in 2025, this probably is an exceptional case.
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.
Also, a VM configured with 4GB doesn't necessarily need to be using all its RAM, virtio-balloon is there to return free memory. In addition to this, macOS is very good (TBH, way more than Linux) at managing its swap memory (having an MacBook M1 Air with 8GB myself, I can attest this is the case), so I'd say this is pretty safe.
| file.read_exact(&mut buffer)?; | ||
|
|
||
| if buffer == QCOW_MAGIC { | ||
| println!("Image detected as Qcow2"); |
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.
More of a general UX question: How do we decide what should be logged and what should be printed to stdout? In other words, why do we want to print this to stdout instead of using info!(), for example?
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.
We don't use println! for logging. This should likely be a debug! rather than println!.
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.
We should never print, as it messes up the console. The only reason those are there it's because this is just an early draft.
|
krunkit run might be worth considering, this docker/ollama style seems to be popular |
|
That'd be a fair amount more work, requiring a management interface for VMs launched with krunkit. What's the use case? |
|
It's just a consideration I guess, FWIW if it was decided the UX should be like this one could start with only implementing "run" and implement the rest later as/if desired. |
|
I suppose I'm not against it, but we'd need a clear use-case to justify it. |
This enables users to easily use VM images by passing them directly as a single argument, like this:
krunkit Fedora-Server-Host-Generic-43-1.6.aarch64.rawFixes #81