[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v4 4/4] cputune_event: queue the event for cputune updates



On 09/22/2014 05:54 PM, John Ferlan wrote:


On 09/18/2014 04:39 AM, Pavel Hrdina wrote:
Now we have universal tunable event so we can use it for reporting
changes to user. The cputune values will be prefixed with "cputune" to
distinguish it from other tunable events.

Signed-off-by: Pavel Hrdina <phrdina redhat com>
---
  src/qemu/qemu_cgroup.c | 18 +++++++++++-
  src/qemu/qemu_driver.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++
  2 files changed, 93 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 9d39370..27dcba3 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -34,6 +34,7 @@
  #include "virscsi.h"
  #include "virstring.h"
  #include "virfile.h"
+#include "virtypedparam.h"

  #define VIR_FROM_THIS VIR_FROM_QEMU

@@ -676,6 +677,10 @@ static int
  qemuSetupCpuCgroup(virDomainObjPtr vm)
  {
      qemuDomainObjPrivatePtr priv = vm->privateData;
+    virObjectEventPtr event = NULL;
+    virTypedParameterPtr eventParams;

Consistency with other defs "eventParams = NULL".

Will fix that, thanks.


+    int eventNparams = 0;
+    int eventMaxparams = 0;

      if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU)) {
         if (vm->def->cputune.sharesSpecified) {
@@ -694,7 +699,18 @@ qemuSetupCpuCgroup(virDomainObjPtr vm)

          if (virCgroupGetCpuShares(priv->cgroup, &val) < 0)
              return -1;
-        vm->def->cputune.shares = val;
+        if (vm->def->cputune.shares != val) {
+            vm->def->cputune.shares = val;
+            if (virTypedParamsAddULLong(&eventParams, &eventNparams,
+                                        &eventMaxparams, "cputune.shares",

Perhaps this name space ('cputune.*') should be what goes into libvirt.h
(mentioned in the v2 3/5 review).

That is define and use

#define VIR_DOMAIN_EVENT_CPUTUNE_SHARES "cputune.shares"

+                                        val) < 0)
+                return -1;
+
+            event = virDomainEventTunableNewFromObj(vm, eventParams, eventNparams);
+        }
+
+        if (event)
+            qemuDomainEventQueue(vm->privateData, event);
      }

      return 0;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5fd89a3..22c6e55 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4538,6 +4538,12 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,
      virBitmapPtr pcpumap = NULL;
      virQEMUDriverConfigPtr cfg = NULL;
      virCapsPtr caps = NULL;
+    virObjectEventPtr event = NULL;
+    char paramField[VIR_TYPED_PARAM_FIELD_LENGTH] = "";
+    char *str = NULL;
+    virTypedParameterPtr eventParams = NULL;
+    int eventNparams = 0;
+    int eventMaxparams = 0;

      virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
                    VIR_DOMAIN_AFFECT_CONFIG, -1);
@@ -4645,6 +4651,18 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,

          if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
              goto cleanup;
+
+        if (snprintf(paramField, VIR_TYPED_PARAM_FIELD_LENGTH,
+                     "cputune.vcpupin%d", vcpu) < 0) {

#define VIR_DOMAIN_EVENT_CPUTUNE_VCPUPIN "cputune.vcpupin%u"

BTW: vcpu is an unsigned...  Somehow have to comment that the event will
*start with* this string with the postfix being the vCPU number as
defined by the guest.

+            goto cleanup;
+        }
+
+        str = virBitmapFormat(pcpumap);
+        if (virTypedParamsAddString(&eventParams, &eventNparams,
+                                    &eventMaxparams, paramField, str) < 0)
+            goto cleanup;
+
+        event = virDomainEventTunableNewFromDom(dom, eventParams, eventNparams);
      }

      if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
@@ -4680,6 +4698,9 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,
          virCgroupFree(&cgroup_vcpu);
      if (vm)
          virObjectUnlock(vm);
+    if (event)
+        qemuDomainEventQueue(driver, event);
+    VIR_FREE(str);
      virBitmapFree(pcpumap);
      virObjectUnref(caps);
      virObjectUnref(cfg);
@@ -4804,6 +4825,12 @@ qemuDomainPinEmulator(virDomainPtr dom,
      virBitmapPtr pcpumap = NULL;
      virQEMUDriverConfigPtr cfg = NULL;
      virCapsPtr caps = NULL;
+    virObjectEventPtr event = NULL;
+    char * str = NULL;
+    virTypedParameterPtr eventParams = NULL;
+    int eventNparams = 0;
+    int eventMaxparams = 0;
+

      virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
                    VIR_DOMAIN_AFFECT_CONFIG, -1);
