Skip to content

Conversation

liulinC
Copy link
Collaborator

@liulinC liulinC commented May 12, 2023

This PR will not be merged until xs9 is ready,
Please just help to review this PR and approve if you are happy, we will merge it later.

Edit: This PR is updated to make gvt-g support configurable, so it can be merged to master as early as possible,
The legacy code can be removed later at proper time.

@johnelse
Copy link
Contributor

So long, old friend 👋

@liulinC
Copy link
Collaborator Author

liulinC commented May 15, 2023

So long, old friend 👋

So long, old friend 😄

@robhoes
Copy link
Member

robhoes commented May 15, 2023

@johnelse is still around and watching PRs here!!

@jonludlam
Copy link
Contributor

Lots of us are still watching! 👀

@robhoes
Copy link
Member

robhoes commented May 15, 2023

Oh dear... we'd better be more careful about what we are saying here... 😬

@liulinC liulinC force-pushed the private/linl/gvt_g branch from 8cca2a1 to e16c21b Compare May 16, 2023 04:26
@liulinC
Copy link
Collaborator Author

liulinC commented May 16, 2023

I rebase to one single commit as the design now has been updated according to @edwintorok ,
We introduce configurable item, and drop/support this vGPU on demand.

@liulinC liulinC changed the title [Do not merge] Drop gvt-g support CP-43131: Make gvt-g support configurable May 16, 2023
@liulinC liulinC requested a review from psafont May 16, 2023 04:32
@liulinC liulinC force-pushed the private/linl/gvt_g branch from e16c21b to ff21643 Compare May 16, 2023 05:41
Copy link
Contributor

@edwintorok edwintorok left a comment

Choose a reason for hiding this comment

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

I think the checks in start/resume/resume_on are redundant and would be covered by the 'allocate_vm_to_host' check.

What happens to the PGPUs though, will XAPI do the right thing on startup and remove the gvt-g ones if not supported due to the change in the 'find_or_create_supported'?

@liulinC
Copy link
Collaborator Author

liulinC commented May 17, 2023

I think the checks in start/resume/resume_on are redundant and would be covered by the 'allocate_vm_to_host' check.

What happens to the PGPUs though, will XAPI do the right thing on startup and remove the gvt-g ones if not supported due to the change in the 'find_or_create_supported'?

  1. Indeed, all the three function will reach allocate_vm_to_host, but that is at very quick deep/later stage, with lots of xapi effort wasted, besides that, that happens when xapi to allocate a host to the VM, thus, the error message looks not straight forward,
# xe vm-start uuid=5e4ff268-c692-b202-6c23-8186266d59bd 
There are no suitable hosts to start this VM on.
The following table provides per-host reasons for why the VM could not be started:

    : Cannot start here [Server_error(VGPU_TYPE_NO_LONGER_SUPPORTED, [ Intel Corporation: Intel GVT-g ])]

With current early check and block,

# xe vm-start uuid=5e4ff268-c692-b202-6c23-8186266d59bd 
VGPU type is no longer supported
type: Intel Corporation: Intel GVT-g

I would like to argue just keep it (yes, we can handle the exception and re-raise at proper place, but that sounds more complicated.)

  1. gvt-g is one type of vGPU, passthrough (intel gvt-d) can still work well.

@liulinC liulinC requested a review from edwintorok May 17, 2023 08:51
@liulinC liulinC force-pushed the private/linl/gvt_g branch from f1151f8 to 6e7cf11 Compare May 19, 2023 01:52
@liulinC
Copy link
Collaborator Author

liulinC commented May 19, 2023

push -f just squash the history and rebase master before merge.

Introduce config gvt-g-supported and default to true
If set to false, then gvt-g vGPU is taken as on longer
supported, thus block VM start/resume/migrate operation
for VM with such vGPU

The gvt-g related code are kept for compatiblility with xs8,
and will be dropped at proper time

Signed-off-by: Lin Liu <lin.liu@citrix.com>
@liulinC liulinC force-pushed the private/linl/gvt_g branch from 6e7cf11 to 7027ef0 Compare May 19, 2023 08:54
@liulinC liulinC merged commit 33b4f5a into xapi-project:master May 19, 2023
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.

6 participants