Skip to content

[ENHANCEMENT] Improve auto-detection of the disk bus type of the VMware importer #72

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

votdev
Copy link
Member

@votdev votdev commented Apr 3, 2025

The IDE disk bus is not supported anymore in the kubevirt version we are using: kubevirt.io/api-reference/v1.1.0/definitions.html#_v1_disktarget

This PR is mapping it to SATA because VirtIO requires the kernel drivers to be installed in the VM which can not be guaranteed.

This is the mapping of the VMware source client:

Bus Example Device ID
SCSI /vm-13010/ParaVirtualSCSIController0:0
SCSI /vm-13011/VirtualBusLogicController0:0
SCSI /vm-13012/VirtualLsiLogicController0:0
SCSI /vm-13013/VirtualLsiLogicSASController0:0
SATA /vm-13767/VirtualAHCIController0:1
IDE /vm-5678/VirtualIDEController1:0
NVMe /vm-2468/VirtualNVMEController0:0
USB /vm-54321/VirtualUSBController0:0

The new DefaultDiskBusType field in VirtualMachineImportSpec allows the user to customize the disk bus type in case the auto-detection fails. Note, the OpenStack source client does not support auto-detection, thus it will always make use of the DefaultDiskBusType field if specified. The DefaultDiskBusType defaults to virtio.

Related Issue:
harvester/harvester#7987

Test plan:
Use added unit tests.

@votdev votdev self-assigned this Apr 3, 2025
@votdev
Copy link
Member Author

votdev commented Apr 3, 2025

The new default value is VirtIO which is different to the latest change done to the adapterType function. See e14eb3e#diff-6bd8e83a30e2ac257f153c634d475a3ee725a17f41bb72c74b0b4493fd292f3cL448. Shall we keep SATA as the default?

@votdev votdev force-pushed the issue_7987_bustype branch from 230e536 to d7d305b Compare April 3, 2025 10:28
@votdev votdev marked this pull request as ready for review April 3, 2025 10:33
@Copilot Copilot AI review requested due to automatic review settings April 3, 2025 10:33
Copilot

This comment was marked as outdated.

@votdev votdev changed the title Bus type auto-detection of the VMware importer does not work correct Improve auto-detection of the bus type of the VMware importer Apr 3, 2025
@votdev votdev force-pushed the issue_7987_bustype branch 2 times, most recently from 48a953b to ddc1579 Compare April 3, 2025 10:51
@votdev votdev force-pushed the issue_7987_bustype branch 2 times, most recently from 2cf5f65 to 24c96d4 Compare April 15, 2025 10:00
@votdev votdev force-pushed the issue_7987_bustype branch from 24c96d4 to 8fc9114 Compare April 29, 2025 11:26
w13915984028
w13915984028 previously approved these changes Apr 29, 2025
Copy link
Member

@w13915984028 w13915984028 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@votdev votdev changed the title Improve auto-detection of the bus type of the VMware importer Improve auto-detection of the disk bus type of the VMware importer Apr 30, 2025
@votdev votdev requested a review from w13915984028 May 5, 2025 06:52
@votdev
Copy link
Member Author

votdev commented May 5, 2025

Thought about adding a ForceDefaultDiskBusType field to force the use of the specified DefaultDiskBusType bus type, but i rejected the idea because you can also change the bus type afterwards. This would only affect the VMware source client.

@votdev votdev force-pushed the issue_7987_bustype branch 2 times, most recently from 95770f8 to c65ca39 Compare May 5, 2025 09:21
w13915984028
w13915984028 previously approved these changes May 5, 2025
Copy link
Member

@w13915984028 w13915984028 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

Copy link

mergify bot commented May 6, 2025

This pull request is now in conflict. Could you fix it @votdev? 🙏

votdev added 3 commits May 6, 2025 10:28
Signed-off-by: Volker Theile <[email protected]>
This allows the user to customize the disk bus type in case the auto-detection fails. Note, the OpenStack source client does not support auto-detection, thus it will always make use of the `DefaultDiskBusType` field if specified. The `DefaultDiskBusType` defaults to `virtio`.

Signed-off-by: Volker Theile <[email protected]>
@votdev votdev changed the title Improve auto-detection of the disk bus type of the VMware importer [ENHANCEMENT] Improve auto-detection of the disk bus type of the VMware importer May 6, 2025
Copy link
Member

@w13915984028 w13915984028 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

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