-
Notifications
You must be signed in to change notification settings - Fork 261
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
Do not allow hot plug CPU with exclusive pinning #540
Conversation
/ost |
Looks good to me. Just one little thing: It may be worth to update the comment in UpdateVmCommand::hotSetCpus. |
I do not think it is necessary, we will never get there because of the validation ( |
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.
Ah right, OK.
@@ -20,6 +20,12 @@ public class VmCommonUtils { | |||
* @return true, if any CPUs are to be hotplugged, false otherwise | |||
*/ | |||
public static boolean isCpusToBeHotpluggedOrUnplugged(VM source, VM destination) { | |||
return cpuSocketsChanged(source, destination) | |||
&& !source.getCpuPinningPolicy().isExclusive() | |||
&& !destination.getCpuPinningPolicy().isExclusive(); |
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.
so we won't be able to change the policy from, let's say, dedicated to none and plug CPUs?
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.
Not on the running VM, no. The VM would need to be restarted for the changes to be applied.
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.
reading it again, my question above was incorrect - I meant to change a running vm that is set with policy=none to policy=dedicated and plug CPUs (it's not that important, just want to understand what's the reason for blocking it)
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.
Every change of cpu pinning policy requires VM reboot. And it is not possible to hot-plug CPUs when using exclusive pinning. Your example would break both conditions :-)
There is no special reason for blocking it, it is just scenario that cannot currently work. How would you imagine going from none to exclusive while running the VM? Would we add pinning at runtime? I do not think VDSM supports that either.
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.
let's say that I take a non-stateless vm with 1 cpu and change it to be stateless with 2 cpus, I'll be able to say that the change to the cpu topology should happen now and I have no option but to wait for the stateless property to change when the vm restart:
similarly, if I take a vm with 1 cpu and policy=none, and change it to 2 cpus and policy=dedicated then I realize that the change to the policy is like the change to the stateless property (and thus, it would be applied on next start) but I would still expect to be able to hot-plug that additional cpu. after restart we won't consider this additional cpu as a hot-plugged one, it would be part of the topology and so the vm should run property after restart with 2 cpus and dedicated policy, no?
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.
I see, thanks for the explanation. I've updated the patch to allow your scenario.
It is not possible to hot plug CPUs when using exclusive CPU pinning. Instead of failing on the backend, the CPU is now listed as "Changes that require Virtual Machine restart" instead of "Changes that can be applied immediately" in the "Pending Virtual Machine changes" dialog. Bug-Url: https://bugzilla.redhat.com/2097717 Bug-Url: https://bugzilla.redhat.com/2111088
/ost |
It is not possible to hot plug CPUs when using exclusive CPU pinning. Instead of failing on the backend, the CPU is now listed as "Changes that require Virtual Machine restart" instead of "Changes that can be applied immediately" in the "Pending Virtual Machine changes" dialog.
Bug-Url: https://bugzilla.redhat.com/2097717
Bug-Url: https://bugzilla.redhat.com/2111088