[libvirt] [PATCH v5 0/7] qemu: Introduce support for new the block_set_io_throttle parameters add in the version 1.7 of qemu.

This series of patches add support for bps_max, bps_rd_max, bps_wr_max, bps_max, bps_rd_max, bps_wr_max, and iops_size in the functions qemuDomainSetBlockIoTune and qemuDomainGetBlockIoTune. The last patch add support for these parameters to the virsh blkdeviotune command. v2: spellfix v3: Merge patch 1/9,2/9,5/9 together. Change the capability detection.(patch 2/7 and 3/7). Try to make the usage of QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX more explicit(patch 3/7). v4: Rebase on HEAD. Update qemu_driver to comply with Pavel's patchs.(patch 3/6) Remove the qemu_monitor_text modification.(remove old patch 5/7) v5: Split patch 1/6 in two. Add documentation for the new xml options (patch 2/7) Change (void) to ATTRIBUTE_UNUSED (patch 4/7) Capability detection of supportMaxOptions move before usage of supportMaxOptions Matthias Gatto (7): qemu: Add define for the news throttle options qemu: Modify the structure _virDomainBlockIoTuneInfo. qemu: Add the capability to detect if the qemu binary have the capability to use bps_max and friends qemu: Add bps_max and friends qemu driver qemu: Add bps_max and friends QMP suport qemu: add bps_max and friends to qemu command generation virsh: Add bps_max and friends to virsh docs/formatdomain.html.in | 25 ++++ docs/schemas/domaincommon.rng | 43 +++++++ include/libvirt/libvirt.h.in | 110 ++++++++++++++++ src/conf/domain_conf.c | 110 +++++++++++++++- src/conf/domain_conf.h | 7 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 57 +++++++- src/qemu/qemu_driver.c | 186 ++++++++++++++++++++++++++- src/qemu/qemu_monitor.c | 10 +- src/qemu/qemu_monitor.h | 6 +- src/qemu/qemu_monitor_json.c | 64 +++++++-- src/qemu/qemu_monitor_json.h | 6 +- tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 + tests/qemumonitorjsontest.c | 6 +- tools/virsh-domain.c | 119 +++++++++++++++++ tools/virsh.pod | 10 ++ 17 files changed, 730 insertions(+), 33 deletions(-) -- 1.8.3.1

