Re: [libvirt] [PATCH 13/17] limit cpu bandwidth only for vcpus

2012-08-07 Thread Eric Blake
On 08/03/2012 12:36 AM, Hu Tao wrote:
 This patch changes the behaviour of xml element cputune.period
 and cputune.quota to limit cpu bandwidth only for vcpus, and no
 longer limit cpu bandwidth for the whole guest.
 
 The reasons to do this are:
 
   - This matches docs of cputune.period and cputune.quota.
   - The other parts excepting vcpus are treated as hypervisor,
 and there are seperate period/quota settings for hypervisor

s/hypervisor/emulator/ throughout,
s/seperate/separate/

Is it worth being able to put a separate quota on the entire domain
(emulator+vcpus), independently from the smaller quotas on a per-vcpu or
per-emulator basis?  While I do think this is an appropriate fix
(normally, what we are trying to rate limit is the guest itself, and
that means run the emulator as fast as possible to let the guest keep
up), I do worry that it means someone that was previously using this to
limit the entire domain would now be oversubscribed, unless we also have
a way to preserve the ability to limit the entire domain.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 13/17] limit cpu bandwidth only for vcpus

2012-08-03 Thread Hu Tao
This patch changes the behaviour of xml element cputune.period
and cputune.quota to limit cpu bandwidth only for vcpus, and no
longer limit cpu bandwidth for the whole guest.

The reasons to do this are:

  - This matches docs of cputune.period and cputune.quota.
  - The other parts excepting vcpus are treated as hypervisor,
and there are seperate period/quota settings for hypervisor
in the subsequent patches
---
 src/qemu/qemu_cgroup.c |   33 ++--
 src/qemu/qemu_driver.c |   65 
 2 files changed, 8 insertions(+), 90 deletions(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 247f987..4d97665 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -556,11 +556,16 @@ int qemuSetupCgroupForVcpu(struct qemud_driver *driver, 
virDomainObjPtr vm)
 unsigned int i;
 unsigned long long period = vm-def-cputune.period;
 long long quota = vm-def-cputune.quota;
-long long vm_quota = 0;
 
 if (driver-cgroup == NULL)
 return 0; /* Not supported, so claim success */
 
+if (!qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) {
+virReportError(VIR_ERR_SYSTEM_ERROR, %s,
+   _(cgroup cpu is not active));
+return -1;
+}
+
 rc = virCgroupForDomain(driver-cgroup, vm-def-name, cgroup, 0);
 if (rc != 0) {
 virReportSystemError(-rc,
@@ -569,25 +574,6 @@ int qemuSetupCgroupForVcpu(struct qemud_driver *driver, 
virDomainObjPtr vm)
 goto cleanup;
 }
 
-/* Set cpu bandwidth for the vm */
-if (period || quota) {
-if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) {
-/* Ensure that we can multiply by vcpus without overflowing. */
-if (quota  LLONG_MAX / vm-def-vcpus) {
-virReportSystemError(EINVAL, %s,
- _(Unable to set cpu bandwidth quota));
-goto cleanup;
-}
-
-if (quota  0)
-vm_quota = quota * vm-def-vcpus;
-else
-vm_quota = quota;
-if (qemuSetupCgroupVcpuBW(cgroup, period, vm_quota)  0)
-goto cleanup;
-}
-}
-
 if (priv-nvcpupids == 0 || priv-vcpupids[0] == vm-pid) {
 /* If we does not know VCPU-PID mapping or all vcpus run in the same
  * thread, we cannot control each vcpu.
@@ -617,10 +603,8 @@ int qemuSetupCgroupForVcpu(struct qemud_driver *driver, 
virDomainObjPtr vm)
 }
 
 if (period || quota) {
-if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_CPU)) 
{
-if (qemuSetupCgroupVcpuBW(cgroup_vcpu, period, quota)  0)
-goto cleanup;
-}
+if (qemuSetupCgroupVcpuBW(cgroup_vcpu, period, quota)  0)
+goto cleanup;
 }
 
 /* Set vcpupin in cgroup if vcpupin xml is provided */
@@ -632,7 +616,6 @@ int qemuSetupCgroupForVcpu(struct qemud_driver *driver, 
virDomainObjPtr vm)
 virCgroupFree(cgroup_vcpu);
 }
 
-virCgroupFree(cgroup_vcpu);
 virCgroupFree(cgroup);
 return 0;
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3b1bf2c..7d1d093 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7386,69 +7386,10 @@ qemuSetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr 
cgroup,
 qemuDomainObjPrivatePtr priv = vm-privateData;
 virCgroupPtr cgroup_vcpu = NULL;
 int rc;
-long long vm_quota = 0;
-long long old_quota = 0;
-unsigned long long old_period = 0;
 
 if (period == 0  quota == 0)
 return 0;
 
-/* Ensure that we can multiply by vcpus without overflowing. */
-if (quota  LLONG_MAX / vm-def-vcpus) {
-virReportSystemError(EINVAL, %s,
- _(Unable to set cpu bandwidth quota));
-goto cleanup;
-}
-
-if (quota  0)
-vm_quota = quota * vm-def-vcpus;
-else
-vm_quota = quota;
-
-rc = virCgroupGetCpuCfsQuota(cgroup, old_quota);
-if (rc  0) {
-virReportSystemError(-rc, %s,
- _(unable to get cpu bandwidth tunable));
-goto cleanup;
-}
-
-rc = virCgroupGetCpuCfsPeriod(cgroup, old_period);
-if (rc  0) {
-virReportSystemError(-rc, %s,
- _(unable to get cpu bandwidth period tunable));
-goto cleanup;
-}
-
-/*
- * If quota will be changed to a small value, we should modify vcpu's quota
- * first. Otherwise, we should modify vm's quota first.
- *
- * If period will be changed to a small value, we should modify vm's period
- * first. Otherwise, we should modify vcpu's period first.
- *
- * If both quota and period will be changed to a big/small value, we cannot
- * modify period and quota together.
- */
-if ((quota != 0)  (period != 0)) {
-if (((quota