@@ -4909,6 +4936,13 @@ qemuDomainPinEmulator(virDomainPtr dom,

          if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
              goto cleanup;
+
+        str = virBitmapFormat(pcpumap);
+        if (virTypedParamsAddString(&eventParams, &eventNparams,
+                                    &eventMaxparams, "cputune.emulatorpin", str) < 0)

#define VIR_DOMAIN_EVENT_CPUTUNE_EMULATORPIN "cputune.emulatorpin"


+            goto cleanup;
+
+        event = virDomainEventTunableNewFromDom(dom, eventParams, eventNparams);
      }

      if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
@@ -4938,6 +4972,9 @@ qemuDomainPinEmulator(virDomainPtr dom,
   cleanup:
      if (cgroup_emulator)
          virCgroupFree(&cgroup_emulator);
+    if (event)
+        qemuDomainEventQueue(driver, event);
+    VIR_FREE(str);
      virBitmapFree(pcpumap);
      virObjectUnref(caps);
      if (vm)
@@ -9096,6 +9133,10 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
      virQEMUDriverConfigPtr cfg = NULL;
      virCapsPtr caps = NULL;
      qemuDomainObjPrivatePtr priv;
+    virObjectEventPtr event = NULL;
+    virTypedParameterPtr eventParams = NULL;
+    int eventNparams = 0;
+    int eventMaxNparams = 0;

      virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
                    VIR_DOMAIN_AFFECT_CONFIG, -1);
@@ -9166,6 +9207,11 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,

                  vm->def->cputune.shares = val;
                  vm->def->cputune.sharesSpecified = true;
+
+                if (virTypedParamsAddULLong(&eventParams, &eventNparams,
+                                            &eventMaxNparams, "cputune.shares",

VIR_DOMAIN_EVENT_CPUTUNE_SHARES

+                                            val) < 0)
+                    goto cleanup;
              }

              if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
@@ -9183,6 +9229,11 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
                      goto cleanup;

                  vm->def->cputune.period = value_ul;
+
+                if (virTypedParamsAddULLong(&eventParams, &eventNparams,
+                                            &eventMaxNparams, "cputune.period",

#define VIR_DOMAIN_EVENT_CPUTUNE_VCPU_PERIOD "cputune.vcpu_period"

+                                            value_ul) < 0)
+                    goto cleanup;
              }

              if (flags & VIR_DOMAIN_AFFECT_CONFIG)
@@ -9197,6 +9248,11 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
                      goto cleanup;

                  vm->def->cputune.quota = value_l;
+
+                if (virTypedParamsAddLLong(&eventParams, &eventNparams,
+                                           &eventMaxNparams, "cputune.quota",

#define VIR_DOMAIN_EVENT_CPUTUNE_VCPU_QUOTA "cputune.vcpu_quota"

+                                           value_l) < 0)
+                    goto cleanup;
              }

              if (flags & VIR_DOMAIN_AFFECT_CONFIG)
@@ -9212,6 +9268,12 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
                      goto cleanup;

                  vm->def->cputune.emulator_period = value_ul;
+
+                if (virTypedParamsAddULLong(&eventParams, &eventNparams,
+                                            &eventMaxNparams,
+                                            "cputune.emulator_period",

#define VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_PERIOD "cputune.emulator_period"

+                                            value_ul) < 0)
+                    goto cleanup;
              }

              if (flags & VIR_DOMAIN_AFFECT_CONFIG)
@@ -9227,6 +9289,12 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
                      goto cleanup;

                  vm->def->cputune.emulator_quota = value_l;
+
+                if (virTypedParamsAddLLong(&eventParams, &eventNparams,
+                                           &eventMaxNparams,
+                                           "cputune.emulator_quota",

#define VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_QUOTA "cputune.emulator_quota"

+                                           value_l) < 0)
+                    goto cleanup;
              }

              if (flags & VIR_DOMAIN_AFFECT_CONFIG)
@@ -9237,6 +9305,12 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
      if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
          goto cleanup;

+    if (eventNparams) {
+        event = virDomainEventTunableNewFromDom(dom, eventParams, eventNparams);

FWIW: This is the code that got me to thinking in 2/4 about the usage of
eventMaxNparams vs. just eventNparams.  They I believe are the same, but
can we "foresee" any reason they could/would differ.

They are not the same. In the eventNparams there is current number of typedParameter stored in eventParams and the eventMaxNparams contains the allocated size of the eventParams and they definitely could be different. The reason is that we are using VIR_RESIZE_N instead of VIR_EXPAND_N and it reallocate the eventParams geometrically to speedup things.


In any case, I think with the usage of libvirt.h #define's this is ACK-able.

I'll add the definitions into the libvirt.h and send another version, thanks.

Pavel


John

+        eventNparams = 0;
+        if (event)
+            qemuDomainEventQueue(driver, event);
+    }

      if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
          rc = virDomainSaveConfig(cfg->configDir, vmdef);
@@ -9253,6 +9327,8 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
      virDomainDefFree(vmdef);
      if (vm)
          virObjectUnlock(vm);
+    if (eventNparams)
+        virTypedParamsFree(eventParams, eventNparams);
      virObjectUnref(caps);
      virObjectUnref(cfg);
      return ret;



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]