[libvirt] [PATCH] qemu: Remove limit enforcing when setting processor count

When setting processor count for a domain using the API libvirt enforced a maximum processor count that was determined using an IOCTL on /dev/kvm. Unfortunately this value isn't representative enough and qemu happily accepts and starts with values greater than the reported value. This patch removes the check so that users are able to use full potential of their big boxes also when setting processor counts with the API not just in XML. --- src/qemu/qemu_driver.c | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8e8e00c..6be3e5f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3595,7 +3595,6 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, virDomainObjPtr vm; virDomainDefPtr persistentDef; const char * type; - int max; int ret = -1; bool maximum; @@ -3645,20 +3644,11 @@ qemuDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, goto endjob; } - if ((max = qemudGetMaxVCPUs(NULL, type)) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("could not determine max vcpus for the domain")); - goto endjob; - } - - if (!maximum && vm->def->maxvcpus < max) { - max = vm->def->maxvcpus; - } - - if (nvcpus > max) { + if (!maximum && nvcpus > vm->def->maxvcpus) { virReportError(VIR_ERR_INVALID_ARG, _("requested vcpus is greater than max allowable" - " vcpus for the domain: %d > %d"), nvcpus, max); + " vcpus for the domain: %d > %d"), + nvcpus, vm->def->maxvcpus); goto endjob; } -- 1.7.12

On 09/07/2012 06:51 AM, Peter Krempa wrote:
When setting processor count for a domain using the API libvirt enforced a maximum processor count that was determined using an IOCTL on /dev/kvm. Unfortunately this value isn't representative enough and qemu happily accepts and starts with values greater than the reported value.
But isn't there still _some_ reasonable limit that we should be checking? That is, although qemu will let me run a guest with 3 vcpus on my 2-cpu laptop, I'm sure that even qemu will reject an attempt to run 1000000 vcpus - how do we know what the real limit is? Also, I'm a bit worried that we may have other places in our code that might need fixing if vcpus > max pcpus, but I guess we'll discover those as we go along. As to the patch itself, the code looks fine; and since it only relaxes constraints, I think it is safe to apply; I'm just worried that we are relaxing too far, so you might want to wait for a second opinion or research further into the max limit enforced by qemu. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 07.09.2012 16:05, Eric Blake wrote:
On 09/07/2012 06:51 AM, Peter Krempa wrote:
When setting processor count for a domain using the API libvirt enforced a maximum processor count that was determined using an IOCTL on /dev/kvm. Unfortunately this value isn't representative enough and qemu happily accepts and starts with values greater than the reported value.
But isn't there still _some_ reasonable limit that we should be checking? That is, although qemu will let me run a guest with 3 vcpus on my 2-cpu laptop, I'm sure that even qemu will reject an attempt to run 1000000 vcpus - how do we know what the real limit is?
Also, I'm a bit worried that we may have other places in our code that might need fixing if vcpus > max pcpus, but I guess we'll discover those as we go along.
As to the patch itself, the code looks fine; and since it only relaxes constraints, I think it is safe to apply; I'm just worried that we are relaxing too far, so you might want to wait for a second opinion or research further into the max limit enforced by qemu.
I am comfortable with taking this in. The VCPU count comes from user. It is different from 'being secure by default' patch I've committed earlier - setting RSS limit for qemu instance; I mean - the difference is qemu can start to leak without any user interference which can lead to host system trashing. However, if users wants to shoot themselves into the leg and start million VCPU domain on a singlecore - well, that's their own <insert-correct-word-here>. Michal
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Fri, Sep 07, 2012 at 02:51:00PM +0200, Peter Krempa wrote:
When setting processor count for a domain using the API libvirt enforced a maximum processor count that was determined using an IOCTL on /dev/kvm. Unfortunately this value isn't representative enough and qemu happily accepts and starts with values greater than the reported value.
Really ? If KVM is accepting a greater vCPU count than it reports via /dev/kvm, then we should fix the latter Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 09/07/2012 09:13 AM, Daniel P. Berrange wrote:
On Fri, Sep 07, 2012 at 02:51:00PM +0200, Peter Krempa wrote:
When setting processor count for a domain using the API libvirt enforced a maximum processor count that was determined using an IOCTL on /dev/kvm. Unfortunately this value isn't representative enough and qemu happily accepts and starts with values greater than the reported value.
Really ? If KVM is accepting a greater vCPU count than it reports via /dev/kvm, then we should fix the latter
Why? We already allow oversubscription (you can run 3 guests with 1 vcpu each on a 2-cpu machine); we also allow pinning a 2-vcpu guest to 1 host cpu; so why can't we allow oversubscription from within a single VM? Sure, performance will be lousier, but that doesn't mean the request is invalid. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Fri, Sep 07, 2012 at 11:07:43AM -0600, Eric Blake wrote:
On 09/07/2012 09:13 AM, Daniel P. Berrange wrote:
On Fri, Sep 07, 2012 at 02:51:00PM +0200, Peter Krempa wrote:
When setting processor count for a domain using the API libvirt enforced a maximum processor count that was determined using an IOCTL on /dev/kvm. Unfortunately this value isn't representative enough and qemu happily accepts and starts with values greater than the reported value.
Really ? If KVM is accepting a greater vCPU count than it reports via /dev/kvm, then we should fix the latter
Why? We already allow oversubscription (you can run 3 guests with 1 vcpu each on a 2-cpu machine); we also allow pinning a 2-vcpu guest to 1 host cpu; so why can't we allow oversubscription from within a single VM? Sure, performance will be lousier, but that doesn't mean the request is invalid.
This isn't anything todo with oversubscription. This check is validating that the user did not request more vCPUs than the hypervisor is able to support, regardless of physical CPU count. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 09/10/12 11:11, Daniel P. Berrange wrote:
On Fri, Sep 07, 2012 at 11:07:43AM -0600, Eric Blake wrote:
On 09/07/2012 09:13 AM, Daniel P. Berrange wrote:
On Fri, Sep 07, 2012 at 02:51:00PM +0200, Peter Krempa wrote:
When setting processor count for a domain using the API libvirt enforced a maximum processor count that was determined using an IOCTL on /dev/kvm. Unfortunately this value isn't representative enough and qemu happily accepts and starts with values greater than the reported value.
Really ? If KVM is accepting a greater vCPU count than it reports via /dev/kvm, then we should fix the latter
Why? We already allow oversubscription (you can run 3 guests with 1 vcpu each on a 2-cpu machine); we also allow pinning a 2-vcpu guest to 1 host cpu; so why can't we allow oversubscription from within a single VM? Sure, performance will be lousier, but that doesn't mean the request is invalid.
This isn't anything todo with oversubscription. This check is validating that the user did not request more vCPUs than the hypervisor is able to support, regardless of physical CPU count.
Yes. But the problem is that even if /dev/kvm reports that it supports 160 CPUs you can start guests with 230 processors with any problem. (Maybe apart from that it will consume a lot of CPU time). $ virsh maxvcpus kvm 160 $ virsh vcpuinfo test VCPU: 0 CPU: 0 State: running CPU time: 10.3s CPU Affinity: yyyy VCPU: 1 CPU: 2 State: running CPU time: 0.3s CPU Affinity: yyyy .... VCPU: 228 CPU: 0 State: running CPU time: 0.2s CPU Affinity: yyyy VCPU: 229 CPU: 2 State: running CPU time: 0.2s CPU Affinity: yyyy and it will eventualy boot successfuly after some crunching.
Daniel

