5:38 p.m.
v1: http://www.redhat.com/archives/libvir-list/2016-September/msg01090.html
Differences since v1:
Patches 1-5 are new...
Patch 1 is essentially the former patch 8.5 added after initial review
based on a review comment. Erik Skultety and I went back and forth
on this one a few times and this is the result
Patch 2 is a result of being frustrated by a generic error message when
I know we could return whatever qemu gives us
Patch 3 is simple code motion - the conf_disk is closer to usage now
Patch 4 & 5 create and use the same algorithm to set the default values
that weren't provided.
Patches 6-7 have already been ACK'd... I've been waiting to push because
they will interfere with libvirt-perl builds and since the 2.3.0 hasn't
been published yet, I'm still holding onto them.
Patch 8 is the former patch 8, but reworked mostly in qemu_driver.c, but
also a change in qemu_monitor_json.c to use "P:" instead of "U:"
for
then length values. For _length a 0 is an invalid value, so we'll use
it to mean "nothing changed"
Patch 9 is the former patch 11 and is mostly intact, with major difference
being the removal of the CHECK_IOTUNE_MAX_LENGTH macro (it was causing
domain disappearance)
Patch 10 is the former patch 12 and is unchanged. It also has been ACKd
Patch 11-12 are new - it's the virsh code.
John Ferlan (12):
qemu: Create a macro to handle setting bytes/iops iotune values
qemu: Return real error message for block_set_io_throttle
qemu: Move setting of conf_disk in qemuDomainSetBlockIoTune
qemu: Introduce qemuDomainSetBlockIoTuneSetDefaults
qemu: Use qemuDomainsetBlockIoTuneSetDefaults for config
include: Add new definitions for duration for bps/iops throttling
caps: Add new capability for the bps/iops throttling length
qemu: Add length for bps/iops throttling parameters to driver
conf: Add support for blkiotune "_length" options
qemu: Add the length options to the iotune command line
virsh: Create a macro to add IOTUNE values
virsh: Add _length parameters to virsh output
docs/formatdomain.html.in | 40 ++-
docs/schemas/domaincommon.rng | 38 +++
include/libvirt/libvirt-domain.h | 102 ++++++
src/conf/domain_conf.c | 24 +-
src/conf/domain_conf.h | 6 +
src/qemu/qemu_capabilities.c | 2 +
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 21 ++
src/qemu/qemu_driver.c | 341 +++++++++++----------
src/qemu/qemu_monitor.c | 7 +-
src/qemu/qemu_monitor.h | 3 +-
src/qemu/qemu_monitor_json.c | 39 ++-
src/qemu/qemu_monitor_json.h | 3 +-
.../caps_2.6.0-gicv2.aarch64.xml | 1 +
.../caps_2.6.0-gicv3.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 1 +
tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 +
tests/qemumonitorjsontest.c | 17 +-
.../qemuxml2argv-blkdeviotune-max-length.args | 34 ++
.../qemuxml2argv-blkdeviotune-max-length.xml | 65 ++++
tests/qemuxml2argvtest.c | 4 +
.../qemuxml2xmlout-blkdeviotune-max-length.xml | 1 +
tests/qemuxml2xmltest.c | 1 +
tools/virsh-domain.c | 196 +++++-------
tools/virsh.pod | 21 ++
26 files changed, 673 insertions(+), 298 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max-length.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max-length.xml
create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-blkdeviotune-max-length.xml
--
2.7.4
5:38 p.m.
New subject: [libvirt] [PATCH v2 01/12] qemu: Create a macro to handle setting bytes/iops iotune values
Create a macros to hide all the comparisons for each of the fields.
Add a 'continue;' for a compiler hint that we only need to find one
this should be similar enough to the if - elseif - elseif logic.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_driver.c | 153 +++++++++++--------------------------------------
1 file changed, 35 insertions(+), 118 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 29a7e3f..3ce3f2d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17371,6 +17371,18 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
VIR_DOMAIN_TUNABLE_BLKDEV_DISK, path) < 0)
goto endjob;
+#define SET_IOTUNE_FIELD(FIELD, BOOL, CONST) \
+ if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_##CONST)) { \
+ info.FIELD = param->value.ul; \
+ BOOL = true; \
+ if (virTypedParamsAddULLong(&eventParams, &eventNparams, \
+ &eventMaxparams, \
+ VIR_DOMAIN_TUNABLE_BLKDEV_##CONST, \
+ param->value.ul) < 0) \
+ goto endjob; \
+ continue; \
+ }
+
for (i = 0; i < nparams; i++) {
virTypedParameterPtr param = ¶ms[i];
@@ -17381,124 +17393,29 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
goto endjob;
}
- if (STREQ(param->field, VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC)) {
- info.total_bytes_sec = param->value.ul;
- set_bytes = true;
- if (virTypedParamsAddULLong(&eventParams, &eventNparams,
- &eventMaxparams,
- VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC,
- param->value.ul) < 0)
- goto endjob;
- } else if (STREQ(param->field,
- VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC)) {
- info.read_bytes_sec = param->value.ul;
- set_bytes = true;
- if (virTypedParamsAddULLong(&eventParams, &eventNparams,
- &eventMaxparams,
- VIR_DOMAIN_TUNABLE_BLKDEV_READ_BYTES_SEC,
- param->value.ul) < 0)
- goto endjob;
- } else if (STREQ(param->field,
- VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC)) {
- info.write_bytes_sec = param->value.ul;
- set_bytes = true;
- if (virTypedParamsAddULLong(&eventParams, &eventNparams,
- &eventMaxparams,
- VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_BYTES_SEC,
- param->value.ul) < 0)
- goto endjob;
- } else if (STREQ(param->field,
- VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC)) {
- info.total_iops_sec = param->value.ul;
- set_iops = true;
- if (virTypedParamsAddULLong(&eventParams, &eventNparams,
- &eventMaxparams,
- VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_IOPS_SEC,
- param->value.ul) < 0)
- goto endjob;
- } else if (STREQ(param->field,
- VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC)) {
- info.read_iops_sec = param->value.ul;
- set_iops = true;
- if (virTypedParamsAddULLong(&eventParams, &eventNparams,
- &eventMaxparams,
- VIR_DOMAIN_TUNABLE_BLKDEV_READ_IOPS_SEC,
- param->value.ul) < 0)
- goto endjob;
- } else if (STREQ(param->field,
- VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC)) {
- info.write_iops_sec = param->value.ul;
- set_iops = true;
- if (virTypedParamsAddULLong(&eventParams, &eventNparams,
- &eventMaxparams,
- VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC,
- param->value.ul) < 0)
- goto endjob;
- } else if (STREQ(param->field,
- VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX)) {
- info.total_bytes_sec_max = param->value.ul;
- set_bytes_max = true;
- if (virTypedParamsAddULLong(&eventParams, &eventNparams,
- &eventMaxparams,
- VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX,
- param->value.ul) < 0)
- goto endjob;
- } else if (STREQ(param->field,
- VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX)) {
- info.read_bytes_sec_max = param->value.ul;
- set_bytes_max = true;
- if (virTypedParamsAddULLong(&eventParams, &eventNparams,
- &eventMaxparams,
- VIR_DOMAIN_TUNABLE_BLKDEV_READ_BYTES_SEC_MAX,
- param->value.ul) < 0)
- goto endjob;
- } else if (STREQ(param->field,
- VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX)) {
- info.write_bytes_sec_max = param->value.ul;
- set_bytes_max = true;
- if (virTypedParamsAddULLong(&eventParams, &eventNparams,
- &eventMaxparams,
- VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_BYTES_SEC_MAX,
- param->value.ul) < 0)
- goto endjob;
- } else if (STREQ(param->field,
- VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX)) {
- info.total_iops_sec_max = param->value.ul;
- set_iops_max = true;
- if (virTypedParamsAddULLong(&eventParams, &eventNparams,
- &eventMaxparams,
- VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX,
- param->value.ul) < 0)
- goto endjob;
- } else if (STREQ(param->field,
- VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX)) {
- info.read_iops_sec_max = param->value.ul;
- set_iops_max = true;
- if (virTypedParamsAddULLong(&eventParams, &eventNparams,
- &eventMaxparams,
- VIR_DOMAIN_TUNABLE_BLKDEV_READ_IOPS_SEC_MAX,
- param->value.ul) < 0)
- goto endjob;
- } else if (STREQ(param->field,
- VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX)) {
- info.write_iops_sec_max = param->value.ul;
- set_iops_max = true;
- if (virTypedParamsAddULLong(&eventParams, &eventNparams,
- &eventMaxparams,
- VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC_MAX,
- param->value.ul) < 0)
- goto endjob;
- } else if (STREQ(param->field,
- VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC)) {
- info.size_iops_sec = param->value.ul;
- set_size_iops = true;
- if (virTypedParamsAddULLong(&eventParams, &eventNparams,
- &eventMaxparams,
- VIR_DOMAIN_TUNABLE_BLKDEV_SIZE_IOPS_SEC,
- param->value.ul) < 0)
- goto endjob;
- }
- }
+ SET_IOTUNE_FIELD(total_bytes_sec, set_bytes, TOTAL_BYTES_SEC);
+ SET_IOTUNE_FIELD(read_bytes_sec, set_bytes, READ_BYTES_SEC);
+ SET_IOTUNE_FIELD(write_bytes_sec, set_bytes, WRITE_BYTES_SEC);
+ SET_IOTUNE_FIELD(total_iops_sec, set_iops, TOTAL_IOPS_SEC);
+ SET_IOTUNE_FIELD(read_iops_sec, set_iops, READ_IOPS_SEC);
+ SET_IOTUNE_FIELD(write_iops_sec, set_iops, WRITE_IOPS_SEC);
+
+ SET_IOTUNE_FIELD(total_bytes_sec_max, set_bytes_max,
+ TOTAL_BYTES_SEC_MAX);
+ SET_IOTUNE_FIELD(read_bytes_sec_max, set_bytes_max,
+ READ_BYTES_SEC_MAX);
+ SET_IOTUNE_FIELD(write_bytes_sec_max, set_bytes_max,
+ WRITE_BYTES_SEC_MAX);
+ SET_IOTUNE_FIELD(total_iops_sec_max, set_iops_max,
+ TOTAL_IOPS_SEC_MAX);
+ SET_IOTUNE_FIELD(read_iops_sec_max, set_iops_max,
+ READ_IOPS_SEC_MAX);
+ SET_IOTUNE_FIELD(write_iops_sec_max, set_iops_max,
+ WRITE_IOPS_SEC_MAX);
+ SET_IOTUNE_FIELD(size_iops_sec, set_size_iops, SIZE_IOPS_SEC);
+ }
+
+#undef SET_IOTUNE_FIELD
if ((info.total_bytes_sec && info.read_bytes_sec) ||
(info.total_bytes_sec && info.write_bytes_sec)) {
--
2.7.4
9:40 a.m.
New subject: [libvirt] [PATCH v2 01/12] qemu: Create a macro to handle setting bytes/iops iotune values
On Thu, Oct 06, 2016 at 06:38:49PM -0400, John Ferlan wrote:
Create a macros to hide all the comparisons for each of the fields.
Add a 'continue;' for a compiler hint that we only need to find one
this should be similar enough to the if - elseif - elseif logic.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_driver.c | 153 +++++++++++--------------------------------------
1 file changed, 35 insertions(+), 118 deletions(-)
ACK
Erik
5:38 p.m.
New subject: [libvirt] [PATCH v2 02/12] qemu: Return real error message for block_set_io_throttle
This patch will also adjust the qemuMonitorJSONSetBlockIoThrottle error
procession so that rather than returning/displaying:
"error: internal error: Unexpected error"
Fetch the actual error message from qemu and display that
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 349a64f..744c878 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4615,15 +4615,19 @@ int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon,
goto cleanup;
if (virJSONValueObjectHasKey(result, "error")) {
- if (qemuMonitorJSONHasError(result, "DeviceNotActive"))
+ if (qemuMonitorJSONHasError(result, "DeviceNotActive")) {
virReportError(VIR_ERR_OPERATION_INVALID,
_("No active operation on device: %s"), device);
- else if (qemuMonitorJSONHasError(result, "NotSupported"))
+ } else if (qemuMonitorJSONHasError(result, "NotSupported")) {
virReportError(VIR_ERR_OPERATION_INVALID,
_("Operation is not supported for device: %s"),
device);
- else
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Unexpected error"));
+ } else {
+ virJSONValuePtr error = virJSONValueObjectGet(result, "error");
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unable to execute '%s', unexpected error:
'%s'"),
+ qemuMonitorJSONCommandName(cmd),
+ qemuMonitorJSONStringifyError(error));
+ }
goto cleanup;
}
--
2.7.4
9:48 a.m.
New subject: [libvirt] [PATCH v2 02/12] qemu: Return real error message for block_set_io_throttle
On Thu, Oct 06, 2016 at 06:38:50PM -0400, John Ferlan wrote:
This patch will also adjust the qemuMonitorJSONSetBlockIoThrottle
error
procession so that rather than returning/displaying:
"error: internal error: Unexpected error"
Fetch the actual error message from qemu and display that
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
ACK
Erik
5:38 p.m.
New subject: [libvirt] [PATCH v2 03/12] qemu: Move setting of conf_disk in qemuDomainSetBlockIoTune
Since persistent_def is the only place that uses it, let's just keep
it closer to where it's used.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_driver.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3ce3f2d..78e917e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17449,15 +17449,6 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
goto endjob;
}
- if (persistentDef) {
- if (!(conf_disk = virDomainDiskByName(persistentDef, path, true))) {
- virReportError(VIR_ERR_INVALID_ARG,
- _("missing persistent configuration for disk
'%s'"),
- path);
- goto endjob;
- }
- }
-
if (def) {
supportMaxOptions = virQEMUCapsGet(priv->qemuCaps,
QEMU_CAPS_DRIVE_IOTUNE_MAX);
@@ -17550,6 +17541,12 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
}
if (persistentDef) {
+ if (!(conf_disk = virDomainDiskByName(persistentDef, path, true))) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("missing persistent configuration for disk
'%s'"),
+ path);
+ goto endjob;
+ }
oldinfo = &conf_disk->blkdeviotune;
if (!set_bytes) {
info.total_bytes_sec = oldinfo->total_bytes_sec;
--
2.7.4
10:06 a.m.
New subject: [libvirt] [PATCH v2 03/12] qemu: Move setting of conf_disk in qemuDomainSetBlockIoTune
On Thu, Oct 06, 2016 at 06:38:51PM -0400, John Ferlan wrote:
Since persistent_def is the only place that uses it, let's just
keep
it closer to where it's used.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_driver.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
ACK
Erik
5:38 p.m.
New subject: [libvirt] [PATCH v2 04/12] qemu: Introduce qemuDomainSetBlockIoTuneSetDefaults
Create a helper to set the bytes/iops iotune default values based on
the current qemu setting
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_driver.c | 66 ++++++++++++++++++++++++++++++--------------------
1 file changed, 40 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 78e917e..fcce3f0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17286,6 +17286,43 @@ qemuDomainOpenGraphicsFD(virDomainPtr dom,
return ret;
}
+
+/* If the user didn't specify bytes limits, inherit previous values;
+ * likewise if the user didn't specify iops limits. */
+static void
+qemuDomainSetBlockIoTuneSetDefaults(virDomainBlockIoTuneInfoPtr newinfo,
+ virDomainBlockIoTuneInfoPtr oldinfo,
+ bool set_bytes,
+ bool set_iops,
+ bool set_bytes_max,
+ bool set_iops_max,
+ bool set_size_iops)
+{
+ if (!set_bytes) {
+ newinfo->total_bytes_sec = oldinfo->total_bytes_sec;
+ newinfo->read_bytes_sec = oldinfo->read_bytes_sec;
+ newinfo->write_bytes_sec = oldinfo->write_bytes_sec;
+ }
+ if (!set_bytes_max) {
+ newinfo->total_bytes_sec_max = oldinfo->total_bytes_sec_max;
+ newinfo->read_bytes_sec_max = oldinfo->read_bytes_sec_max;
+ newinfo->write_bytes_sec_max = oldinfo->write_bytes_sec_max;
+ }
+ if (!set_iops) {
+ newinfo->total_iops_sec = oldinfo->total_iops_sec;
+ newinfo->read_iops_sec = oldinfo->read_iops_sec;
+ newinfo->write_iops_sec = oldinfo->write_iops_sec;
+ }
+ if (!set_iops_max) {
+ newinfo->total_iops_sec_max = oldinfo->total_iops_sec_max;
+ newinfo->read_iops_sec_max = oldinfo->read_iops_sec_max;
+ newinfo->write_iops_sec_max = oldinfo->write_iops_sec_max;
+ }
+ if (!set_size_iops)
+ newinfo->size_iops_sec = oldinfo->size_iops_sec;
+}
+
+
static int
qemuDomainSetBlockIoTune(virDomainPtr dom,
const char *path,
@@ -17473,32 +17510,9 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
if (!(device = qemuAliasFromDisk(disk)))
goto endjob;
- /* If the user didn't specify bytes limits, inherit previous
- * values; likewise if the user didn't specify iops
- * limits. */
- oldinfo = &disk->blkdeviotune;
- if (!set_bytes) {
- info.total_bytes_sec = oldinfo->total_bytes_sec;
- info.read_bytes_sec = oldinfo->read_bytes_sec;
- info.write_bytes_sec = oldinfo->write_bytes_sec;
- }
- if (!set_bytes_max) {
- info.total_bytes_sec_max = oldinfo->total_bytes_sec_max;
- info.read_bytes_sec_max = oldinfo->read_bytes_sec_max;
- info.write_bytes_sec_max = oldinfo->write_bytes_sec_max;
- }
- if (!set_iops) {
- info.total_iops_sec = oldinfo->total_iops_sec;
- info.read_iops_sec = oldinfo->read_iops_sec;
- info.write_iops_sec = oldinfo->write_iops_sec;
- }
- if (!set_iops_max) {
- info.total_iops_sec_max = oldinfo->total_iops_sec_max;
- info.read_iops_sec_max = oldinfo->read_iops_sec_max;
- info.write_iops_sec_max = oldinfo->write_iops_sec_max;
- }
- if (!set_size_iops)
- info.size_iops_sec = oldinfo->size_iops_sec;
+ qemuDomainSetBlockIoTuneSetDefaults(&info, &disk->blkdeviotune,
+ set_bytes, set_iops, set_bytes_max,
+ set_iops_max, set_size_iops);
#define CHECK_MAX(val) \
do { \
--
2.7.4
8:05 a.m.
New subject: [libvirt] [PATCH v2 04/12] qemu: Introduce qemuDomainSetBlockIoTuneSetDefaults
On Thu, Oct 06, 2016 at 06:38:52PM -0400, John Ferlan wrote:
Create a helper to set the bytes/iops iotune default values based on
the current qemu setting
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_driver.c | 66 ++++++++++++++++++++++++++++++--------------------
1 file changed, 40 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 78e917e..fcce3f0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17286,6 +17286,43 @@ qemuDomainOpenGraphicsFD(virDomainPtr dom,
return ret;
}
+
+/* If the user didn't specify bytes limits, inherit previous values;
+ * likewise if the user didn't specify iops limits. */
+static void
+qemuDomainSetBlockIoTuneSetDefaults(virDomainBlockIoTuneInfoPtr newinfo,
+ virDomainBlockIoTuneInfoPtr oldinfo,
+ bool set_bytes,
+ bool set_iops,
+ bool set_bytes_max,
+ bool set_iops_max,
+ bool set_size_iops)
+{
Could this be a macro instead? 5 booleans, it just seems a tiny bit odd.
+ if (!set_bytes) {
+ newinfo->total_bytes_sec = oldinfo->total_bytes_sec;
+ newinfo->read_bytes_sec = oldinfo->read_bytes_sec;
+ newinfo->write_bytes_sec = oldinfo->write_bytes_sec;
+ }
And then you'll end up just with snippet like ^^this. Or you could use
bitwise OR'd flags instead of the booleans, as it scales better (in terms of
number of arguments), though I'm not very fond of that even though it works.
ACK with the adjustment, but see 5/12 as well before pushing.
Erik
8:28 a.m.
New subject: [libvirt] [PATCH v2 04/12] qemu: Introduce qemuDomainSetBlockIoTuneSetDefaults
On 10/25/2016 09:05 AM, Erik Skultety wrote:
On Thu, Oct 06, 2016 at 06:38:52PM -0400, John Ferlan wrote:
> Create a helper to set the bytes/iops iotune default values based on
> the current qemu setting
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/qemu/qemu_driver.c | 66 ++++++++++++++++++++++++++++++--------------------
> 1 file changed, 40 insertions(+), 26 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 78e917e..fcce3f0 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -17286,6 +17286,43 @@ qemuDomainOpenGraphicsFD(virDomainPtr dom,
> return ret;
> }
>
> +
> +/* If the user didn't specify bytes limits, inherit previous values;
> + * likewise if the user didn't specify iops limits. */
> +static void
> +qemuDomainSetBlockIoTuneSetDefaults(virDomainBlockIoTuneInfoPtr newinfo,
> + virDomainBlockIoTuneInfoPtr oldinfo,
> + bool set_bytes,
> + bool set_iops,
> + bool set_bytes_max,
> + bool set_iops_max,
> + bool set_size_iops)
> +{
Could this be a macro instead? 5 booleans, it just seems a tiny bit odd.
Probably - I can try... Obvious impact later when _length is added.
> + if (!set_bytes) {
> + newinfo->total_bytes_sec = oldinfo->total_bytes_sec;
> + newinfo->read_bytes_sec = oldinfo->read_bytes_sec;
> + newinfo->write_bytes_sec = oldinfo->write_bytes_sec;
> + }
And then you'll end up just with snippet like ^^this. Or you could use
bitwise OR'd flags instead of the booleans, as it scales better (in terms of
number of arguments), though I'm not very fond of that even though it works.
ACK with the adjustment, but see 5/12 as well before pushing.
There is no official bug report - it was one of those visual inspection
and oh sh*t moments during testing when I didn't release my domain
wasn't started and my max settings were zero'd out.
I can merge the two together as well - not a problem.
John
5:38 p.m.
New subject: [libvirt] [PATCH v2 05/12] qemu: Use qemuDomainsetBlockIoTuneSetDefaults for config
Rather than repeat the lines in the persistent def, use the same helper
to set config values.
This also fixes a bug where config values for *_max and size_iops_sec would
be set back to 0 if a config value was set.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_driver.c | 15 +++------------
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index fcce3f0..a5f7d0c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17336,7 +17336,6 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
virDomainDefPtr def = NULL;
virDomainDefPtr persistentDef = NULL;
virDomainBlockIoTuneInfo info;
- virDomainBlockIoTuneInfo *oldinfo;
char *device = NULL;
int ret = -1;
size_t i;
@@ -17561,17 +17560,9 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
path);
goto endjob;
}
- oldinfo = &conf_disk->blkdeviotune;
- if (!set_bytes) {
- info.total_bytes_sec = oldinfo->total_bytes_sec;
- info.read_bytes_sec = oldinfo->read_bytes_sec;
- info.write_bytes_sec = oldinfo->write_bytes_sec;
- }
- if (!set_iops) {
- info.total_iops_sec = oldinfo->total_iops_sec;
- info.read_iops_sec = oldinfo->read_iops_sec;
- info.write_iops_sec = oldinfo->write_iops_sec;
- }
+ qemuDomainSetBlockIoTuneSetDefaults(&info, &conf_disk->blkdeviotune,
+ set_bytes, set_iops, set_bytes_max,
+ set_iops_max, set_size_iops);
conf_disk->blkdeviotune = info;
ret = virDomainSaveConfig(cfg->configDir, driver->caps, persistentDef);
if (ret < 0)
--
2.7.4
8:04 a.m.
New subject: [libvirt] [PATCH v2 05/12] qemu: Use qemuDomainsetBlockIoTuneSetDefaults for config
On Thu, Oct 06, 2016 at 06:38:53PM -0400, John Ferlan wrote:
Rather than repeat the lines in the persistent def, use the same
helper
to set config values.
This also fixes a bug where config values for *_max and size_iops_sec would
be set back to 0 if a config value was set.
Is there a BZ for this^^, if yes, then please add it here, if not, then
I think this can be easily squashed to 4/12.
ACK anyway.
Erik
5:38 p.m.
New subject: [libvirt] [PATCH v2 06/12] include: Add new definitions for duration for bps/iops throttling
Add new options to allow proving a duration/length in seconds to allow the
bps/iops (and friends) to occur:
total_bytes_sec_max_length
write_bytes_sec_max_length
read_bytes_sec_max_length
total_iops_sec_max_length
write_iops_sec_max_length
read_iops_sec_max_length
Add continue for compiler hint to return to for control
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
include/libvirt/libvirt-domain.h | 48 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index ec94cdf..6fe92e2 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -2322,6 +2322,54 @@ int virDomainBlockCommit(virDomainPtr dom, const char *disk, const
char *base,
# define VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX "write_iops_sec_max"
/**
+ * VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX_LENGTH:
+ *
+ * Macro for the BlockIoTune tunable weight: it represents the duration in
+ * seconds for the burst allowed by total_bytes_sec_max, as a ullong.
+ */
+# define VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX_LENGTH
"total_bytes_sec_max_length"
+
+/**
+ * VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX_LENGTH:
+ *
+ * Macro for the BlockIoTune tunable weight: it represents the duration in
+ * seconds for the burst allowed by read_bytes_sec_max, as a ullong.
+ */
+# define VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX_LENGTH
"read_bytes_sec_max_length"
+
+/**
+ * VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX_LENGTH:
+ *
+ * Macro for the BlockIoTune tunable weight: it represents the duration in
+ * seconds for the burst allowed by write_bytes_sec_max, as a ullong.
+ */
+# define VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX_LENGTH
"write_bytes_sec_max_length"
+
+/**
+ * VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX_LENGTH:
+ *
+ * Macro for the BlockIoTune tunable weight: it represents the duration in
+ * seconds for the burst allowed by total_iops_sec_max, as a ullong.
+ */
+# define VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX_LENGTH
"total_iops_sec_max_length"
+
+/**
+ * VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX_LENGTH:
+ *
+ * Macro for the BlockIoTune tunable weight: it represents the duration in
+ * seconds for the burst allowed by read_iops_sec_max, as a ullong.
+ */
+# define VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX_LENGTH
"read_iops_sec_max_length"
+
+/**
+ * VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX_LENGTH:
+ *
+ * Macro for the BlockIoTune tunable weight: it represents the duration in
+ * seconds for the burst allowed by write_iops_sec_max, as a ullong.
+ */
+# define VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX_LENGTH
"write_iops_sec_max_length"
+
+/**
* VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC:
* Macro for the BlockIoTune tunable weight: it represents the size
* I/O operations per second permitted through a block device, as a ullong.
--
2.7.4
10:20 a.m.
New subject: [libvirt] [PATCH v2 06/12] include: Add new definitions for duration for bps/iops throttling
On Thu, Oct 06, 2016 at 06:38:54PM -0400, John Ferlan wrote:
Add new options to allow proving a duration/length in seconds to
allow the
bps/iops (and friends) to occur:
total_bytes_sec_max_length
write_bytes_sec_max_length
read_bytes_sec_max_length
total_iops_sec_max_length
write_iops_sec_max_length
read_iops_sec_max_length
Add continue for compiler hint to return to for control
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
ACK
Erik
PS: I'll continue with the review tomorrow, but it's looking good so far.
5:38 p.m.
New subject: [libvirt] [PATCH v2 07/12] caps: Add new capability for the bps/iops throttling length
Add the capability to detect if the qemu binary can support the feature
to use bps-max-length and friends.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 1 +
tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 +
7 files changed, 8 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index cc8ec58..eb78d92 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -344,6 +344,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
"query-hotpluggable-cpus",
"virtio-net.rx_queue_size", /* 235 */
+ "drive-iotune-max-length",
);
@@ -2852,6 +2853,7 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] =
{
{ "name", "debug-threads", QEMU_CAPS_NAME_DEBUG_THREADS },
{ "name", "guest", QEMU_CAPS_NAME_GUEST },
{ "spice", "unix", QEMU_CAPS_SPICE_UNIX },
+ { "drive", "throttling.bps-total-max-length",
QEMU_CAPS_DRIVE_IOTUNE_MAX_LENGTH },
};
static int
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index ba0ef48..d99d61d 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -378,6 +378,7 @@ typedef enum {
/* 235 */
QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE, /* virtio-net-*.rx_queue_size */
+ QEMU_CAPS_DRIVE_IOTUNE_MAX_LENGTH, /* -drive bps_max_length = and friends */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml
b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml
index fd14665..d5089dc 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml
@@ -159,6 +159,7 @@
<flag name='display'/>
<flag name='smm'/>
<flag name='virtio-pci-disable-legacy'/>
+ <flag name='drive-iotune-max-length'/>
<version>2005094</version>
<kvmVersion>0</kvmVersion>
<package></package>
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml
b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml
index eb708f8..3417996 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml
@@ -159,6 +159,7 @@
<flag name='display'/>
<flag name='smm'/>
<flag name='virtio-pci-disable-legacy'/>
+ <flag name='drive-iotune-max-length'/>
<version>2005094</version>
<kvmVersion>0</kvmVersion>
<package></package>
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml
b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml
index 482b384..6a653ae 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml
@@ -153,6 +153,7 @@
<flag name='display'/>
<flag name='smm'/>
<flag name='virtio-pci-disable-legacy'/>
+ <flag name='drive-iotune-max-length'/>
<version>2005094</version>
<kvmVersion>0</kvmVersion>
<package></package>
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml
index 60f1fcf..cbea49e 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml
@@ -196,6 +196,7 @@
<flag name='intel-iommu'/>
<flag name='smm'/>
<flag name='virtio-pci-disable-legacy'/>
+ <flag name='drive-iotune-max-length'/>
<version>2006000</version>
<kvmVersion>0</kvmVersion>
<package></package>
diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml
index 98f3762..d9e9a78 100644
--- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml
@@ -197,6 +197,7 @@
<flag name='smm'/>
<flag name='virtio-pci-disable-legacy'/>
<flag name='query-hotpluggable-cpus'/>
+ <flag name='drive-iotune-max-length'/>
<version>2007000</version>
<kvmVersion>0</kvmVersion>
<package> (v2.7.0)</package>
--
2.7.4
8:32 a.m.
New subject: [libvirt] [PATCH v2 07/12] caps: Add new capability for the bps/iops throttling length
On Thu, Oct 06, 2016 at 06:38:55PM -0400, John Ferlan wrote:
Add the capability to detect if the qemu binary can support the
feature
to use bps-max-length and friends.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
this patch caused a rebase conflict but you'd come across that for sure.
ACK (when I read the cover letter it was already too late as at that time I'd
already sent an ACK to 6/12, so I'll do it here as well to stay consistent)
Erik
5:38 p.m.
New subject: [libvirt] [PATCH v2 08/12] qemu: Add length for bps/iops throttling parameters to driver
Add support for a duration/length for the bps/iops and friends.
Modify the API in order to add the "blkdeviotune." specific definitions
for the iotune throttling duration/length options
total_bytes_sec_max_length
write_bytes_sec_max_length
read_bytes_sec_max_length
total_iops_sec_max_length
write_iops_sec_max_length
read_iops_sec_max_length
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
include/libvirt/libvirt-domain.h | 54 +++++++++++++++++++++
src/conf/domain_conf.h | 6 +++
src/qemu/qemu_driver.c | 100 +++++++++++++++++++++++++++++++++++++--
src/qemu/qemu_monitor.c | 7 ++-
src/qemu/qemu_monitor.h | 3 +-
src/qemu/qemu_monitor_json.c | 25 +++++++++-
src/qemu/qemu_monitor_json.h | 3 +-
tests/qemumonitorjsontest.c | 17 ++++++-
8 files changed, 202 insertions(+), 13 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 6fe92e2..d5d7d47 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -3846,6 +3846,60 @@ typedef void
(*virConnectDomainEventJobCompletedCallback)(virConnectPtr conn,
# define VIR_DOMAIN_TUNABLE_BLKDEV_SIZE_IOPS_SEC "blkdeviotune.size_iops_sec"
/**
+ * VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX_LENGTH:
+ *
+ * Macro represents the length in seconds allowed for a burst period
+ * for the blkdeviotune.total_bytes_sec_max,
+ * as VIR_TYPED_PARAM_ULLONG.
+ */
+# define VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX_LENGTH
"blkdeviotune.total_bytes_sec_max_length"
+
+/**
+ * VIR_DOMAIN_TUNABLE_BLKDEV_READ_BYTES_SEC_MAX_LENGTH:
+ *
+ * Macro represents the length in seconds allowed for a burst period
+ * for the blkdeviotune.read_bytes_sec_max
+ * as VIR_TYPED_PARAM_ULLONG.
+ */
+# define VIR_DOMAIN_TUNABLE_BLKDEV_READ_BYTES_SEC_MAX_LENGTH
"blkdeviotune.read_bytes_sec_max_length"
+
+/**
+ * VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_BYTES_SEC_MAX_LENGTH:
+ *
+ * Macro represents the length in seconds allowed for a burst period
+ * for the blkdeviotune.write_bytes_sec_max
+ * as VIR_TYPED_PARAM_ULLONG.
+ */
+# define VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_BYTES_SEC_MAX_LENGTH
"blkdeviotune.write_bytes_sec_max_length"
+
+/**
+ * VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX_LENGTH:
+ *
+ * Macro represents the length in seconds allowed for a burst period
+ * for the blkdeviotune.total_iops_sec_max
+ * as VIR_TYPED_PARAM_ULLONG.
+ */
+# define VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX_LENGTH
"blkdeviotune.total_iops_sec_max_length"
+
+/**
+ * VIR_DOMAIN_TUNABLE_BLKDEV_READ_IOPS_SEC_MAX_LENGTH:
+ *
+ * Macro represents the length in seconds allowed for a burst period
+ * for the blkdeviotune.read_iops_sec_max
+ * as VIR_TYPED_PARAM_ULLONG.
+ */
+# define VIR_DOMAIN_TUNABLE_BLKDEV_READ_IOPS_SEC_MAX_LENGTH
"blkdeviotune.read_iops_sec_max_length"
+
+/**
+ * VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC_MAX_LENGTH:
+ *
+ * Macro represents the length in seconds allowed for a burst period
+ * for the blkdeviotune.write_iops_sec_max
+ * as VIR_TYPED_PARAM_ULLONG.
+ */
+# define VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC_MAX_LENGTH
"blkdeviotune.write_iops_sec_max_length"
+
+/**
* virConnectDomainEventTunableCallback:
* @conn: connection object
* @dom: domain on which the event occurred
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a70bc21..7e182c1 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -548,6 +548,12 @@ struct _virDomainBlockIoTuneInfo {
unsigned long long read_iops_sec_max;
unsigned long long write_iops_sec_max;
unsigned long long size_iops_sec;
+ unsigned long long total_bytes_sec_max_length;
+ unsigned long long read_bytes_sec_max_length;
+ unsigned long long write_bytes_sec_max_length;
+ unsigned long long total_iops_sec_max_length;
+ unsigned long long read_iops_sec_max_length;
+ unsigned long long write_iops_sec_max_length;
};
typedef virDomainBlockIoTuneInfo *virDomainBlockIoTuneInfoPtr;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a5f7d0c..8345d58 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -114,6 +114,7 @@ VIR_LOG_INIT("qemu.qemu_driver");
#define QEMU_NB_BLOCK_IO_TUNE_PARAM 6
#define QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX 13
+#define QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX_LENGTH 19
#define QEMU_NB_NUMA_PARAM 2
@@ -17296,7 +17297,9 @@ qemuDomainSetBlockIoTuneSetDefaults(virDomainBlockIoTuneInfoPtr
newinfo,
bool set_iops,
bool set_bytes_max,
bool set_iops_max,
- bool set_size_iops)
+ bool set_size_iops,
+ bool set_bytes_max_length,
+ bool set_iops_max_length)
{
if (!set_bytes) {
newinfo->total_bytes_sec = oldinfo->total_bytes_sec;
@@ -17320,6 +17323,36 @@ qemuDomainSetBlockIoTuneSetDefaults(virDomainBlockIoTuneInfoPtr
newinfo,
}
if (!set_size_iops)
newinfo->size_iops_sec = oldinfo->size_iops_sec;
+
+ /* The length field is handled a bit differently. If not defined/set,
+ * QEMU will default these to 0 or 1 depending on whether something in
+ * the same family is set or not.
+ *
+ * Similar to other values, if nothing in the family is defined/set,
+ * then take whatever is in the oldinfo.
+ *
+ * To clear an existing limit, a 0 is provided; however, passing that
+ * 0 onto QEMU if there's a family value defined/set (or defaulted)
+ * will cause an error. So, to mimic that, if our oldinfo was set and
+ * our newinfo is clearing, then set max_length based on whether we
+ * have a value in the family set/defined. */
+#define SET_MAX_LENGTH(BOOL, FIELD) \
+ if (!BOOL) \
+ newinfo->FIELD##_max_length = oldinfo->FIELD##_max_length; \
+ else if (BOOL && oldinfo->FIELD##_max_length &&
\
+ !newinfo->FIELD##_max_length) \
+ newinfo->FIELD##_max_length = (newinfo->FIELD || \
+ newinfo->FIELD##_max) ? 1 : 0;
+
+ SET_MAX_LENGTH(set_bytes_max_length, total_bytes_sec);
+ SET_MAX_LENGTH(set_bytes_max_length, read_bytes_sec);
+ SET_MAX_LENGTH(set_bytes_max_length, write_bytes_sec);
+ SET_MAX_LENGTH(set_iops_max_length, total_iops_sec);
+ SET_MAX_LENGTH(set_iops_max_length, read_iops_sec);
+ SET_MAX_LENGTH(set_iops_max_length, write_iops_sec);
+
+#undef SET_MAX_LENGTH
+
}
@@ -17346,7 +17379,10 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
bool set_bytes_max = false;
bool set_iops_max = false;
bool set_size_iops = false;
+ bool set_bytes_max_length = false;
+ bool set_iops_max_length = false;
bool supportMaxOptions = true;
+ bool supportMaxLengthOptions = true;
virQEMUDriverConfigPtr cfg = NULL;
virObjectEventPtr event = NULL;
virTypedParameterPtr eventParams = NULL;
@@ -17382,6 +17418,18 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
VIR_TYPED_PARAM_ULLONG,
VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC,
VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX_LENGTH,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX_LENGTH,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX_LENGTH,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX_LENGTH,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX_LENGTH,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX_LENGTH,
+ VIR_TYPED_PARAM_ULLONG,
NULL) < 0)
return -1;
@@ -17449,6 +17497,19 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
SET_IOTUNE_FIELD(write_iops_sec_max, set_iops_max,
WRITE_IOPS_SEC_MAX);
SET_IOTUNE_FIELD(size_iops_sec, set_size_iops, SIZE_IOPS_SEC);
+
+ SET_IOTUNE_FIELD(total_bytes_sec_max_length, set_bytes_max_length,
+ TOTAL_BYTES_SEC_MAX_LENGTH);
+ SET_IOTUNE_FIELD(read_bytes_sec_max_length, set_bytes_max_length,
+ READ_BYTES_SEC_MAX_LENGTH);
+ SET_IOTUNE_FIELD(write_bytes_sec_max_length, set_bytes_max_length,
+ WRITE_BYTES_SEC_MAX_LENGTH);
+ SET_IOTUNE_FIELD(total_iops_sec_max_length, set_iops_max_length,
+ TOTAL_IOPS_SEC_MAX_LENGTH);
+ SET_IOTUNE_FIELD(read_iops_sec_max_length, set_iops_max_length,
+ READ_IOPS_SEC_MAX_LENGTH);
+ SET_IOTUNE_FIELD(write_iops_sec_max_length, set_iops_max_length,
+ WRITE_IOPS_SEC_MAX_LENGTH);
}
#undef SET_IOTUNE_FIELD
@@ -17488,6 +17549,9 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
if (def) {
supportMaxOptions = virQEMUCapsGet(priv->qemuCaps,
QEMU_CAPS_DRIVE_IOTUNE_MAX);
+ supportMaxLengthOptions =
+ virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX_LENGTH);
+
if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("block I/O throttling not supported with this "
@@ -17503,6 +17567,14 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
goto endjob;
}
+ if (!supportMaxLengthOptions &&
+ (set_iops_max_length || set_bytes_max_length)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("a block I/O throttling length parameter is not "
+ "supported with this QEMU binary"));
+ goto endjob;
+ }
+
if (!(disk = qemuDomainDiskByName(def, path)))
goto endjob;
@@ -17511,7 +17583,9 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
qemuDomainSetBlockIoTuneSetDefaults(&info, &disk->blkdeviotune,
set_bytes, set_iops, set_bytes_max,
- set_iops_max, set_size_iops);
+ set_iops_max, set_size_iops,
+ set_bytes_max_length,
+ set_iops_max_length);
#define CHECK_MAX(val) \
do { \
@@ -17533,9 +17607,13 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
#undef CHECK_MAX
+ /* NB: Let's let QEMU decide how to handle issues with _length
+ * via the JSON error code from the block_set_io_throttle call */
+
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorSetBlockIoThrottle(priv->mon, device,
- &info, supportMaxOptions);
+ &info, supportMaxOptions,
+ supportMaxLengthOptions);
if (qemuDomainObjExitMonitor(driver, vm) < 0)
ret = -1;
if (ret < 0)
@@ -17562,7 +17640,9 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
}
qemuDomainSetBlockIoTuneSetDefaults(&info, &conf_disk->blkdeviotune,
set_bytes, set_iops, set_bytes_max,
- set_iops_max, set_size_iops);
+ set_iops_max, set_size_iops,
+ set_bytes_max_length,
+ set_iops_max_length);
conf_disk->blkdeviotune = info;
ret = virDomainSaveConfig(cfg->configDir, driver->caps, persistentDef);
if (ret < 0)
@@ -17598,7 +17678,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
virDomainBlockIoTuneInfo reply;
char *device = NULL;
int ret = -1;
- int maxparams = QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX;
+ int maxparams = QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX_LENGTH;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
VIR_DOMAIN_AFFECT_CONFIG |
@@ -17634,6 +17714,9 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX))
maxparams = QEMU_NB_BLOCK_IO_TUNE_PARAM;
+ else if (!virQEMUCapsGet(priv->qemuCaps,
+ QEMU_CAPS_DRIVE_IOTUNE_MAX_LENGTH))
+ maxparams = QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX;
}
if (*nparams == 0) {
@@ -17697,6 +17780,13 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
BLOCK_IOTUNE_ASSIGN(SIZE_IOPS_SEC, size_iops_sec);
+ BLOCK_IOTUNE_ASSIGN(TOTAL_BYTES_SEC_MAX_LENGTH, total_bytes_sec_max_length);
+ BLOCK_IOTUNE_ASSIGN(READ_BYTES_SEC_MAX_LENGTH, read_bytes_sec_max_length);
+ BLOCK_IOTUNE_ASSIGN(WRITE_BYTES_SEC_MAX_LENGTH, write_bytes_sec_max_length);
+
+ BLOCK_IOTUNE_ASSIGN(TOTAL_IOPS_SEC_MAX_LENGTH, total_iops_sec_max_length);
+ BLOCK_IOTUNE_ASSIGN(READ_IOPS_SEC_MAX_LENGTH, read_iops_sec_max_length);
+ BLOCK_IOTUNE_ASSIGN(WRITE_IOPS_SEC_MAX_LENGTH, write_iops_sec_max_length);
#undef BLOCK_IOTUNE_ASSIGN
ret = 0;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 5175f4e..961cfd8 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3423,14 +3423,17 @@ int
qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon,
const char *device,
virDomainBlockIoTuneInfoPtr info,
- bool supportMaxOptions)
+ bool supportMaxOptions,
+ bool supportMaxLengthOptions)
{
VIR_DEBUG("device=%p, info=%p", device, info);
QEMU_CHECK_MONITOR(mon);
if (mon->json)
- return qemuMonitorJSONSetBlockIoThrottle(mon, device, info, supportMaxOptions);
+ return qemuMonitorJSONSetBlockIoThrottle(mon, device, info,
+ supportMaxOptions,
+ supportMaxLengthOptions);
else
return qemuMonitorTextSetBlockIoThrottle(mon, device, info);
}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 7d78e5b..700dff9 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -873,7 +873,8 @@ int qemuMonitorOpenGraphics(qemuMonitorPtr mon,
int qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon,
const char *device,
virDomainBlockIoTuneInfoPtr info,
- bool supportMaxOptions);
+ bool supportMaxOptions,
+ bool supportMaxLengthOptions);
int qemuMonitorGetBlockIoThrottle(qemuMonitorPtr mon,
const char *device,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 744c878..25b0569 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4560,6 +4560,12 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result,
GET_THROTTLE_STATS_OPTIONAL("iops_rd_max", read_iops_sec_max);
GET_THROTTLE_STATS_OPTIONAL("iops_wr_max", write_iops_sec_max);
GET_THROTTLE_STATS_OPTIONAL("iops_size", size_iops_sec);
+ GET_THROTTLE_STATS_OPTIONAL("bps_max_length",
total_bytes_sec_max_length);
+ GET_THROTTLE_STATS_OPTIONAL("bps_rd_max_length",
read_bytes_sec_max_length);
+ GET_THROTTLE_STATS_OPTIONAL("bps_wr_max_length",
write_bytes_sec_max_length);
+ GET_THROTTLE_STATS_OPTIONAL("iops_max_length",
total_iops_sec_max_length);
+ GET_THROTTLE_STATS_OPTIONAL("iops_rd_max_length",
read_iops_sec_max_length);
+ GET_THROTTLE_STATS_OPTIONAL("iops_wr_max_length",
write_iops_sec_max_length);
break;
}
@@ -4581,7 +4587,8 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result,
int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon,
const char *device,
virDomainBlockIoTuneInfoPtr info,
- bool supportMaxOptions)
+ bool supportMaxOptions,
+ bool supportMaxLengthOptions)
{
int ret = -1;
virJSONValuePtr cmd = NULL;
@@ -4590,7 +4597,8 @@ int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon,
/* The qemu capability check has already been made in
* qemuDomainSetBlockIoTune. NB, once a NULL is found in
* the sequence, qemuMonitorJSONMakeCommand will stop. So
- * let's make use of that when !supportMaxOptions */
+ * let's make use of that when !supportMaxOptions and
+ * similarly when !supportMaxLengthOptions */
cmd = qemuMonitorJSONMakeCommand("block_set_io_throttle",
"s:device", device,
"U:bps", info->total_bytes_sec,
@@ -4607,6 +4615,19 @@ int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon,
"U:iops_rd_max",
info->read_iops_sec_max,
"U:iops_wr_max",
info->write_iops_sec_max,
"U:iops_size", info->size_iops_sec,
+ !supportMaxLengthOptions ? NULL :
+ "P:bps_max_length",
+ info->total_bytes_sec_max_length,
+ "P:bps_rd_max_length",
+ info->read_bytes_sec_max_length,
+ "P:bps_wr_max_length",
+ info->write_bytes_sec_max_length,
+ "P:iops_max_length",
+ info->total_iops_sec_max_length,
+ "P:iops_rd_max_length",
+ info->read_iops_sec_max_length,
+ "P:iops_wr_max_length",
+ info->write_iops_sec_max_length,
NULL);
if (!cmd)
return -1;
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 6a5eb3b..77b2e02 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -327,7 +327,8 @@ int qemuMonitorJSONOpenGraphics(qemuMonitorPtr mon,
int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon,
const char *device,
virDomainBlockIoTuneInfoPtr info,
- bool supportMaxOptions);
+ bool supportMaxOptions,
+ bool supportMaxLengthOptions);
int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr mon,
const char *device,
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 4e7bb71..bd5b7c3 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -67,6 +67,12 @@ const char *queryBlockReply =
" \"iops_rd_max\": 11,"
" \"iops_wr_max\": 12,"
" \"iops_size\": 13,"
+" \"bps_max_length\": 14,"
+" \"bps_rd_max_length\": 15,"
+" \"bps_wr_max_length\": 16,"
+" \"iops_max_length\": 17,"
+" \"iops_rd_max_length\": 18,"
+" \"iops_wr_max_length\": 19,"
" \"file\":
\"/home/zippy/work/tmp/gentoo.qcow2\","
" \"encryption_key_missing\": false"
" },"
@@ -1885,7 +1891,7 @@ testQemuMonitorJSONqemuMonitorJSONSetBlockIoThrottle(const void
*data)
if (!test)
return -1;
- expectedInfo = (virDomainBlockIoTuneInfo) {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12,
13};
+ expectedInfo = (virDomainBlockIoTuneInfo) {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13,
14, 15, 16, 17, 18, 19};
if (qemuMonitorTestAddItem(test, "query-block", queryBlockReply) < 0 ||
qemuMonitorTestAddItemParams(test, "block_set_io_throttle",
@@ -1897,6 +1903,12 @@ testQemuMonitorJSONqemuMonitorJSONSetBlockIoThrottle(const void
*data)
"bps_wr_max", "9",
"iops_max", "10",
"iops_rd_max", "11",
"iops_wr_max", "12",
"iops_size", "13",
+ "bps_max_length", "14",
+ "bps_rd_max_length", "15",
+ "bps_wr_max_length", "16",
+ "iops_max_length", "17",
+ "iops_rd_max_length", "18",
+ "iops_wr_max_length", "19",
NULL, NULL) < 0)
goto cleanup;
@@ -1911,7 +1923,8 @@ testQemuMonitorJSONqemuMonitorJSONSetBlockIoThrottle(const void
*data)
}
if (qemuMonitorJSONSetBlockIoThrottle(qemuMonitorTestGetMonitor(test),
- "drive-virtio-disk1", &info,
true) < 0)
+ "drive-virtio-disk1", &info,
true,
+ true) < 0)
goto cleanup;
ret = 0;
--
2.7.4
12:30 p.m.
New subject: [libvirt] [PATCH v2 08/12] qemu: Add length for bps/iops throttling parameters to driver
On Thu, Oct 06, 2016 at 06:38:56PM -0400, John Ferlan wrote:
Add support for a duration/length for the bps/iops and friends.
Modify the API in order to add the "blkdeviotune." specific definitions
for the iotune throttling duration/length options
total_bytes_sec_max_length
write_bytes_sec_max_length
read_bytes_sec_max_length
total_iops_sec_max_length
write_iops_sec_max_length
read_iops_sec_max_length
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
include/libvirt/libvirt-domain.h | 54 +++++++++++++++++++++
src/conf/domain_conf.h | 6 +++
src/qemu/qemu_driver.c | 100 +++++++++++++++++++++++++++++++++++++--
src/qemu/qemu_monitor.c | 7 ++-
src/qemu/qemu_monitor.h | 3 +-
src/qemu/qemu_monitor_json.c | 25 +++++++++-
src/qemu/qemu_monitor_json.h | 3 +-
tests/qemumonitorjsontest.c | 17 ++++++-
8 files changed, 202 insertions(+), 13 deletions(-)
[...]
@@ -17296,7 +17297,9 @@
qemuDomainSetBlockIoTuneSetDefaults(virDomainBlockIoTuneInfoPtr newinfo,
just a nitpick, are both the 'Set's necessary in the name, unless one of them
is a verb and the other a noun, I think the first one could be dropped.
bool set_iops,
bool set_bytes_max,
bool set_iops_max,
- bool set_size_iops)
+ bool set_size_iops,
+ bool set_bytes_max_length,
+ bool set_iops_max_length)
{
if (!set_bytes) {
newinfo->total_bytes_sec = oldinfo->total_bytes_sec;
@@ -17320,6 +17323,36 @@ qemuDomainSetBlockIoTuneSetDefaults(virDomainBlockIoTuneInfoPtr
newinfo,
}
if (!set_size_iops)
newinfo->size_iops_sec = oldinfo->size_iops_sec;
+
+ /* The length field is handled a bit differently. If not defined/set,
+ * QEMU will default these to 0 or 1 depending on whether something in
+ * the same family is set or not.
+ *
+ * Similar to other values, if nothing in the family is defined/set,
+ * then take whatever is in the oldinfo.
+ *
+ * To clear an existing limit, a 0 is provided; however, passing that
+ * 0 onto QEMU if there's a family value defined/set (or defaulted)
+ * will cause an error. So, to mimic that, if our oldinfo was set and
+ * our newinfo is clearing, then set max_length based on whether we
+ * have a value in the family set/defined. */
Hmm. You can disregard my comment in 4/12 about replacing the whole function
with a macro, instead I suggest keeping the function and making the macro below
also work for all the settings above, otherwise it just doesn't look right, one
part of the function being expanded while the other covered by a macro because
the behaviour is slightly different (I have to admit though, it was kinda
painful to comprehend what's going on on the qemu side)
+#define SET_MAX_LENGTH(BOOL, FIELD)
\
+ if (!BOOL) \
+ newinfo->FIELD##_max_length = oldinfo->FIELD##_max_length; \
+ else if (BOOL && oldinfo->FIELD##_max_length &&
\
+ !newinfo->FIELD##_max_length) \
+ newinfo->FIELD##_max_length = (newinfo->FIELD || \
+ newinfo->FIELD##_max) ? 1 : 0;
+
+ SET_MAX_LENGTH(set_bytes_max_length, total_bytes_sec);
+ SET_MAX_LENGTH(set_bytes_max_length, read_bytes_sec);
+ SET_MAX_LENGTH(set_bytes_max_length, write_bytes_sec);
+ SET_MAX_LENGTH(set_iops_max_length, total_iops_sec);
+ SET_MAX_LENGTH(set_iops_max_length, read_iops_sec);
+ SET_MAX_LENGTH(set_iops_max_length, write_iops_sec);
+
+#undef SET_MAX_LENGTH
+
}
@@ -17346,7 +17379,10 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
bool set_bytes_max = false;
bool set_iops_max = false;
bool set_size_iops = false;
+ bool set_bytes_max_length = false;
+ bool set_iops_max_length = false;
bool supportMaxOptions = true;
+ bool supportMaxLengthOptions = true;
virQEMUDriverConfigPtr cfg = NULL;
virObjectEventPtr event = NULL;
virTypedParameterPtr eventParams = NULL;
@@ -17382,6 +17418,18 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
VIR_TYPED_PARAM_ULLONG,
VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC,
VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX_LENGTH,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX_LENGTH,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX_LENGTH,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX_LENGTH,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX_LENGTH,
+ VIR_TYPED_PARAM_ULLONG,
+ VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX_LENGTH,
+ VIR_TYPED_PARAM_ULLONG,
NULL) < 0)
return -1;
@@ -17449,6 +17497,19 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
SET_IOTUNE_FIELD(write_iops_sec_max, set_iops_max,
WRITE_IOPS_SEC_MAX);
SET_IOTUNE_FIELD(size_iops_sec, set_size_iops, SIZE_IOPS_SEC);
+
+ SET_IOTUNE_FIELD(total_bytes_sec_max_length, set_bytes_max_length,
+ TOTAL_BYTES_SEC_MAX_LENGTH);
+ SET_IOTUNE_FIELD(read_bytes_sec_max_length, set_bytes_max_length,
+ READ_BYTES_SEC_MAX_LENGTH);
+ SET_IOTUNE_FIELD(write_bytes_sec_max_length, set_bytes_max_length,
+ WRITE_BYTES_SEC_MAX_LENGTH);
+ SET_IOTUNE_FIELD(total_iops_sec_max_length, set_iops_max_length,
+ TOTAL_IOPS_SEC_MAX_LENGTH);
+ SET_IOTUNE_FIELD(read_iops_sec_max_length, set_iops_max_length,
+ READ_IOPS_SEC_MAX_LENGTH);
+ SET_IOTUNE_FIELD(write_iops_sec_max_length, set_iops_max_length,
+ WRITE_IOPS_SEC_MAX_LENGTH);
}
#undef SET_IOTUNE_FIELD
@@ -17488,6 +17549,9 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
if (def) {
supportMaxOptions = virQEMUCapsGet(priv->qemuCaps,
QEMU_CAPS_DRIVE_IOTUNE_MAX);
+ supportMaxLengthOptions =
+ virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX_LENGTH);
+
Basically I have nothing against this^^, just a note that should the
capabilities covering iotune be extended in the future I can image this to be
handled as OR'd flags rather than individual variables, but it's okay now.
ACK with the macro adjustment above.
Erik
4:31 p.m.
New subject: [libvirt] [PATCH v2 08/12] qemu: Add length for bps/iops throttling parameters to driver
On 10/25/2016 01:30 PM, Erik Skultety wrote:
On Thu, Oct 06, 2016 at 06:38:56PM -0400, John Ferlan wrote:
> Add support for a duration/length for the bps/iops and friends.
>
> Modify the API in order to add the "blkdeviotune." specific definitions
> for the iotune throttling duration/length options
>
> total_bytes_sec_max_length
> write_bytes_sec_max_length
> read_bytes_sec_max_length
> total_iops_sec_max_length
> write_iops_sec_max_length
> read_iops_sec_max_length
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> include/libvirt/libvirt-domain.h | 54 +++++++++++++++++++++
> src/conf/domain_conf.h | 6 +++
> src/qemu/qemu_driver.c | 100 +++++++++++++++++++++++++++++++++++++--
> src/qemu/qemu_monitor.c | 7 ++-
> src/qemu/qemu_monitor.h | 3 +-
> src/qemu/qemu_monitor_json.c | 25 +++++++++-
> src/qemu/qemu_monitor_json.h | 3 +-
> tests/qemumonitorjsontest.c | 17 ++++++-
> 8 files changed, 202 insertions(+), 13 deletions(-)
>
[...]
> @@ -17296,7 +17297,9 @@
qemuDomainSetBlockIoTuneSetDefaults(virDomainBlockIoTuneInfoPtr newinfo,
just a nitpick, are both the 'Set's necessary in the name, unless one of them
is a verb and the other a noun, I think the first one could be dropped.
D'oh... The name was so long I missed the double set!
> bool set_iops,
> bool set_bytes_max,
> bool set_iops_max,
> - bool set_size_iops)
> + bool set_size_iops,
> + bool set_bytes_max_length,
> + bool set_iops_max_length)
> {
> if (!set_bytes) {
> newinfo->total_bytes_sec = oldinfo->total_bytes_sec;
> @@ -17320,6 +17323,36 @@
qemuDomainSetBlockIoTuneSetDefaults(virDomainBlockIoTuneInfoPtr newinfo,
> }
> if (!set_size_iops)
> newinfo->size_iops_sec = oldinfo->size_iops_sec;
> +
> + /* The length field is handled a bit differently. If not defined/set,
> + * QEMU will default these to 0 or 1 depending on whether something in
> + * the same family is set or not.
> + *
> + * Similar to other values, if nothing in the family is defined/set,
> + * then take whatever is in the oldinfo.
> + *
> + * To clear an existing limit, a 0 is provided; however, passing that
> + * 0 onto QEMU if there's a family value defined/set (or defaulted)
> + * will cause an error. So, to mimic that, if our oldinfo was set and
> + * our newinfo is clearing, then set max_length based on whether we
> + * have a value in the family set/defined. */
Hmm. You can disregard my comment in 4/12 about replacing the whole function
with a macro, instead I suggest keeping the function and making the macro below
also work for all the settings above, otherwise it just doesn't look right, one
part of the function being expanded while the other covered by a macro because
the behaviour is slightly different (I have to admit though, it was kinda
painful to comprehend what's going on on the qemu side)
Yes, quite painful - especially when I realized it after the initial
series... The difference is I went with a function.
I create a macro for the setting of the defaults
> +#define SET_MAX_LENGTH(BOOL, FIELD)
\
> + if (!BOOL) \
> + newinfo->FIELD##_max_length = oldinfo->FIELD##_max_length;
\
> + else if (BOOL && oldinfo->FIELD##_max_length &&
\
> + !newinfo->FIELD##_max_length) \
> + newinfo->FIELD##_max_length = (newinfo->FIELD ||
\
> + newinfo->FIELD##_max) ? 1 : 0;
> +
> + SET_MAX_LENGTH(set_bytes_max_length, total_bytes_sec);
> + SET_MAX_LENGTH(set_bytes_max_length, read_bytes_sec);
> + SET_MAX_LENGTH(set_bytes_max_length, write_bytes_sec);
> + SET_MAX_LENGTH(set_iops_max_length, total_iops_sec);
> + SET_MAX_LENGTH(set_iops_max_length, read_iops_sec);
> + SET_MAX_LENGTH(set_iops_max_length, write_iops_sec);
> +
> +#undef SET_MAX_LENGTH
> +
> }
>
>
> @@ -17346,7 +17379,10 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
> bool set_bytes_max = false;
> bool set_iops_max = false;
> bool set_size_iops = false;
> + bool set_bytes_max_length = false;
> + bool set_iops_max_length = false;
> bool supportMaxOptions = true;
> + bool supportMaxLengthOptions = true;
> virQEMUDriverConfigPtr cfg = NULL;
> virObjectEventPtr event = NULL;
> virTypedParameterPtr eventParams = NULL;
> @@ -17382,6 +17418,18 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
> VIR_TYPED_PARAM_ULLONG,
> VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC,
> VIR_TYPED_PARAM_ULLONG,
> + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX_LENGTH,
> + VIR_TYPED_PARAM_ULLONG,
> + VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX_LENGTH,
> + VIR_TYPED_PARAM_ULLONG,
> + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX_LENGTH,
> + VIR_TYPED_PARAM_ULLONG,
> + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX_LENGTH,
> + VIR_TYPED_PARAM_ULLONG,
> + VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX_LENGTH,
> + VIR_TYPED_PARAM_ULLONG,
> + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX_LENGTH,
> + VIR_TYPED_PARAM_ULLONG,
> NULL) < 0)
> return -1;
>
> @@ -17449,6 +17497,19 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
> SET_IOTUNE_FIELD(write_iops_sec_max, set_iops_max,
> WRITE_IOPS_SEC_MAX);
> SET_IOTUNE_FIELD(size_iops_sec, set_size_iops, SIZE_IOPS_SEC);
> +
> + SET_IOTUNE_FIELD(total_bytes_sec_max_length, set_bytes_max_length,
> + TOTAL_BYTES_SEC_MAX_LENGTH);
> + SET_IOTUNE_FIELD(read_bytes_sec_max_length, set_bytes_max_length,
> + READ_BYTES_SEC_MAX_LENGTH);
> + SET_IOTUNE_FIELD(write_bytes_sec_max_length, set_bytes_max_length,
> + WRITE_BYTES_SEC_MAX_LENGTH);
> + SET_IOTUNE_FIELD(total_iops_sec_max_length, set_iops_max_length,
> + TOTAL_IOPS_SEC_MAX_LENGTH);
> + SET_IOTUNE_FIELD(read_iops_sec_max_length, set_iops_max_length,
> + READ_IOPS_SEC_MAX_LENGTH);
> + SET_IOTUNE_FIELD(write_iops_sec_max_length, set_iops_max_length,
> + WRITE_IOPS_SEC_MAX_LENGTH);
> }
>
> #undef SET_IOTUNE_FIELD
> @@ -17488,6 +17549,9 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
> if (def) {
> supportMaxOptions = virQEMUCapsGet(priv->qemuCaps,
> QEMU_CAPS_DRIVE_IOTUNE_MAX);
> + supportMaxLengthOptions =
> + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX_LENGTH);
> +
Basically I have nothing against this^^, just a note that should the
capabilities covering iotune be extended in the future I can image this to be
handled as OR'd flags rather than individual variables, but it's okay now.
Ironically - there is another field, but it was added before the -length
values were supported. That's the *next* series I'll be posting...
The tricky part of the OR'd flags will be how the command line logic is
put together. It's a rather unique hack that ultimately is in
qemuMonitorJSONSetBlockIoThrottle regarding how the command line is put
together with a bit of "knowledge" regarding "when" the boolean was
added.
Coming to the theater of the absurd soon is "max" (added in qemu 1.7),
"group" (added in qemu 2.4), and "max_length" (added in qemu 2.6).
Tks -
John
ACK with the macro adjustment above.
Erik
5:38 p.m.
New subject: [libvirt] [PATCH v2 09/12] conf: Add support for blkiotune "_length" options
Modify _virDomainBlockIoTuneInfo and rng schema to support the _length
options for bps/iops throttling values. Document the new values.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
docs/formatdomain.html.in | 40 ++++++++++++-
docs/schemas/domaincommon.rng | 38 +++++++++++++
src/conf/domain_conf.c | 24 +++++++-
.../qemuxml2argv-blkdeviotune-max-length.xml | 65 ++++++++++++++++++++++
.../qemuxml2xmlout-blkdeviotune-max-length.xml | 1 +
tests/qemuxml2xmltest.c | 1 +
6 files changed, 166 insertions(+), 3 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max-length.xml
create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-blkdeviotune-max-length.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 1266e9d..274b9bb 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2617,7 +2617,45 @@
maximum write I/O operations per second.</dd>
<dt><code>size_iops_sec</code></dt>
<dd>The optional <code>size_iops_sec</code> element is the
- size of I/O operations per second.</dd>
+ size of I/O operations per second.
+ <p>
+ <span class="since">Throughput limits since 1.2.11 and QEMU
1.7</span>
+ </p>
+ </dd>
+ <dt><code>total_bytes_sec_max_length</code></dt>
+ <dd>The optional <code>total_bytes_sec_max_length</code>
+ element is the maximum duration in seconds for the
+ <code>total_bytes_sec_max</code> burst period. Only valid
+ when the <code>total_bytes_sec_max</code> is set.</dd>
+ <dt><code>read_bytes_sec_max_length</code></dt>
+ <dd>The optional <code>read_bytes_sec_max_length</code>
+ element is the maximum duration in seconds for the
+ <code>read_bytes_sec_max</code> burst period. Only valid
+ when the <code>read_bytes_sec_max</code> is set.</dd>
+ <dt><code>write_bytes_sec_max</code></dt>
+ <dd>The optional <code>write_bytes_sec_max_length</code>
+ element is the maximum duration in seconds for the
+ <code>write_bytes_sec_max</code> burst period. Only valid
+ when the if <code>write_bytes_sec_max</code> is set.</dd>
+ <dt><code>total_iops_sec_max_length</code></dt>
+ <dd>The optional <code>total_iops_sec_max_length</code>
+ element is the maximum duration in seconds for the
+ <code>total_iops_sec_max</code> burst period. Only valid
+ when the <code>total_iops_sec_max</code> is set.</dd>
+ <dt><code>read_iops_sec_max_length</code></dt>
+ <dd>The optional <code>read_iops_sec_max_length</code>
+ element is the maximum duration in seconds for the
+ <code>read_iops_sec_max</code> burst period. Only valid
+ when the <code>read_iops_sec_max</code> is set.</dd>
+ <dt><code>write_iops_sec_max</code></dt>
+ <dd>The optional <code>write_iops_sec_max_length</code>
+ element is the maximum duration in seconds for the
+ <code>write_iops_sec_max</code> burst period. Only valid
+ when the <code>write_iops_sec_max</code> is set.
+ <p>
+ <span class="since">Throughput length since 2.4.0 and QEMU
2.6</span>
+ </p>
+ </dd>
</dl>
</dd>
<dt><code>driver</code></dt>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 6eeb4e9..d648702 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4960,6 +4960,44 @@
<data type="unsignedLong"/>
</element>
</optional>
+ <choice>
+ <element name="total_bytes_sec_max_length">
+ <data type="unsignedLong"/>
+ </element>
+ <group>
+ <interleave>
+ <optional>
+ <element name="read_bytes_sec_max_length">
+ <data type="unsignedLong"/>
+ </element>
+ </optional>
+ <optional>
+ <element name="write_bytes_sec_max_length">
+ <data type="unsignedLong"/>
+ </element>
+ </optional>
+ </interleave>
+ </group>
+ </choice>
+ <choice>
+ <element name="total_iops_sec_max_length">
+ <data type="unsignedLong"/>
+ </element>
+ <group>
+ <interleave>
+ <optional>
+ <element name="read_iops_sec_max_length">
+ <data type="unsignedLong"/>
+ </element>
+ </optional>
+ <optional>
+ <element name="write_iops_sec_max_length">
+ <data type="unsignedLong"/>
+ </element>
+ </optional>
+ </interleave>
+ </group>
+ </choice>
</interleave>
</element>
</define>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6a772c6..9dce51d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7137,7 +7137,7 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
virReportError(VIR_ERR_XML_ERROR, \
_("disk iotune field '%s' must be an integer"),
#val); \
return -1; \
- }
+ } \
static int
virDomainDiskDefIotuneParse(virDomainDiskDefPtr def,
@@ -7159,6 +7159,13 @@ virDomainDiskDefIotuneParse(virDomainDiskDefPtr def,
PARSE_IOTUNE(size_iops_sec);
+ PARSE_IOTUNE(total_bytes_sec_max_length);
+ PARSE_IOTUNE(read_bytes_sec_max_length);
+ PARSE_IOTUNE(write_bytes_sec_max_length);
+ PARSE_IOTUNE(total_iops_sec_max_length);
+ PARSE_IOTUNE(read_iops_sec_max_length);
+ PARSE_IOTUNE(write_iops_sec_max_length);
+
if ((def->blkdeviotune.total_bytes_sec &&
def->blkdeviotune.read_bytes_sec) ||
(def->blkdeviotune.total_bytes_sec &&
@@ -20272,7 +20279,13 @@ virDomainDiskDefFormat(virBufferPtr buf,
def->blkdeviotune.total_iops_sec_max ||
def->blkdeviotune.read_iops_sec_max ||
def->blkdeviotune.write_iops_sec_max ||
- def->blkdeviotune.size_iops_sec) {
+ def->blkdeviotune.size_iops_sec ||
+ def->blkdeviotune.total_bytes_sec_max_length ||
+ def->blkdeviotune.read_bytes_sec_max_length ||
+ def->blkdeviotune.write_bytes_sec_max_length ||
+ def->blkdeviotune.total_iops_sec_max_length ||
+ def->blkdeviotune.read_iops_sec_max_length ||
+ def->blkdeviotune.write_iops_sec_max_length) {
virBufferAddLit(buf, "<iotune>\n");
virBufferAdjustIndent(buf, 2);
@@ -20295,6 +20308,13 @@ virDomainDiskDefFormat(virBufferPtr buf,
def->blkdeviotune.size_iops_sec);
}
+ FORMAT_IOTUNE(total_bytes_sec_max_length);
+ FORMAT_IOTUNE(read_bytes_sec_max_length);
+ FORMAT_IOTUNE(write_bytes_sec_max_length);
+ FORMAT_IOTUNE(total_iops_sec_max_length);
+ FORMAT_IOTUNE(read_iops_sec_max_length);
+ FORMAT_IOTUNE(write_iops_sec_max_length);
+
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "</iotune>\n");
}
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max-length.xml
b/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max-length.xml
new file mode 100644
index 0000000..c740751
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max-length.xml
@@ -0,0 +1,65 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219100</memory>
+ <currentMemory unit='KiB'>219100</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='qcow2' cache='none'/>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <iotune>
+ <total_bytes_sec>5000</total_bytes_sec>
+ <total_iops_sec>6000</total_iops_sec>
+ <total_bytes_sec_max>10000</total_bytes_sec_max>
+ <total_iops_sec_max>11000</total_iops_sec_max>
+ <total_bytes_sec_max_length>3</total_bytes_sec_max_length>
+ <total_iops_sec_max_length>5</total_iops_sec_max_length>
+ </iotune>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='qcow2' cache='none'/>
+ <source dev='/dev/HostVG/QEMUGuest2'/>
+ <target dev='hdb' bus='ide'/>
+ <iotune>
+ <read_bytes_sec>5000</read_bytes_sec>
+ <write_bytes_sec>5500</write_bytes_sec>
+ <read_iops_sec>3500</read_iops_sec>
+ <write_iops_sec>4000</write_iops_sec>
+ <read_bytes_sec_max>6000</read_bytes_sec_max>
+ <write_bytes_sec_max>6500</write_bytes_sec_max>
+ <read_iops_sec_max>7000</read_iops_sec_max>
+ <write_iops_sec_max>7500</write_iops_sec_max>
+ <size_iops_sec>2000</size_iops_sec>
+ <read_bytes_sec_max_length>3</read_bytes_sec_max_length>
+ <write_bytes_sec_max_length>5</write_bytes_sec_max_length>
+ <read_iops_sec_max_length>7</read_iops_sec_max_length>
+ <write_iops_sec_max_length>9</write_iops_sec_max_length>
+ </iotune>
+ <address type='drive' controller='0' bus='0'
target='0' unit='1'/>
+ </disk>
+ <controller type='usb' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='ide' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x1'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='virtio'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
+ </memballoon>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-blkdeviotune-max-length.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-blkdeviotune-max-length.xml
new file mode 120000
index 0000000..67f3e9a
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-blkdeviotune-max-length.xml
@@ -0,0 +1 @@
+../qemuxml2argvdata/qemuxml2argv-blkdeviotune-max-length.xml
\ No newline at end of file
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 4b58986..5c50512 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -608,6 +608,7 @@ mymain(void)
DO_TEST("usb-redir-filter-version", NONE);
DO_TEST("blkdeviotune", NONE);
DO_TEST("blkdeviotune-max", NONE);
+ DO_TEST("blkdeviotune-max-length", NONE);
DO_TEST("controller-usb-order", NONE);
DO_TEST_FULL("seclabel-dynamic-baselabel", WHEN_INACTIVE, GIC_NONE, NONE);
--
2.7.4
12:33 p.m.
New subject: [libvirt] [PATCH v2 09/12] conf: Add support for blkiotune "_length" options
On Thu, Oct 06, 2016 at 06:38:57PM -0400, John Ferlan wrote:
Modify _virDomainBlockIoTuneInfo and rng schema to support the
_length
options for bps/iops throttling values. Document the new values.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
docs/formatdomain.html.in | 40 ++++++++++++-
docs/schemas/domaincommon.rng | 38 +++++++++++++
src/conf/domain_conf.c | 24 +++++++-
.../qemuxml2argv-blkdeviotune-max-length.xml | 65 ++++++++++++++++++++++
.../qemuxml2xmlout-blkdeviotune-max-length.xml | 1 +
tests/qemuxml2xmltest.c | 1 +
6 files changed, 166 insertions(+), 3 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max-length.xml
create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-blkdeviotune-max-length.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 1266e9d..274b9bb 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2617,7 +2617,45 @@
maximum write I/O operations per second.</dd>
<dt><code>size_iops_sec</code></dt>
<dd>The optional <code>size_iops_sec</code> element is the
- size of I/O operations per second.</dd>
+ size of I/O operations per second.
+ <p>
+ <span class="since">Throughput limits since 1.2.11 and QEMU
1.7</span>
+ </p>
+ </dd>
+ <dt><code>total_bytes_sec_max_length</code></dt>
+ <dd>The optional <code>total_bytes_sec_max_length</code>
+ element is the maximum duration in seconds for the
+ <code>total_bytes_sec_max</code> burst period. Only valid
+ when the <code>total_bytes_sec_max</code> is set.</dd>
+ <dt><code>read_bytes_sec_max_length</code></dt>
+ <dd>The optional <code>read_bytes_sec_max_length</code>
+ element is the maximum duration in seconds for the
+ <code>read_bytes_sec_max</code> burst period. Only valid
+ when the <code>read_bytes_sec_max</code> is set.</dd>
+ <dt><code>write_bytes_sec_max</code></dt>
+ <dd>The optional <code>write_bytes_sec_max_length</code>
+ element is the maximum duration in seconds for the
+ <code>write_bytes_sec_max</code> burst period. Only valid
+ when the if <code>write_bytes_sec_max</code> is set.</dd>
s/if //
ACK
Erik
5:38 p.m.
New subject: [libvirt] [PATCH v2 10/12] qemu: Add the length options to the iotune command line
Add in the block I/O throttling length/duration parameter to the command
line if supported. If not supported, fail command creation.
Add the xml2argvtest for testing.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 21 +++++++++++++
.../qemuxml2argv-blkdeviotune-max-length.args | 34 ++++++++++++++++++++++
tests/qemuxml2argvtest.c | 4 +++
3 files changed, 59 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max-length.args
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 578ff8b..6d2e15d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1747,6 +1747,20 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
goto error;
}
+ /* block I/O throttling length 2.6 */
+ if ((disk->blkdeviotune.total_bytes_sec_max_length ||
+ disk->blkdeviotune.read_bytes_sec_max_length ||
+ disk->blkdeviotune.write_bytes_sec_max_length ||
+ disk->blkdeviotune.total_iops_sec_max_length ||
+ disk->blkdeviotune.read_iops_sec_max_length ||
+ disk->blkdeviotune.write_iops_sec_max_length) &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX_LENGTH)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("there are some block I/O throttling length parameters
"
+ "that are not supported with this QEMU binary"));
+ goto error;
+ }
+
if (disk->blkdeviotune.total_bytes_sec > QEMU_BLOCK_IOTUNE_MAX ||
disk->blkdeviotune.read_bytes_sec > QEMU_BLOCK_IOTUNE_MAX ||
disk->blkdeviotune.write_bytes_sec > QEMU_BLOCK_IOTUNE_MAX ||
@@ -1788,6 +1802,13 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
IOTUNE_ADD(size_iops_sec, "iops-size");
+ IOTUNE_ADD(total_bytes_sec_max_length, "bps-total-max-length");
+ IOTUNE_ADD(read_bytes_sec_max_length, "bps-read-max-length");
+ IOTUNE_ADD(write_bytes_sec_max_length, "bps-write-max-length");
+ IOTUNE_ADD(total_iops_sec_max_length, "iops-total-max-length");
+ IOTUNE_ADD(read_iops_sec_max_length, "iops-read-max-length");
+ IOTUNE_ADD(write_iops_sec_max_length, "iops-write-max-length");
+
#undef IOTUNE_ADD
if (virBufferCheckError(&opt) < 0)
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max-length.args
b/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max-length.args
new file mode 100644
index 0000000..b656aea
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max-length.args
@@ -0,0 +1,34 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
+-no-acpi \
+-boot c \
+-usb \
+-drive file=/dev/HostVG/QEMUGuest1,format=qcow2,if=none,id=drive-ide0-0-0,\
+cache=none,throttling.bps-total=5000,throttling.iops-total=6000,\
+throttling.bps-total-max=10000,throttling.iops-total-max=11000,\
+throttling.bps-total-max-length=3,throttling.iops-total-max-length=5 \
+-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
+-drive file=/dev/HostVG/QEMUGuest2,format=qcow2,if=none,id=drive-ide0-0-1,\
+cache=none,throttling.bps-read=5000,throttling.bps-write=5500,\
+throttling.iops-read=3500,throttling.iops-write=4000,\
+throttling.bps-read-max=6000,throttling.bps-write-max=6500,\
+throttling.iops-read-max=7000,throttling.iops-write-max=7500,\
+throttling.iops-size=2000,throttling.bps-read-max-length=3,\
+throttling.bps-write-max-length=5,throttling.iops-read-max-length=7,\
+throttling.iops-write-max-length=9 \
+-device ide-drive,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 4b9ecb8..6523a07 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1492,6 +1492,10 @@ mymain(void)
DO_TEST("blkdeviotune-max",
QEMU_CAPS_DRIVE_IOTUNE,
QEMU_CAPS_DRIVE_IOTUNE_MAX);
+ DO_TEST("blkdeviotune-max-length",
+ QEMU_CAPS_DRIVE_IOTUNE,
+ QEMU_CAPS_DRIVE_IOTUNE_MAX,
+ QEMU_CAPS_DRIVE_IOTUNE_MAX_LENGTH);
DO_TEST("multifunction-pci-device",
QEMU_CAPS_NODEFCONFIG,
--
2.7.4
12:38 p.m.
New subject: [libvirt] [PATCH v2 10/12] qemu: Add the length options to the iotune command line
On Thu, Oct 06, 2016 at 06:38:58PM -0400, John Ferlan wrote:
Add in the block I/O throttling length/duration parameter to the
command
line if supported. If not supported, fail command creation.
Add the xml2argvtest for testing.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 21 +++++++++++++
.../qemuxml2argv-blkdeviotune-max-length.args | 34 ++++++++++++++++++++++
tests/qemuxml2argvtest.c | 4 +++
3 files changed, 59 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max-length.args
ACK
Erik
5:38 p.m.
New subject: [libvirt] [PATCH v2 11/12] virsh: Create a macro to add IOTUNE values
Rework the repetitive lines to add iotune values into easier to read macro
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
tools/virsh-domain.c | 141 +++++++++------------------------------------------
1 file changed, 25 insertions(+), 116 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 2ce0a06..0643dfb 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1300,122 +1300,31 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptStringReq(ctl, cmd, "device", &disk) < 0)
goto cleanup;
- if ((rv = vshCommandOptScaledInt(ctl, cmd, "total-bytes-sec", &value,
1, ULLONG_MAX)) < 0) {
- goto interror;
- } else if (rv > 0) {
- if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
- VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC,
- value) < 0)
- goto save_error;
- }
-
- if ((rv = vshCommandOptScaledInt(ctl, cmd, "read-bytes-sec", &value, 1,
ULLONG_MAX)) < 0) {
- goto interror;
- } else if (rv > 0) {
- if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
- VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC,
- value) < 0)
- goto save_error;
- }
-
- if ((rv = vshCommandOptScaledInt(ctl, cmd, "write-bytes-sec", &value,
1, ULLONG_MAX)) < 0) {
- goto interror;
- } else if (rv > 0) {
- if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
- VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC,
- value) < 0)
- goto save_error;
- }
-
- if ((rv = vshCommandOptScaledInt(ctl, cmd, "total-bytes-sec-max",
&value, 1, ULLONG_MAX)) < 0) {
- goto interror;
- } else if (rv > 0) {
- if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
- VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX,
- value) < 0)
- goto save_error;
- }
-
- if ((rv = vshCommandOptScaledInt(ctl, cmd, "read-bytes-sec-max",
&value, 1, ULLONG_MAX)) < 0) {
- goto interror;
- } else if (rv > 0) {
- if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
- VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX,
- value) < 0)
- goto save_error;
- }
-
- if ((rv = vshCommandOptScaledInt(ctl, cmd, "write-bytes-sec-max",
&value, 1, ULLONG_MAX)) < 0) {
- goto interror;
- } else if (rv > 0) {
- if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
- VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX,
- value) < 0)
- goto save_error;
- }
-
- if ((rv = vshCommandOptULongLong(ctl, cmd, "total-iops-sec", &value))
< 0) {
- goto interror;
- } else if (rv > 0) {
- if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
- VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC,
- value) < 0)
- goto save_error;
- }
-
- if ((rv = vshCommandOptULongLong(ctl, cmd, "read-iops-sec", &value))
< 0) {
- goto interror;
- } else if (rv > 0) {
- if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
- VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC,
- value) < 0)
- goto save_error;
- }
-
- if ((rv = vshCommandOptULongLong(ctl, cmd, "write-iops-sec", &value))
< 0) {
- goto interror;
- } else if (rv > 0) {
- if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
- VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC,
- value) < 0)
- goto save_error;
- }
-
- if ((rv = vshCommandOptULongLong(ctl, cmd, "write-iops-sec-max",
&value)) < 0) {
- goto interror;
- } else if (rv > 0) {
- if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
- VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX,
- value) < 0)
- goto save_error;
- }
-
- if ((rv = vshCommandOptULongLong(ctl, cmd, "read-iops-sec-max",
&value)) < 0) {
- goto interror;
- } else if (rv > 0) {
- if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
- VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX,
- value) < 0)
- goto save_error;
- }
-
- if ((rv = vshCommandOptULongLong(ctl, cmd, "total-iops-sec-max",
&value)) < 0) {
- goto interror;
- } else if (rv > 0) {
- if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
- VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX,
- value) < 0)
- goto save_error;
- }
-
- if ((rv = vshCommandOptULongLong(ctl, cmd, "size-iops-sec", &value))
< 0) {
- goto interror;
- } else if (rv > 0) {
- if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
- VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC,
- value) < 0)
- goto save_error;
- }
+#define VSH_ADD_IOTUNE(PARAM, CONST) \
+ if ((rv = vshCommandOptScaledInt(ctl, cmd, #PARAM, &value, \
+ 1, ULLONG_MAX)) < 0) { \
+ goto interror; \
+ } else if (rv > 0) { \
+ if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
\
+ VIR_DOMAIN_BLOCK_IOTUNE_##CONST, \
+ value) < 0) \
+ goto save_error; \
+ } \
+
+ VSH_ADD_IOTUNE(total-bytes-sec, TOTAL_BYTES_SEC);
+ VSH_ADD_IOTUNE(read-bytes-sec, READ_BYTES_SEC);
+ VSH_ADD_IOTUNE(write-bytes-sec, WRITE_BYTES_SEC);
+ VSH_ADD_IOTUNE(total-iops-sec, TOTAL_IOPS_SEC);
+ VSH_ADD_IOTUNE(read-iops-sec, READ_IOPS_SEC);
+ VSH_ADD_IOTUNE(write-iops-sec, WRITE_IOPS_SEC);
+
+ VSH_ADD_IOTUNE(total-bytes-sec-max, TOTAL_BYTES_SEC_MAX);
+ VSH_ADD_IOTUNE(read-bytes-sec-max, READ_BYTES_SEC_MAX);
+ VSH_ADD_IOTUNE(write-bytes-sec-max, WRITE_BYTES_SEC_MAX);
+ VSH_ADD_IOTUNE(total-iops-sec-max, TOTAL_IOPS_SEC_MAX);
+ VSH_ADD_IOTUNE(read-iops-sec-max, READ_IOPS_SEC_MAX);
+ VSH_ADD_IOTUNE(write-iops-sec-max, WRITE_IOPS_SEC_MAX);
+ VSH_ADD_IOTUNE(size-iops-sec, SIZE_IOPS_SEC);
if (nparams == 0) {
if (virDomainGetBlockIoTune(dom, NULL, NULL, &nparams, flags) != 0) {
--
2.7.4
12:44 p.m.
New subject: [libvirt] [PATCH v2 11/12] virsh: Create a macro to add IOTUNE values
On Thu, Oct 06, 2016 at 06:38:59PM -0400, John Ferlan wrote:
Rework the repetitive lines to add iotune values into easier to read
macro
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
tools/virsh-domain.c | 141 +++++++++------------------------------------------
1 file changed, 25 insertions(+), 116 deletions(-)
[...]
+#define VSH_ADD_IOTUNE(PARAM, CONST)
\
+ if ((rv = vshCommandOptScaledInt(ctl, cmd, #PARAM, &value, \
+ 1, ULLONG_MAX)) < 0) { \
Again a nitpick, I guess the -length setting should not get scaled when one
tries e.g. 'kib' to pass along with the setting. In terms of a cleanup, yes,
it's very welcome, so maybe add a 'SCALE' argument - your call.
ACK
Erik
+ goto interror;
\
+ } else if (rv > 0) { \
+ if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
\
+ VIR_DOMAIN_BLOCK_IOTUNE_##CONST, \
+ value) < 0) \
+ goto save_error; \
+ } \
+
+ VSH_ADD_IOTUNE(total-bytes-sec, TOTAL_BYTES_SEC);
+ VSH_ADD_IOTUNE(read-bytes-sec, READ_BYTES_SEC);
+ VSH_ADD_IOTUNE(write-bytes-sec, WRITE_BYTES_SEC);
+ VSH_ADD_IOTUNE(total-iops-sec, TOTAL_IOPS_SEC);
+ VSH_ADD_IOTUNE(read-iops-sec, READ_IOPS_SEC);
+ VSH_ADD_IOTUNE(write-iops-sec, WRITE_IOPS_SEC);
+
+ VSH_ADD_IOTUNE(total-bytes-sec-max, TOTAL_BYTES_SEC_MAX);
+ VSH_ADD_IOTUNE(read-bytes-sec-max, READ_BYTES_SEC_MAX);
+ VSH_ADD_IOTUNE(write-bytes-sec-max, WRITE_BYTES_SEC_MAX);
+ VSH_ADD_IOTUNE(total-iops-sec-max, TOTAL_IOPS_SEC_MAX);
+ VSH_ADD_IOTUNE(read-iops-sec-max, READ_IOPS_SEC_MAX);
+ VSH_ADD_IOTUNE(write-iops-sec-max, WRITE_IOPS_SEC_MAX);
+ VSH_ADD_IOTUNE(size-iops-sec, SIZE_IOPS_SEC);
if (nparams == 0) {
if (virDomainGetBlockIoTune(dom, NULL, NULL, &nparams, flags) != 0) {
--
2.7.4
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
4:31 p.m.
New subject: [libvirt] [PATCH v2 11/12] virsh: Create a macro to add IOTUNE values
On 10/25/2016 01:44 PM, Erik Skultety wrote:
On Thu, Oct 06, 2016 at 06:38:59PM -0400, John Ferlan wrote:
> Rework the repetitive lines to add iotune values into easier to read macro
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> tools/virsh-domain.c | 141 +++++++++------------------------------------------
> 1 file changed, 25 insertions(+), 116 deletions(-)
>
[...]
> +#define VSH_ADD_IOTUNE(PARAM, CONST) \
> + if ((rv = vshCommandOptScaledInt(ctl, cmd, #PARAM, &value,
\
> + 1, ULLONG_MAX)) < 0) { \
Again a nitpick, I guess the -length setting should not get scaled when one
Upon closer inspect, good catch and it's not just -length... The iops
values aren't scaled either!
There's now two macros... I renamed the current one to add a _SCALED and
created a new one that uses vshCommandOptULongLong (I don't want to mess
with another SCALE argument)
tries e.g. 'kib' to pass along with the setting. In terms of
a cleanup, yes,
it's very welcome, so maybe add a 'SCALE' argument - your call.
and the -length values will all be part of the non scaled version..
Thanks for the review - the series is now pushed.
John
ACK
Erik
> + goto interror; \
> + } else if (rv > 0) { \
> + if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
\
> + VIR_DOMAIN_BLOCK_IOTUNE_##CONST, \
> + value) < 0) \
> + goto save_error; \
> + } \
> +
> + VSH_ADD_IOTUNE(total-bytes-sec, TOTAL_BYTES_SEC);
> + VSH_ADD_IOTUNE(read-bytes-sec, READ_BYTES_SEC);
> + VSH_ADD_IOTUNE(write-bytes-sec, WRITE_BYTES_SEC);
> + VSH_ADD_IOTUNE(total-iops-sec, TOTAL_IOPS_SEC);
> + VSH_ADD_IOTUNE(read-iops-sec, READ_IOPS_SEC);
> + VSH_ADD_IOTUNE(write-iops-sec, WRITE_IOPS_SEC);
> +
> + VSH_ADD_IOTUNE(total-bytes-sec-max, TOTAL_BYTES_SEC_MAX);
> + VSH_ADD_IOTUNE(read-bytes-sec-max, READ_BYTES_SEC_MAX);
> + VSH_ADD_IOTUNE(write-bytes-sec-max, WRITE_BYTES_SEC_MAX);
> + VSH_ADD_IOTUNE(total-iops-sec-max, TOTAL_IOPS_SEC_MAX);
> + VSH_ADD_IOTUNE(read-iops-sec-max, READ_IOPS_SEC_MAX);
> + VSH_ADD_IOTUNE(write-iops-sec-max, WRITE_IOPS_SEC_MAX);
> + VSH_ADD_IOTUNE(size-iops-sec, SIZE_IOPS_SEC);
>
> if (nparams == 0) {
> if (virDomainGetBlockIoTune(dom, NULL, NULL, &nparams, flags) != 0) {
> --
> 2.7.4
>
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
5:39 p.m.
New subject: [libvirt] [PATCH v2 12/12] virsh: Add _length parameters to virsh output
https://bugzilla.redhat.com/show_bug.cgi?id=1349898
Add the duration parameters to the virsh input/output for blkdeviotune
command and describe them in the pod file.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
tools/virsh-domain.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++
tools/virsh.pod | 21 ++++++++++++++++++++
2 files changed, 76 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 0643dfb..2a9dc77 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1263,6 +1263,54 @@ static const vshCmdOptDef opts_blkdeviotune[] = {
.type = VSH_OT_INT,
.help = N_("I/O size in bytes")
},
+ {.name = "total_bytes_sec_max_length",
+ .type = VSH_OT_ALIAS,
+ .help = "total-bytes-sec-max-length"
+ },
+ {.name = "total-bytes-sec-max-length",
+ .type = VSH_OT_INT,
+ .help = N_("duration in seconds to allow total max bytes")
+ },
+ {.name = "read_bytes_sec_max_length",
+ .type = VSH_OT_ALIAS,
+ .help = "read-bytes-sec-max-length"
+ },
+ {.name = "read-bytes-sec-max-length",
+ .type = VSH_OT_INT,
+ .help = N_("duration in seconds to allow read max bytes")
+ },
+ {.name = "write_bytes_sec_max_length",
+ .type = VSH_OT_ALIAS,
+ .help = "write-bytes-sec-max-length"
+ },
+ {.name = "write-bytes-sec-max-length",
+ .type = VSH_OT_INT,
+ .help = N_("duration in seconds to allow write max bytes")
+ },
+ {.name = "total_iops_sec_max_length",
+ .type = VSH_OT_ALIAS,
+ .help = "total-iops-sec-max-length"
+ },
+ {.name = "total-iops-sec-max-length",
+ .type = VSH_OT_INT,
+ .help = N_("duration in seconds to allow total I/O operations max")
+ },
+ {.name = "read_iops_sec_max_length",
+ .type = VSH_OT_ALIAS,
+ .help = "read-iops-sec-max-length"
+ },
+ {.name = "read-iops-sec-max-length",
+ .type = VSH_OT_INT,
+ .help = N_("duration in seconds to allow read I/O operations max")
+ },
+ {.name = "write_iops_sec_max_length",
+ .type = VSH_OT_ALIAS,
+ .help = "write-iops-sec-max-length"
+ },
+ {.name = "write-iops-sec-max-length",
+ .type = VSH_OT_INT,
+ .help = N_("duration in seconds to allow write I/O operations max")
+ },
VIRSH_COMMON_OPT_DOMAIN_CONFIG,
VIRSH_COMMON_OPT_DOMAIN_LIVE,
VIRSH_COMMON_OPT_DOMAIN_CURRENT,
@@ -1326,6 +1374,13 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
VSH_ADD_IOTUNE(write-iops-sec-max, WRITE_IOPS_SEC_MAX);
VSH_ADD_IOTUNE(size-iops-sec, SIZE_IOPS_SEC);
+ VSH_ADD_IOTUNE(total-bytes-sec-max-length, TOTAL_BYTES_SEC_MAX_LENGTH);
+ VSH_ADD_IOTUNE(read-bytes-sec-max-length, READ_BYTES_SEC_MAX_LENGTH);
+ VSH_ADD_IOTUNE(write-bytes-sec-max-length, WRITE_BYTES_SEC_MAX_LENGTH);
+ VSH_ADD_IOTUNE(total-iops-sec-max-length, TOTAL_IOPS_SEC_MAX_LENGTH);
+ VSH_ADD_IOTUNE(read-iops-sec-max-length, READ_IOPS_SEC_MAX_LENGTH);
+ VSH_ADD_IOTUNE(write-iops-sec-max-length, WRITE_IOPS_SEC_MAX_LENGTH);
+
if (nparams == 0) {
if (virDomainGetBlockIoTune(dom, NULL, NULL, &nparams, flags) != 0) {
vshError(ctl, "%s",
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 227c9b2..99620b1 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1127,6 +1127,10 @@ command.
[[I<total-iops-sec>] | [I<read-iops-sec>] [I<write-iops-sec>]]
[[I<total-bytes-sec-max>] | [I<read-bytes-sec-max>]
[I<write-bytes-sec-max>]]
[[I<total-iops-sec-max>] | [I<read-iops-sec-max>]
[I<write-iops-sec-max>]]
+[[I<total-bytes-sec-max-length>] |
+[I<read-bytes-sec-max-length>] [I<write-bytes-sec-max-length>]]
+[[I<total-iops-sec-max-length>] |
+[I<read-iops-sec-max-length>] [I<write-iops-sec-max-length>]]
[I<size-iops-sec>]
Set or query the block disk io parameters for a block device of I<domain>.
@@ -1154,6 +1158,18 @@ integer, the default being bytes per second if no suffix is
specified.
I<--total-iops-sec-max> specifies maximum total I/O operations limit per second.
I<--read-iops-sec-max> specifies maximum read I/O operations limit per second.
I<--write-iops-sec-max> specifies maximum write I/O operations limit per second.
+I<--total-bytes-sec-max-length> specifies duration in seconds to allow maximum
+total throughput limit.
+I<--read-bytes-sec-max-length> specifies duration in seconds to allow maximum
+read throughput limit.
+I<--write-bytes-sec-max-length> specifies duration in seconds to allow maximum
+write throughput limit.
+I<--total-iops-sec-max-length> specifies duration in seconds to allow maximum
+total I/O operations limit.
+I<--read-iops-sec-max-length> specifies duration in seconds to allow maximum
+read I/O operations limit.
+I<--write-iops-sec-max-length> specifies duration in seconds to allow maximum
+write I/O operations limit.
I<--size-iops-sec> specifies size I/O operations limit per second.
Older versions of virsh only accepted these options with underscore
@@ -1164,6 +1180,11 @@ as --read-bytes-sec) resets the other two in that category to
unlimited.
An explicit 0 also clears any limit. A non-zero value for a given total
cannot be mixed with non-zero values for read or write.
+It is up to the hypervisor to determine how to handle the length values.
+For the qemu hypervisor, if an I/O limit value or maximum value is set,
+then the default value of 1 second will be displayed. Supplying a 0 will
+reset the value back to the default.
+
If I<--live> is specified, affect a running guest.
If I<--config> is specified, affect the next boot of a persistent guest.
If I<--current> is specified, affect the current guest state.
--
2.7.4
12:50 p.m.
New subject: [libvirt] [PATCH v2 12/12] virsh: Add _length parameters to virsh output
On Thu, Oct 06, 2016 at 06:39:00PM -0400, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1349898
Add the duration parameters to the virsh input/output for blkdeviotune
command and describe them in the pod file.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
tools/virsh-domain.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++
tools/virsh.pod | 21 ++++++++++++++++++++
2 files changed, 76 insertions(+)
ACK
Erik
9:23 p.m.
ping? Tks (I have a follow up series for group that I'd like to see
considered before the end of the release cycle too, but this needs to be
done first).
John
On 10/06/2016 06:38 PM, John Ferlan wrote:
v1:
http://www.redhat.com/archives/libvir-list/2016-September/msg01090.html
Differences since v1:
Patches 1-5 are new...
Patch 1 is essentially the former patch 8.5 added after initial review
based on a review comment. Erik Skultety and I went back and forth
on this one a few times and this is the result
Patch 2 is a result of being frustrated by a generic error message when
I know we could return whatever qemu gives us
Patch 3 is simple code motion - the conf_disk is closer to usage now
Patch 4 & 5 create and use the same algorithm to set the default values
that weren't provided.
Patches 6-7 have already been ACK'd... I've been waiting to push because
they will interfere with libvirt-perl builds and since the 2.3.0 hasn't
been published yet, I'm still holding onto them.
Patch 8 is the former patch 8, but reworked mostly in qemu_driver.c, but
also a change in qemu_monitor_json.c to use "P:" instead of "U:"
for
then length values. For _length a 0 is an invalid value, so we'll use
it to mean "nothing changed"
Patch 9 is the former patch 11 and is mostly intact, with major difference
being the removal of the CHECK_IOTUNE_MAX_LENGTH macro (it was causing
domain disappearance)
Patch 10 is the former patch 12 and is unchanged. It also has been ACKd
Patch 11-12 are new - it's the virsh code.
John Ferlan (12):
qemu: Create a macro to handle setting bytes/iops iotune values
qemu: Return real error message for block_set_io_throttle
qemu: Move setting of conf_disk in qemuDomainSetBlockIoTune
qemu: Introduce qemuDomainSetBlockIoTuneSetDefaults
qemu: Use qemuDomainsetBlockIoTuneSetDefaults for config
include: Add new definitions for duration for bps/iops throttling
caps: Add new capability for the bps/iops throttling length
qemu: Add length for bps/iops throttling parameters to driver
conf: Add support for blkiotune "_length" options
qemu: Add the length options to the iotune command line
virsh: Create a macro to add IOTUNE values
virsh: Add _length parameters to virsh output
docs/formatdomain.html.in | 40 ++-
docs/schemas/domaincommon.rng | 38 +++
include/libvirt/libvirt-domain.h | 102 ++++++
src/conf/domain_conf.c | 24 +-
src/conf/domain_conf.h | 6 +
src/qemu/qemu_capabilities.c | 2 +
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 21 ++
src/qemu/qemu_driver.c | 341 +++++++++++----------
src/qemu/qemu_monitor.c | 7 +-
src/qemu/qemu_monitor.h | 3 +-
src/qemu/qemu_monitor_json.c | 39 ++-
src/qemu/qemu_monitor_json.h | 3 +-
.../caps_2.6.0-gicv2.aarch64.xml | 1 +
.../caps_2.6.0-gicv3.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 1 +
tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 +
tests/qemumonitorjsontest.c | 17 +-
.../qemuxml2argv-blkdeviotune-max-length.args | 34 ++
.../qemuxml2argv-blkdeviotune-max-length.xml | 65 ++++
tests/qemuxml2argvtest.c | 4 +
.../qemuxml2xmlout-blkdeviotune-max-length.xml | 1 +
tests/qemuxml2xmltest.c | 1 +
tools/virsh-domain.c | 196 +++++-------
tools/virsh.pod | 21 ++
26 files changed, 673 insertions(+), 298 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max-length.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max-length.xml
create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-blkdeviotune-max-length.xml
6:24 a.m.
ping^2?
Thanks -
John
On 10/06/2016 06:38 PM, John Ferlan wrote:
v1:
http://www.redhat.com/archives/libvir-list/2016-September/msg01090.html
Differences since v1:
Patches 1-5 are new...
Patch 1 is essentially the former patch 8.5 added after initial review
based on a review comment. Erik Skultety and I went back and forth
on this one a few times and this is the result
Patch 2 is a result of being frustrated by a generic error message when
I know we could return whatever qemu gives us
Patch 3 is simple code motion - the conf_disk is closer to usage now
Patch 4 & 5 create and use the same algorithm to set the default values
that weren't provided.
Patches 6-7 have already been ACK'd... I've been waiting to push because
they will interfere with libvirt-perl builds and since the 2.3.0 hasn't
been published yet, I'm still holding onto them.
Patch 8 is the former patch 8, but reworked mostly in qemu_driver.c, but
also a change in qemu_monitor_json.c to use "P:" instead of "U:"
for
then length values. For _length a 0 is an invalid value, so we'll use
it to mean "nothing changed"
Patch 9 is the former patch 11 and is mostly intact, with major difference
being the removal of the CHECK_IOTUNE_MAX_LENGTH macro (it was causing
domain disappearance)
Patch 10 is the former patch 12 and is unchanged. It also has been ACKd
Patch 11-12 are new - it's the virsh code.
John Ferlan (12):
qemu: Create a macro to handle setting bytes/iops iotune values
qemu: Return real error message for block_set_io_throttle
qemu: Move setting of conf_disk in qemuDomainSetBlockIoTune
qemu: Introduce qemuDomainSetBlockIoTuneSetDefaults
qemu: Use qemuDomainsetBlockIoTuneSetDefaults for config
include: Add new definitions for duration for bps/iops throttling
caps: Add new capability for the bps/iops throttling length
qemu: Add length for bps/iops throttling parameters to driver
conf: Add support for blkiotune "_length" options
qemu: Add the length options to the iotune command line
virsh: Create a macro to add IOTUNE values
virsh: Add _length parameters to virsh output
docs/formatdomain.html.in | 40 ++-
docs/schemas/domaincommon.rng | 38 +++
include/libvirt/libvirt-domain.h | 102 ++++++
src/conf/domain_conf.c | 24 +-
src/conf/domain_conf.h | 6 +
src/qemu/qemu_capabilities.c | 2 +
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 21 ++
src/qemu/qemu_driver.c | 341 +++++++++++----------
src/qemu/qemu_monitor.c | 7 +-
src/qemu/qemu_monitor.h | 3 +-
src/qemu/qemu_monitor_json.c | 39 ++-
src/qemu/qemu_monitor_json.h | 3 +-
.../caps_2.6.0-gicv2.aarch64.xml | 1 +
.../caps_2.6.0-gicv3.aarch64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml | 1 +
tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml | 1 +
tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml | 1 +
tests/qemumonitorjsontest.c | 17 +-
.../qemuxml2argv-blkdeviotune-max-length.args | 34 ++
.../qemuxml2argv-blkdeviotune-max-length.xml | 65 ++++
tests/qemuxml2argvtest.c | 4 +
.../qemuxml2xmlout-blkdeviotune-max-length.xml | 1 +
tests/qemuxml2xmltest.c | 1 +
tools/virsh-domain.c | 196 +++++-------
tools/virsh.pod | 21 ++
26 files changed, 673 insertions(+), 298 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max-length.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-blkdeviotune-max-length.xml
create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-blkdeviotune-max-length.xml
2950
days inactive
2969
days old
29 comments
2 participants
participants (2)
-
Erik Skultety -
John Ferlan