[libvirt] [PATCH] qemu_driver: don't set cpu pin for TCG domain
From: Chen Hanxiao <chenhanxiao@gmail.com> We don't support pinning cpu for TCG domain. But we could set it by vcpupin command, which result in a failed startup. Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> --- src/qemu/qemu_driver.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 93ea5e2..98cfcab 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5177,6 +5177,14 @@ qemuDomainPinVcpuFlags(virDomainPtr dom, if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0) goto endjob; + if ((def && def->virtType == VIR_DOMAIN_VIRT_QEMU) || + (persistentDef && persistentDef->virtType == VIR_DOMAIN_VIRT_QEMU)) + { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("Virt type 'Qemu'(TCG) did not support CPU pin")); + goto endjob; + } + if (persistentDef && !(vcpuinfo = virDomainDefGetVcpu(persistentDef, vcpu))) { virReportError(VIR_ERR_INVALID_ARG, -- 1.8.3.1
On Sat, Oct 22, 2016 at 05:49:38PM +0800, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@gmail.com>
We don't support pinning cpu for TCG domain. But we could set it by vcpupin command, which result in a failed startup.
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> --- src/qemu/qemu_driver.c | 8 ++++++++ 1 file changed, 8 insertions(+)
Honestly, I think it makes more sense the way it's done now. The API allows the same thing you would be able to do by 'virsh edit' anyway. The fact that TCG doesn't support that now doesn't mean it will not support that in the future and the way it's done now will ensure it works in that case, I believe. So I would vote for keeping it this way... Maybe a domain capability element could show whether this is possible or not, but that seems like an overkill now. Is that any particular major issue you are trying to resolve by this? E.g. in a mgmt app?
At 2016-10-27 15:41:39, "Martin Kletzander" <mkletzan@redhat.com> wrote:
On Sat, Oct 22, 2016 at 05:49:38PM +0800, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@gmail.com>
We don't support pinning cpu for TCG domain. But we could set it by vcpupin command, which result in a failed startup.
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> --- src/qemu/qemu_driver.c | 8 ++++++++ 1 file changed, 8 insertions(+)
Honestly, I think it makes more sense the way it's done now. The API allows the same thing you would be able to do by 'virsh edit' anyway. The fact that TCG doesn't support that now doesn't mean it will not support that in the future and the way it's done now will ensure it works in that case, I believe.
We've already did the same check in qemuDomainRefreshVcpuInfo. If TCG supports pin feature in the future, we could remove these restrictions: 1) in config phase as this patch did. 2) in runtime.
So I would vote for keeping it this way... Maybe a domain capability element could show whether this is possible or not, but that seems like an overkill now. Is that any particular major issue you are trying to resolve by this? E.g. in a mgmt app?
Do we have API for showing domain's virt type? I had some VMs with either tcg or kvm virt type. PinVcpu API bring me some troubles then. Regards, - Chen
On Thu, Oct 27, 2016 at 04:28:57PM +0800, Chen Hanxiao wrote:
At 2016-10-27 15:41:39, "Martin Kletzander" <mkletzan@redhat.com> wrote:
On Sat, Oct 22, 2016 at 05:49:38PM +0800, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@gmail.com>
We don't support pinning cpu for TCG domain. But we could set it by vcpupin command, which result in a failed startup.
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> --- src/qemu/qemu_driver.c | 8 ++++++++ 1 file changed, 8 insertions(+)
Honestly, I think it makes more sense the way it's done now. The API allows the same thing you would be able to do by 'virsh edit' anyway. The fact that TCG doesn't support that now doesn't mean it will not support that in the future and the way it's done now will ensure it works in that case, I believe.
We've already did the same check in qemuDomainRefreshVcpuInfo. If TCG supports pin feature in the future, we could remove these restrictions: 1) in config phase as this patch did. 2) in runtime.
Yeah, we'd have to change it again. Not that it's a big deal, it's just not that clean. But since we've done that already once :-/ I guess fine...
So I would vote for keeping it this way... Maybe a domain capability element could show whether this is possible or not, but that seems like an overkill now. Is that any particular major issue you are trying to resolve by this? E.g. in a mgmt app?
Do we have API for showing domain's virt type? I had some VMs with either tcg or kvm virt type. PinVcpu API bring me some troubles then.
I meant more like output of either virsh capabilities or virsh domcapabilities. I don't think there is an API that returns exactly what the domain's virt type is. Anyway, you made your case =) I'll push this in a while, with few style and rewording fixes.
Regards, - Chen
participants (2)
-
Chen Hanxiao -
Martin Kletzander