Add defines for the news options total_bytes_sec_max, write_bytes_sec_max, read_bytes_sec_max, total_iops_sec_max, write_iops_sec_max, read_iops_sec_max, size_iops_sec. Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com> --- include/libvirt/libvirt.h.in | 54 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index c910b31..459a292 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2800,6 +2800,60 @@ int virDomainBlockCommit(virDomainPtr dom, const char *disk, const char *base, */ #define VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC "write_iops_sec" +/** + * VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX: + * + * Macro for the BlockIoTune tunable weight: it represents the maximum total + * bytes per second permitted through a block device, as a ullong. + */ +#define VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX "total_bytes_sec_max" + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX: + * + * Macro for the BlockIoTune tunable weight: it represents the maximum read + * bytes per second permitted through a block device, as a ullong. + */ +#define VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX "read_bytes_sec_max" + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX: + * + * Macro for the BlockIoTune tunable weight: it represents the maximum write + * bytes per second permitted through a block device, as a ullong. + */ +#define VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX "write_bytes_sec_max" + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX: + * + * Macro for the BlockIoTune tunable weight: it represents the maximum + * I/O operations per second permitted through a block device, as a ullong. + */ +#define VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX "total_iops_sec_max" + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX: + * + * Macro for the BlockIoTune tunable weight: it represents the maximum read + * I/O operations per second permitted through a block device, as a ullong. + */ +#define VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX "read_iops_sec_max" + +/** + * VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX: + * Macro for the BlockIoTune tunable weight: it represents the maximum write + * I/O operations per second permitted through a block device, as a ullong. + */ +#define VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX "write_iops_sec_max" + +/** + * 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. + */ +#define VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC "size_iops_sec" + int virDomainSetBlockIoTune(virDomainPtr dom, const char *disk, -- 1.8.3.1

On 10/07/2014 05:14 AM, Matthias Gatto wrote:
Add defines for the news options total_bytes_sec_max, write_bytes_sec_max, read_bytes_sec_max, total_iops_sec_max, write_iops_sec_max, read_iops_sec_max, size_iops_sec.
Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com> --- include/libvirt/libvirt.h.in | 54 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+)
Oops, because I took my time reviewing this, Dan has refactored this file in the meantime, and the patch doesn't apply cleanly any more. At least the conflict resolution is obvious. ACK. I'll push this after reviewing the rest of the series. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 10/07/2014 05:14 AM, Matthias Gatto wrote:
Add defines for the news options total_bytes_sec_max, write_bytes_sec_max, read_bytes_sec_max, total_iops_sec_max, write_iops_sec_max, read_iops_sec_max, size_iops_sec.
s/news/new/ Also, please wrap your commit messages to around 72 columns or so (git log likes to indent messages, and lots of people still like to use 80-column messages, so anything longer results in annoying truncation or wraparound on the display) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Modify the structure _virDomainBlockIoTuneInfo to support these the new options. Change the initialization of the variable expectedInfo in qemumonitorjsontest.c to avoid compiling problem. Add documentation about the new xml options Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com> --- docs/formatdomain.html.in | 25 ++++++++++ docs/schemas/domaincommon.rng | 43 +++++++++++++++++ src/conf/domain_conf.c | 110 +++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 7 +++ tests/qemumonitorjsontest.c | 2 +- 5 files changed, 185 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 45b0f61..6ec7b6b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2103,6 +2103,31 @@ <dt><code>write_iops_sec</code></dt> <dd>The optional <code>write_iops_sec</code> element is the write I/O operations per second.</dd> + <dt><code>total_bytes_sec_max</code></dt> + <dd>The optional <code>total_bytes_sec_max</code> element is the + maximum total throughput limit in bytes per second. This cannot + appear with <code>read_bytes_sec_max</code> + or <code>write_bytes_sec_max</code>.</dd> + <dt><code>read_bytes_sec_max</code></dt> + <dd>The optional <code>read_bytes_sec_max</code> element is the + maximum read throughput limit in bytes per second.</dd> + <dt><code>write_bytes_sec_max</code></dt> + <dd>The optional <code>write_bytes_sec_max</code> element is the + maximum write throughput limit in bytes per second.</dd> + <dt><code>total_iops_sec_max</code></dt> + <dd>The optional <code>total_iops_sec_max</code> element is the + maximum total I/O operations per second. This cannot + appear with <code>read_iops_sec_max</code> + or <code>write_iops_sec_max</code>.</dd> + <dt><code>read_iops_sec_max</code></dt> + <dd>The optional <code>read_iops_sec_max</code> element is the + maximum read I/O operations per second.</dd> + <dt><code>write_iops_sec_max</code></dt> + <dd>The optional <code>write_iops_sec_max</code> element is the + 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> </dl> </dd> <dt><code>driver</code></dt> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 6b69fd1..5ab796b 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4538,6 +4538,49 @@ </interleave> </group> </choice> + <choice> + <element name="total_bytes_sec_max"> + <data type="unsignedLong"/> + </element> + <group> + <interleave> + <optional> + <element name="read_bytes_sec_max"> + <data type="unsignedLong"/> + </element> + </optional> + <optional> + <element name="write_bytes_sec_max"> + <data type="unsignedLong"/> + </element> + </optional> + </interleave> + </group> + </choice> + <choice> + <element name="total_iops_sec_max"> + <data type="unsignedLong"/> + </element> + <group> + <interleave> + <optional> + <element name="read_iops_sec_max"> + <data type="unsignedLong"/> + </element> + </optional> + <optional> + <element name="write_iops_sec_max"> + <data type="unsignedLong"/> + </element> + </optional> + </interleave> + </group> + </choice> + <optional> + <element name="size_iops_sec"> + <data type="unsignedLong"/> + </element> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a9c6f05..5c22ddd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5815,6 +5815,49 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, def->blkdeviotune.write_iops_sec = 0; } + if (virXPathULongLong("string(./iotune/total_bytes_sec_max)", + ctxt, + &def->blkdeviotune.total_bytes_sec_max) < 0) { + def->blkdeviotune.total_bytes_sec_max = 0; + } + + if (virXPathULongLong("string(./iotune/read_bytes_sec_max)", + ctxt, + &def->blkdeviotune.read_bytes_sec_max) < 0) { + def->blkdeviotune.read_bytes_sec_max = 0; + } + + if (virXPathULongLong("string(./iotune/write_bytes_sec_max)", + ctxt, + &def->blkdeviotune.write_bytes_sec_max) < 0) { + def->blkdeviotune.write_bytes_sec_max = 0; + } + + if (virXPathULongLong("string(./iotune/total_iops_sec_max)", + ctxt, + &def->blkdeviotune.total_iops_sec_max) < 0) { + def->blkdeviotune.total_iops_sec_max = 0; + } + + if (virXPathULongLong("string(./iotune/read_iops_sec_max)", + ctxt, + &def->blkdeviotune.read_iops_sec_max) < 0) { + def->blkdeviotune.read_iops_sec_max = 0; + } + + if (virXPathULongLong("string(./iotune/write_iops_sec_max)", + ctxt, + &def->blkdeviotune.write_iops_sec_max) < 0) { + def->blkdeviotune.write_iops_sec_max = 0; + } + + if (virXPathULongLong("string(./iotune/size_iops_sec)", + ctxt, + &def->blkdeviotune.size_iops_sec) < 0) { + def->blkdeviotune.size_iops_sec = 0; + } + + if ((def->blkdeviotune.total_bytes_sec && def->blkdeviotune.read_bytes_sec) || (def->blkdeviotune.total_bytes_sec && @@ -5834,6 +5877,27 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, "cannot be set at the same time")); goto error; } + + if ((def->blkdeviotune.total_bytes_sec_max && + def->blkdeviotune.read_bytes_sec_max) || + (def->blkdeviotune.total_bytes_sec_max && + def->blkdeviotune.write_bytes_sec_max)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("total and read/write bytes_sec_max " + "cannot be set at the same time")); + goto error; + } + + if ((def->blkdeviotune.total_iops_sec_max && + def->blkdeviotune.read_iops_sec_max) || + (def->blkdeviotune.total_iops_sec_max && + def->blkdeviotune.write_iops_sec_max)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("total and read/write iops_sec_max " + "cannot be set at the same time")); + goto error; + } + } else if (xmlStrEqual(cur->name, BAD_CAST "readonly")) { def->src->readonly = true; } else if (xmlStrEqual(cur->name, BAD_CAST "shareable")) { @@ -16218,7 +16282,14 @@ virDomainDiskDefFormat(virBufferPtr buf, def->blkdeviotune.write_bytes_sec || def->blkdeviotune.total_iops_sec || def->blkdeviotune.read_iops_sec || - def->blkdeviotune.write_iops_sec) { + def->blkdeviotune.write_iops_sec || + def->blkdeviotune.total_bytes_sec_max || + def->blkdeviotune.read_bytes_sec_max || + def->blkdeviotune.write_bytes_sec_max || + def->blkdeviotune.total_iops_sec_max || + def->blkdeviotune.read_iops_sec_max || + def->blkdeviotune.write_iops_sec_max || + def->blkdeviotune.size_iops_sec) { virBufferAddLit(buf, "<iotune>\n"); virBufferAdjustIndent(buf, 2); if (def->blkdeviotune.total_bytes_sec) { @@ -16251,6 +16322,43 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferAsprintf(buf, "<write_iops_sec>%llu</write_iops_sec>\n", def->blkdeviotune.write_iops_sec); } + + if (def->blkdeviotune.total_bytes_sec_max) { + virBufferAsprintf(buf, "<total_bytes_sec_max>%llu</total_bytes_sec_max>\n", + def->blkdeviotune.total_bytes_sec_max); + } + + if (def->blkdeviotune.read_bytes_sec_max) { + virBufferAsprintf(buf, "<read_bytes_sec_max>%llu</read_bytes_sec_max>\n", + def->blkdeviotune.read_bytes_sec_max); + + } + + if (def->blkdeviotune.write_bytes_sec_max) { + virBufferAsprintf(buf, "<write_bytes_sec_max>%llu</write_bytes_sec_max>\n", + def->blkdeviotune.write_bytes_sec_max); + } + + if (def->blkdeviotune.total_iops_sec_max) { + virBufferAsprintf(buf, "<total_iops_sec_max>%llu</total_iops_sec_max>\n", + def->blkdeviotune.total_iops_sec_max); + } + + if (def->blkdeviotune.read_iops_sec_max) { + virBufferAsprintf(buf, "<read_iops_sec_max>%llu</read_iops_sec_max>\n", + def->blkdeviotune.read_iops_sec_max); + } + + if (def->blkdeviotune.write_iops_sec_max) { + virBufferAsprintf(buf, "<write_iops_sec_max>%llu</write_iops_sec_max>\n", + def->blkdeviotune.write_iops_sec_max); + } + + if (def->blkdeviotune.size_iops_sec) { + virBufferAsprintf(buf, "<size_iops_sec>%llu</size_iops_sec>\n", + def->blkdeviotune.size_iops_sec); + } + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</iotune>\n"); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d73d654..0692852 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -612,6 +612,13 @@ struct _virDomainBlockIoTuneInfo { unsigned long long total_iops_sec; unsigned long long read_iops_sec; unsigned long long write_iops_sec; + unsigned long long total_bytes_sec_max; + unsigned long long read_bytes_sec_max; + unsigned long long write_bytes_sec_max; + unsigned long long total_iops_sec_max; + unsigned long long read_iops_sec_max; + unsigned long long write_iops_sec_max; + unsigned long long size_iops_sec; }; typedef virDomainBlockIoTuneInfo *virDomainBlockIoTuneInfoPtr; diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index b8177c0..ad33c10 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1835,7 +1835,7 @@ testQemuMonitorJSONqemuMonitorJSONSetBlockIoThrottle(const void *data) if (!test) return -1; - expectedInfo = (virDomainBlockIoTuneInfo) {1, 2, 3, 4, 5, 6}; + expectedInfo = (virDomainBlockIoTuneInfo) {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13}; if (qemuMonitorTestAddItem(test, "query-block", queryBlockReply) < 0 || qemuMonitorTestAddItemParams(test, "block_set_io_throttle", -- 1.8.3.1

On 10/07/2014 05:14 AM, Matthias Gatto wrote:
Modify the structure _virDomainBlockIoTuneInfo to support these the new options. Change the initialization of the variable expectedInfo in qemumonitorjsontest.c to avoid compiling problem. Add documentation about the new xml options
Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com> --- docs/formatdomain.html.in | 25 ++++++++++ docs/schemas/domaincommon.rng | 43 +++++++++++++++++ src/conf/domain_conf.c | 110 +++++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 7 +++ tests/qemumonitorjsontest.c | 2 +- 5 files changed, 185 insertions(+), 2 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 45b0f61..6ec7b6b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2103,6 +2103,31 @@ <dt><code>write_iops_sec</code></dt> <dd>The optional <code>write_iops_sec</code> element is the write I/O operations per second.</dd> + <dt><code>total_bytes_sec_max</code></dt> + <dd>The optional <code>total_bytes_sec_max</code> element is the + maximum total throughput limit in bytes per second. This cannot + appear with <code>read_bytes_sec_max</code> + or <code>write_bytes_sec_max</code>.</dd>
Would be worth a 'since 1.2.9' designation throughout this section.
+ + if (def->blkdeviotune.read_bytes_sec_max) { + virBufferAsprintf(buf, "<read_bytes_sec_max>%llu</read_bytes_sec_max>\n", + def->blkdeviotune.read_bytes_sec_max); + + }
Awkward blank line before the }. ACK; I can fix up the docs and the blank line. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Add a value in the enum virQEMUCapsFlags for the qemu capability. Set it with virQEMUCapsSet if the binary suport bps_max and they friends. Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 + 3 files changed, 4 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 6fcb5c7..73fd812 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -271,6 +271,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "iothread", "migrate-rdma", "ivshmem", + "drive-iotune-max", ); @@ -2450,6 +2451,7 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { { "spice", "disable-agent-file-xfer", QEMU_CAPS_SPICE_FILE_XFER_DISABLE }, { "msg", "timestamp", QEMU_CAPS_MSG_TIMESTAMP }, { "numa", NULL, QEMU_CAPS_NUMA }, + { "drive", "throttling.bps-total-max", QEMU_CAPS_DRIVE_IOTUNE_MAX}, }; static int diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 0214f30..9161088 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -218,6 +218,7 @@ typedef enum { QEMU_CAPS_OBJECT_IOTHREAD = 176, /* -object iothread */ QEMU_CAPS_MIGRATE_RDMA = 177, /* have rdma migration */ QEMU_CAPS_DEVICE_IVSHMEM = 178, /* -device ivshmem */ + QEMU_CAPS_DRIVE_IOTUNE_MAX = 179, /* -drive bps_max= and friends */ QEMU_CAPS_LAST, /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps index 7fc654d..7003ad8 100644 --- a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps +++ b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps @@ -160,4 +160,5 @@ <flag name='iothread'/> <flag name='migrate-rdma'/> <flag name='ivshmem'/> + <flag name='drive-iotune-max'/> </qemuCaps> -- 1.8.3.1

Add support for bps_max and friends in the driver part. In the part checking if a qemu is running, check if the running binary support bps_max, if not print an error message, if yes add it to "info" variable Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com> --- include/libvirt/libvirt.h.in | 56 +++++++++++++ src/qemu/qemu_driver.c | 186 +++++++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_monitor.c | 10 ++- src/qemu/qemu_monitor.h | 6 +- src/qemu/qemu_monitor_json.c | 11 ++- src/qemu/qemu_monitor_json.h | 6 +- tests/qemumonitorjsontest.c | 4 +- 7 files changed, 258 insertions(+), 21 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 459a292..c1590d6 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -5379,6 +5379,62 @@ typedef void (*virConnectDomainEventDeviceRemovedCallback)(virConnectPtr conn, #define VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC "blkdeviotune.write_iops_sec" /** + * VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX: + * + * Marco represents the total throughput limit in maximum bytes per second, + * as VIR_TYPED_PARAM_ULLONG. + */ +#define VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_BYTES_SEC_MAX "blkdeviotune.total_bytes_sec_max" + +/** + * VIR_DOMAIN_TUNABLE_BLKDEV_READ_BYTES_SEC_MAX: + * + * Marco represents the read throughput limit in maximum bytes per second, + * as VIR_TYPED_PARAM_ULLONG. + */ +#define VIR_DOMAIN_TUNABLE_BLKDEV_READ_BYTES_SEC_MAX "blkdeviotune.read_bytes_sec_max" + +/** + * VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_BYTES_SEC_MAX: + * + * Macro represents the write throughput limit in maximum bytes per second, + * as VIR_TYPED_PARAM_ULLONG. + */ +#define VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_BYTES_SEC_MAX "blkdeviotune.write_bytes_sec_max" + +/** + * VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX: + * + * Macro represents the total maximum I/O operations per second, + * as VIR_TYPED_PARAM_ULLONG. + */ +#define VIR_DOMAIN_TUNABLE_BLKDEV_TOTAL_IOPS_SEC_MAX "blkdeviotune.total_iops_sec_max" + +/** + * VIR_DOMAIN_TUNABLE_BLKDEV_READ_IOPS_SEC_MAX: + * + * Macro represents the read maximum I/O operations per second, + * as VIR_TYPED_PARAM_ULLONG. + */ +#define VIR_DOMAIN_TUNABLE_BLKDEV_READ_IOPS_SEC_MAX "blkdeviotune.read_iops_sec_max" + +/** + * VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC_MAX: + * + * Macro represents the write maximum I/O operations per second, + * as VIR_TYPED_PARAM_ULLONG. + */ +#define VIR_DOMAIN_TUNABLE_BLKDEV_WRITE_IOPS_SEC_MAX "blkdeviotune.write_iops_sec_max" + +/** + * VIR_DOMAIN_TUNABLE_BLKDEV_SIZE_IOPS_SEC: + * + * Macro represents the size maximum I/O operations per second, + * as VIR_TYPED_PARAM_ULLONG. + */ +#define VIR_DOMAIN_TUNABLE_BLKDEV_SIZE_IOPS_SEC "blkdeviotune.size_iops_sec" + +/** * virConnectDomainEventTunableCallback: * @conn: connection object * @dom: domain on which the event occurred diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d111ccd..a913a7d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -105,6 +105,7 @@ VIR_LOG_INIT("qemu.qemu_driver"); #define QEMU_NB_MEM_PARAM 3 #define QEMU_NB_BLOCK_IO_TUNE_PARAM 6 +#define QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX 13 #define QEMU_NB_NUMA_PARAM 2 @@ -16284,6 +16285,10 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, int conf_idx = -1; bool set_bytes = false; bool set_iops = false; + bool set_bytes_max = false; + bool set_iops_max = false; + bool set_size_iops = false; + bool supportMaxOptions = true; virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; virObjectEventPtr event = NULL; @@ -16306,6 +16311,20 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, VIR_TYPED_PARAM_ULLONG, VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC, VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX, + VIR_TYPED_PARAM_ULLONG, + VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC, + VIR_TYPED_PARAM_ULLONG, NULL) < 0) return -1; @@ -16398,6 +16417,69 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, 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; } } @@ -16415,6 +16497,20 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, goto endjob; } + if ((info.total_bytes_sec_max && info.read_bytes_sec_max) || + (info.total_bytes_sec_max && info.write_bytes_sec_max)) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("total and read/write of bytes_sec_max cannot be set at the same time")); + goto endjob; + } + + if ((info.total_iops_sec_max && info.read_iops_sec_max) || + (info.total_iops_sec_max && info.write_iops_sec_max)) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("total and read/write of iops_sec_max cannot be set at the same time")); + goto endjob; + } + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { if ((conf_idx = virDomainDiskIndexByName(persistentDef, disk, true)) < 0) { virReportError(VIR_ERR_INVALID_ARG, @@ -16425,6 +16521,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, } if (flags & VIR_DOMAIN_AFFECT_LIVE) { + supportMaxOptions = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE); if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("block I/O throttling not supported with this " @@ -16432,6 +16529,13 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, goto endjob; } + if (!supportMaxOptions && (set_iops_max || set_bytes_max)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("a block I/O throttling parameter is not supported with this " + "QEMU binary")); + goto endjob; + } + if (!(device = qemuDiskPathToAlias(vm, disk, &idx))) goto endjob; @@ -16444,13 +16548,25 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, 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; qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorSetBlockIoThrottle(priv->mon, device, &info); + ret = qemuMonitorSetBlockIoThrottle(priv->mon, device, &info, supportMaxOptions); qemuDomainObjExitMonitor(driver, vm); if (ret < 0) goto endjob; @@ -16494,6 +16610,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, } endjob: + if (!qemuDomainObjEndJob(driver, vm)) vm = NULL; @@ -16517,13 +16634,14 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm = NULL; - qemuDomainObjPrivatePtr priv; + qemuDomainObjPrivatePtr priv = NULL; virDomainDefPtr persistentDef = NULL; virDomainBlockIoTuneInfo reply; char *device = NULL; int ret = -1; size_t i; virCapsPtr caps = NULL; + bool supportMaxOptions = true; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG | @@ -16541,9 +16659,14 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; + if (flags & VIR_DOMAIN_AFFECT_LIVE) { + priv = vm->privateData; + supportMaxOptions = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX); + } + if ((*nparams) == 0) { /* Current number of parameters supported by QEMU Block I/O Throttling */ - *nparams = QEMU_NB_BLOCK_IO_TUNE_PARAM; + *nparams = supportMaxOptions ? QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX : QEMU_NB_BLOCK_IO_TUNE_PARAM; ret = 0; goto cleanup; } @@ -16561,9 +16684,8 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, } if (flags & VIR_DOMAIN_AFFECT_LIVE) { - priv = vm->privateData; qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, &reply); + ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, &reply, supportMaxOptions); qemuDomainObjExitMonitor(driver, vm); if (ret < 0) goto endjob; @@ -16580,7 +16702,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, reply = persistentDef->disks[idx]->blkdeviotune; } - for (i = 0; i < QEMU_NB_BLOCK_IO_TUNE_PARAM && i < *nparams; i++) { + for (i = 0; i < QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX && i < *nparams; i++) { virTypedParameterPtr param = ¶ms[i]; switch (i) { @@ -16626,14 +16748,64 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, reply.write_iops_sec) < 0) goto endjob; break; + case 6: + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC_MAX, + VIR_TYPED_PARAM_ULLONG, + reply.total_bytes_sec_max) < 0) + goto endjob; + break; + case 7: + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC_MAX, + VIR_TYPED_PARAM_ULLONG, + reply.read_bytes_sec_max) < 0) + goto endjob; + break; + case 8: + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC_MAX, + VIR_TYPED_PARAM_ULLONG, + reply.write_bytes_sec_max) < 0) + goto endjob; + break; + case 9: + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC_MAX, + VIR_TYPED_PARAM_ULLONG, + reply.total_iops_sec_max) < 0) + goto endjob; + break; + case 10: + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC_MAX, + VIR_TYPED_PARAM_ULLONG, + reply.read_iops_sec_max) < 0) + goto endjob; + break; + case 11: + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC_MAX, + VIR_TYPED_PARAM_ULLONG, + reply.write_iops_sec_max) < 0) + goto endjob; + break; + case 12: + if (virTypedParameterAssign(param, + VIR_DOMAIN_BLOCK_IOTUNE_SIZE_IOPS_SEC, + VIR_TYPED_PARAM_ULLONG, + reply.size_iops_sec) < 0) + goto endjob; /* coverity[dead_error_begin] */ default: break; } } - if (*nparams > QEMU_NB_BLOCK_IO_TUNE_PARAM) + if (!supportMaxOptions && *nparams > QEMU_NB_BLOCK_IO_TUNE_PARAM) *nparams = QEMU_NB_BLOCK_IO_TUNE_PARAM; + else if (*nparams > QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX) + *nparams = QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX; ret = 0; endjob: diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 00c62f7..2e2e8ea 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3427,14 +3427,15 @@ qemuMonitorBlockJobInfo(qemuMonitorPtr mon, int qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr info) + virDomainBlockIoTuneInfoPtr info, + bool supportMaxOptions) { int ret; VIR_DEBUG("mon=%p, device=%p, info=%p", mon, device, info); if (mon->json) { - ret = qemuMonitorJSONSetBlockIoThrottle(mon, device, info); + ret = qemuMonitorJSONSetBlockIoThrottle(mon, device, info, supportMaxOptions); } else { ret = qemuMonitorTextSetBlockIoThrottle(mon, device, info); } @@ -3443,14 +3444,15 @@ int qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon, int qemuMonitorGetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr reply) + virDomainBlockIoTuneInfoPtr reply, + bool supportMaxOptions) { int ret; VIR_DEBUG("mon=%p, device=%p, reply=%p", mon, device, reply); if (mon->json) { - ret = qemuMonitorJSONGetBlockIoThrottle(mon, device, reply); + ret = qemuMonitorJSONGetBlockIoThrottle(mon, device, reply, supportMaxOptions); } else { ret = qemuMonitorTextGetBlockIoThrottle(mon, device, reply); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 63e14cc..3388e64 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -752,11 +752,13 @@ int qemuMonitorOpenGraphics(qemuMonitorPtr mon, int qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr info); + virDomainBlockIoTuneInfoPtr info, + bool supportMaxOptions); int qemuMonitorGetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr reply); + virDomainBlockIoTuneInfoPtr reply, + bool supportMaxOptions); int qemuMonitorSystemWakeup(qemuMonitorPtr mon); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 6504d15..a118c1f 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4161,7 +4161,8 @@ int qemuMonitorJSONOpenGraphics(qemuMonitorPtr mon, static int qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result, const char *device, - virDomainBlockIoTuneInfoPtr reply) + virDomainBlockIoTuneInfoPtr reply, + bool supportMaxOptions ATTRIBUTE_UNUSED) { virJSONValuePtr io_throttle; int ret = -1; @@ -4232,7 +4233,8 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result, int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr info) + virDomainBlockIoTuneInfoPtr info, + bool supportMaxOptions ATTRIBUTE_UNUSED) { int ret = -1; virJSONValuePtr cmd = NULL; @@ -4272,7 +4274,8 @@ int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon, int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr reply) + virDomainBlockIoTuneInfoPtr reply, + bool supportMaxOptions) { int ret = -1; virJSONValuePtr cmd = NULL; @@ -4299,7 +4302,7 @@ int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr mon, } if (ret == 0) - ret = qemuMonitorJSONBlockIoThrottleInfo(result, device, reply); + ret = qemuMonitorJSONBlockIoThrottleInfo(result, device, reply, supportMaxOptions); virJSONValueFree(cmd); virJSONValueFree(result); diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index c7dd416..ba977aa 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -314,11 +314,13 @@ int qemuMonitorJSONOpenGraphics(qemuMonitorPtr mon, int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr info); + virDomainBlockIoTuneInfoPtr info, + bool supportMaxOptions); int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr mon, const char *device, - virDomainBlockIoTuneInfoPtr reply); + virDomainBlockIoTuneInfoPtr reply, + bool supportMaxOptions); int qemuMonitorJSONSystemWakeup(qemuMonitorPtr mon); diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index ad33c10..1b202fb 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1847,7 +1847,7 @@ testQemuMonitorJSONqemuMonitorJSONSetBlockIoThrottle(const void *data) goto cleanup; if (qemuMonitorJSONGetBlockIoThrottle(qemuMonitorTestGetMonitor(test), - "drive-virtio-disk0", &info) < 0) + "drive-virtio-disk0", &info, false) < 0) goto cleanup; if (memcmp(&info, &expectedInfo, sizeof(info) != 0)) { @@ -1857,7 +1857,7 @@ testQemuMonitorJSONqemuMonitorJSONSetBlockIoThrottle(const void *data) } if (qemuMonitorJSONSetBlockIoThrottle(qemuMonitorTestGetMonitor(test), - "drive-virtio-disk1", &info) < 0) + "drive-virtio-disk1", &info, false) < 0) goto cleanup; ret = 0; -- 1.8.3.1

Detect if the the qemu binary currently in use suport the bps_max option, If yes add it to the command, if not, just ignore the options. Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com> --- src/qemu/qemu_monitor_json.c | 57 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 46 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index a118c1f..50f5888 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4148,6 +4148,12 @@ int qemuMonitorJSONOpenGraphics(qemuMonitorPtr mon, } +#define GET_THROTTLE_STATS_OPTIONAL(FIELD, STORE) \ + if (virJSONValueObjectGetNumberUlong(inserted, \ + FIELD, \ + &reply->STORE) < 0) { \ + reply->STORE = 0; \ + } #define GET_THROTTLE_STATS(FIELD, STORE) \ if (virJSONValueObjectGetNumberUlong(inserted, \ FIELD, \ @@ -4162,7 +4168,7 @@ static int qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result, const char *device, virDomainBlockIoTuneInfoPtr reply, - bool supportMaxOptions ATTRIBUTE_UNUSED) + bool supportMaxOptions) { virJSONValuePtr io_throttle; int ret = -1; @@ -4214,6 +4220,15 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result, GET_THROTTLE_STATS("iops", total_iops_sec); GET_THROTTLE_STATS("iops_rd", read_iops_sec); GET_THROTTLE_STATS("iops_wr", write_iops_sec); + if (supportMaxOptions) { + GET_THROTTLE_STATS_OPTIONAL("bps_max", total_bytes_sec_max); + GET_THROTTLE_STATS_OPTIONAL("bps_rd_max", read_bytes_sec_max); + GET_THROTTLE_STATS_OPTIONAL("bps_wr_max", write_bytes_sec_max); + GET_THROTTLE_STATS_OPTIONAL("iops_max", total_iops_sec_max); + 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); + } break; } @@ -4230,25 +4245,45 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result, return ret; } #undef GET_THROTTLE_STATS +#undef GET_THROTTLE_STATS_OPTIONAL int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon, const char *device, virDomainBlockIoTuneInfoPtr info, - bool supportMaxOptions ATTRIBUTE_UNUSED) + bool supportMaxOptions) { int ret = -1; virJSONValuePtr cmd = NULL; virJSONValuePtr result = NULL; - cmd = qemuMonitorJSONMakeCommand("block_set_io_throttle", - "s:device", device, - "U:bps", info->total_bytes_sec, - "U:bps_rd", info->read_bytes_sec, - "U:bps_wr", info->write_bytes_sec, - "U:iops", info->total_iops_sec, - "U:iops_rd", info->read_iops_sec, - "U:iops_wr", info->write_iops_sec, - NULL); + if (supportMaxOptions) { + cmd = qemuMonitorJSONMakeCommand("block_set_io_throttle", + "s:device", device, + "U:bps", info->total_bytes_sec, + "U:bps_rd", info->read_bytes_sec, + "U:bps_wr", info->write_bytes_sec, + "U:iops", info->total_iops_sec, + "U:iops_rd", info->read_iops_sec, + "U:iops_wr", info->write_iops_sec, + "U:bps_max", info->total_bytes_sec_max, + "U:bps_rd_max", info->read_bytes_sec_max, + "U:bps_wr_max", info->write_bytes_sec_max, + "U:iops_max", info->total_iops_sec_max, + "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, + NULL); + } else { + cmd = qemuMonitorJSONMakeCommand("block_set_io_throttle", + "s:device", device, + "U:bps", info->total_bytes_sec, + "U:bps_rd", info->read_bytes_sec, + "U:bps_wr", info->write_bytes_sec, + "U:iops", info->total_iops_sec, + "U:iops_rd", info->read_iops_sec, + "U:iops_wr", info->write_iops_sec, + NULL); + } if (!cmd) return -1; -- 1.8.3.1

On 10/07/2014 05:14 AM, Matthias Gatto wrote:
Detect if the the qemu binary currently in use suport the bps_max option, If yes add it to the command, if not, just ignore the options.
Ignoring options is wrong. If a user explicitly requested something in XML but the hypervisor cannot honor it, we want to error out. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Sat, Oct 25, 2014 at 6:30 AM, Eric Blake <eblake@redhat.com> wrote:
On 10/07/2014 05:14 AM, Matthias Gatto wrote:
Detect if the the qemu binary currently in use suport the bps_max option, If yes add it to the command, if not, just ignore the options.
Ignoring options is wrong. If a user explicitly requested something in XML but the hypervisor cannot honor it, we want to error out.
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
But I've already check this in qemu_driver.c We can check it twice, but will it be really useful ? Maybe I should write the check has already been made in the commit message ?

On 10/27/2014 09:51 AM, Matthias Gatto wrote:
On Sat, Oct 25, 2014 at 6:30 AM, Eric Blake <eblake@redhat.com> wrote:
On 10/07/2014 05:14 AM, Matthias Gatto wrote:
Detect if the the qemu binary currently in use suport the bps_max option,
s/suport/supports/
If yes add it to the command, if not, just ignore the options.
Ignoring options is wrong. If a user explicitly requested something in XML but the hypervisor cannot honor it, we want to error out.
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
But I've already check this in qemu_driver.c We can check it twice, but will it be really useful ?
We have to check in two places: when constructing the command line (qemu_command.c) and when hotplugging a new value (qemu_driver.c calling into qemu_monitor.c). In general, if the user has requested the XML, but qemu can't support it, then we must error out. I haven't fully read the series to see if you adequately filtered things out earlier; but if the filter happened earlier, then a comment in the code pointing to the other location that did the filtering would help (not just the commit message).
Maybe I should write the check has already been made in the commit message ?
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Check the arability of the options with the current qemu binary, add them in the varable opt if yes, print a message if not. Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com> --- src/qemu/qemu_command.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8cb0865..22c6a92 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3669,12 +3669,32 @@ qemuBuildDriveStr(virConnectPtr conn, goto error; } + /* block I/O throttling 1.7 */ + if ((disk->blkdeviotune.total_bytes_sec_max || + disk->blkdeviotune.read_bytes_sec_max || + disk->blkdeviotune.write_bytes_sec_max || + disk->blkdeviotune.total_iops_sec_max || + disk->blkdeviotune.read_iops_sec_max || + disk->blkdeviotune.write_iops_sec_max) && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_IOTUNE_MAX)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("there is some block I/O throttling paramater that are not supported with this " + "QEMU binary(need QEMU 1.7 or superior)")); + goto error; + } + if (disk->blkdeviotune.total_bytes_sec > LLONG_MAX || disk->blkdeviotune.read_bytes_sec > LLONG_MAX || disk->blkdeviotune.write_bytes_sec > LLONG_MAX || disk->blkdeviotune.total_iops_sec > LLONG_MAX || disk->blkdeviotune.read_iops_sec > LLONG_MAX || - disk->blkdeviotune.write_iops_sec > LLONG_MAX) { + disk->blkdeviotune.write_iops_sec > LLONG_MAX || + disk->blkdeviotune.total_bytes_sec_max > LLONG_MAX || + disk->blkdeviotune.read_bytes_sec_max > LLONG_MAX || + disk->blkdeviotune.write_bytes_sec_max > LLONG_MAX || + disk->blkdeviotune.total_iops_sec_max > LLONG_MAX || + disk->blkdeviotune.read_iops_sec_max > LLONG_MAX || + disk->blkdeviotune.write_iops_sec_max > LLONG_MAX) { virReportError(VIR_ERR_OVERFLOW, _("block I/O throttle limit must " "be less than %llu using QEMU"), LLONG_MAX); @@ -3711,6 +3731,41 @@ qemuBuildDriveStr(virConnectPtr conn, disk->blkdeviotune.write_iops_sec); } + if (disk->blkdeviotune.total_bytes_sec_max) { + virBufferAsprintf(&opt, ",bps_max=%llu", + disk->blkdeviotune.total_bytes_sec_max); + } + + if (disk->blkdeviotune.read_bytes_sec_max) { + virBufferAsprintf(&opt, ",bps_rd_max=%llu", + disk->blkdeviotune.read_bytes_sec_max); + } + + if (disk->blkdeviotune.write_bytes_sec_max) { + virBufferAsprintf(&opt, ",bps_wr_max=%llu", + disk->blkdeviotune.write_bytes_sec_max); + } + + if (disk->blkdeviotune.total_iops_sec_max) { + virBufferAsprintf(&opt, ",iops_max=%llu", + disk->blkdeviotune.total_iops_sec_max); + } + + if (disk->blkdeviotune.read_iops_sec_max) { + virBufferAsprintf(&opt, ",iops_rd_max=%llu", + disk->blkdeviotune.read_iops_sec_max); + } + + if (disk->blkdeviotune.write_iops_sec_max) { + virBufferAsprintf(&opt, ",iops_wr_max=%llu", + disk->blkdeviotune.write_iops_sec_max); + } + + if (disk->blkdeviotune.write_iops_sec_max) { + virBufferAsprintf(&opt, ",iops_size=%llu", + disk->blkdeviotune.size_iops_sec); + } + if (virBufferCheckError(&opt) < 0) goto error; -- 1.8.3.1

Add the new throttle options to virsh, and send them to libvirt. Signed-off-by: Matthias Gatto <matthias.gatto@outscale.com> --- tools/virsh-domain.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 10 +++++ 2 files changed, 129 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 12550ff..9eba253 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1111,6 +1111,62 @@ static const vshCmdOptDef opts_blkdeviotune[] = { .type = VSH_OT_INT, .help = N_("write I/O operations limit per second") }, + {.name = "total_bytes_sec_max", + .type = VSH_OT_ALIAS, + .help = "total-bytes-sec-max" + }, + {.name = "total-bytes-sec-max", + .type = VSH_OT_INT, + .help = N_("total max in bytes") + }, + {.name = "read_bytes_sec_max", + .type = VSH_OT_ALIAS, + .help = "read-bytes-sec-max" + }, + {.name = "read-bytes-sec-max", + .type = VSH_OT_INT, + .help = N_("read max in bytes") + }, + {.name = "write_bytes_sec_max", + .type = VSH_OT_ALIAS, + .help = "write-bytes-sec-max" + }, + {.name = "write-bytes-sec-max", + .type = VSH_OT_INT, + .help = N_("write max in bytes") + }, + {.name = "total_iops_sec_max", + .type = VSH_OT_ALIAS, + .help = "total-iops-sec-max" + }, + {.name = "total-iops-sec-max", + .type = VSH_OT_INT, + .help = N_("total I/O operations max") + }, + {.name = "read_iops_sec_max", + .type = VSH_OT_ALIAS, + .help = "read-iops-sec-max" + }, + {.name = "read-iops-sec-max", + .type = VSH_OT_INT, + .help = N_("read I/O operations max") + }, + {.name = "write_iops_sec_max", + .type = VSH_OT_ALIAS, + .help = "write-iops-sec-max" + }, + {.name = "write-iops-sec-max", + .type = VSH_OT_INT, + .help = N_("write I/O operations max") + }, + {.name = "size_iops_sec", + .type = VSH_OT_ALIAS, + .help = "size-iops-sec" + }, + {.name = "size-iops-sec", + .type = VSH_OT_INT, + .help = N_("I/O size in bytes") + }, {.name = "config", .type = VSH_OT_BOOL, .help = N_("affect next boot") @@ -1184,6 +1240,33 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) goto save_error; } + if ((rv = vshCommandOptULongLong(cmd, "total-bytes-sec-max", &value)) < 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 = vshCommandOptULongLong(cmd, "read-bytes-sec-max", &value)) < 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 = vshCommandOptULongLong(cmd, "write-bytes-sec-max", &value)) < 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(cmd, "total-iops-sec", &value)) < 0) { goto interror; } else if (rv > 0) { @@ -1211,6 +1294,42 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) goto save_error; } + if ((rv = vshCommandOptULongLong(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(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(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(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; + } + 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 eae9195..1fa3c52 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1052,6 +1052,9 @@ convert it to the maximum value allowed. [[I<--config>] [I<--live>] | [I<--current>]] [[I<total-bytes-sec>] | [I<read-bytes-sec>] [I<write-bytes-sec>]] [[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<size-iops-sec>] Set or query the block disk io parameters for a block device of I<domain>. I<device> specifies a unique target name (<target dev='name'/>) or source @@ -1066,6 +1069,13 @@ I<--write-bytes-sec> specifies write throughput limit in bytes per second. I<--total-iops-sec> specifies total I/O operations limit per second. I<--read-iops-sec> specifies read I/O operations limit per second. I<--write-iops-sec> specifies write I/O operations limit per second. +I<--total-bytes-sec-max> specifies maximum total throughput limit in bytes per second. +I<--read-bytes-sec-max> specifies maximum read throughput limit in bytes per second. +I<--write-bytes-sec-max> specifies maximum write throughput limit in bytes per second. +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<--size-iops-sec> specifies size I/O operations limit per second. Older versions of virsh only accepted these options with underscore instead of dash, as in I<--total_bytes_sec>. -- 1.8.3.1

On Tue, Oct 7, 2014 at 1:14 PM, Matthias Gatto <matthias.gatto@outscale.com> wrote:
This series of patches add support for bps_max, bps_rd_max, bps_wr_max, bps_max, bps_rd_max, bps_wr_max, and iops_size in the functions qemuDomainSetBlockIoTune and qemuDomainGetBlockIoTune. The last patch add support for these parameters to the virsh blkdeviotune command.
v2: spellfix
v3: Merge patch 1/9,2/9,5/9 together. Change the capability detection.(patch 2/7 and 3/7). Try to make the usage of QEMU_NB_BLOCK_IO_TUNE_PARAM_MAX more explicit(patch 3/7).
v4: Rebase on HEAD. Update qemu_driver to comply with Pavel's patchs.(patch 3/6) Remove the qemu_monitor_text modification.(remove old patch 5/7)
v5: Split patch 1/6 in two. Add documentation for the new xml options (patch 2/7) Change (void) to ATTRIBUTE_UNUSED (patch 4/7) Capability detection of supportMaxOptions move before usage of supportMaxOptions
Matthias Gatto (7): qemu: Add define for the news throttle options qemu: Modify the structure _virDomainBlockIoTuneInfo. qemu: Add the capability to detect if the qemu binary have the capability to use bps_max and friends qemu: Add bps_max and friends qemu driver qemu: Add bps_max and friends QMP suport qemu: add bps_max and friends to qemu command generation virsh: Add bps_max and friends to virsh
docs/formatdomain.html.in | 25 ++++ docs/schemas/domaincommon.rng | 43 +++++++ include/libvirt/libvirt.h.in | 110 ++++++++++++++++ src/conf/domain_conf.c | 110 +++++++++++++++- src/conf/domain_conf.h | 7 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 57 +++++++- src/qemu/qemu_driver.c | 186 ++++++++++++++++++++++++++- src/qemu/qemu_monitor.c | 10 +- src/qemu/qemu_monitor.h | 6 +- src/qemu/qemu_monitor_json.c | 64 +++++++-- src/qemu/qemu_monitor_json.h | 6 +- tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 + tests/qemumonitorjsontest.c | 6 +- tools/virsh-domain.c | 119 +++++++++++++++++ tools/virsh.pod | 10 ++ 17 files changed, 730 insertions(+), 33 deletions(-)
-- 1.8.3.1
ping
participants (2)
-
Eric Blake
-
Matthias Gatto