-
Notifications
You must be signed in to change notification settings - Fork 97
docs: add VM restart and addon management warnings for PCI devices #973
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 warning to restart VMs immediately after modifying PCI/vGPU devices - Document annotation to skip addon disable check when VMs are stopped with devices attached - Explain synchronization importance between VM spec and running state Signed-off-by: Jack Yu <[email protected]>
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 request overview
Adds user-facing warnings to Harvester advanced add-on docs to reduce misconfiguration risk when managing PCI/vGPU devices on VMs, aligning with the problem described in harvester/harvester#9531.
Changes:
- Add a warning to restart VMs immediately after adding/removing PCI devices.
- Document an annotation-based escape hatch to bypass the add-on disable check for the PCI devices controller.
- Add equivalent warning + annotation guidance for vGPU management via the NVIDIA Driver Toolkit add-on.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| docs/advanced/addons/pcidevices.md | Adds a warning about VM restart after PCI device changes and documents a skip-check annotation for add-on disable operations. |
| docs/advanced/addons/nvidiadrivertoolkit.md | Adds a warning about VM restart after vGPU changes and documents a skip-check annotation for add-on disable operations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
w13915984028
left a comment
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.
Thanks, some suggestions.
BTW, in the create vm main page https://docs.harvesterhci.io/v1.7/vm/index, there is no link to gpu/pci/usb ..., @jillian-maroket should we add links to addons? thanks.
|
|
||
| This is accomplished by using the `pcidevices-controller` addon. | ||
|
|
||
| To use the PCI devices feature, users need to enable the `pcidevices-controller` addon first. |
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.
could we add one line on L22 about the new disable check on v180? new L62 could be moved to here or kept.
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 can see how @jillian-maroket would like to arrange this.
| @@ -32,6 +32,16 @@ On pod startup, the ENTRYPOINT script will download the NVIDIA driver from the s | |||
|
|
|||
| The `PCIDevices` addon can now leverage this addon to manage the lifecycle of the vGPU devices on nodes containing supported GPU [devices](../vgpusupport.md). | |||
|
|
|||
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.
Similar, L34 about the new check when disabling addon
|
|
||
| **Always restart the VM immediately after attaching or detaching vGPU devices.** | ||
|
|
||
| Although rebooting the VM after editing its spec is not mandatory, we strongly recommend doing so to ensure proper synchronization. Without an immediate reboot, the addon disable check might not accurately detect devices in use. |
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.
instant rebooting helps check on addon rebooting, also avoids potential resources conflicts even when addon is still enabled
WebberHuang1118
left a comment
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.
LGTM, thanks for the enhancement.
harvester/harvester#9531