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

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

 



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@xxxxxxxxxx>
---
  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


--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]