On Mon, Sep 10, 2012 at 11:26:50AM +0200, Peter Krempa wrote:
On 09/10/12 11:11, Daniel P. Berrange wrote:
On Fri, Sep 07, 2012 at 11:07:43AM -0600, Eric Blake wrote:
On 09/07/2012 09:13 AM, Daniel P. Berrange wrote:
On Fri, Sep 07, 2012 at 02:51:00PM +0200, Peter Krempa wrote:
When setting processor count for a domain using the API libvirt enforced a maximum processor count that was determined using an IOCTL on /dev/kvm. Unfortunately this value isn't representative enough and qemu happily accepts and starts with values greater than the reported value.
Really ? If KVM is accepting a greater vCPU count than it reports via /dev/kvm, then we should fix the latter
Why? We already allow oversubscription (you can run 3 guests with 1 vcpu each on a 2-cpu machine); we also allow pinning a 2-vcpu guest to 1 host cpu; so why can't we allow oversubscription from within a single VM? Sure, performance will be lousier, but that doesn't mean the request is invalid.
This isn't anything todo with oversubscription. This check is validating that the user did not request more vCPUs than the hypervisor is able to support, regardless of physical CPU count.
Yes. But the problem is that even if /dev/kvm reports that it supports 160 CPUs you can start guests with 230 processors with any problem. (Maybe apart from that it will consume a lot of CPU time).
As I said earlier, if that is correct, then it is a KVM bug that should be fixed. We should not ignore the limit KVM reports to us just because it appears to work with more vCPUs. We need to ask the KVM guys to explain what's going on here. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (4)
-
Daniel P. Berrange
-
Eric Blake
-
Michal Privoznik
-
Peter Krempa