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

Re: [libvirt] [PATCH v2 5/5] cputune_event: queue the event for cputune updates



On 09/12/2014 10:31 PM, John Ferlan wrote:

s//"something to describe what's being done!"

Might be nice to see what the event would look like when it's triggered.
May help in that documentation effort in the future.

On 09/10/2014 08:08 AM, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina redhat com>
---
  src/qemu/qemu_cgroup.c | 18 ++++++++++++-
  src/qemu/qemu_driver.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++
  2 files changed, 90 insertions(+), 1 deletion(-)


So this is only going to occur as asked in 4/4 in v1 when/if the
cputune.shares value is adjusted during qemuProcessStart.


Yes, that's right.

Also events are only triggered when someone uses the api that virsh
vcpupin, emulatorpin, and schedinfo(set) call.  Which is "ok' by me for
the onlist iothreadpin changes which don't yet have a similar
iothreadpin api.  Meaning my other series shouldn't be affected



[..]

@@ -4569,6 +4575,18 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,

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

As pointed out for 1/5 - this really could just be an snprintf()


+            goto cleanup;
+        }

The extra {} ^^^  and vvv are probably not necessary


[..]

@@ -4833,6 +4860,14 @@ qemuDomainPinEmulator(virDomainPtr dom,

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

ditto on the {}


[..]

@@ -9205,12 +9272,18 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,
          vmdef = NULL;
      }

+

Extraneous line


ACK with the adjustments mentioned.  I think you should just skip the
virSnprintf for this series and add one in another series to encompass
all callers (and of course add the 'rule' like Eric suggests in 1/5


Will fix the brackets and the extra line, thanks.

Pavel


John



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