Skip to content

NAS-134717 / 25.10 / Allow specifying image OS for virt VMs #16228

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

Merged
merged 1 commit into from
Apr 22, 2025

Conversation

Qubad786
Copy link
Contributor

@Qubad786 Qubad786 commented Apr 9, 2025

Context

RTC for incus VMs are by default set to UTC, which is fine for linux base VMs but it is problematic for Windows VMs because Windows requires RTC to be set to localtime in order to work correctly. To resolve this, it is required that image.os property is set to Windows, by doing this, incus will set rtc to localtime internally.

This PR introduces a image_os field which a user can specify at the time of creating an instance or updating it. We specify a few popular OS choices but let the user specify anything else he might want as well.

@Qubad786 Qubad786 requested a review from a team April 9, 2025 20:44
@bugclerk bugclerk changed the title Allow specifying image OS for virt VMs NAS-134717 / 25.10 / Allow specifying image OS for virt VMs Apr 9, 2025
@bugclerk
Copy link
Contributor

bugclerk commented Apr 9, 2025

@@ -274,6 +277,9 @@ def __data_to_config(self, instance_name: str, data: dict, raw: dict = None, ins
config['user.autostart'] = str(data['autostart']).lower()

if instance_type == 'VM':
if data.get('image_os'):
config['image.os'] = data['image_os'].capitalize()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this clearly defined somewhere in incus specs/docs that we can use it this way? Should we be using user.ix_image_os instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is documented here https://github.com/lxc/incus/pull/1767/files

We are investigating upgrading incus version as well and that should hopefully pull this fix in too when that happens

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Specifically this part

When creating a Windows virtual machine, make sure to set the `image.os` property to something starting with `Windows`.
Doing so will tell Incus to expect Windows to be running inside of the virtual machine and to tweak behavior accordingly.

@yocalebo
Copy link
Contributor

@Qubad786 friendly ping

@Qubad786 Qubad786 requested review from anodos325 and removed request for anodos325 April 15, 2025 12:46
@Qubad786 Qubad786 merged commit 43a97e8 into master Apr 22, 2025
2 checks passed
@Qubad786 Qubad786 deleted the NAS-134717 branch April 22, 2025 11:49
@bugclerk
Copy link
Contributor

This PR has been merged and conversations have been locked.
If you would like to discuss more about this issue please use our forums or raise a Jira ticket.

@truenas truenas locked as resolved and limited conversation to collaborators Apr 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants