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(a)redhat.com&gt; >--- > 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.