4:49 a.m.
From: Chen Hanxiao <chenhanxiao(a)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(a)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
2:41 a.m.
On Sat, Oct 22, 2016 at 05:49:38PM +0800, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao(a)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(a)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?
4:28 p.m.
At 2016-10-27 15:41:39, "Martin Kletzander" <mkletzan(a)redhat.com> wrote:
On Sat, Oct 22, 2016 at 05:49:38PM +0800, Chen Hanxiao wrote:
>From: Chen Hanxiao <chenhanxiao(a)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(a)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
8:21 a.m.
On Thu, Oct 27, 2016 at 04:28:57PM +0800, Chen Hanxiao wrote:
At 2016-10-27 15:41:39, "Martin Kletzander" <mkletzan(a)redhat.com> wrote:
>On Sat, Oct 22, 2016 at 05:49:38PM +0800, Chen Hanxiao wrote:
>>From: Chen Hanxiao <chenhanxiao(a)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(a)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
2950
days inactive
2955
days old
3 comments
2 participants
participants (2)
-
Chen Hanxiao -
Martin Kletzander