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

Support USB Passthrough #1069

Merged
merged 20 commits into from
Sep 24, 2024
Merged

Support USB Passthrough #1069

merged 20 commits into from
Sep 24, 2024

Conversation

torchiaf
Copy link
Collaborator

@torchiaf torchiaf commented Jul 16, 2024

Summary

PR Checklist

  • Is this a multi-tenancy feature/bug?
    • Yes, the relevant RBAC changes are at:
  • Do we need to backport changes to the old Rancher UI, such as RKE1?
    • Yes, the relevant PR is at:
  • Are backend engineers aware of UI changes?
    • Yes, the backend owner is:

Related Issue harvester/harvester#5763

Occurred changes and/or fixed issues

Technical notes summary

Areas or cases that should be tested

  • Disable/Enable PCI addon -> USB page visibility
  • Enable/Disable USB device
  • Assign PCI/USB devices to VMs

Areas which could experience regressions

PCI devices page

Screenshot/Video

USB devices

image

Groups

image

image

VM edit mode

image

@torchiaf torchiaf changed the title 5763 usb Support USB Passthrough Jul 16, 2024
@a110605 a110605 added Draft PR The PR still no ready New Feature New feature for up coming release labels Aug 26, 2024
@torchiaf torchiaf force-pushed the 5763-usb branch 2 times, most recently from 9141d87 to d7ed6b0 Compare September 8, 2024 16:13
@torchiaf torchiaf marked this pull request as ready for review September 8, 2024 16:13
@torchiaf torchiaf removed the Draft PR The PR still no ready label Sep 9, 2024
@a110605 a110605 requested a review from Yu-Jack September 9, 2024 09:12
@Yu-Jack
Copy link
Collaborator

Yu-Jack commented Sep 9, 2024

Few questions, not sure whether it's Harvester version issue. In my local Harvester, I just enabled pcidevices-controller Add-on. Did I miss something?

I opened the USB Device Page with embedded Rancher and enabled one of them. Other USB devices show weird status.

image

I tried to create a virtual machine. After I clicked the Create button in virtual machine image page. It showed this error message. It seems some variables are undefined in some cases.

image

@torchiaf
Copy link
Collaborator Author

torchiaf commented Sep 9, 2024

I opened the USB Device Page with embedded Rancher and enabled one of them. Other USB devices show weird status.

I'm trying to replicate it.

I tried to create a virtual machine. After I clicked the Create button in virtual machine image page. It showed this error message. It seems some variables are undefined in some cases.

This is a UI bug, b84a4f5 should fix it.

@torchiaf torchiaf force-pushed the 5763-usb branch 2 times, most recently from 7c0de51 to 02fe957 Compare September 9, 2024 10:40
@torchiaf
Copy link
Collaborator Author

torchiaf commented Sep 9, 2024

I opened the USB Device Page with embedded Rancher and enabled one of them. Other USB devices show weird status.

It's fixed in 02fe957

@Yu-Jack
Copy link
Collaborator

Yu-Jack commented Sep 10, 2024

Found a issue. After I enabled the USB Device, the Claimed By showed nothing.

image

But, the usbdevicecalim contains it.

image

@torchiaf
Copy link
Collaborator Author

Found a issue. After I enabled the USB Device, the Claimed By showed nothing.

Fixed, thanks!

@a110605
Copy link
Collaborator

a110605 commented Sep 11, 2024

The enable link is jump to error when pci-addon is disabled.
Missing USB_DEVICE_CONTROLLER in ADD_ONS.

Screenshot 2024-09-11 at 2 01 11 PM Screenshot 2024-09-11 at 2 01 34 PM

@a110605
Copy link
Collaborator

a110605 commented Sep 11, 2024

Not sure why but sort the columns cause enable/disable passthrough button switch.

button_switch.webm

@a110605
Copy link
Collaborator

a110605 commented Sep 11, 2024

Enable group dialog is empty.

image

@a110605
Copy link
Collaborator

a110605 commented Sep 11, 2024

Why I can't disable usb device ? Same for disable group button
cc @Yu-Jack

Screenshot 2024-09-11 at 2 40 06 PM

@Yu-Jack
Copy link
Collaborator

Yu-Jack commented Sep 11, 2024

Why I can't disable usb device ? Same for disable group button cc @Yu-Jack

Screenshot 2024-09-11 at 2 40 06 PM

@a110605 @torchiaf It seems the Add-on is restarted. Because I patched the daemonset image directly, so it will be reset if Add-on is restarted. Please check the daemonset(harvester-system/harvester-pcidevices-controller) image is jk82421/pcidevices:v0.4.0.10 or not before testing.

@a110605
Copy link
Collaborator

a110605 commented Sep 11, 2024

It's nice to block user create VM with un-existed or un-enabled device name if directly edit YAML. cc @Yu-Jack

Screenshot 2024-09-11 at 3 18 00 PM

Copy link

mergify bot commented Sep 12, 2024

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

torchiaf and others added 20 commits September 23, 2024 12:54
Signed-off-by: Francesco Torchia <francesco.torchia@suse.com>
Signed-off-by: Francesco Torchia <francesco.torchia@suse.com>
Signed-off-by: Francesco Torchia <francesco.torchia@suse.com>
Signed-off-by: Francesco Torchia <francesco.torchia@suse.com>
…aim type

Signed-off-by: Francesco Torchia <francesco.torchia@suse.com>
Signed-off-by: Francesco Torchia <francesco.torchia@suse.com>
Signed-off-by: Francesco Torchia <francesco.torchia@suse.com>
Signed-off-by: Francesco Torchia <francesco.torchia@suse.com>
- Add columns and filters
- Filter other vm devices in pci and usb devices pages when selecting devices

Signed-off-by: Francesco Torchia <francesco.torchia@suse.com>
Signed-off-by: Francesco Torchia <francesco.torchia@suse.com>
Signed-off-by: Francesco Torchia <francesco.torchia@suse.com>
Signed-off-by: Francesco Torchia <francesco.torchia@suse.com>
Signed-off-by: Francesco Torchia <francesco.torchia@suse.com>
Signed-off-by: Francesco Torchia <francesco.torchia@suse.com>
…BDevices/index.vue

Co-authored-by: Jack Yu <jack.yu@suse.com>
Signed-off-by: Francesco Torchia <francesco.torchia@suse.com>
Signed-off-by: Francesco Torchia <francesco.torchia@suse.com>
Signed-off-by: Francesco Torchia <francesco.torchia@suse.com>
Signed-off-by: Francesco Torchia <francesco.torchia@suse.com>
Signed-off-by: Francesco Torchia <francesco.torchia@suse.com>
Copy link
Collaborator

@a110605 a110605 left a comment

Choose a reason for hiding this comment

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

LGTM

@torchiaf torchiaf merged commit a2a67e7 into harvester:master Sep 24, 2024
7 checks passed
@torchiaf
Copy link
Collaborator Author

@mergify backport release-harvester-v1.4

@mergify mergify bot mentioned this pull request Sep 24, 2024
3 tasks
Copy link

mergify bot commented Sep 24, 2024

backport release-harvester-v1.4

✅ Backports have been created

torchiaf added a commit that referenced this pull request Sep 24, 2024
…v1.4/pr-1069

 Support USB Passthrough (backport #1069)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Feature New feature for up coming release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants