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

Re: [libvirt] [PATCH] Rename tunable event constants



On 09/25/2014 06:51 PM, Daniel P. Berrange wrote:
For the new VIR_DOMAIN_EVENT_ID_TUNABLE event we have a bunch of
constants added

    VIR_DOMAIN_EVENT_CPUTUNE_<blah>
    VIR_DOMAIN_EVENT_BLKDEVTUNE_<blah>

This naming convention is bad for two reasons

   - There is no common prefix unique for the events to both
     relate them, and distinguish them from other event
     constants

   - The values associated with the constants were chosen
     to match the names used with virConnectGetAllDomainStats
     so having EVENT in the constant name is not applicable in
     that respect

This patch proposes renaming the constants to

     VIR_DOMAIN_TUNABLE_CPU_<blah>
     VIR_DOMAIN_TUNABLE_BLKDEV_<blah>

ie, given them a common VIR_DOMAIN_TUNABLE prefix.

Signed-off-by: Daniel P. Berrange <berrange redhat com>
---
  include/libvirt/libvirt.h.in | 56 ++++++++++++++++++++++----------------------
  src/qemu/qemu_cgroup.c       |  2 +-
  src/qemu/qemu_driver.c       | 28 +++++++++++-----------
  3 files changed, 43 insertions(+), 43 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 93c9a48..2ff5204 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -5204,119 +5204,119 @@ typedef void (*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn,
                                                             void *opaque);

  /**
- * VIR_DOMAIN_EVENT_CPUTUNE_VCPUPIN:
+ * VIR_DOMAIN_TUNABLE_CPU_VCPUPIN:
   *
   * Macro represents formatted pinning for one vcpu specified by id which is
   * appended to the parameter name, for example "cputune.vcpupin1",
   * as VIR_TYPED_PARAM_STRING.
   */
-#define VIR_DOMAIN_EVENT_CPUTUNE_VCPUPIN "cputune.vcpupin%u"
+#define VIR_DOMAIN_TUNABLE_CPU_VCPUPIN "cputune.vcpupin%u"

  /**
- * VIR_DOMAIN_EVENT_CPUTUNE_EMULATORIN:
+ * VIR_DOMAIN_TUNABLE_CPU_EMULATORIN:
   *
   * Macro represents formatted pinning for emulator process,
   * as VIR_TYPED_PARAM_STRING.
   */
-#define VIR_DOMAIN_EVENT_CPUTUNE_EMULATORIN "cputune.emulatorpin"
+#define VIR_DOMAIN_TUNABLE_CPU_EMULATORIN "cputune.emulatorpin"

  /**
- * VIR_DOMAIN_EVENT_CPUTUNE_CPU_SHARES:
+ * VIR_DOMAIN_TUNABLE_CPU_CPU_SHARES:
   *
   * Macro represents proportional weight of the scheduler used on the
   * host cpu, when using the posix scheduler, as VIR_TYPED_PARAM_ULLONG.
   */
-#define VIR_DOMAIN_EVENT_CPUTUNE_CPU_SHARES "cputune.cpu_shares"
+#define VIR_DOMAIN_TUNABLE_CPU_CPU_SHARES "cputune.cpu_shares"

  /**
- * VIR_DOMAIN_EVENT_CPUTUNE_VCPU_PERIOD:
+ * VIR_DOMAIN_TUNABLE_CPU_VCPU_PERIOD:
   *
   * Macro represents the enforcement period for a quota, in microseconds,
   * for vcpus only, when using the posix scheduler, as VIR_TYPED_PARAM_ULLONG.
   */
-#define VIR_DOMAIN_EVENT_CPUTUNE_VCPU_PERIOD "cputune.vcpu_period"
+#define VIR_DOMAIN_TUNABLE_CPU_VCPU_PERIOD "cputune.vcpu_period"

  /**
- * VIR_DOMAIN_EVENT_CPUTUNE_VCPU_QUOTA:
+ * VIR_DOMAIN_TUNABLE_CPU_VCPU_QUOTA:
   *
   * Macro represents the maximum bandwidth to be used within a period for
   * vcpus only, when using the posix scheduler, as VIR_TYPED_PARAM_LLONG.
   */
-#define VIR_DOMAIN_EVENT_CPUTUNE_VCPU_QUOTA "cputune.vcpu_quota"
+#define VIR_DOMAIN_TUNABLE_CPU_VCPU_QUOTA "cputune.vcpu_quota"

  /**
- * VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_PERIOD:
+ * VIR_DOMAIN_TUNABLE_CPU_EMULATOR_PERIOD:
   *
   * Macro represents the enforcement period for a quota in microseconds,
   * when using the posix scheduler, for all emulator activity not tied to
   * vcpus, as VIR_TYPED_PARAM_ULLONG.
   */
-#define VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_PERIOD "cputune.emulator_period"
+#define VIR_DOMAIN_TUNABLE_CPU_EMULATOR_PERIOD "cputune.emulator_period"

  /**
- * VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_QUOTA:
+ * VIR_DOMAIN_TUNABLE_CPU_EMULATOR_QUOTA:
   *
   * Macro represents the maximum bandwidth to be used within a period for
   * all emulator activity not tied to vcpus, when using the posix scheduler,
   * as an VIR_TYPED_PARAM_LLONG.
   */
-#define VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_QUOTA "cputune.emulator_quota"
+#define VIR_DOMAIN_TUNABLE_CPU_EMULATOR_QUOTA "cputune.emulator_quota"

  /**
- * VIR_DOMAIN_EVENT_BLKDEVIOTUNE_DISK:
+ * VIR_DOMAIN_TUNABLE_BLKDEV_DISK:
   *
   * Macro represents the name of guest disk for which the values are updated,
   * as VIR_TYPED_PARAM_STRING.
   */
-#define VIR_DOMAIN_EVENT_BLKDEVIOTUNE_DISK "blkdeviotune.disk"
+#define VIR_DOMAIN_TUNABLE_BLKDEV_DISK "blkdeviotune.disk"

  /**
- * VIR_DOMAIN_EVENT_BLKDEVIOTUNE_TOTAL_BYTES_SEC:
+ * VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC:
   *
   * Marco represents the total throughput limit in bytes per second,
   * as VIR_TYPED_PARAM_ULLONG.
   */
-#define VIR_DOMAIN_EVENT_BLKDEVIOTUNE_TOTAL_BYTES_SEC "blkdeviotune.total_bytes_sec"
+#define VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC "blkdeviotune.total_bytes_sec"

  /**
- * VIR_DOMAIN_EVENT_BLKDEVIOTUNE_READ_BYTES_SEC:
+ * VIR_DOMAIN_TUNABLE_BLKDEV_READ_BYTES_SEC:
   *
   * Marco represents the read throughput limit in bytes per second,
   * as VIR_TYPED_PARAM_ULLONG.
   */
-#define VIR_DOMAIN_EVENT_BLKDEVIOTUNE_READ_BYTES_SEC "blkdeviotune.read_bytes_sec"
+#define VIR_DOMAIN_TUNABLE_BLKDEV_READ_BYTES_SEC "blkdeviotune.read_bytes_sec"

  /**
- * VIR_DOMAIN_EVENT_BLKDEVIOTUNE_WRITE_BYTES_SEC:
+ * VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_BYTES_SEC:
   *
   * Macro represents the write throughput limit in bytes per second,
   * as VIR_TYPED_PARAM_ULLONG.
   */
-#define VIR_DOMAIN_EVENT_BLKDEVIOTUNE_WRITE_BYTES_SEC "blkdeviotune.write_bytes_sec"
+#define VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_BYTES_SEC "blkdeviotune.write_bytes_sec"

  /**
- * VIR_DOMAIN_EVENT_BLKDEVIOTUNE_TOTAL_IOPS_SEC:
+ * VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_IOPS_SEC:
   *
   * Macro represents the total I/O operations per second,
   * as VIR_TYPED_PARAM_ULLONG.
   */
-#define VIR_DOMAIN_EVENT_BLKDEVIOTUNE_TOTAL_IOPS_SEC "blkdeviotune.total_iops_sec"
+#define VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_IOPS_SEC "blkdeviotune.total_iops_sec"

  /**
- * VIR_DOMAIN_EVENT_BLKDEVIOTUNE_READ_IOPS_SEC:
+ * VIR_DOMAIN_TUNABLE_BLKDEV_READ_IOPS_SEC:
   *
   * Macro represents the read I/O operations per second,
   * as VIR_TYPED_PARAM_ULLONG.
   */
-#define VIR_DOMAIN_EVENT_BLKDEVIOTUNE_READ_IOPS_SEC "blkdeviotune.read_iops_sec"
+#define VIR_DOMAIN_TUNABLE_BLKDEV_READ_IOPS_SEC "blkdeviotune.read_iops_sec"

  /**
- * VIR_DOMAIN_EVENT_BLKDEVIOTUNE_WRITE_IOPS_SEC:
+ * VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC:
   *
   * Macro represents the write I/O operations per second,
   * as VIR_TYPED_PARAM_ULLONG.
   */
-#define VIR_DOMAIN_EVENT_BLKDEVIOTUNE_WRITE_IOPS_SEC "blkdeviotune.write_iops_sec"
+#define VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC "blkdeviotune.write_iops_sec"

  /**
   * virConnectDomainEventTunableCallback:
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 300946a..8819943 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -703,7 +703,7 @@ qemuSetupCpuCgroup(virDomainObjPtr vm)
              vm->def->cputune.shares = val;
              if (virTypedParamsAddULLong(&eventParams, &eventNparams,
                                          &eventMaxparams,
-                                        VIR_DOMAIN_EVENT_CPUTUNE_CPU_SHARES,
+                                        VIR_DOMAIN_TUNABLE_CPU_CPU_SHARES,
                                          val) < 0)
                  return -1;

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 117138a..6e792a6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4653,7 +4653,7 @@ qemuDomainPinVcpuFlags(virDomainPtr dom,
              goto cleanup;

          if (snprintf(paramField, VIR_TYPED_PARAM_FIELD_LENGTH,
-                     VIR_DOMAIN_EVENT_CPUTUNE_VCPUPIN, vcpu) < 0) {
+                     VIR_DOMAIN_TUNABLE_CPU_VCPUPIN, vcpu) < 0) {
              goto cleanup;
          }

@@ -4940,7 +4940,7 @@ qemuDomainPinEmulator(virDomainPtr dom,
          str = virBitmapFormat(pcpumap);
          if (virTypedParamsAddString(&eventParams, &eventNparams,
                                      &eventMaxparams,
-                                    VIR_DOMAIN_EVENT_CPUTUNE_EMULATORIN,
+                                    VIR_DOMAIN_TUNABLE_CPU_EMULATORIN,
                                      str) < 0)
              goto cleanup;

@@ -9318,7 +9318,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,

                  if (virTypedParamsAddULLong(&eventParams, &eventNparams,
                                              &eventMaxNparams,
-                                            VIR_DOMAIN_EVENT_CPUTUNE_CPU_SHARES,
+                                            VIR_DOMAIN_TUNABLE_CPU_CPU_SHARES,
                                              val) < 0)
                      goto cleanup;
              }
@@ -9341,7 +9341,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,

                  if (virTypedParamsAddULLong(&eventParams, &eventNparams,
                                              &eventMaxNparams,
-                                            VIR_DOMAIN_EVENT_CPUTUNE_VCPU_PERIOD,
+                                            VIR_DOMAIN_TUNABLE_CPU_VCPU_PERIOD,
                                              value_ul) < 0)
                      goto cleanup;
              }
@@ -9361,7 +9361,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,

                  if (virTypedParamsAddLLong(&eventParams, &eventNparams,
                                             &eventMaxNparams,
-                                           VIR_DOMAIN_EVENT_CPUTUNE_VCPU_QUOTA,
+                                           VIR_DOMAIN_TUNABLE_CPU_VCPU_QUOTA,
                                             value_l) < 0)
                      goto cleanup;
              }
@@ -9382,7 +9382,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,

                  if (virTypedParamsAddULLong(&eventParams, &eventNparams,
                                              &eventMaxNparams,
-                                            VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_PERIOD,
+                                            VIR_DOMAIN_TUNABLE_CPU_EMULATOR_PERIOD,
                                              value_ul) < 0)
                      goto cleanup;
              }
@@ -9403,7 +9403,7 @@ qemuDomainSetSchedulerParametersFlags(virDomainPtr dom,

                  if (virTypedParamsAddLLong(&eventParams, &eventNparams,
                                             &eventMaxNparams,
-                                           VIR_DOMAIN_EVENT_CPUTUNE_EMULATOR_QUOTA,
+                                           VIR_DOMAIN_TUNABLE_CPU_EMULATOR_QUOTA,
                                             value_l) < 0)
                      goto cleanup;
              }
@@ -16321,7 +16321,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
          goto endjob;

      if (virTypedParamsAddString(&eventParams, &eventNparams, &eventMaxparams,
-                                VIR_DOMAIN_EVENT_BLKDEVIOTUNE_DISK, disk) < 0)
+                                VIR_DOMAIN_TUNABLE_BLKDEV_DISK, disk) < 0)
          goto endjob;

      for (i = 0; i < nparams; i++) {
@@ -16339,7 +16339,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
              set_bytes = true;
              if (virTypedParamsAddULLong(&eventParams, &eventNparams,
                                          &eventMaxparams,
-                                        VIR_DOMAIN_EVENT_BLKDEVIOTUNE_TOTAL_BYTES_SEC,
+                                        VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC,
                                          param->value.ul) < 0)
                  goto endjob;
          } else if (STREQ(param->field,
@@ -16348,7 +16348,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
              set_bytes = true;
              if (virTypedParamsAddULLong(&eventParams, &eventNparams,
                                          &eventMaxparams,
-                                        VIR_DOMAIN_EVENT_BLKDEVIOTUNE_READ_BYTES_SEC,
+                                        VIR_DOMAIN_TUNABLE_BLKDEV_READ_BYTES_SEC,
                                          param->value.ul) < 0)
                  goto endjob;
          } else if (STREQ(param->field,
@@ -16357,7 +16357,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
              set_bytes = true;
              if (virTypedParamsAddULLong(&eventParams, &eventNparams,
                                          &eventMaxparams,
-                                        VIR_DOMAIN_EVENT_BLKDEVIOTUNE_WRITE_BYTES_SEC,
+                                        VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_BYTES_SEC,
                                          param->value.ul) < 0)
                  goto endjob;
          } else if (STREQ(param->field,
@@ -16366,7 +16366,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
              set_iops = true;
              if (virTypedParamsAddULLong(&eventParams, &eventNparams,
                                          &eventMaxparams,
-                                        VIR_DOMAIN_EVENT_BLKDEVIOTUNE_TOTAL_IOPS_SEC,
+                                        VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_IOPS_SEC,
                                          param->value.ul) < 0)
                  goto endjob;
          } else if (STREQ(param->field,
@@ -16375,7 +16375,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
              set_iops = true;
              if (virTypedParamsAddULLong(&eventParams, &eventNparams,
                                          &eventMaxparams,
-                                        VIR_DOMAIN_EVENT_BLKDEVIOTUNE_READ_IOPS_SEC,
+                                        VIR_DOMAIN_TUNABLE_BLKDEV_READ_IOPS_SEC,
                                          param->value.ul) < 0)
                  goto endjob;
          } else if (STREQ(param->field,
@@ -16384,7 +16384,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
              set_iops = true;
              if (virTypedParamsAddULLong(&eventParams, &eventNparams,
                                          &eventMaxparams,
-                                        VIR_DOMAIN_EVENT_BLKDEVIOTUNE_WRITE_IOPS_SEC,
+                                        VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC,
                                          param->value.ul) < 0)
                  goto endjob;
          }



I would probably leave the "IO" in the macro name:

    VIR_DOMAIN_TUNABLE_BLKDEVIO_DISK

to make it clear that it's for tuning I/O values.

ACK whether you change the BLKDEV to BLKDEVIO or not.

Pavel


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