7:34 a.m.
For some archs and machines, the maxvcpus defined in the kernel can be different
from the qemu-caps maxvcpus. Just reporting the kernel defined maxvcpus is not
be sufficient. virsh domacapabilities and virsh maxvcpus --type kvm return
different maxvcpus values and is confusing as to know what actually works.
The minimum of the two values is what actually works.
For example on PPC64, the KVM_MAX_VCPUS is defined to be 1024 in kernel
where as qemu has MAX_CPUMASK_BITS defined at 255 in include/sysemu/sysemu.h.
The guest can go upto 256 vcpus here.
Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com>
---
src/qemu/qemu_driver.c | 27 ++++++++++++++++++++++++---
1 file changed, 24 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3d0c7c8..2716af8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1255,10 +1255,31 @@ static int qemuConnectIsAlive(virConnectPtr conn
ATTRIBUTE_UNUSED)
static int
-kvmGetMaxVCPUs(void)
+kvmGetMaxVCPUs(virConnectPtr conn)
{
int fd;
int ret;
+ int qemuCapsMaxVcpus = 0;
+ virArch arch = virArchFromHost();
+ virQEMUCapsPtr qemuCaps = NULL;
+ virQEMUDriverPtr driver = conn->privateData;
+ const char *machine;
+
+ if (!(qemuCaps = virQEMUCapsCacheLookupByArch(driver->qemuCapsCache,
+ arch))) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("unable to find any emulator to serve '%s' "
+ "architecture"), virArchToString(arch));
+ return -1;
+ }
+
+ if (!(machine = virQEMUCapsGetDefaultMachine(qemuCaps))) {
+ virObjectUnref(qemuCaps);
+ return -1;
+ }
+
+ qemuCapsMaxVcpus = virQEMUCapsGetMachineMaxCpus(qemuCaps, machine);
+ virObjectUnref(qemuCaps);
if ((fd = open(KVM_DEVICE, O_RDONLY)) < 0) {
virReportSystemError(errno, _("Unable to open %s"), KVM_DEVICE);
@@ -1282,7 +1303,7 @@ kvmGetMaxVCPUs(void)
cleanup:
VIR_FORCE_CLOSE(fd);
- return ret;
+ return ret > qemuCapsMaxVcpus ? qemuCapsMaxVcpus : ret;
}
@@ -1323,7 +1344,7 @@ qemuConnectGetMaxVcpus(virConnectPtr conn ATTRIBUTE_UNUSED, const
char *type)
return 16;
if (STRCASEEQ(type, "kvm"))
- return kvmGetMaxVCPUs();
+ return kvmGetMaxVCPUs(conn);
if (STRCASEEQ(type, "kqemu"))
return 1;
9:12 a.m.
On 05/04/2016 08:34 AM, Shivaprasad G Bhat wrote:
For some archs and machines, the maxvcpus defined in the kernel can
be different
from the qemu-caps maxvcpus. Just reporting the kernel defined maxvcpus is not
be sufficient. virsh domacapabilities and virsh maxvcpus --type kvm return
different maxvcpus values and is confusing as to know what actually works.
The minimum of the two values is what actually works.
For example on PPC64, the KVM_MAX_VCPUS is defined to be 1024 in kernel
where as qemu has MAX_CPUMASK_BITS defined at 255 in include/sysemu/sysemu.h.
The guest can go upto 256 vcpus here.
Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com>
---
src/qemu/qemu_driver.c | 27 ++++++++++++++++++++++++---
1 file changed, 24 insertions(+), 3 deletions(-)
You'll also need to extend the virConnectGetMaxVcpus doc string in
src/libvirt-host.c to indicate it may limit the reported CPUs based on host
arch and default machine type.
Additionally, there's also virDomainGetMaxVcpus which is just
virConnectGetMaxVcpus(def->type), however we should extend the way that is
handled internally to also abide the domain's arch and machine type in this case.
One comment below:
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3d0c7c8..2716af8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1255,10 +1255,31 @@ static int qemuConnectIsAlive(virConnectPtr conn
ATTRIBUTE_UNUSED)
static int
-kvmGetMaxVCPUs(void)
+kvmGetMaxVCPUs(virConnectPtr conn)
{
int fd;
int ret;
+ int qemuCapsMaxVcpus = 0;
+ virArch arch = virArchFromHost();
+ virQEMUCapsPtr qemuCaps = NULL;
+ virQEMUDriverPtr driver = conn->privateData;
+ const char *machine;
+
+ if (!(qemuCaps = virQEMUCapsCacheLookupByArch(driver->qemuCapsCache,
+ arch))) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("unable to find any emulator to serve '%s' "
+ "architecture"), virArchToString(arch));
+ return -1;
+ }
+
+ if (!(machine = virQEMUCapsGetDefaultMachine(qemuCaps))) {
+ virObjectUnref(qemuCaps);
+ return -1;
+ }
+
+ qemuCapsMaxVcpus = virQEMUCapsGetMachineMaxCpus(qemuCaps, machine);
+ virObjectUnref(qemuCaps);
if ((fd = open(KVM_DEVICE, O_RDONLY)) < 0) {
virReportSystemError(errno, _("Unable to open %s"), KVM_DEVICE);
@@ -1282,7 +1303,7 @@ kvmGetMaxVCPUs(void)
cleanup:
VIR_FORCE_CLOSE(fd);
- return ret;
+ return ret > qemuCapsMaxVcpus ? qemuCapsMaxVcpus : ret;
}
You can use MIN() here
Thanks,
Cole
9:24 a.m.
On Wed, May 04, 2016 at 10:12:24 -0400, Cole Robinson wrote:
On 05/04/2016 08:34 AM, Shivaprasad G Bhat wrote:
> For some archs and machines, the maxvcpus defined in the kernel can be different
> from the qemu-caps maxvcpus. Just reporting the kernel defined maxvcpus is not
> be sufficient. virsh domacapabilities and virsh maxvcpus --type kvm return
> different maxvcpus values and is confusing as to know what actually works.
> The minimum of the two values is what actually works.
>
> For example on PPC64, the KVM_MAX_VCPUS is defined to be 1024 in kernel
> where as qemu has MAX_CPUMASK_BITS defined at 255 in include/sysemu/sysemu.h.
> The guest can go upto 256 vcpus here.
>
> Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com>
> ---
> src/qemu/qemu_driver.c | 27 ++++++++++++++++++++++++---
> 1 file changed, 24 insertions(+), 3 deletions(-)
>
You'll also need to extend the virConnectGetMaxVcpus doc string in
src/libvirt-host.c to indicate it may limit the reported CPUs based on host
arch and default machine type.
Hmm, I think it is a very bad idea to limit the CPUs according to
something which is hidden and you cannot influence. In other words, the
API itself is not able to report the maximum depending on emulator
binary, machine type, whatever so it shouldn't do that at all. Using
default emulator and machine type is even more confusing that reporting
just the kernel limits.
The virConnectGetDomainCapabilities API seems like a much better place
for reporting maximum number of usable CPUs as it can get kvm/qemu,
arch, emulator binary, and machine type as optional parameters so that
the user gets the answer they want rather than just (in various ways)
random number reported by virConnectGetMaxVcpus.
Jirka
9:44 a.m.
On 05/04/2016 10:24 AM, Jiri Denemark wrote:
On Wed, May 04, 2016 at 10:12:24 -0400, Cole Robinson wrote:
> On 05/04/2016 08:34 AM, Shivaprasad G Bhat wrote:
>> For some archs and machines, the maxvcpus defined in the kernel can be different
>> from the qemu-caps maxvcpus. Just reporting the kernel defined maxvcpus is not
>> be sufficient. virsh domacapabilities and virsh maxvcpus --type kvm return
>> different maxvcpus values and is confusing as to know what actually works.
>> The minimum of the two values is what actually works.
>>
>> For example on PPC64, the KVM_MAX_VCPUS is defined to be 1024 in kernel
>> where as qemu has MAX_CPUMASK_BITS defined at 255 in include/sysemu/sysemu.h.
>> The guest can go upto 256 vcpus here.
>>
>> Signed-off-by: Shivaprasad G Bhat <sbhat(a)linux.vnet.ibm.com>
>> ---
>> src/qemu/qemu_driver.c | 27 ++++++++++++++++++++++++---
>> 1 file changed, 24 insertions(+), 3 deletions(-)
>>
>
> You'll also need to extend the virConnectGetMaxVcpus doc string in
> src/libvirt-host.c to indicate it may limit the reported CPUs based on host
> arch and default machine type.
Hmm, I think it is a very bad idea to limit the CPUs according to
something which is hidden and you cannot influence. In other words, the
API itself is not able to report the maximum depending on emulator
binary, machine type, whatever so it shouldn't do that at all. Using
default emulator and machine type is even more confusing that reporting
just the kernel limits.
Is it not confusing that MaxVCPUs will return a value that no VM can actually
support, because it hits qemu limits? That was the original complaint.
Maybe instead we do MIN(kvm_vcpus, max_vcpus_supported_by_any_qemu_machine)
Even if we don't change the logic, the API documentation could use some work
to clarify the existing behavior
The virConnectGetDomainCapabilities API seems like a much better place
for reporting maximum number of usable CPUs as it can get kvm/qemu,
arch, emulator binary, and machine type as optional parameters so that
the user gets the answer they want rather than just (in various ways)
random number reported by virConnectGetMaxVcpus.
I agree the proper place for reporting fully accurate info is in domcapabilities
- Cole
12:27 p.m.
On Wed, 2016-05-04 at 10:44 -0400, Cole Robinson wrote:
> Hmm, I think it is a very bad idea to limit the CPUs according
to
> something which is hidden and you cannot influence. In other words, the
> API itself is not able to report the maximum depending on emulator
> binary, machine type, whatever so it shouldn't do that at all. Using
> default emulator and machine type is even more confusing that reporting
> just the kernel limits.
Is it not confusing that MaxVCPUs will return a value that no VM can actually
support, because it hits qemu limits? That was the original complaint.
Maybe instead we do MIN(kvm_vcpus, max_vcpus_supported_by_any_qemu_machine)
Even if we don't change the logic, the API documentation could use some work
to clarify the existing behavior
> The virConnectGetDomainCapabilities API seems like a much better place
> for reporting maximum number of usable CPUs as it can get kvm/qemu,
> arch, emulator binary, and machine type as optional parameters so that
> the user gets the answer they want rather than just (in various ways)
> random number reported by virConnectGetMaxVcpus.
I agree the proper place for reporting fully accurate info is in domcapabilities
How about something like
<vcpus max='255'>
<kvm>
<soft_limit>96</soft_limit>
<hard_limit>2048</hard_limit>
</kvm>
<qemu>
<hard_limit>255</hard_limit>
</qemu>
</vcpus>
Hopefully someone can come up with a better XML than that, but
you get the idea :)
This would include one bit of information that AFAIK we're
currently missing, which is the recommended number of vCPUs;
the maximum value would of course be the smallest amoung all
hard limits.
And of course we can't change virConnectGetMaxVcpus() because
it's public API, but we can extend 'virsh maxvcpus' to accept
the same options as 'virsh domcapabilities' and actually use
the domcapabilities XML behind the scenes.
That, and update the API documentation for
virConnectGetMaxVcpus() to notify applications developers that
they should really use the domcapabilities XML instead.
--
Andrea Bolognani
Software Engineer - Virtualization Team
1:52 a.m.
On Wed, May 04, 2016 at 19:27:41 +0200, Andrea Bolognani wrote:
How about something like
<vcpus max='255'>
<kvm>
<soft_limit>96</soft_limit>
<hard_limit>2048</hard_limit>
</kvm>
<qemu>
<hard_limit>255</hard_limit>
</qemu>
</vcpus>
No, domcapabilities are bound to an arch/binary/type/machine-type
combination so providing both kvm and qemu stuff there is wrong. If it's
a result of asking for kvm type, //vcpus@max should contain the kvm
limit. If the type of the domain is qemu, it should contain qemu limit.
And in both cases, the limit is constrained to just the machine type
reported in the domcapabilities XML.
This would include one bit of information that AFAIK we're
currently missing, which is the recommended number of vCPUs;
the maximum value would of course be the smallest amoung all
hard limits.
We could perhaps somehow incorporate the "recommended number of CPUs" in
there, but that's a separate thing really.
And of course we can't change virConnectGetMaxVcpus() because
it's public API, but we can extend 'virsh maxvcpus' to accept
the same options as 'virsh domcapabilities' and actually use
the domcapabilities XML behind the scenes.
I don't think this is a good idea.
That, and update the API documentation for
virConnectGetMaxVcpus() to notify applications developers that
they should really use the domcapabilities XML instead.
But yeah, we should update the docs this way.
Jirka
2:34 a.m.
On Thu, 2016-05-05 at 08:52 +0200, Jiri Denemark wrote:
On Wed, May 04, 2016 at 19:27:41 +0200, Andrea Bolognani wrote:
>
> How about something like
>
> <vcpus max='255'>
> <kvm>
> <soft_limit>96</soft_limit>
> <hard_limit>2048</hard_limit>
> </kvm>
> <qemu>
> <hard_limit>255</hard_limit>
> </qemu>
> </vcpus>
No, domcapabilities are bound to an arch/binary/type/machine-type
combination so providing both kvm and qemu stuff there is wrong. If it's
a result of asking for kvm type, //vcpus@max should contain the kvm
limit. If the type of the domain is qemu, it should contain qemu limit.
And in both cases, the limit is constrained to just the machine type
reported in the domcapabilities XML.
The fact is that the QEMU limit *does* influence the actual
limit for KVM guests: see the example above, taken from a
POWER host, where KVM would happily create 2048 vCPUs but QEMU
is limited to 255.
I didn't say it explicitly, but it was implied that QEMU limits
would only be present for QEMU and KVM guests, and KVM limits
only for KVM guests.
I guess we could skip all the details and just give the user
<vcpus max='255' suggested='96'/>
so they don't have to implement the logic themselves. But
reporting the KVM limits without taking QEMU limits into
consideration is not the way to go, I think.
> This would include one bit of information that AFAIK we're
> currently missing, which is the recommended number of vCPUs;
> the maximum value would of course be the smallest amoung all
> hard limits.
We could perhaps somehow incorporate the "recommended number of CPUs" in
there, but that's a separate thing really.
It seems like a valuable information an a good fit for the
element, but if you have a better placement in mind please do
share :)
> And of course we can't change virConnectGetMaxVcpus()
because
> it's public API, but we can extend 'virsh maxvcpus' to accept
> the same options as 'virsh domcapabilities' and actually use
> the domcapabilities XML behind the scenes.
I don't think this is a good idea.
Why not? virsh is supposed to be both a uselful management tool
and a showcase of our API... If we document that a function does
not provide optimal data, and document the preferred approach
instead, why shouldn't virsh be updated to reflect the changes
we're raccomending external users, thus making it better both
as a management tool and as an API showcase?
--
Andrea Bolognani
Software Engineer - Virtualization Team
3 a.m.
On Thu, May 05, 2016 at 09:34:21 +0200, Andrea Bolognani wrote:
On Thu, 2016-05-05 at 08:52 +0200, Jiri Denemark wrote:
> On Wed, May 04, 2016 at 19:27:41 +0200, Andrea Bolognani wrote:
> >
> > How about something like
> >
> > <vcpus max='255'>
> > <kvm>
> > <soft_limit>96</soft_limit>
> > <hard_limit>2048</hard_limit>
> > </kvm>
> > <qemu>
> > <hard_limit>255</hard_limit>
> > </qemu>
> > </vcpus>
>
> No, domcapabilities are bound to an arch/binary/type/machine-type
> combination so providing both kvm and qemu stuff there is wrong. If it's
> a result of asking for kvm type, //vcpus@max should contain the kvm
> limit. If the type of the domain is qemu, it should contain qemu limit.
> And in both cases, the limit is constrained to just the machine type
> reported in the domcapabilities XML.
The fact is that the QEMU limit *does* influence the actual
limit for KVM guests: see the example above, taken from a
POWER host, where KVM would happily create 2048 vCPUs but QEMU
is limited to 255.
I didn't say it explicitly, but it was implied that QEMU limits
would only be present for QEMU and KVM guests, and KVM limits
only for KVM guests.
I guess we could skip all the details and just give the user
<vcpus max='255' suggested='96'/>
so they don't have to implement the logic themselves. But
reporting the KVM limits without taking QEMU limits into
consideration is not the way to go, I think.
Yes. I didn't want to say we should only report the number from KVM, I
meant we should only show the number that is applicable to a KVM domain
of the particular machine type. That is, both KVM and QEMU limits
combined into a single number which gives the real limit.
Jirka
3:39 a.m.
On Thu, 2016-05-05 at 10:00 +0200, Jiri Denemark wrote:
>
> I guess we could skip all the details and just give the user
>
> <vcpus max='255' suggested='96'/>
>
> so they don't have to implement the logic themselves. But
> reporting the KVM limits without taking QEMU limits into
> consideration is not the way to go, I think.
Yes. I didn't want to say we should only report the number from KVM, I
meant we should only show the number that is applicable to a KVM domain
of the particular machine type. That is, both KVM and QEMU limits
combined into a single number which gives the real limit.
Okay :)
But if we don't change 'virsh maxvcpus --type kvm' to report
the computed limit, instead of just the KVM limit as it does
now, the problem reported by Shivaprasad will still exist...
--
Andrea Bolognani
Software Engineer - Virtualization Team
4 a.m.
On Thu, May 05, 2016 at 10:39:56 +0200, Andrea Bolognani wrote:
On Thu, 2016-05-05 at 10:00 +0200, Jiri Denemark wrote:
> >
> > I guess we could skip all the details and just give the user
> >
> > <vcpus max='255' suggested='96'/>
> >
> > so they don't have to implement the logic themselves. But
> > reporting the KVM limits without taking QEMU limits into
> > consideration is not the way to go, I think.
>
> Yes. I didn't want to say we should only report the number from KVM, I
> meant we should only show the number that is applicable to a KVM domain
> of the particular machine type. That is, both KVM and QEMU limits
> combined into a single number which gives the real limit.
Okay :)
But if we don't change 'virsh maxvcpus --type kvm' to report
the computed limit, instead of just the KVM limit as it does
now, the problem reported by Shivaprasad will still exist...
I think that should be fixed by documenting that the old API and virsh
command report bad results and should not be used rather than trying to
add syntax sugar that will pull the data from a different place.
4:34 a.m.
On Thu, 2016-05-05 at 11:00 +0200, Peter Krempa wrote:
>
> But if we don't change 'virsh maxvcpus --type kvm' to report
> the computed limit, instead of just the KVM limit as it does
> now, the problem reported by Shivaprasad will still exist...
I think that should be fixed by documenting that the old API and virsh
command report bad results and should not be used rather than trying to
add syntax sugar that will pull the data from a different place.
I disagree. We can't extend virConnectGetMaxVcpus() in any
sensible way because that would break the ABI, but we can do so
for the maxvcpus command: --type is already the same as the
domcapabilities command, we just have to add --emulatorbin,
--arch and --machine and trivially change the implementation.
People using 'virsh maxvcpus' without passing any of the
new options will get better data automatically, in a completely
backward-compatible fashion, and we will have one less place
where different APIs return slightly different results for no
apparent reason (I'm looking at you, topology information).
While virsh commands generally map directly to the libvirt
function of the same name, that's not necessarily a strict
requirement IMHO - especially when we're asking application
developers *not* to use that API anymore, and pointing them
to a different solution. We should lead by example here.
--
Andrea Bolognani
Software Engineer - Virtualization Team
1:55 a.m.
On Wed, May 04, 2016 at 10:44:37 -0400, Cole Robinson wrote:
On 05/04/2016 10:24 AM, Jiri Denemark wrote:
> Hmm, I think it is a very bad idea to limit the CPUs according to
> something which is hidden and you cannot influence. In other words, the
> API itself is not able to report the maximum depending on emulator
> binary, machine type, whatever so it shouldn't do that at all. Using
> default emulator and machine type is even more confusing that reporting
> just the kernel limits.
Is it not confusing that MaxVCPUs will return a value that no VM can actually
support, because it hits qemu limits? That was the original complaint.
Maybe instead we do MIN(kvm_vcpus, max_vcpus_supported_by_any_qemu_machine)
Even if we don't change the logic, the API documentation could use some work
to clarify the existing behavior
I think this is the only part that really needs changing. There's no way
we can fix the API to report anything useful. The result will always be
confusing in some way. I'd rather keep the confusion consistent and let
the API report what it always reported and explain that in the
documentation with a pointer to domain capabilities API.
Jirka
3199
days inactive
3200
days old
11 comments
5 participants
participants (5)
-
Andrea Bolognani -
Cole Robinson -
Jiri Denemark -
Peter Krempa -
Shivaprasad G Bhat