[libvirt] [PATCH v2 00/12] Add length (duration) params for iotune throttling
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
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@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
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@redhat.com> --- src/qemu/qemu_driver.c | 153 +++++++++++-------------------------------------- 1 file changed, 35 insertions(+), 118 deletions(-)
ACK Erik
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@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
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@redhat.com> --- src/qemu/qemu_monitor_json.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
ACK Erik
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@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
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@redhat.com> --- src/qemu/qemu_driver.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)
ACK Erik
Create a helper to set the bytes/iops iotune default values based on the current qemu setting Signed-off-by: John Ferlan <jferlan@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
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@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
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@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
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@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
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
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@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
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@redhat.com>
ACK Erik PS: I'll continue with the review tomorrow, but it's looking good so far.
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@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
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@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
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@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
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@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
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@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
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@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
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@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
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@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
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@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
Rework the repetitive lines to add iotune values into easier to read macro Signed-off-by: John Ferlan <jferlan@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
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@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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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@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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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@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
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@redhat.com> --- tools/virsh-domain.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 21 ++++++++++++++++++++ 2 files changed, 76 insertions(+)
ACK Erik
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
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
participants (2)
-
Erik Skultety -
John Ferlan