* [Qemu-devel] [PATCH v11 0/6] fsdev: qmp interface for io throttling
@ 2017-09-14 10:40 Pradeep Jagadeesh
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code Pradeep Jagadeesh
` (5 more replies)
0 siblings, 6 replies; 25+ messages in thread
From: Pradeep Jagadeesh @ 2017-09-14 10:40 UTC (permalink / raw)
To: eric blake, greg kurz
Cc: Pradeep Jagadeesh, alberto garcia, Markus Armbruster,
Dr. David Alan Gilbert, jani kokkonen, qemu-devel
These patches provide the qmp interface, to query the io throttle
status of the all fsdev devices that are present in a vm.
also, it provides an interface to set the io throttle parameters of a
fsdev to a required value. Some of the patches also remove the
duplicate code that was present in block and fsdev files.
Pradeep Jagadeesh (6):
throttle: factor out duplicate code
qmp: Use ThrottleLimits structure
qmp: factor out throttle code to reuse code
hmp: create a throttle initialization function for code reuse
fsdev: QMP interface for throttling
fsdev: hmp interface for throttling
Makefile | 3 +-
blockdev.c | 97 ++--------------------------
fsdev/qemu-fsdev-dummy.c | 11 ++++
fsdev/qemu-fsdev-throttle.c | 140 ++++++++++++++++++++++++++--------------
fsdev/qemu-fsdev-throttle.h | 9 ++-
fsdev/qemu-fsdev.c | 30 +++++++++
hmp-commands-info.hx | 18 ++++++
hmp-commands.hx | 19 ++++++
hmp.c | 79 +++++++++++++++++++++--
hmp.h | 4 ++
include/qemu/throttle-options.h | 3 +
include/qemu/throttle.h | 4 +-
include/qemu/typedefs.h | 1 +
monitor.c | 5 ++
qapi-schema.json | 4 ++
qapi/block-core.json | 78 ++--------------------
qapi/fsdev.json | 81 +++++++++++++++++++++++
qmp.c | 14 ++++
util/throttle.c | 52 +++++++++++++++
19 files changed, 426 insertions(+), 226 deletions(-)
create mode 100644 qapi/fsdev.json
v0 -> v1:
Addressed comments from Eric Blake, Greg Kurz and Daniel P.Berrange
Mainly renaming the functions and removing the redundant code.
v1 -> v2:
Addressed comments from Eric Blake and Greg Kurz.
As per the suggestion I split the patches into smaller patches.
Removed some more duplicate code.
v2 -> v3:
Addresssed comments from Alberto Garcia.
Changed the comment from block to iothrottle in the iothrottle.json
Added the dummy functions in qemu-fsdev-dummy.c to address the compilation
issues that were observed.
v3 -> v4:
Addressed comments from Eric Blake and Greg Kurz
Re-ordered the patches
Added the dummy functions in qmp.c to address the cross compilation issues
v4 -> v5:
Addressed comments from Eric Blake and Greg Kurz
Split the fsdev qmp patch into hmp and qmp related patches
Moved the common functionalities to throttle.c instead of creating
a new file
v5 -> v6:
Addressed comments from Greg Kurz and Markus Armbruster
Split the commits to specific to hmp and throttle as suggested by Greg
Moved ThrottleConfig typedef to qemu/typedefs.h
Addressed compilation issue on FreeBSD by adding flags in qmp.c
v6 -> v7:
Addressed comments from Albert Garcia and Dr. David Alan Gilbert
Fixed the hmp-commands-info.hx and hmp-commands.hx as per Dr. David's
comments.
Fixed the bug with the hmp fsdev_set_io_throttle and info fsdev_iothrottle
v7 -> v8:
Addressed comments from Markus Armbruster and Eric Blake
Removed unwanted headers from qmp-fsdev-throttle.h
v8 -> v9:
Addressed comments from Markus Armbruster and Eric Blake
Removed the iothrottle.json and pushed the iothrottle struct to
block-core.json
v9 -> v10:
Addressed comments from Albert Garcia
Fixed issue related to dynamically passing throttle configuration
Removed some unused code
v10 -> v11:
Addressed the comments from Markus Armbruster and Eric Blake
Rebased the patches over 2.10
--
1.8.3.1
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code
2017-09-14 10:40 [Qemu-devel] [PATCH v11 0/6] fsdev: qmp interface for io throttling Pradeep Jagadeesh
@ 2017-09-14 10:40 ` Pradeep Jagadeesh
2017-09-14 11:12 ` Greg Kurz
2017-09-18 16:20 ` Manos Pitsidianakis
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 2/6] qmp: Use ThrottleLimits structure Pradeep Jagadeesh
` (4 subsequent siblings)
5 siblings, 2 replies; 25+ messages in thread
From: Pradeep Jagadeesh @ 2017-09-14 10:40 UTC (permalink / raw)
To: eric blake, greg kurz
Cc: Pradeep Jagadeesh, alberto garcia, Markus Armbruster,
jani kokkonen, qemu-devel
This patch factors out the duplicate throttle code that was still
present in block and fsdev devices.
Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
blockdev.c | 44 +---------------------------------
fsdev/qemu-fsdev-throttle.c | 44 ++--------------------------------
include/qemu/throttle-options.h | 3 +++
include/qemu/throttle.h | 4 ++--
include/qemu/typedefs.h | 1 +
util/throttle.c | 52 +++++++++++++++++++++++++++++++++++++++++
6 files changed, 61 insertions(+), 87 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 56a6b24..9d33c25 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -387,49 +387,7 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
}
if (throttle_cfg) {
- throttle_config_init(throttle_cfg);
- throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
- qemu_opt_get_number(opts, "throttling.bps-total", 0);
- throttle_cfg->buckets[THROTTLE_BPS_READ].avg =
- qemu_opt_get_number(opts, "throttling.bps-read", 0);
- throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
- qemu_opt_get_number(opts, "throttling.bps-write", 0);
- throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
- qemu_opt_get_number(opts, "throttling.iops-total", 0);
- throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
- qemu_opt_get_number(opts, "throttling.iops-read", 0);
- throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
- qemu_opt_get_number(opts, "throttling.iops-write", 0);
-
- throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
- qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
- throttle_cfg->buckets[THROTTLE_BPS_READ].max =
- qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
- throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
- qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
- throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
- qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
- throttle_cfg->buckets[THROTTLE_OPS_READ].max =
- qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
- throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
- qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
-
- throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
- qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
- throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =
- qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
- throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
- qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
- throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
- qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
- throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
- qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
- throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
- qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
-
- throttle_cfg->op_size =
- qemu_opt_get_number(opts, "throttling.iops-size", 0);
-
+ throttle_parse_options(throttle_cfg, opts);
if (!throttle_is_valid(throttle_cfg, errp)) {
return;
}
diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
index 49eebb5..0e6fb86 100644
--- a/fsdev/qemu-fsdev-throttle.c
+++ b/fsdev/qemu-fsdev-throttle.c
@@ -16,6 +16,7 @@
#include "qemu/error-report.h"
#include "qemu-fsdev-throttle.h"
#include "qemu/iov.h"
+#include "qemu/throttle-options.h"
static void fsdev_throttle_read_timer_cb(void *opaque)
{
@@ -31,48 +32,7 @@ static void fsdev_throttle_write_timer_cb(void *opaque)
void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error **errp)
{
- throttle_config_init(&fst->cfg);
- fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =
- qemu_opt_get_number(opts, "throttling.bps-total", 0);
- fst->cfg.buckets[THROTTLE_BPS_READ].avg =
- qemu_opt_get_number(opts, "throttling.bps-read", 0);
- fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =
- qemu_opt_get_number(opts, "throttling.bps-write", 0);
- fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =
- qemu_opt_get_number(opts, "throttling.iops-total", 0);
- fst->cfg.buckets[THROTTLE_OPS_READ].avg =
- qemu_opt_get_number(opts, "throttling.iops-read", 0);
- fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =
- qemu_opt_get_number(opts, "throttling.iops-write", 0);
-
- fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =
- qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
- fst->cfg.buckets[THROTTLE_BPS_READ].max =
- qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
- fst->cfg.buckets[THROTTLE_BPS_WRITE].max =
- qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
- fst->cfg.buckets[THROTTLE_OPS_TOTAL].max =
- qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
- fst->cfg.buckets[THROTTLE_OPS_READ].max =
- qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
- fst->cfg.buckets[THROTTLE_OPS_WRITE].max =
- qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
-
- fst->cfg.buckets[THROTTLE_BPS_TOTAL].burst_length =
- qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
- fst->cfg.buckets[THROTTLE_BPS_READ].burst_length =
- qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
- fst->cfg.buckets[THROTTLE_BPS_WRITE].burst_length =
- qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
- fst->cfg.buckets[THROTTLE_OPS_TOTAL].burst_length =
- qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
- fst->cfg.buckets[THROTTLE_OPS_READ].burst_length =
- qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
- fst->cfg.buckets[THROTTLE_OPS_WRITE].burst_length =
- qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
- fst->cfg.op_size =
- qemu_opt_get_number(opts, "throttling.iops-size", 0);
-
+ throttle_parse_options(&fst->cfg, opts);
throttle_is_valid(&fst->cfg, errp);
}
diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h
index 3528a8f..9709dcd 100644
--- a/include/qemu/throttle-options.h
+++ b/include/qemu/throttle-options.h
@@ -9,6 +9,7 @@
*/
#ifndef THROTTLE_OPTIONS_H
#define THROTTLE_OPTIONS_H
+#include "typedefs.h"
#define QEMU_OPT_IOPS_TOTAL "iops-total"
#define QEMU_OPT_IOPS_TOTAL_MAX "iops-total-max"
@@ -111,4 +112,6 @@
.help = "when limiting by iops max size of an I/O in bytes",\
}
+void throttle_parse_options(ThrottleConfig *, QemuOpts *);
+
#endif
diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
index 8c93237..b6ebc6d 100644
--- a/include/qemu/throttle.h
+++ b/include/qemu/throttle.h
@@ -89,10 +89,10 @@ typedef struct LeakyBucket {
* However it allows to keep the code clean and the bucket field is reset to
* zero at the right time.
*/
-typedef struct ThrottleConfig {
+struct ThrottleConfig {
LeakyBucket buckets[BUCKETS_COUNT]; /* leaky buckets */
uint64_t op_size; /* size of an operation in bytes */
-} ThrottleConfig;
+};
typedef struct ThrottleState {
ThrottleConfig cfg; /* configuration */
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 39bc835..90fe0f9 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -100,6 +100,7 @@ typedef struct uWireSlave uWireSlave;
typedef struct VirtIODevice VirtIODevice;
typedef struct Visitor Visitor;
typedef struct node_info NodeInfo;
+typedef struct ThrottleConfig ThrottleConfig;
typedef void SaveStateHandler(QEMUFile *f, void *opaque);
typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);
diff --git a/util/throttle.c b/util/throttle.c
index 06bf916..9ef28c4 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -27,6 +27,7 @@
#include "qemu/throttle.h"
#include "qemu/timer.h"
#include "block/aio.h"
+#include "qemu/throttle-options.h"
/* This function make a bucket leak
*
@@ -635,3 +636,54 @@ void throttle_config_to_limits(ThrottleConfig *cfg, ThrottleLimits *var)
var->has_iops_write_max_length = true;
var->has_iops_size = true;
}
+
+/* parse the throttle options
+ *
+ * @opts: qemu options
+ * @throttle_cfg: throttle configuration
+ */
+void throttle_parse_options(ThrottleConfig *throttle_cfg, QemuOpts *opts)
+{
+ throttle_config_init(throttle_cfg);
+ throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
+ qemu_opt_get_number(opts, "throttling.bps-total", 0);
+ throttle_cfg->buckets[THROTTLE_BPS_READ].avg =
+ qemu_opt_get_number(opts, "throttling.bps-read", 0);
+ throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
+ qemu_opt_get_number(opts, "throttling.bps-write", 0);
+ throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
+ qemu_opt_get_number(opts, "throttling.iops-total", 0);
+ throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
+ qemu_opt_get_number(opts, "throttling.iops-read", 0);
+ throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
+ qemu_opt_get_number(opts, "throttling.iops-write", 0);
+
+ throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
+ qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
+ throttle_cfg->buckets[THROTTLE_BPS_READ].max =
+ qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
+ throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
+ qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
+ throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
+ qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
+ throttle_cfg->buckets[THROTTLE_OPS_READ].max =
+ qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
+ throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
+ qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
+
+ throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
+ qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
+ throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =
+ qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
+ throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
+ qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
+ throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
+ qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
+ throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
+ qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
+ throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
+ qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
+
+ throttle_cfg->op_size =
+ qemu_opt_get_number(opts, "throttling.iops-size", 0);
+}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v11 2/6] qmp: Use ThrottleLimits structure
2017-09-14 10:40 [Qemu-devel] [PATCH v11 0/6] fsdev: qmp interface for io throttling Pradeep Jagadeesh
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code Pradeep Jagadeesh
@ 2017-09-14 10:40 ` Pradeep Jagadeesh
2017-09-14 11:19 ` Greg Kurz
` (3 more replies)
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 3/6] qmp: factor out throttle code to reuse code Pradeep Jagadeesh
` (3 subsequent siblings)
5 siblings, 4 replies; 25+ messages in thread
From: Pradeep Jagadeesh @ 2017-09-14 10:40 UTC (permalink / raw)
To: eric blake, greg kurz
Cc: Pradeep Jagadeesh, alberto garcia, jani kokkonen, qemu-devel
This patch factors out code to use the ThrottleLimits
strurcture.
Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
qapi/block-core.json | 78 +++-------------------------------------------------
1 file changed, 4 insertions(+), 74 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index bb11815..d0ccfda 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1826,84 +1826,13 @@
#
# @device: Block device name (deprecated, use @id instead)
#
-# @id: The name or QOM path of the guest device (since: 2.8)
-#
-# @bps: total throughput limit in bytes per second
-#
-# @bps_rd: read throughput limit in bytes per second
-#
-# @bps_wr: write throughput limit in bytes per second
-#
-# @iops: total I/O operations per second
-#
-# @iops_rd: read I/O operations per second
-#
-# @iops_wr: write I/O operations per second
-#
-# @bps_max: total throughput limit during bursts,
-# in bytes (Since 1.7)
-#
-# @bps_rd_max: read throughput limit during bursts,
-# in bytes (Since 1.7)
-#
-# @bps_wr_max: write throughput limit during bursts,
-# in bytes (Since 1.7)
-#
-# @iops_max: total I/O operations per second during bursts,
-# in bytes (Since 1.7)
-#
-# @iops_rd_max: read I/O operations per second during bursts,
-# in bytes (Since 1.7)
-#
-# @iops_wr_max: write I/O operations per second during bursts,
-# in bytes (Since 1.7)
-#
-# @bps_max_length: maximum length of the @bps_max burst
-# period, in seconds. It must only
-# be set if @bps_max is set as well.
-# Defaults to 1. (Since 2.6)
-#
-# @bps_rd_max_length: maximum length of the @bps_rd_max
-# burst period, in seconds. It must only
-# be set if @bps_rd_max is set as well.
-# Defaults to 1. (Since 2.6)
-#
-# @bps_wr_max_length: maximum length of the @bps_wr_max
-# burst period, in seconds. It must only
-# be set if @bps_wr_max is set as well.
-# Defaults to 1. (Since 2.6)
-#
-# @iops_max_length: maximum length of the @iops burst
-# period, in seconds. It must only
-# be set if @iops_max is set as well.
-# Defaults to 1. (Since 2.6)
-#
-# @iops_rd_max_length: maximum length of the @iops_rd_max
-# burst period, in seconds. It must only
-# be set if @iops_rd_max is set as well.
-# Defaults to 1. (Since 2.6)
-#
-# @iops_wr_max_length: maximum length of the @iops_wr_max
-# burst period, in seconds. It must only
-# be set if @iops_wr_max is set as well.
-# Defaults to 1. (Since 2.6)
-#
-# @iops_size: an I/O size in bytes (Since 1.7)
-#
# @group: throttle group name (Since 2.4)
#
# Since: 1.1
##
{ 'struct': 'BlockIOThrottle',
- 'data': { '*device': 'str', '*id': 'str', 'bps': 'int', 'bps_rd': 'int',
- 'bps_wr': 'int', 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
- '*bps_max': 'int', '*bps_rd_max': 'int',
- '*bps_wr_max': 'int', '*iops_max': 'int',
- '*iops_rd_max': 'int', '*iops_wr_max': 'int',
- '*bps_max_length': 'int', '*bps_rd_max_length': 'int',
- '*bps_wr_max_length': 'int', '*iops_max_length': 'int',
- '*iops_rd_max_length': 'int', '*iops_wr_max_length': 'int',
- '*iops_size': 'int', '*group': 'str' } }
+ 'base': 'ThrottleLimits',
+ 'data': { '*device': 'str', '*group': 'str' } }
##
# @ThrottleLimits:
@@ -1913,6 +1842,7 @@
# transaction. All fields are optional. When setting limits, if a field is
# missing the current value is not changed.
#
+# @id: device id
# @iops-total: limit total I/O operations per second
# @iops-total-max: I/O operations burst
# @iops-total-max-length: length of the iops-total-max burst period, in seconds
@@ -1942,7 +1872,7 @@
# Since: 2.11
##
{ 'struct': 'ThrottleLimits',
- 'data': { '*iops-total' : 'int', '*iops-total-max' : 'int',
+ 'data': { '*id' : 'str', '*iops-total' : 'int', '*iops-total-max' : 'int',
'*iops-total-max-length' : 'int', '*iops-read' : 'int',
'*iops-read-max' : 'int', '*iops-read-max-length' : 'int',
'*iops-write' : 'int', '*iops-write-max' : 'int',
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v11 3/6] qmp: factor out throttle code to reuse code
2017-09-14 10:40 [Qemu-devel] [PATCH v11 0/6] fsdev: qmp interface for io throttling Pradeep Jagadeesh
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code Pradeep Jagadeesh
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 2/6] qmp: Use ThrottleLimits structure Pradeep Jagadeesh
@ 2017-09-14 10:40 ` Pradeep Jagadeesh
2017-09-14 11:22 ` Greg Kurz
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 4/6] hmp: create a throttle initialization function for code reuse Pradeep Jagadeesh
` (2 subsequent siblings)
5 siblings, 1 reply; 25+ messages in thread
From: Pradeep Jagadeesh @ 2017-09-14 10:40 UTC (permalink / raw)
To: eric blake, greg kurz
Cc: Pradeep Jagadeesh, alberto garcia, Markus Armbruster,
jani kokkonen, qemu-devel
This patch reuses the code to set throttle limits.
Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
---
blockdev.c | 53 +++--------------------------------------------------
1 file changed, 3 insertions(+), 50 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 9d33c25..2bd8ebd 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2569,6 +2569,7 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp)
BlockDriverState *bs;
BlockBackend *blk;
AioContext *aio_context;
+ ThrottleLimits *tlimit;
blk = qmp_get_blk(arg->has_device ? arg->device : NULL,
arg->has_id ? arg->id : NULL,
@@ -2586,56 +2587,8 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp)
goto out;
}
- throttle_config_init(&cfg);
- cfg.buckets[THROTTLE_BPS_TOTAL].avg = arg->bps;
- cfg.buckets[THROTTLE_BPS_READ].avg = arg->bps_rd;
- cfg.buckets[THROTTLE_BPS_WRITE].avg = arg->bps_wr;
-
- cfg.buckets[THROTTLE_OPS_TOTAL].avg = arg->iops;
- cfg.buckets[THROTTLE_OPS_READ].avg = arg->iops_rd;
- cfg.buckets[THROTTLE_OPS_WRITE].avg = arg->iops_wr;
-
- if (arg->has_bps_max) {
- cfg.buckets[THROTTLE_BPS_TOTAL].max = arg->bps_max;
- }
- if (arg->has_bps_rd_max) {
- cfg.buckets[THROTTLE_BPS_READ].max = arg->bps_rd_max;
- }
- if (arg->has_bps_wr_max) {
- cfg.buckets[THROTTLE_BPS_WRITE].max = arg->bps_wr_max;
- }
- if (arg->has_iops_max) {
- cfg.buckets[THROTTLE_OPS_TOTAL].max = arg->iops_max;
- }
- if (arg->has_iops_rd_max) {
- cfg.buckets[THROTTLE_OPS_READ].max = arg->iops_rd_max;
- }
- if (arg->has_iops_wr_max) {
- cfg.buckets[THROTTLE_OPS_WRITE].max = arg->iops_wr_max;
- }
-
- if (arg->has_bps_max_length) {
- cfg.buckets[THROTTLE_BPS_TOTAL].burst_length = arg->bps_max_length;
- }
- if (arg->has_bps_rd_max_length) {
- cfg.buckets[THROTTLE_BPS_READ].burst_length = arg->bps_rd_max_length;
- }
- if (arg->has_bps_wr_max_length) {
- cfg.buckets[THROTTLE_BPS_WRITE].burst_length = arg->bps_wr_max_length;
- }
- if (arg->has_iops_max_length) {
- cfg.buckets[THROTTLE_OPS_TOTAL].burst_length = arg->iops_max_length;
- }
- if (arg->has_iops_rd_max_length) {
- cfg.buckets[THROTTLE_OPS_READ].burst_length = arg->iops_rd_max_length;
- }
- if (arg->has_iops_wr_max_length) {
- cfg.buckets[THROTTLE_OPS_WRITE].burst_length = arg->iops_wr_max_length;
- }
-
- if (arg->has_iops_size) {
- cfg.op_size = arg->iops_size;
- }
+ tlimit = qapi_BlockIOThrottle_base(arg);
+ throttle_config_to_limits(&cfg, tlimit);
if (!throttle_is_valid(&cfg, errp)) {
goto out;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v11 4/6] hmp: create a throttle initialization function for code reuse
2017-09-14 10:40 [Qemu-devel] [PATCH v11 0/6] fsdev: qmp interface for io throttling Pradeep Jagadeesh
` (2 preceding siblings ...)
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 3/6] qmp: factor out throttle code to reuse code Pradeep Jagadeesh
@ 2017-09-14 10:40 ` Pradeep Jagadeesh
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 5/6] fsdev: QMP interface for throttling Pradeep Jagadeesh
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 6/6] fsdev: hmp " Pradeep Jagadeesh
5 siblings, 0 replies; 25+ messages in thread
From: Pradeep Jagadeesh @ 2017-09-14 10:40 UTC (permalink / raw)
To: eric blake, greg kurz
Cc: Pradeep Jagadeesh, alberto garcia, Dr. David Alan Gilbert,
jani kokkonen, qemu-devel
This patch creates a throttle initialization function to maximize the
code reusability. The same code is also used by fsdev.
Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
hmp.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/hmp.c b/hmp.c
index cd046c6..acaf0e6 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1752,20 +1752,28 @@ void hmp_change(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, &err);
}
+static void hmp_initialize_throttle_limits(ThrottleLimits *iot,
+ const QDict *qdict)
+{
+ iot->bps_total = qdict_get_int(qdict, "bps");
+ iot->bps_read = qdict_get_int(qdict, "bps_rd");
+ iot->bps_write = qdict_get_int(qdict, "bps_wr");
+ iot->iops_total = qdict_get_int(qdict, "iops");
+ iot->iops_read = qdict_get_int(qdict, "iops_rd");
+ iot->iops_write = qdict_get_int(qdict, "iops_wr");
+}
+
void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict)
{
Error *err = NULL;
+ ThrottleLimits *tlimits;
BlockIOThrottle throttle = {
.has_device = true,
.device = (char *) qdict_get_str(qdict, "device"),
- .bps = qdict_get_int(qdict, "bps"),
- .bps_rd = qdict_get_int(qdict, "bps_rd"),
- .bps_wr = qdict_get_int(qdict, "bps_wr"),
- .iops = qdict_get_int(qdict, "iops"),
- .iops_rd = qdict_get_int(qdict, "iops_rd"),
- .iops_wr = qdict_get_int(qdict, "iops_wr"),
};
+ tlimits = qapi_BlockIOThrottle_base(&throttle);
+ hmp_initialize_throttle_limits(tlimits, qdict);
qmp_block_set_io_throttle(&throttle, &err);
hmp_handle_error(mon, &err);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v11 5/6] fsdev: QMP interface for throttling
2017-09-14 10:40 [Qemu-devel] [PATCH v11 0/6] fsdev: qmp interface for io throttling Pradeep Jagadeesh
` (3 preceding siblings ...)
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 4/6] hmp: create a throttle initialization function for code reuse Pradeep Jagadeesh
@ 2017-09-14 10:40 ` Pradeep Jagadeesh
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 6/6] fsdev: hmp " Pradeep Jagadeesh
5 siblings, 0 replies; 25+ messages in thread
From: Pradeep Jagadeesh @ 2017-09-14 10:40 UTC (permalink / raw)
To: eric blake, greg kurz
Cc: Pradeep Jagadeesh, Markus Armbruster, alberto garcia,
Dr. David Alan Gilbert, jani kokkonen, qemu-devel
This patch introduces qmp interfaces for the fsdev
devices. This provides two interfaces one
for querying info of all the fsdev devices. The second one
to set the IO limits for the required fsdev device.
Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
Makefile | 3 +-
fsdev/qemu-fsdev-dummy.c | 11 ++++++
fsdev/qemu-fsdev-throttle.c | 96 ++++++++++++++++++++++++++++++++++++++++++---
fsdev/qemu-fsdev-throttle.h | 9 ++++-
fsdev/qemu-fsdev.c | 30 ++++++++++++++
monitor.c | 5 +++
qapi-schema.json | 4 ++
qapi/fsdev.json | 81 ++++++++++++++++++++++++++++++++++++++
qmp.c | 14 +++++++
9 files changed, 244 insertions(+), 9 deletions(-)
create mode 100644 qapi/fsdev.json
diff --git a/Makefile b/Makefile
index 337a1f6..6556dbf 100644
--- a/Makefile
+++ b/Makefile
@@ -421,7 +421,8 @@ qapi-modules = $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/qapi/common.json \
$(SRC_PATH)/qapi/tpm.json \
$(SRC_PATH)/qapi/trace.json \
$(SRC_PATH)/qapi/transaction.json \
- $(SRC_PATH)/qapi/ui.json
+ $(SRC_PATH)/qapi/ui.json \
+ $(SRC_PATH)/qapi/fsdev.json
qapi-types.c qapi-types.h :\
$(qapi-modules) $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
diff --git a/fsdev/qemu-fsdev-dummy.c b/fsdev/qemu-fsdev-dummy.c
index 6dc0fbc..dc5cb6c 100644
--- a/fsdev/qemu-fsdev-dummy.c
+++ b/fsdev/qemu-fsdev-dummy.c
@@ -14,8 +14,19 @@
#include "qemu-fsdev.h"
#include "qemu/config-file.h"
#include "qemu/module.h"
+#include "qmp-commands.h"
int qemu_fsdev_add(QemuOpts *opts)
{
return 0;
}
+
+void qmp_fsdev_set_io_throttle(ThrottleLimits *arg, Error **errp)
+{
+ return;
+}
+
+ThrottleLimitsList *qmp_query_fsdev_io_throttle(Error **errp)
+{
+ return NULL;
+}
diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
index 0e6fb86..fb7a6fa 100644
--- a/fsdev/qemu-fsdev-throttle.c
+++ b/fsdev/qemu-fsdev-throttle.c
@@ -16,6 +16,7 @@
#include "qemu/error-report.h"
#include "qemu-fsdev-throttle.h"
#include "qemu/iov.h"
+#include "qemu/main-loop.h"
#include "qemu/throttle-options.h"
static void fsdev_throttle_read_timer_cb(void *opaque)
@@ -30,6 +31,92 @@ static void fsdev_throttle_write_timer_cb(void *opaque)
qemu_co_enter_next(&fst->throttled_reqs[true]);
}
+typedef struct {
+ FsThrottle *fst;
+ bool is_write;
+} RestartData;
+
+static bool coroutine_fn throttle_co_restart_queue(FsThrottle *fst,
+ bool is_write)
+{
+ return qemu_co_queue_next(&fst->throttled_reqs[is_write]);
+}
+
+static void schedule_next_request(FsThrottle *fst, bool is_write)
+{
+ bool must_wait = throttle_schedule_timer(&fst->ts, &fst->tt, is_write);
+ if (!must_wait) {
+ if (qemu_in_coroutine() &&
+ throttle_co_restart_queue(fst, is_write)) {
+ return;
+ } else {
+ int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
+ timer_mod(fst->tt.timers[is_write], now);
+ }
+ }
+}
+
+static void coroutine_fn throttle_restart_queue_entry(void *opaque)
+{
+ RestartData *data = opaque;
+ bool is_write = data->is_write;
+ bool empty_queue = !throttle_co_restart_queue(data->fst, is_write);
+ if (empty_queue) {
+ schedule_next_request(data->fst, is_write);
+ }
+}
+
+static void throttle_restart_queues(FsThrottle *fst)
+{
+ Coroutine *co;
+ RestartData rd = {
+ .fst = fst,
+ .is_write = true
+ };
+
+ co = qemu_coroutine_create(throttle_restart_queue_entry, &rd);
+ aio_co_enter(fst->ctx, co);
+
+ rd.is_write = false;
+
+ co = qemu_coroutine_create(throttle_restart_queue_entry, &rd);
+ aio_co_enter(fst->ctx, co);
+}
+
+static void coroutine_fn fsdev_throttle_config(FsThrottle *fst)
+{
+ if (throttle_enabled(&fst->cfg)) {
+ throttle_config(&fst->ts, QEMU_CLOCK_REALTIME, &fst->cfg);
+ } else {
+ throttle_restart_queues(fst);
+ }
+}
+
+void fsdev_set_io_throttle(ThrottleLimits *arg, FsThrottle *fst, Error **errp)
+{
+ ThrottleConfig cfg;
+
+ throttle_get_config(&fst->ts, &cfg);
+ throttle_limits_to_config(arg, &cfg, errp);
+
+ if (throttle_is_valid(&cfg, errp)) {
+ fst->cfg = cfg;
+ fsdev_throttle_config(fst);
+ }
+}
+
+void fsdev_get_io_throttle(FsThrottle *fst, ThrottleLimits **fs9pcfg,
+ char *fsdevice)
+{
+ ThrottleConfig cfg = fst->cfg;
+ ThrottleLimits *fscfg = g_malloc0(sizeof(*fscfg));
+
+ fscfg->has_id = true;
+ fscfg->id = g_strdup(fsdevice);
+ throttle_config_to_limits(&cfg, fscfg);
+ *fs9pcfg = fscfg;
+}
+
void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error **errp)
{
throttle_parse_options(&fst->cfg, opts);
@@ -40,8 +127,9 @@ void fsdev_throttle_init(FsThrottle *fst)
{
if (throttle_enabled(&fst->cfg)) {
throttle_init(&fst->ts);
+ fst->ctx = qemu_get_aio_context();
throttle_timers_init(&fst->tt,
- qemu_get_aio_context(),
+ fst->ctx,
QEMU_CLOCK_REALTIME,
fsdev_throttle_read_timer_cb,
fsdev_throttle_write_timer_cb,
@@ -62,11 +150,7 @@ void coroutine_fn fsdev_co_throttle_request(FsThrottle *fst, bool is_write,
}
throttle_account(&fst->ts, is_write, iov_size(iov, iovcnt));
-
- if (!qemu_co_queue_empty(&fst->throttled_reqs[is_write]) &&
- !throttle_schedule_timer(&fst->ts, &fst->tt, is_write)) {
- qemu_co_queue_next(&fst->throttled_reqs[is_write]);
- }
+ schedule_next_request(fst, is_write);
}
}
diff --git a/fsdev/qemu-fsdev-throttle.h b/fsdev/qemu-fsdev-throttle.h
index e418643..fc7409c 100644
--- a/fsdev/qemu-fsdev-throttle.h
+++ b/fsdev/qemu-fsdev-throttle.h
@@ -15,8 +15,6 @@
#ifndef _FSDEV_THROTTLE_H
#define _FSDEV_THROTTLE_H
-#include "block/aio.h"
-#include "qemu/main-loop.h"
#include "qemu/coroutine.h"
#include "qapi/error.h"
#include "qemu/throttle.h"
@@ -25,6 +23,7 @@ typedef struct FsThrottle {
ThrottleState ts;
ThrottleTimers tt;
ThrottleConfig cfg;
+ AioContext *ctx;
CoQueue throttled_reqs[2];
} FsThrottle;
@@ -36,4 +35,10 @@ void coroutine_fn fsdev_co_throttle_request(FsThrottle *, bool ,
struct iovec *, int);
void fsdev_throttle_cleanup(FsThrottle *);
+
+void fsdev_set_io_throttle(ThrottleLimits *, FsThrottle *, Error **errp);
+
+void fsdev_get_io_throttle(FsThrottle *, ThrottleLimits **iothp,
+ char *);
+
#endif /* _FSDEV_THROTTLE_H */
diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c
index 266e442..07bb005 100644
--- a/fsdev/qemu-fsdev.c
+++ b/fsdev/qemu-fsdev.c
@@ -16,6 +16,7 @@
#include "qemu-common.h"
#include "qemu/config-file.h"
#include "qemu/error-report.h"
+#include "qmp-commands.h"
static QTAILQ_HEAD(FsDriverEntry_head, FsDriverListEntry) fsdriver_entries =
QTAILQ_HEAD_INITIALIZER(fsdriver_entries);
@@ -98,3 +99,32 @@ FsDriverEntry *get_fsdev_fsentry(char *id)
}
return NULL;
}
+
+void qmp_fsdev_set_io_throttle(ThrottleLimits *arg, Error **errp)
+{
+
+ FsDriverEntry *fse;
+
+ fse = get_fsdev_fsentry(arg->has_id ? arg->id : NULL);
+ if (!fse) {
+ error_setg(errp, "Not a valid fsdev device");
+ return;
+ }
+
+ fsdev_set_io_throttle(arg, &fse->fst, errp);
+}
+
+ThrottleLimitsList *qmp_query_fsdev_io_throttle(Error **errp)
+{
+ ThrottleLimitsList *head = NULL, *p_next;
+ struct FsDriverListEntry *fsle;
+
+ QTAILQ_FOREACH(fsle, &fsdriver_entries, next) {
+ p_next = g_new0(ThrottleLimitsList, 1);
+ fsdev_get_io_throttle(&fsle->fse.fst, &p_next->value,
+ fsle->fse.fsdev_id);
+ p_next->next = head;
+ head = p_next;
+ }
+ return head;
+}
diff --git a/monitor.c b/monitor.c
index 9239f7a..d12334f 100644
--- a/monitor.c
+++ b/monitor.c
@@ -998,6 +998,11 @@ static void qmp_unregister_commands_hack(void)
&& !defined(TARGET_S390X)
qmp_unregister_command(&qmp_commands, "query-cpu-definitions");
#endif
+#ifndef CONFIG_VIRTFS
+ qmp_unregister_command(&qmp_commands, "fsdev-set-io-throttle");
+ qmp_unregister_command(&qmp_commands, "query-fsdev-io-throttle");
+#endif
+
}
void monitor_init_qmp_commands(void)
diff --git a/qapi-schema.json b/qapi-schema.json
index f3af2cb..bd55190 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -93,6 +93,10 @@
{ 'include': 'qapi/trace.json' }
{ 'include': 'qapi/introspect.json' }
+# QAPI fsdev definitions
+{ 'include': 'qapi/fsdev.json' }
+
+
##
# = Miscellanea
##
diff --git a/qapi/fsdev.json b/qapi/fsdev.json
new file mode 100644
index 0000000..7babc30
--- /dev/null
+++ b/qapi/fsdev.json
@@ -0,0 +1,81 @@
+# -*- Mode: Python -*-
+
+##
+# == QAPI fsdev definitions
+##
+
+##
+# @fsdev-set-io-throttle:
+#
+# Change I/O limits for a 9p/fsdev device.
+#
+# I/O limits can be enabled by setting throttle value to non-zero number.
+#
+# I/O limits can be disabled by setting all throttle values to 0.
+#
+# Returns: Nothing on success
+# If @device is not a valid fsdev device, GenericError
+#
+# Since: 2.11
+#
+# Example:
+#
+# -> { "execute": "fsdev-set-io-throttle",
+# "arguments": { "id": "id0-1-0",
+# "bps": 1000000,
+# "bps_rd": 0,
+# "bps_wr": 0,
+# "iops": 0,
+# "iops_rd": 0,
+# "iops_wr": 0,
+# "bps_max": 8000000,
+# "bps_rd_max": 0,
+# "bps_wr_max": 0,
+# "iops_max": 0,
+# "iops_rd_max": 0,
+# "iops_wr_max": 0,
+# "bps_max_length": 60,
+# "iops_size": 0 } }
+# <- { "returns": {} }
+##
+{ 'command': 'fsdev-set-io-throttle', 'boxed': true,
+ 'data': 'ThrottleLimits' }
+##
+# @query-fsdev-io-throttle:
+#
+# Returns: a list of @IOThrottle describing I/O throttle
+# values of each fsdev device
+#
+# Since: 2.11
+#
+# Example:
+#
+# -> { "Execute": "query-fsdev-io-throttle" }
+# <- { "returns" : [
+# {
+# "id": "id0-hd0",
+# "bps":1000000,
+# "bps_rd":0,
+# "bps_wr":0,
+# "iops":1000000,
+# "iops_rd":0,
+# "iops_wr":0,
+# "bps_max": 8000000,
+# "bps_rd_max": 0,
+# "bps_wr_max": 0,
+# "iops_max": 0,
+# "iops_rd_max": 0,
+# "iops_wr_max": 0,
+# "bps_max_length": 0,
+# "bps_rd_max_length": 0,
+# "bps_wr_max_length": 0,
+# "iops_max_length": 0,
+# "iops_rd_max_length": 0,
+# "iops_wr_max_length": 0,
+# "iops_size": 0
+# }
+# ]
+# }
+#
+##
+{ 'command': 'query-fsdev-io-throttle', 'returns': [ 'ThrottleLimits' ] }
diff --git a/qmp.c b/qmp.c
index b86201e..8de2e23 100644
--- a/qmp.c
+++ b/qmp.c
@@ -130,6 +130,20 @@ void qmp_cpu_add(int64_t id, Error **errp)
}
}
+#if defined(_WIN64) || defined(_WIN32) || defined(__FreeBSD__)
+
+void qmp_fsdev_set_io_throttle(ThrottleLimits *arg, Error **errp)
+{
+ return;
+}
+
+ThrottleLimitsList *qmp_query_fsdev_io_throttle(Error **errp)
+{
+ return NULL;
+}
+
+#endif
+
#ifndef CONFIG_VNC
/* If VNC support is enabled, the "true" query-vnc command is
defined in the VNC subsystem */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [Qemu-devel] [PATCH v11 6/6] fsdev: hmp interface for throttling
2017-09-14 10:40 [Qemu-devel] [PATCH v11 0/6] fsdev: qmp interface for io throttling Pradeep Jagadeesh
` (4 preceding siblings ...)
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 5/6] fsdev: QMP interface for throttling Pradeep Jagadeesh
@ 2017-09-14 10:40 ` Pradeep Jagadeesh
2017-09-14 12:05 ` Alberto Garcia
5 siblings, 1 reply; 25+ messages in thread
From: Pradeep Jagadeesh @ 2017-09-14 10:40 UTC (permalink / raw)
To: eric blake, greg kurz
Cc: Pradeep Jagadeesh, alberto garcia, Dr. David Alan Gilbert,
jani kokkonen, qemu-devel
This patch introduces hmp interfaces for the fsdev
devices.
Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
hmp-commands-info.hx | 18 ++++++++++++++++
hmp-commands.hx | 19 +++++++++++++++++
hmp.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++
hmp.h | 4 ++++
4 files changed, 100 insertions(+)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 4ab7fce..54f1968 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -84,6 +84,24 @@ STEXI
Show block device statistics.
ETEXI
+#if defined(CONFIG_VIRTFS)
+
+ {
+ .name = "fsdev_iothrottle",
+ .args_type = "",
+ .params = "",
+ .help = "show fsdev iothrottle information",
+ .cmd = hmp_info_fsdev_iothrottle,
+ },
+
+#endif
+
+STEXI
+@item info fsdev_iothrottle
+@findex fsdev_iothrottle
+Show fsdev device throttle info.
+ETEXI
+
{
.name = "block-jobs",
.args_type = "",
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 1941e19..aef9f79 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1680,6 +1680,25 @@ STEXI
Change I/O throttle limits for a block drive to @var{bps} @var{bps_rd} @var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
ETEXI
+#if defined(CONFIG_VIRTFS)
+
+ {
+ .name = "fsdev_set_io_throttle",
+ .args_type = "device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l",
+ .params = "device bps bps_rd bps_wr iops iops_rd iops_wr",
+ .help = "change I/O throttle limits for a fs devices",
+ .cmd = hmp_fsdev_set_io_throttle,
+ },
+
+#endif
+
+STEXI
+@item fsdev_set_io_throttle @var{device} @var{bps} @var{bps_rd} @var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
+@findex fsdev_set_io_throttle
+Change I/O throttle limits for a fs devices to @var{bps} @var{bps_rd} @var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
+ETEXI
+
+
{
.name = "set_password",
.args_type = "protocol:s,password:s,connected:s?",
diff --git a/hmp.c b/hmp.c
index acaf0e6..ceec6c9 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1778,6 +1778,65 @@ void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, &err);
}
+#ifdef CONFIG_VIRTFS
+
+void hmp_fsdev_set_io_throttle(Monitor *mon, const QDict *qdict)
+{
+ Error *err = NULL;
+ ThrottleLimits throttle = {
+ .has_id = true,
+ .id = (char *) qdict_get_str(qdict, "device"),
+ };
+
+ hmp_initialize_throttle_limits(&throttle, qdict);
+ qmp_fsdev_set_io_throttle(&throttle, &err);
+ hmp_handle_error(mon, &err);
+}
+
+static void print_fsdev_throttle_config(Monitor *mon, ThrottleLimits *fscfg)
+{
+ monitor_printf(mon, "%s", fscfg->id);
+ monitor_printf(mon, " I/O throttling:"
+ " bps=%" PRId64
+ " bps_rd=%" PRId64 " bps_wr=%" PRId64
+ " bps_max=%" PRId64
+ " bps_rd_max=%" PRId64
+ " bps_wr_max=%" PRId64
+ " iops=%" PRId64 " iops_rd=%" PRId64
+ " iops_wr=%" PRId64
+ " iops_max=%" PRId64
+ " iops_rd_max=%" PRId64
+ " iops_wr_max=%" PRId64
+ " iops_size=%" PRId64
+ "\n",
+ fscfg->bps_total,
+ fscfg->bps_read,
+ fscfg->bps_write,
+ fscfg->bps_total_max,
+ fscfg->bps_read_max,
+ fscfg->bps_write_max,
+ fscfg->iops_total,
+ fscfg->iops_read,
+ fscfg->iops_write,
+ fscfg->iops_total_max,
+ fscfg->iops_read_max,
+ fscfg->iops_write_max,
+ fscfg->iops_size);
+}
+
+void hmp_info_fsdev_iothrottle(Monitor *mon, const QDict *qdict)
+{
+ ThrottleLimitsList *fsdev_list, *info;
+ fsdev_list = qmp_query_fsdev_io_throttle(NULL);
+
+ for (info = fsdev_list; info; info = info->next) {
+ print_fsdev_throttle_config(mon, info->value);
+ }
+ qapi_free_ThrottleLimitsList(fsdev_list);
+}
+
+#endif
+
void hmp_block_stream(Monitor *mon, const QDict *qdict)
{
Error *error = NULL;
diff --git a/hmp.h b/hmp.h
index 1ff4552..d700d7d 100644
--- a/hmp.h
+++ b/hmp.h
@@ -81,6 +81,10 @@ void hmp_set_password(Monitor *mon, const QDict *qdict);
void hmp_expire_password(Monitor *mon, const QDict *qdict);
void hmp_eject(Monitor *mon, const QDict *qdict);
void hmp_change(Monitor *mon, const QDict *qdict);
+#ifdef CONFIG_VIRTFS
+void hmp_fsdev_set_io_throttle(Monitor *mon, const QDict *qdict);
+void hmp_info_fsdev_iothrottle(Monitor *mon, const QDict *qdict);
+#endif
void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict);
void hmp_block_stream(Monitor *mon, const QDict *qdict);
void hmp_block_job_set_speed(Monitor *mon, const QDict *qdict);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code Pradeep Jagadeesh
@ 2017-09-14 11:12 ` Greg Kurz
2017-09-18 16:20 ` Manos Pitsidianakis
1 sibling, 0 replies; 25+ messages in thread
From: Greg Kurz @ 2017-09-14 11:12 UTC (permalink / raw)
To: Pradeep Jagadeesh
Cc: eric blake, Pradeep Jagadeesh, alberto garcia, Markus Armbruster,
jani kokkonen, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 12212 bytes --]
On Thu, 14 Sep 2017 06:40:05 -0400
Pradeep Jagadeesh <pradeepkiruvale@gmail.com> wrote:
> This patch factors out the duplicate throttle code that was still
> present in block and fsdev devices.
>
> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
> Reviewed-by: Alberto Garcia <berto@igalia.com>
> Reviewed-by: Greg Kurz <groug@kaod.org>
I see you took my remarks into account, except for the patch title
which is still the very same as commit:
a2a7862ca9ab throttle: factor out duplicate code
:-\
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
> blockdev.c | 44 +---------------------------------
> fsdev/qemu-fsdev-throttle.c | 44 ++--------------------------------
> include/qemu/throttle-options.h | 3 +++
> include/qemu/throttle.h | 4 ++--
> include/qemu/typedefs.h | 1 +
> util/throttle.c | 52 +++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 61 insertions(+), 87 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index 56a6b24..9d33c25 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -387,49 +387,7 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
> }
>
> if (throttle_cfg) {
> - throttle_config_init(throttle_cfg);
> - throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
> - qemu_opt_get_number(opts, "throttling.bps-total", 0);
> - throttle_cfg->buckets[THROTTLE_BPS_READ].avg =
> - qemu_opt_get_number(opts, "throttling.bps-read", 0);
> - throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
> - qemu_opt_get_number(opts, "throttling.bps-write", 0);
> - throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
> - qemu_opt_get_number(opts, "throttling.iops-total", 0);
> - throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
> - qemu_opt_get_number(opts, "throttling.iops-read", 0);
> - throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
> - qemu_opt_get_number(opts, "throttling.iops-write", 0);
> -
> - throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
> - qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
> - throttle_cfg->buckets[THROTTLE_BPS_READ].max =
> - qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
> - throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
> - qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
> - throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
> - qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
> - throttle_cfg->buckets[THROTTLE_OPS_READ].max =
> - qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
> - throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
> - qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
> -
> - throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
> - qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
> - throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =
> - qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
> - throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
> - qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
> - throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
> - qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
> - throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
> - qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
> - throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
> - qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
> -
> - throttle_cfg->op_size =
> - qemu_opt_get_number(opts, "throttling.iops-size", 0);
> -
> + throttle_parse_options(throttle_cfg, opts);
> if (!throttle_is_valid(throttle_cfg, errp)) {
> return;
> }
> diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
> index 49eebb5..0e6fb86 100644
> --- a/fsdev/qemu-fsdev-throttle.c
> +++ b/fsdev/qemu-fsdev-throttle.c
> @@ -16,6 +16,7 @@
> #include "qemu/error-report.h"
> #include "qemu-fsdev-throttle.h"
> #include "qemu/iov.h"
> +#include "qemu/throttle-options.h"
>
> static void fsdev_throttle_read_timer_cb(void *opaque)
> {
> @@ -31,48 +32,7 @@ static void fsdev_throttle_write_timer_cb(void *opaque)
>
> void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error **errp)
> {
> - throttle_config_init(&fst->cfg);
> - fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =
> - qemu_opt_get_number(opts, "throttling.bps-total", 0);
> - fst->cfg.buckets[THROTTLE_BPS_READ].avg =
> - qemu_opt_get_number(opts, "throttling.bps-read", 0);
> - fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =
> - qemu_opt_get_number(opts, "throttling.bps-write", 0);
> - fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =
> - qemu_opt_get_number(opts, "throttling.iops-total", 0);
> - fst->cfg.buckets[THROTTLE_OPS_READ].avg =
> - qemu_opt_get_number(opts, "throttling.iops-read", 0);
> - fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =
> - qemu_opt_get_number(opts, "throttling.iops-write", 0);
> -
> - fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =
> - qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
> - fst->cfg.buckets[THROTTLE_BPS_READ].max =
> - qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
> - fst->cfg.buckets[THROTTLE_BPS_WRITE].max =
> - qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
> - fst->cfg.buckets[THROTTLE_OPS_TOTAL].max =
> - qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
> - fst->cfg.buckets[THROTTLE_OPS_READ].max =
> - qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
> - fst->cfg.buckets[THROTTLE_OPS_WRITE].max =
> - qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
> -
> - fst->cfg.buckets[THROTTLE_BPS_TOTAL].burst_length =
> - qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
> - fst->cfg.buckets[THROTTLE_BPS_READ].burst_length =
> - qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
> - fst->cfg.buckets[THROTTLE_BPS_WRITE].burst_length =
> - qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
> - fst->cfg.buckets[THROTTLE_OPS_TOTAL].burst_length =
> - qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
> - fst->cfg.buckets[THROTTLE_OPS_READ].burst_length =
> - qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
> - fst->cfg.buckets[THROTTLE_OPS_WRITE].burst_length =
> - qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
> - fst->cfg.op_size =
> - qemu_opt_get_number(opts, "throttling.iops-size", 0);
> -
> + throttle_parse_options(&fst->cfg, opts);
> throttle_is_valid(&fst->cfg, errp);
> }
>
> diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h
> index 3528a8f..9709dcd 100644
> --- a/include/qemu/throttle-options.h
> +++ b/include/qemu/throttle-options.h
> @@ -9,6 +9,7 @@
> */
> #ifndef THROTTLE_OPTIONS_H
> #define THROTTLE_OPTIONS_H
> +#include "typedefs.h"
>
> #define QEMU_OPT_IOPS_TOTAL "iops-total"
> #define QEMU_OPT_IOPS_TOTAL_MAX "iops-total-max"
> @@ -111,4 +112,6 @@
> .help = "when limiting by iops max size of an I/O in bytes",\
> }
>
> +void throttle_parse_options(ThrottleConfig *, QemuOpts *);
> +
> #endif
> diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
> index 8c93237..b6ebc6d 100644
> --- a/include/qemu/throttle.h
> +++ b/include/qemu/throttle.h
> @@ -89,10 +89,10 @@ typedef struct LeakyBucket {
> * However it allows to keep the code clean and the bucket field is reset to
> * zero at the right time.
> */
> -typedef struct ThrottleConfig {
> +struct ThrottleConfig {
> LeakyBucket buckets[BUCKETS_COUNT]; /* leaky buckets */
> uint64_t op_size; /* size of an operation in bytes */
> -} ThrottleConfig;
> +};
>
> typedef struct ThrottleState {
> ThrottleConfig cfg; /* configuration */
> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> index 39bc835..90fe0f9 100644
> --- a/include/qemu/typedefs.h
> +++ b/include/qemu/typedefs.h
> @@ -100,6 +100,7 @@ typedef struct uWireSlave uWireSlave;
> typedef struct VirtIODevice VirtIODevice;
> typedef struct Visitor Visitor;
> typedef struct node_info NodeInfo;
> +typedef struct ThrottleConfig ThrottleConfig;
> typedef void SaveStateHandler(QEMUFile *f, void *opaque);
> typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);
>
> diff --git a/util/throttle.c b/util/throttle.c
> index 06bf916..9ef28c4 100644
> --- a/util/throttle.c
> +++ b/util/throttle.c
> @@ -27,6 +27,7 @@
> #include "qemu/throttle.h"
> #include "qemu/timer.h"
> #include "block/aio.h"
> +#include "qemu/throttle-options.h"
>
> /* This function make a bucket leak
> *
> @@ -635,3 +636,54 @@ void throttle_config_to_limits(ThrottleConfig *cfg, ThrottleLimits *var)
> var->has_iops_write_max_length = true;
> var->has_iops_size = true;
> }
> +
> +/* parse the throttle options
> + *
> + * @opts: qemu options
> + * @throttle_cfg: throttle configuration
> + */
> +void throttle_parse_options(ThrottleConfig *throttle_cfg, QemuOpts *opts)
> +{
> + throttle_config_init(throttle_cfg);
> + throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
> + qemu_opt_get_number(opts, "throttling.bps-total", 0);
> + throttle_cfg->buckets[THROTTLE_BPS_READ].avg =
> + qemu_opt_get_number(opts, "throttling.bps-read", 0);
> + throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
> + qemu_opt_get_number(opts, "throttling.bps-write", 0);
> + throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
> + qemu_opt_get_number(opts, "throttling.iops-total", 0);
> + throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
> + qemu_opt_get_number(opts, "throttling.iops-read", 0);
> + throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
> + qemu_opt_get_number(opts, "throttling.iops-write", 0);
> +
> + throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
> + qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
> + throttle_cfg->buckets[THROTTLE_BPS_READ].max =
> + qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
> + throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
> + qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
> + throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
> + qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
> + throttle_cfg->buckets[THROTTLE_OPS_READ].max =
> + qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
> + throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
> + qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
> +
> + throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
> + qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
> + throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =
> + qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
> + throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
> + qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
> + throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
> + qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
> + throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
> + qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
> + throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
> + qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
> +
> + throttle_cfg->op_size =
> + qemu_opt_get_number(opts, "throttling.iops-size", 0);
> +}
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v11 2/6] qmp: Use ThrottleLimits structure
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 2/6] qmp: Use ThrottleLimits structure Pradeep Jagadeesh
@ 2017-09-14 11:19 ` Greg Kurz
2017-09-14 11:23 ` Pradeep Jagadeesh
2017-09-18 16:04 ` Manos Pitsidianakis
` (2 subsequent siblings)
3 siblings, 1 reply; 25+ messages in thread
From: Greg Kurz @ 2017-09-14 11:19 UTC (permalink / raw)
To: Pradeep Jagadeesh
Cc: eric blake, jani kokkonen, alberto garcia, Pradeep Jagadeesh, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 5448 bytes --]
On Thu, 14 Sep 2017 06:40:06 -0400
Pradeep Jagadeesh <pradeepkiruvale@gmail.com> wrote:
> This patch factors out code to use the ThrottleLimits
> strurcture.
>
> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Alberto Garcia <berto@igalia.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> ---
> qapi/block-core.json | 78 +++-------------------------------------------------
Uhhh... what happened here ? Where did the lines go ? I've never reviewed this
patch before...
> 1 file changed, 4 insertions(+), 74 deletions(-)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index bb11815..d0ccfda 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1826,84 +1826,13 @@
> #
> # @device: Block device name (deprecated, use @id instead)
> #
> -# @id: The name or QOM path of the guest device (since: 2.8)
> -#
> -# @bps: total throughput limit in bytes per second
> -#
> -# @bps_rd: read throughput limit in bytes per second
> -#
> -# @bps_wr: write throughput limit in bytes per second
> -#
> -# @iops: total I/O operations per second
> -#
> -# @iops_rd: read I/O operations per second
> -#
> -# @iops_wr: write I/O operations per second
> -#
> -# @bps_max: total throughput limit during bursts,
> -# in bytes (Since 1.7)
> -#
> -# @bps_rd_max: read throughput limit during bursts,
> -# in bytes (Since 1.7)
> -#
> -# @bps_wr_max: write throughput limit during bursts,
> -# in bytes (Since 1.7)
> -#
> -# @iops_max: total I/O operations per second during bursts,
> -# in bytes (Since 1.7)
> -#
> -# @iops_rd_max: read I/O operations per second during bursts,
> -# in bytes (Since 1.7)
> -#
> -# @iops_wr_max: write I/O operations per second during bursts,
> -# in bytes (Since 1.7)
> -#
> -# @bps_max_length: maximum length of the @bps_max burst
> -# period, in seconds. It must only
> -# be set if @bps_max is set as well.
> -# Defaults to 1. (Since 2.6)
> -#
> -# @bps_rd_max_length: maximum length of the @bps_rd_max
> -# burst period, in seconds. It must only
> -# be set if @bps_rd_max is set as well.
> -# Defaults to 1. (Since 2.6)
> -#
> -# @bps_wr_max_length: maximum length of the @bps_wr_max
> -# burst period, in seconds. It must only
> -# be set if @bps_wr_max is set as well.
> -# Defaults to 1. (Since 2.6)
> -#
> -# @iops_max_length: maximum length of the @iops burst
> -# period, in seconds. It must only
> -# be set if @iops_max is set as well.
> -# Defaults to 1. (Since 2.6)
> -#
> -# @iops_rd_max_length: maximum length of the @iops_rd_max
> -# burst period, in seconds. It must only
> -# be set if @iops_rd_max is set as well.
> -# Defaults to 1. (Since 2.6)
> -#
> -# @iops_wr_max_length: maximum length of the @iops_wr_max
> -# burst period, in seconds. It must only
> -# be set if @iops_wr_max is set as well.
> -# Defaults to 1. (Since 2.6)
> -#
> -# @iops_size: an I/O size in bytes (Since 1.7)
> -#
> # @group: throttle group name (Since 2.4)
> #
> # Since: 1.1
> ##
> { 'struct': 'BlockIOThrottle',
> - 'data': { '*device': 'str', '*id': 'str', 'bps': 'int', 'bps_rd': 'int',
> - 'bps_wr': 'int', 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
> - '*bps_max': 'int', '*bps_rd_max': 'int',
> - '*bps_wr_max': 'int', '*iops_max': 'int',
> - '*iops_rd_max': 'int', '*iops_wr_max': 'int',
> - '*bps_max_length': 'int', '*bps_rd_max_length': 'int',
> - '*bps_wr_max_length': 'int', '*iops_max_length': 'int',
> - '*iops_rd_max_length': 'int', '*iops_wr_max_length': 'int',
> - '*iops_size': 'int', '*group': 'str' } }
> + 'base': 'ThrottleLimits',
> + 'data': { '*device': 'str', '*group': 'str' } }
>
> ##
> # @ThrottleLimits:
> @@ -1913,6 +1842,7 @@
> # transaction. All fields are optional. When setting limits, if a field is
> # missing the current value is not changed.
> #
> +# @id: device id
> # @iops-total: limit total I/O operations per second
> # @iops-total-max: I/O operations burst
> # @iops-total-max-length: length of the iops-total-max burst period, in seconds
> @@ -1942,7 +1872,7 @@
> # Since: 2.11
> ##
> { 'struct': 'ThrottleLimits',
> - 'data': { '*iops-total' : 'int', '*iops-total-max' : 'int',
> + 'data': { '*id' : 'str', '*iops-total' : 'int', '*iops-total-max' : 'int',
> '*iops-total-max-length' : 'int', '*iops-read' : 'int',
> '*iops-read-max' : 'int', '*iops-read-max-length' : 'int',
> '*iops-write' : 'int', '*iops-write-max' : 'int',
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v11 3/6] qmp: factor out throttle code to reuse code
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 3/6] qmp: factor out throttle code to reuse code Pradeep Jagadeesh
@ 2017-09-14 11:22 ` Greg Kurz
0 siblings, 0 replies; 25+ messages in thread
From: Greg Kurz @ 2017-09-14 11:22 UTC (permalink / raw)
To: Pradeep Jagadeesh
Cc: eric blake, qemu-devel, jani kokkonen, alberto garcia,
Pradeep Jagadeesh, Markus Armbruster
[-- Attachment #1: Type: text/plain, Size: 3209 bytes --]
On Thu, 14 Sep 2017 06:40:07 -0400
Pradeep Jagadeesh <pradeepkiruvale@gmail.com> wrote:
> This patch reuses the code to set throttle limits.
>
> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
> Reviewed-by: Alberto Garcia <berto@igalia.com>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> ---
> blockdev.c | 53 +++--------------------------------------------------
Same remarks as for the previous patch...
> 1 file changed, 3 insertions(+), 50 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index 9d33c25..2bd8ebd 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2569,6 +2569,7 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp)
> BlockDriverState *bs;
> BlockBackend *blk;
> AioContext *aio_context;
> + ThrottleLimits *tlimit;
>
> blk = qmp_get_blk(arg->has_device ? arg->device : NULL,
> arg->has_id ? arg->id : NULL,
> @@ -2586,56 +2587,8 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp)
> goto out;
> }
>
> - throttle_config_init(&cfg);
> - cfg.buckets[THROTTLE_BPS_TOTAL].avg = arg->bps;
> - cfg.buckets[THROTTLE_BPS_READ].avg = arg->bps_rd;
> - cfg.buckets[THROTTLE_BPS_WRITE].avg = arg->bps_wr;
> -
> - cfg.buckets[THROTTLE_OPS_TOTAL].avg = arg->iops;
> - cfg.buckets[THROTTLE_OPS_READ].avg = arg->iops_rd;
> - cfg.buckets[THROTTLE_OPS_WRITE].avg = arg->iops_wr;
> -
> - if (arg->has_bps_max) {
> - cfg.buckets[THROTTLE_BPS_TOTAL].max = arg->bps_max;
> - }
> - if (arg->has_bps_rd_max) {
> - cfg.buckets[THROTTLE_BPS_READ].max = arg->bps_rd_max;
> - }
> - if (arg->has_bps_wr_max) {
> - cfg.buckets[THROTTLE_BPS_WRITE].max = arg->bps_wr_max;
> - }
> - if (arg->has_iops_max) {
> - cfg.buckets[THROTTLE_OPS_TOTAL].max = arg->iops_max;
> - }
> - if (arg->has_iops_rd_max) {
> - cfg.buckets[THROTTLE_OPS_READ].max = arg->iops_rd_max;
> - }
> - if (arg->has_iops_wr_max) {
> - cfg.buckets[THROTTLE_OPS_WRITE].max = arg->iops_wr_max;
> - }
> -
> - if (arg->has_bps_max_length) {
> - cfg.buckets[THROTTLE_BPS_TOTAL].burst_length = arg->bps_max_length;
> - }
> - if (arg->has_bps_rd_max_length) {
> - cfg.buckets[THROTTLE_BPS_READ].burst_length = arg->bps_rd_max_length;
> - }
> - if (arg->has_bps_wr_max_length) {
> - cfg.buckets[THROTTLE_BPS_WRITE].burst_length = arg->bps_wr_max_length;
> - }
> - if (arg->has_iops_max_length) {
> - cfg.buckets[THROTTLE_OPS_TOTAL].burst_length = arg->iops_max_length;
> - }
> - if (arg->has_iops_rd_max_length) {
> - cfg.buckets[THROTTLE_OPS_READ].burst_length = arg->iops_rd_max_length;
> - }
> - if (arg->has_iops_wr_max_length) {
> - cfg.buckets[THROTTLE_OPS_WRITE].burst_length = arg->iops_wr_max_length;
> - }
> -
> - if (arg->has_iops_size) {
> - cfg.op_size = arg->iops_size;
> - }
> + tlimit = qapi_BlockIOThrottle_base(arg);
> + throttle_config_to_limits(&cfg, tlimit);
>
> if (!throttle_is_valid(&cfg, errp)) {
> goto out;
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v11 2/6] qmp: Use ThrottleLimits structure
2017-09-14 11:19 ` Greg Kurz
@ 2017-09-14 11:23 ` Pradeep Jagadeesh
0 siblings, 0 replies; 25+ messages in thread
From: Pradeep Jagadeesh @ 2017-09-14 11:23 UTC (permalink / raw)
To: Greg Kurz, Pradeep Jagadeesh
Cc: eric blake, jani kokkonen, alberto garcia, qemu-devel
On 9/14/2017 1:19 PM, Greg Kurz wrote:
> On Thu, 14 Sep 2017 06:40:06 -0400
> Pradeep Jagadeesh <pradeepkiruvale@gmail.com> wrote:
>
>> This patch factors out code to use the ThrottleLimits
>> strurcture.
>>
>> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
>> Reviewed-by: Greg Kurz <groug@kaod.org>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> Reviewed-by: Alberto Garcia <berto@igalia.com>
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> qapi/block-core.json | 78 +++-------------------------------------------------
>
> Uhhh... what happened here ? Where did the lines go ? I've never reviewed this
> patch before...
I did rebase the code on 2.10
Now I am using the ThrottleLimits structure that was introduced IN 2.10.
This is same as the IOThrottle structure.
So, many things got changed in this version.
-Pradeep
>
>> 1 file changed, 4 insertions(+), 74 deletions(-)
>>
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index bb11815..d0ccfda 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -1826,84 +1826,13 @@
>> #
>> # @device: Block device name (deprecated, use @id instead)
>> #
>> -# @id: The name or QOM path of the guest device (since: 2.8)
>> -#
>> -# @bps: total throughput limit in bytes per second
>> -#
>> -# @bps_rd: read throughput limit in bytes per second
>> -#
>> -# @bps_wr: write throughput limit in bytes per second
>> -#
>> -# @iops: total I/O operations per second
>> -#
>> -# @iops_rd: read I/O operations per second
>> -#
>> -# @iops_wr: write I/O operations per second
>> -#
>> -# @bps_max: total throughput limit during bursts,
>> -# in bytes (Since 1.7)
>> -#
>> -# @bps_rd_max: read throughput limit during bursts,
>> -# in bytes (Since 1.7)
>> -#
>> -# @bps_wr_max: write throughput limit during bursts,
>> -# in bytes (Since 1.7)
>> -#
>> -# @iops_max: total I/O operations per second during bursts,
>> -# in bytes (Since 1.7)
>> -#
>> -# @iops_rd_max: read I/O operations per second during bursts,
>> -# in bytes (Since 1.7)
>> -#
>> -# @iops_wr_max: write I/O operations per second during bursts,
>> -# in bytes (Since 1.7)
>> -#
>> -# @bps_max_length: maximum length of the @bps_max burst
>> -# period, in seconds. It must only
>> -# be set if @bps_max is set as well.
>> -# Defaults to 1. (Since 2.6)
>> -#
>> -# @bps_rd_max_length: maximum length of the @bps_rd_max
>> -# burst period, in seconds. It must only
>> -# be set if @bps_rd_max is set as well.
>> -# Defaults to 1. (Since 2.6)
>> -#
>> -# @bps_wr_max_length: maximum length of the @bps_wr_max
>> -# burst period, in seconds. It must only
>> -# be set if @bps_wr_max is set as well.
>> -# Defaults to 1. (Since 2.6)
>> -#
>> -# @iops_max_length: maximum length of the @iops burst
>> -# period, in seconds. It must only
>> -# be set if @iops_max is set as well.
>> -# Defaults to 1. (Since 2.6)
>> -#
>> -# @iops_rd_max_length: maximum length of the @iops_rd_max
>> -# burst period, in seconds. It must only
>> -# be set if @iops_rd_max is set as well.
>> -# Defaults to 1. (Since 2.6)
>> -#
>> -# @iops_wr_max_length: maximum length of the @iops_wr_max
>> -# burst period, in seconds. It must only
>> -# be set if @iops_wr_max is set as well.
>> -# Defaults to 1. (Since 2.6)
>> -#
>> -# @iops_size: an I/O size in bytes (Since 1.7)
>> -#
>> # @group: throttle group name (Since 2.4)
>> #
>> # Since: 1.1
>> ##
>> { 'struct': 'BlockIOThrottle',
>> - 'data': { '*device': 'str', '*id': 'str', 'bps': 'int', 'bps_rd': 'int',
>> - 'bps_wr': 'int', 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
>> - '*bps_max': 'int', '*bps_rd_max': 'int',
>> - '*bps_wr_max': 'int', '*iops_max': 'int',
>> - '*iops_rd_max': 'int', '*iops_wr_max': 'int',
>> - '*bps_max_length': 'int', '*bps_rd_max_length': 'int',
>> - '*bps_wr_max_length': 'int', '*iops_max_length': 'int',
>> - '*iops_rd_max_length': 'int', '*iops_wr_max_length': 'int',
>> - '*iops_size': 'int', '*group': 'str' } }
>> + 'base': 'ThrottleLimits',
>> + 'data': { '*device': 'str', '*group': 'str' } }
>>
>> ##
>> # @ThrottleLimits:
>> @@ -1913,6 +1842,7 @@
>> # transaction. All fields are optional. When setting limits, if a field is
>> # missing the current value is not changed.
>> #
>> +# @id: device id
>> # @iops-total: limit total I/O operations per second
>> # @iops-total-max: I/O operations burst
>> # @iops-total-max-length: length of the iops-total-max burst period, in seconds
>> @@ -1942,7 +1872,7 @@
>> # Since: 2.11
>> ##
>> { 'struct': 'ThrottleLimits',
>> - 'data': { '*iops-total' : 'int', '*iops-total-max' : 'int',
>> + 'data': { '*id' : 'str', '*iops-total' : 'int', '*iops-total-max' : 'int',
>> '*iops-total-max-length' : 'int', '*iops-read' : 'int',
>> '*iops-read-max' : 'int', '*iops-read-max-length' : 'int',
>> '*iops-write' : 'int', '*iops-write-max' : 'int',
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v11 6/6] fsdev: hmp interface for throttling
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 6/6] fsdev: hmp " Pradeep Jagadeesh
@ 2017-09-14 12:05 ` Alberto Garcia
0 siblings, 0 replies; 25+ messages in thread
From: Alberto Garcia @ 2017-09-14 12:05 UTC (permalink / raw)
To: Pradeep Jagadeesh, eric blake, greg kurz
Cc: Pradeep Jagadeesh, Dr. David Alan Gilbert, jani kokkonen, qemu-devel
On Thu 14 Sep 2017 12:40:10 PM CEST, Pradeep Jagadeesh wrote:
> This patch introduces hmp interfaces for the fsdev
> devices.
>
> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Berto
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v11 2/6] qmp: Use ThrottleLimits structure
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 2/6] qmp: Use ThrottleLimits structure Pradeep Jagadeesh
2017-09-14 11:19 ` Greg Kurz
@ 2017-09-18 16:04 ` Manos Pitsidianakis
2017-09-20 8:46 ` Pradeep Jagadeesh
2017-09-18 16:25 ` Manos Pitsidianakis
2017-09-18 17:10 ` Eric Blake
3 siblings, 1 reply; 25+ messages in thread
From: Manos Pitsidianakis @ 2017-09-18 16:04 UTC (permalink / raw)
To: Pradeep Jagadeesh
Cc: eric blake, greg kurz, jani kokkonen, alberto garcia,
Pradeep Jagadeesh, qemu-devel, Markus Armbruster
[-- Attachment #1: Type: text/plain, Size: 5534 bytes --]
On Thu, Sep 14, 2017 at 06:40:06AM -0400, Pradeep Jagadeesh wrote:
>This patch factors out code to use the ThrottleLimits
>strurcture.
>
>Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
>Reviewed-by: Greg Kurz <groug@kaod.org>
>Reviewed-by: Eric Blake <eblake@redhat.com>
>Reviewed-by: Alberto Garcia <berto@igalia.com>
>Reviewed-by: Markus Armbruster <armbru@redhat.com>
>---
> qapi/block-core.json | 78 +++-------------------------------------------------
> 1 file changed, 4 insertions(+), 74 deletions(-)
>
>diff --git a/qapi/block-core.json b/qapi/block-core.json
>index bb11815..d0ccfda 100644
>--- a/qapi/block-core.json
>+++ b/qapi/block-core.json
>@@ -1826,84 +1826,13 @@
> #
> # @device: Block device name (deprecated, use @id instead)
> #
>-# @id: The name or QOM path of the guest device (since: 2.8)
>-#
>-# @bps: total throughput limit in bytes per second
>-#
>-# @bps_rd: read throughput limit in bytes per second
>-#
>-# @bps_wr: write throughput limit in bytes per second
>-#
>-# @iops: total I/O operations per second
>-#
>-# @iops_rd: read I/O operations per second
>-#
>-# @iops_wr: write I/O operations per second
>-#
>-# @bps_max: total throughput limit during bursts,
>-# in bytes (Since 1.7)
>-#
>-# @bps_rd_max: read throughput limit during bursts,
>-# in bytes (Since 1.7)
>-#
>-# @bps_wr_max: write throughput limit during bursts,
>-# in bytes (Since 1.7)
>-#
>-# @iops_max: total I/O operations per second during bursts,
>-# in bytes (Since 1.7)
>-#
>-# @iops_rd_max: read I/O operations per second during bursts,
>-# in bytes (Since 1.7)
>-#
>-# @iops_wr_max: write I/O operations per second during bursts,
>-# in bytes (Since 1.7)
>-#
>-# @bps_max_length: maximum length of the @bps_max burst
>-# period, in seconds. It must only
>-# be set if @bps_max is set as well.
>-# Defaults to 1. (Since 2.6)
>-#
>-# @bps_rd_max_length: maximum length of the @bps_rd_max
>-# burst period, in seconds. It must only
>-# be set if @bps_rd_max is set as well.
>-# Defaults to 1. (Since 2.6)
>-#
>-# @bps_wr_max_length: maximum length of the @bps_wr_max
>-# burst period, in seconds. It must only
>-# be set if @bps_wr_max is set as well.
>-# Defaults to 1. (Since 2.6)
>-#
>-# @iops_max_length: maximum length of the @iops burst
>-# period, in seconds. It must only
>-# be set if @iops_max is set as well.
>-# Defaults to 1. (Since 2.6)
>-#
>-# @iops_rd_max_length: maximum length of the @iops_rd_max
>-# burst period, in seconds. It must only
>-# be set if @iops_rd_max is set as well.
>-# Defaults to 1. (Since 2.6)
>-#
>-# @iops_wr_max_length: maximum length of the @iops_wr_max
>-# burst period, in seconds. It must only
>-# be set if @iops_wr_max is set as well.
>-# Defaults to 1. (Since 2.6)
>-#
>-# @iops_size: an I/O size in bytes (Since 1.7)
>-#
> # @group: throttle group name (Since 2.4)
BlockIOThrottle and ThrottleLimits are not directly compatible, for
example here we have iops_rd_max_length as a name,
> #
> # Since: 1.1
> ##
> { 'struct': 'BlockIOThrottle',
>- 'data': { '*device': 'str', '*id': 'str', 'bps': 'int', 'bps_rd': 'int',
>- 'bps_wr': 'int', 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
>- '*bps_max': 'int', '*bps_rd_max': 'int',
>- '*bps_wr_max': 'int', '*iops_max': 'int',
>- '*iops_rd_max': 'int', '*iops_wr_max': 'int',
>- '*bps_max_length': 'int', '*bps_rd_max_length': 'int',
>- '*bps_wr_max_length': 'int', '*iops_max_length': 'int',
>- '*iops_rd_max_length': 'int', '*iops_wr_max_length': 'int',
>- '*iops_size': 'int', '*group': 'str' } }
>+ 'base': 'ThrottleLimits',
>+ 'data': { '*device': 'str', '*group': 'str' } }
>
> ##
> # @ThrottleLimits:
>@@ -1913,6 +1842,7 @@
> # transaction. All fields are optional. When setting limits, if a field is
> # missing the current value is not changed.
> #
>+# @id: device id
> # @iops-total: limit total I/O operations per second
> # @iops-total-max: I/O operations burst
> # @iops-total-max-length: length of the iops-total-max burst period, in seconds
>@@ -1942,7 +1872,7 @@
> # Since: 2.11
> ##
> { 'struct': 'ThrottleLimits',
>- 'data': { '*iops-total' : 'int', '*iops-total-max' : 'int',
>+ 'data': { '*id' : 'str', '*iops-total' : 'int', '*iops-total-max' : 'int',
> '*iops-total-max-length' : 'int', '*iops-read' : 'int',
> '*iops-read-max' : 'int', '*iops-read-max-length' : 'int',
> '*iops-write' : 'int', '*iops-write-max' : 'int',
And here it's iops-read-max-length. This breaks the block layer's old
throttling API. Do we want the old API replaced for 2.11? I don't know
which maintainer is responsible for this, so I CC'd armbru.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code Pradeep Jagadeesh
2017-09-14 11:12 ` Greg Kurz
@ 2017-09-18 16:20 ` Manos Pitsidianakis
2017-09-20 8:57 ` Pradeep Jagadeesh
2017-09-22 11:31 ` Pradeep Jagadeesh
1 sibling, 2 replies; 25+ messages in thread
From: Manos Pitsidianakis @ 2017-09-18 16:20 UTC (permalink / raw)
To: Pradeep Jagadeesh
Cc: eric blake, greg kurz, qemu-devel, jani kokkonen, alberto garcia,
Pradeep Jagadeesh, Markus Armbruster
[-- Attachment #1: Type: text/plain, Size: 12212 bytes --]
On Thu, Sep 14, 2017 at 06:40:05AM -0400, Pradeep Jagadeesh wrote:
>This patch factors out the duplicate throttle code that was still
>present in block and fsdev devices.
>
>Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
>Reviewed-by: Alberto Garcia <berto@igalia.com>
>Reviewed-by: Greg Kurz <groug@kaod.org>
>Reviewed-by: Eric Blake <eblake@redhat.com>
>---
> blockdev.c | 44 +---------------------------------
> fsdev/qemu-fsdev-throttle.c | 44 ++--------------------------------
> include/qemu/throttle-options.h | 3 +++
> include/qemu/throttle.h | 4 ++--
> include/qemu/typedefs.h | 1 +
> util/throttle.c | 52 +++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 61 insertions(+), 87 deletions(-)
>
>diff --git a/blockdev.c b/blockdev.c
>index 56a6b24..9d33c25 100644
>--- a/blockdev.c
>+++ b/blockdev.c
>@@ -387,49 +387,7 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
> }
>
> if (throttle_cfg) {
>- throttle_config_init(throttle_cfg);
>- throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
>- qemu_opt_get_number(opts, "throttling.bps-total", 0);
>- throttle_cfg->buckets[THROTTLE_BPS_READ].avg =
>- qemu_opt_get_number(opts, "throttling.bps-read", 0);
>- throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
>- qemu_opt_get_number(opts, "throttling.bps-write", 0);
>- throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
>- qemu_opt_get_number(opts, "throttling.iops-total", 0);
>- throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
>- qemu_opt_get_number(opts, "throttling.iops-read", 0);
>- throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
>- qemu_opt_get_number(opts, "throttling.iops-write", 0);
>-
>- throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
>- qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
>- throttle_cfg->buckets[THROTTLE_BPS_READ].max =
>- qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
>- throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
>- qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
>- throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
>- qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
>- throttle_cfg->buckets[THROTTLE_OPS_READ].max =
>- qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
>- throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
>- qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
>-
>- throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
>- qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
>- throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =
>- qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
>- throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
>- qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
>- throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
>- qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
>- throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
>- qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
>- throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
>- qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
>-
>- throttle_cfg->op_size =
>- qemu_opt_get_number(opts, "throttling.iops-size", 0);
>-
>+ throttle_parse_options(throttle_cfg, opts);
> if (!throttle_is_valid(throttle_cfg, errp)) {
> return;
> }
>diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
>index 49eebb5..0e6fb86 100644
>--- a/fsdev/qemu-fsdev-throttle.c
>+++ b/fsdev/qemu-fsdev-throttle.c
>@@ -16,6 +16,7 @@
> #include "qemu/error-report.h"
> #include "qemu-fsdev-throttle.h"
> #include "qemu/iov.h"
>+#include "qemu/throttle-options.h"
>
> static void fsdev_throttle_read_timer_cb(void *opaque)
> {
>@@ -31,48 +32,7 @@ static void fsdev_throttle_write_timer_cb(void *opaque)
>
> void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error **errp)
> {
>- throttle_config_init(&fst->cfg);
>- fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =
>- qemu_opt_get_number(opts, "throttling.bps-total", 0);
>- fst->cfg.buckets[THROTTLE_BPS_READ].avg =
>- qemu_opt_get_number(opts, "throttling.bps-read", 0);
>- fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =
>- qemu_opt_get_number(opts, "throttling.bps-write", 0);
>- fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =
>- qemu_opt_get_number(opts, "throttling.iops-total", 0);
>- fst->cfg.buckets[THROTTLE_OPS_READ].avg =
>- qemu_opt_get_number(opts, "throttling.iops-read", 0);
>- fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =
>- qemu_opt_get_number(opts, "throttling.iops-write", 0);
>-
>- fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =
>- qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
>- fst->cfg.buckets[THROTTLE_BPS_READ].max =
>- qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
>- fst->cfg.buckets[THROTTLE_BPS_WRITE].max =
>- qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
>- fst->cfg.buckets[THROTTLE_OPS_TOTAL].max =
>- qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
>- fst->cfg.buckets[THROTTLE_OPS_READ].max =
>- qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
>- fst->cfg.buckets[THROTTLE_OPS_WRITE].max =
>- qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
>-
>- fst->cfg.buckets[THROTTLE_BPS_TOTAL].burst_length =
>- qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
>- fst->cfg.buckets[THROTTLE_BPS_READ].burst_length =
>- qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
>- fst->cfg.buckets[THROTTLE_BPS_WRITE].burst_length =
>- qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
>- fst->cfg.buckets[THROTTLE_OPS_TOTAL].burst_length =
>- qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
>- fst->cfg.buckets[THROTTLE_OPS_READ].burst_length =
>- qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
>- fst->cfg.buckets[THROTTLE_OPS_WRITE].burst_length =
>- qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
>- fst->cfg.op_size =
>- qemu_opt_get_number(opts, "throttling.iops-size", 0);
>-
>+ throttle_parse_options(&fst->cfg, opts);
> throttle_is_valid(&fst->cfg, errp);
> }
>
>diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h
>index 3528a8f..9709dcd 100644
>--- a/include/qemu/throttle-options.h
>+++ b/include/qemu/throttle-options.h
>@@ -9,6 +9,7 @@
> */
> #ifndef THROTTLE_OPTIONS_H
> #define THROTTLE_OPTIONS_H
>+#include "typedefs.h"
>
> #define QEMU_OPT_IOPS_TOTAL "iops-total"
> #define QEMU_OPT_IOPS_TOTAL_MAX "iops-total-max"
>@@ -111,4 +112,6 @@
> .help = "when limiting by iops max size of an I/O in bytes",\
> }
>
>+void throttle_parse_options(ThrottleConfig *, QemuOpts *);
>+
> #endif
>diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
>index 8c93237..b6ebc6d 100644
>--- a/include/qemu/throttle.h
>+++ b/include/qemu/throttle.h
>@@ -89,10 +89,10 @@ typedef struct LeakyBucket {
> * However it allows to keep the code clean and the bucket field is reset to
> * zero at the right time.
> */
>-typedef struct ThrottleConfig {
>+struct ThrottleConfig {
> LeakyBucket buckets[BUCKETS_COUNT]; /* leaky buckets */
> uint64_t op_size; /* size of an operation in bytes */
>-} ThrottleConfig;
>+};
>
> typedef struct ThrottleState {
> ThrottleConfig cfg; /* configuration */
>diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
>index 39bc835..90fe0f9 100644
>--- a/include/qemu/typedefs.h
>+++ b/include/qemu/typedefs.h
>@@ -100,6 +100,7 @@ typedef struct uWireSlave uWireSlave;
> typedef struct VirtIODevice VirtIODevice;
> typedef struct Visitor Visitor;
> typedef struct node_info NodeInfo;
>+typedef struct ThrottleConfig ThrottleConfig;
Is this really needed? Just include include/qemu/throttle.h wherever you
need.
> typedef void SaveStateHandler(QEMUFile *f, void *opaque);
> typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);
>
>diff --git a/util/throttle.c b/util/throttle.c
>index 06bf916..9ef28c4 100644
>--- a/util/throttle.c
>+++ b/util/throttle.c
>@@ -27,6 +27,7 @@
> #include "qemu/throttle.h"
> #include "qemu/timer.h"
> #include "block/aio.h"
>+#include "qemu/throttle-options.h"
>
> /* This function make a bucket leak
> *
>@@ -635,3 +636,54 @@ void throttle_config_to_limits(ThrottleConfig *cfg, ThrottleLimits *var)
> var->has_iops_write_max_length = true;
> var->has_iops_size = true;
> }
>+
>+/* parse the throttle options
>+ *
>+ * @opts: qemu options
>+ * @throttle_cfg: throttle configuration
>+ */
>+void throttle_parse_options(ThrottleConfig *throttle_cfg, QemuOpts *opts)
>+{
>+ throttle_config_init(throttle_cfg);
>+ throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
>+ qemu_opt_get_number(opts, "throttling.bps-total", 0);
>+ throttle_cfg->buckets[THROTTLE_BPS_READ].avg =
>+ qemu_opt_get_number(opts, "throttling.bps-read", 0);
>+ throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
>+ qemu_opt_get_number(opts, "throttling.bps-write", 0);
>+ throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
>+ qemu_opt_get_number(opts, "throttling.iops-total", 0);
>+ throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
>+ qemu_opt_get_number(opts, "throttling.iops-read", 0);
>+ throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
>+ qemu_opt_get_number(opts, "throttling.iops-write", 0);
>+
>+ throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
>+ qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
>+ throttle_cfg->buckets[THROTTLE_BPS_READ].max =
>+ qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
>+ throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
>+ qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
>+ throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
>+ qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
>+ throttle_cfg->buckets[THROTTLE_OPS_READ].max =
>+ qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
>+ throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
>+ qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
>+
>+ throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
>+ qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
>+ throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =
>+ qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
>+ throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
>+ qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
>+ throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
>+ qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
>+ throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
>+ qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
>+ throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
>+ qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
>+
>+ throttle_cfg->op_size =
>+ qemu_opt_get_number(opts, "throttling.iops-size", 0);
>+}
You should reuse the #define's in include/qemu/throttle-options.h
See throttle_extract_options() in this patch:
http://patchwork.ozlabs.org/patch/803919/ Disregard the UINT_MAX checks,
unsigned ints in LeakyBucket were replaced by uint64_t.
Don't forget to drop the reviews when you change a patch! The original
reviews will probably not be valid anymore.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v11 2/6] qmp: Use ThrottleLimits structure
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 2/6] qmp: Use ThrottleLimits structure Pradeep Jagadeesh
2017-09-14 11:19 ` Greg Kurz
2017-09-18 16:04 ` Manos Pitsidianakis
@ 2017-09-18 16:25 ` Manos Pitsidianakis
2017-09-19 8:07 ` Pradeep Jagadeesh
2017-09-18 17:10 ` Eric Blake
3 siblings, 1 reply; 25+ messages in thread
From: Manos Pitsidianakis @ 2017-09-18 16:25 UTC (permalink / raw)
To: Pradeep Jagadeesh
Cc: eric blake, greg kurz, jani kokkonen, alberto garcia,
Pradeep Jagadeesh, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 5410 bytes --]
On Thu, Sep 14, 2017 at 06:40:06AM -0400, Pradeep Jagadeesh wrote:
>This patch factors out code to use the ThrottleLimits
>strurcture.
>
>Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
>Reviewed-by: Greg Kurz <groug@kaod.org>
>Reviewed-by: Eric Blake <eblake@redhat.com>
>Reviewed-by: Alberto Garcia <berto@igalia.com>
>Reviewed-by: Markus Armbruster <armbru@redhat.com>
>---
> qapi/block-core.json | 78 +++-------------------------------------------------
> 1 file changed, 4 insertions(+), 74 deletions(-)
>
>diff --git a/qapi/block-core.json b/qapi/block-core.json
>index bb11815..d0ccfda 100644
>--- a/qapi/block-core.json
>+++ b/qapi/block-core.json
>@@ -1826,84 +1826,13 @@
> #
> # @device: Block device name (deprecated, use @id instead)
> #
>-# @id: The name or QOM path of the guest device (since: 2.8)
>-#
>-# @bps: total throughput limit in bytes per second
>-#
>-# @bps_rd: read throughput limit in bytes per second
>-#
>-# @bps_wr: write throughput limit in bytes per second
>-#
>-# @iops: total I/O operations per second
>-#
>-# @iops_rd: read I/O operations per second
>-#
>-# @iops_wr: write I/O operations per second
>-#
>-# @bps_max: total throughput limit during bursts,
>-# in bytes (Since 1.7)
>-#
>-# @bps_rd_max: read throughput limit during bursts,
>-# in bytes (Since 1.7)
>-#
>-# @bps_wr_max: write throughput limit during bursts,
>-# in bytes (Since 1.7)
>-#
>-# @iops_max: total I/O operations per second during bursts,
>-# in bytes (Since 1.7)
>-#
>-# @iops_rd_max: read I/O operations per second during bursts,
>-# in bytes (Since 1.7)
>-#
>-# @iops_wr_max: write I/O operations per second during bursts,
>-# in bytes (Since 1.7)
>-#
>-# @bps_max_length: maximum length of the @bps_max burst
>-# period, in seconds. It must only
>-# be set if @bps_max is set as well.
>-# Defaults to 1. (Since 2.6)
>-#
>-# @bps_rd_max_length: maximum length of the @bps_rd_max
>-# burst period, in seconds. It must only
>-# be set if @bps_rd_max is set as well.
>-# Defaults to 1. (Since 2.6)
>-#
>-# @bps_wr_max_length: maximum length of the @bps_wr_max
>-# burst period, in seconds. It must only
>-# be set if @bps_wr_max is set as well.
>-# Defaults to 1. (Since 2.6)
>-#
>-# @iops_max_length: maximum length of the @iops burst
>-# period, in seconds. It must only
>-# be set if @iops_max is set as well.
>-# Defaults to 1. (Since 2.6)
>-#
>-# @iops_rd_max_length: maximum length of the @iops_rd_max
>-# burst period, in seconds. It must only
>-# be set if @iops_rd_max is set as well.
>-# Defaults to 1. (Since 2.6)
>-#
>-# @iops_wr_max_length: maximum length of the @iops_wr_max
>-# burst period, in seconds. It must only
>-# be set if @iops_wr_max is set as well.
>-# Defaults to 1. (Since 2.6)
>-#
>-# @iops_size: an I/O size in bytes (Since 1.7)
>-#
> # @group: throttle group name (Since 2.4)
> #
> # Since: 1.1
> ##
> { 'struct': 'BlockIOThrottle',
>- 'data': { '*device': 'str', '*id': 'str', 'bps': 'int', 'bps_rd': 'int',
>- 'bps_wr': 'int', 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
>- '*bps_max': 'int', '*bps_rd_max': 'int',
>- '*bps_wr_max': 'int', '*iops_max': 'int',
>- '*iops_rd_max': 'int', '*iops_wr_max': 'int',
>- '*bps_max_length': 'int', '*bps_rd_max_length': 'int',
>- '*bps_wr_max_length': 'int', '*iops_max_length': 'int',
>- '*iops_rd_max_length': 'int', '*iops_wr_max_length': 'int',
>- '*iops_size': 'int', '*group': 'str' } }
>+ 'base': 'ThrottleLimits',
>+ 'data': { '*device': 'str', '*group': 'str' } }
>
> ##
> # @ThrottleLimits:
>@@ -1913,6 +1842,7 @@
> # transaction. All fields are optional. When setting limits, if a field is
> # missing the current value is not changed.
> #
>+# @id: device id
> # @iops-total: limit total I/O operations per second
> # @iops-total-max: I/O operations burst
> # @iops-total-max-length: length of the iops-total-max burst period, in seconds
>@@ -1942,7 +1872,7 @@
> # Since: 2.11
> ##
> { 'struct': 'ThrottleLimits',
>- 'data': { '*iops-total' : 'int', '*iops-total-max' : 'int',
>+ 'data': { '*id' : 'str', '*iops-total' : 'int', '*iops-total-max' : 'int',
> '*iops-total-max-length' : 'int', '*iops-read' : 'int',
> '*iops-read-max' : 'int', '*iops-read-max-length' : 'int',
> '*iops-write' : 'int', '*iops-write-max' : 'int',
>--
>1.8.3.1
>
Why is id needed? ThrottleLimits is not per group, not per device. And
as I mentioned in another reply, this changes the block_set_io_throttle
command which is not simple refactoring.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v11 2/6] qmp: Use ThrottleLimits structure
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 2/6] qmp: Use ThrottleLimits structure Pradeep Jagadeesh
` (2 preceding siblings ...)
2017-09-18 16:25 ` Manos Pitsidianakis
@ 2017-09-18 17:10 ` Eric Blake
2017-09-19 10:06 ` Pradeep Jagadeesh
3 siblings, 1 reply; 25+ messages in thread
From: Eric Blake @ 2017-09-18 17:10 UTC (permalink / raw)
To: Pradeep Jagadeesh, greg kurz
Cc: Pradeep Jagadeesh, alberto garcia, jani kokkonen, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 643 bytes --]
On 09/14/2017 05:40 AM, Pradeep Jagadeesh wrote:
> This patch factors out code to use the ThrottleLimits
> strurcture.
s/strurcture/structure/
>
> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> Reviewed-by: Eric Blake <eblake@redhat.com>
Echoing the sentiments made elsewhere - this is a significant rewrite
from the earlier version, so you WANT reviewers to look at it fresh
rather than assuming that previous reviews still apply.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v11 2/6] qmp: Use ThrottleLimits structure
2017-09-18 16:25 ` Manos Pitsidianakis
@ 2017-09-19 8:07 ` Pradeep Jagadeesh
0 siblings, 0 replies; 25+ messages in thread
From: Pradeep Jagadeesh @ 2017-09-19 8:07 UTC (permalink / raw)
To: Manos Pitsidianakis, Pradeep Jagadeesh, eric blake, greg kurz,
jani kokkonen, alberto garcia, qemu-devel
On 9/18/2017 6:25 PM, Manos Pitsidianakis wrote:
> On Thu, Sep 14, 2017 at 06:40:06AM -0400, Pradeep Jagadeesh wrote:
>> This patch factors out code to use the ThrottleLimits
>> strurcture.
>>
>> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
>> Reviewed-by: Greg Kurz <groug@kaod.org>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> Reviewed-by: Alberto Garcia <berto@igalia.com>
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> qapi/block-core.json | 78
>> +++-------------------------------------------------
>> 1 file changed, 4 insertions(+), 74 deletions(-)
>>
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index bb11815..d0ccfda 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -1826,84 +1826,13 @@
>> #
>> # @device: Block device name (deprecated, use @id instead)
>> #
>> -# @id: The name or QOM path of the guest device (since: 2.8)
>> -#
>> -# @bps: total throughput limit in bytes per second
>> -#
>> -# @bps_rd: read throughput limit in bytes per second
>> -#
>> -# @bps_wr: write throughput limit in bytes per second
>> -#
>> -# @iops: total I/O operations per second
>> -#
>> -# @iops_rd: read I/O operations per second
>> -#
>> -# @iops_wr: write I/O operations per second
>> -#
>> -# @bps_max: total throughput limit during bursts,
>> -# in bytes (Since 1.7)
>> -#
>> -# @bps_rd_max: read throughput limit during bursts,
>> -# in bytes (Since 1.7)
>> -#
>> -# @bps_wr_max: write throughput limit during bursts,
>> -# in bytes (Since 1.7)
>> -#
>> -# @iops_max: total I/O operations per second during bursts,
>> -# in bytes (Since 1.7)
>> -#
>> -# @iops_rd_max: read I/O operations per second during bursts,
>> -# in bytes (Since 1.7)
>> -#
>> -# @iops_wr_max: write I/O operations per second during bursts,
>> -# in bytes (Since 1.7)
>> -#
>> -# @bps_max_length: maximum length of the @bps_max burst
>> -# period, in seconds. It must only
>> -# be set if @bps_max is set as well.
>> -# Defaults to 1. (Since 2.6)
>> -#
>> -# @bps_rd_max_length: maximum length of the @bps_rd_max
>> -# burst period, in seconds. It must only
>> -# be set if @bps_rd_max is set as well.
>> -# Defaults to 1. (Since 2.6)
>> -#
>> -# @bps_wr_max_length: maximum length of the @bps_wr_max
>> -# burst period, in seconds. It must only
>> -# be set if @bps_wr_max is set as well.
>> -# Defaults to 1. (Since 2.6)
>> -#
>> -# @iops_max_length: maximum length of the @iops burst
>> -# period, in seconds. It must only
>> -# be set if @iops_max is set as well.
>> -# Defaults to 1. (Since 2.6)
>> -#
>> -# @iops_rd_max_length: maximum length of the @iops_rd_max
>> -# burst period, in seconds. It must only
>> -# be set if @iops_rd_max is set as well.
>> -# Defaults to 1. (Since 2.6)
>> -#
>> -# @iops_wr_max_length: maximum length of the @iops_wr_max
>> -# burst period, in seconds. It must only
>> -# be set if @iops_wr_max is set as well.
>> -# Defaults to 1. (Since 2.6)
>> -#
>> -# @iops_size: an I/O size in bytes (Since 1.7)
>> -#
>> # @group: throttle group name (Since 2.4)
>> #
>> # Since: 1.1
>> ##
>> { 'struct': 'BlockIOThrottle',
>> - 'data': { '*device': 'str', '*id': 'str', 'bps': 'int', 'bps_rd':
>> 'int',
>> - 'bps_wr': 'int', 'iops': 'int', 'iops_rd': 'int',
>> 'iops_wr': 'int',
>> - '*bps_max': 'int', '*bps_rd_max': 'int',
>> - '*bps_wr_max': 'int', '*iops_max': 'int',
>> - '*iops_rd_max': 'int', '*iops_wr_max': 'int',
>> - '*bps_max_length': 'int', '*bps_rd_max_length': 'int',
>> - '*bps_wr_max_length': 'int', '*iops_max_length': 'int',
>> - '*iops_rd_max_length': 'int', '*iops_wr_max_length': 'int',
>> - '*iops_size': 'int', '*group': 'str' } }
>> + 'base': 'ThrottleLimits',
>> + 'data': { '*device': 'str', '*group': 'str' } }
>>
>> ##
>> # @ThrottleLimits:
>> @@ -1913,6 +1842,7 @@
>> # transaction. All fields are optional. When setting limits, if a
>> field is
>> # missing the current value is not changed.
>> #
>> +# @id: device id
>> # @iops-total: limit total I/O operations per second
>> # @iops-total-max: I/O operations burst
>> # @iops-total-max-length: length of the iops-total-max burst period,
>> in seconds
>> @@ -1942,7 +1872,7 @@
>> # Since: 2.11
>> ##
>> { 'struct': 'ThrottleLimits',
>> - 'data': { '*iops-total' : 'int', '*iops-total-max' : 'int',
>> + 'data': { '*id' : 'str', '*iops-total' : 'int', '*iops-total-max' :
>> 'int',
>> '*iops-total-max-length' : 'int', '*iops-read' : 'int',
>> '*iops-read-max' : 'int', '*iops-read-max-length' : 'int',
>> '*iops-write' : 'int', '*iops-write-max' : 'int',
>> --
>> 1.8.3.1
>>
>
> Why is id needed? ThrottleLimits is not per group, not per device. And
> as I mentioned in another reply, this changes the block_set_io_throttle
> command which is not simple refactoring.
When you use qmp/hmp interface the id is used to set/get throttle
configuration. So, I thought if id is part of the throttle config, it
will be useful. Now I will try to move it to just for fsdeviothrottle.
-Pradeep
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v11 2/6] qmp: Use ThrottleLimits structure
2017-09-18 17:10 ` Eric Blake
@ 2017-09-19 10:06 ` Pradeep Jagadeesh
2017-09-19 13:10 ` Eric Blake
2017-09-19 13:32 ` Greg Kurz
0 siblings, 2 replies; 25+ messages in thread
From: Pradeep Jagadeesh @ 2017-09-19 10:06 UTC (permalink / raw)
To: Eric Blake, Pradeep Jagadeesh, greg kurz
Cc: alberto garcia, jani kokkonen, qemu-devel
On 9/18/2017 7:10 PM, Eric Blake wrote:
> On 09/14/2017 05:40 AM, Pradeep Jagadeesh wrote:
>> This patch factors out code to use the ThrottleLimits
>> strurcture.
>
> s/strurcture/structure/
>
>>
>> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
>> Reviewed-by: Greg Kurz <groug@kaod.org>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>
> Echoing the sentiments made elsewhere - this is a significant rewrite
> from the earlier version, so you WANT reviewers to look at it fresh
> rather than assuming that previous reviews still apply.
>
OK, but shall I send them as V12 or V01 again?
-Pradeep
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v11 2/6] qmp: Use ThrottleLimits structure
2017-09-19 10:06 ` Pradeep Jagadeesh
@ 2017-09-19 13:10 ` Eric Blake
2017-09-19 13:32 ` Greg Kurz
1 sibling, 0 replies; 25+ messages in thread
From: Eric Blake @ 2017-09-19 13:10 UTC (permalink / raw)
To: Pradeep Jagadeesh, Pradeep Jagadeesh, greg kurz
Cc: alberto garcia, jani kokkonen, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 891 bytes --]
On 09/19/2017 05:06 AM, Pradeep Jagadeesh wrote:
> On 9/18/2017 7:10 PM, Eric Blake wrote:
>> On 09/14/2017 05:40 AM, Pradeep Jagadeesh wrote:
>>> This patch factors out code to use the ThrottleLimits
>>> strurcture.
>>
>> s/strurcture/structure/
>>
>>>
>>> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
>>> Reviewed-by: Greg Kurz <groug@kaod.org>
>>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>
>> Echoing the sentiments made elsewhere - this is a significant rewrite
>> from the earlier version, so you WANT reviewers to look at it fresh
>> rather than assuming that previous reviews still apply.
>>
> OK, but shall I send them as V12 or V01 again?
v12 is fine - you really are revising the patches a twelfth time.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v11 2/6] qmp: Use ThrottleLimits structure
2017-09-19 10:06 ` Pradeep Jagadeesh
2017-09-19 13:10 ` Eric Blake
@ 2017-09-19 13:32 ` Greg Kurz
1 sibling, 0 replies; 25+ messages in thread
From: Greg Kurz @ 2017-09-19 13:32 UTC (permalink / raw)
To: Pradeep Jagadeesh
Cc: Eric Blake, Pradeep Jagadeesh, alberto garcia, jani kokkonen, qemu-devel
On Tue, 19 Sep 2017 12:06:26 +0200
Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com> wrote:
> On 9/18/2017 7:10 PM, Eric Blake wrote:
> > On 09/14/2017 05:40 AM, Pradeep Jagadeesh wrote:
> >> This patch factors out code to use the ThrottleLimits
> >> strurcture.
> >
> > s/strurcture/structure/
> >
> >>
> >> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
> >> Reviewed-by: Greg Kurz <groug@kaod.org>
> >> Reviewed-by: Eric Blake <eblake@redhat.com>
> >
> > Echoing the sentiments made elsewhere - this is a significant rewrite
> > from the earlier version, so you WANT reviewers to look at it fresh
> > rather than assuming that previous reviews still apply.
> >
> OK, but shall I send them as V12 or V01 again?
>
The version is rather tied to the whole patchset than to each
individual patch. The next round will be the 12th, so all
patches should have v12 in the Subject.
> -Pradeep
>
--
Gregory Kurz kurzgreg@fr.ibm.com
gkurz@linux.vnet.ibm.com
Software Engineer @ IBM/LTC http://www.ibm.com
Tel 33-5-6218-1607
"Anarchy is about taking complete responsibility for yourself."
Alan Moore.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v11 2/6] qmp: Use ThrottleLimits structure
2017-09-18 16:04 ` Manos Pitsidianakis
@ 2017-09-20 8:46 ` Pradeep Jagadeesh
0 siblings, 0 replies; 25+ messages in thread
From: Pradeep Jagadeesh @ 2017-09-20 8:46 UTC (permalink / raw)
To: Manos Pitsidianakis, Pradeep Jagadeesh, eric blake, greg kurz,
jani kokkonen, alberto garcia, qemu-devel, Markus Armbruster
On 9/18/2017 6:04 PM, Manos Pitsidianakis wrote:
> On Thu, Sep 14, 2017 at 06:40:06AM -0400, Pradeep Jagadeesh wrote:
>> This patch factors out code to use the ThrottleLimits
>> strurcture.
>>
>> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
>> Reviewed-by: Greg Kurz <groug@kaod.org>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> Reviewed-by: Alberto Garcia <berto@igalia.com>
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> qapi/block-core.json | 78
>> +++-------------------------------------------------
>> 1 file changed, 4 insertions(+), 74 deletions(-)
>>
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index bb11815..d0ccfda 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -1826,84 +1826,13 @@
>> #
>> # @device: Block device name (deprecated, use @id instead)
>> #
>> -# @id: The name or QOM path of the guest device (since: 2.8)
>> -#
>> -# @bps: total throughput limit in bytes per second
>> -#
>> -# @bps_rd: read throughput limit in bytes per second
>> -#
>> -# @bps_wr: write throughput limit in bytes per second
>> -#
>> -# @iops: total I/O operations per second
>> -#
>> -# @iops_rd: read I/O operations per second
>> -#
>> -# @iops_wr: write I/O operations per second
>> -#
>> -# @bps_max: total throughput limit during bursts,
>> -# in bytes (Since 1.7)
>> -#
>> -# @bps_rd_max: read throughput limit during bursts,
>> -# in bytes (Since 1.7)
>> -#
>> -# @bps_wr_max: write throughput limit during bursts,
>> -# in bytes (Since 1.7)
>> -#
>> -# @iops_max: total I/O operations per second during bursts,
>> -# in bytes (Since 1.7)
>> -#
>> -# @iops_rd_max: read I/O operations per second during bursts,
>> -# in bytes (Since 1.7)
>> -#
>> -# @iops_wr_max: write I/O operations per second during bursts,
>> -# in bytes (Since 1.7)
>> -#
>> -# @bps_max_length: maximum length of the @bps_max burst
>> -# period, in seconds. It must only
>> -# be set if @bps_max is set as well.
>> -# Defaults to 1. (Since 2.6)
>> -#
>> -# @bps_rd_max_length: maximum length of the @bps_rd_max
>> -# burst period, in seconds. It must only
>> -# be set if @bps_rd_max is set as well.
>> -# Defaults to 1. (Since 2.6)
>> -#
>> -# @bps_wr_max_length: maximum length of the @bps_wr_max
>> -# burst period, in seconds. It must only
>> -# be set if @bps_wr_max is set as well.
>> -# Defaults to 1. (Since 2.6)
>> -#
>> -# @iops_max_length: maximum length of the @iops burst
>> -# period, in seconds. It must only
>> -# be set if @iops_max is set as well.
>> -# Defaults to 1. (Since 2.6)
>> -#
>> -# @iops_rd_max_length: maximum length of the @iops_rd_max
>> -# burst period, in seconds. It must only
>> -# be set if @iops_rd_max is set as well.
>> -# Defaults to 1. (Since 2.6)
>> -#
>> -# @iops_wr_max_length: maximum length of the @iops_wr_max
>> -# burst period, in seconds. It must only
>> -# be set if @iops_wr_max is set as well.
>> -# Defaults to 1. (Since 2.6)
>> -#
>> -# @iops_size: an I/O size in bytes (Since 1.7)
>> -#
>> # @group: throttle group name (Since 2.4)
>
> BlockIOThrottle and ThrottleLimits are not directly compatible, for
> example here we have iops_rd_max_length as a name,
>
>> #
>> # Since: 1.1
>> ##
>> { 'struct': 'BlockIOThrottle',
>> - 'data': { '*device': 'str', '*id': 'str', 'bps': 'int', 'bps_rd':
>> 'int',
>> - 'bps_wr': 'int', 'iops': 'int', 'iops_rd': 'int',
>> 'iops_wr': 'int',
>> - '*bps_max': 'int', '*bps_rd_max': 'int',
>> - '*bps_wr_max': 'int', '*iops_max': 'int',
>> - '*iops_rd_max': 'int', '*iops_wr_max': 'int',
>> - '*bps_max_length': 'int', '*bps_rd_max_length': 'int',
>> - '*bps_wr_max_length': 'int', '*iops_max_length': 'int',
>> - '*iops_rd_max_length': 'int', '*iops_wr_max_length': 'int',
>> - '*iops_size': 'int', '*group': 'str' } }
>> + 'base': 'ThrottleLimits',
>> + 'data': { '*device': 'str', '*group': 'str' } }
>>
>> ##
>> # @ThrottleLimits:
>> @@ -1913,6 +1842,7 @@
>> # transaction. All fields are optional. When setting limits, if a
>> field is
>> # missing the current value is not changed.
>> #
>> +# @id: device id
>> # @iops-total: limit total I/O operations per second
>> # @iops-total-max: I/O operations burst
>> # @iops-total-max-length: length of the iops-total-max burst period,
>> in seconds
>> @@ -1942,7 +1872,7 @@
>> # Since: 2.11
>> ##
>> { 'struct': 'ThrottleLimits',
>> - 'data': { '*iops-total' : 'int', '*iops-total-max' : 'int',
>> + 'data': { '*id' : 'str', '*iops-total' : 'int', '*iops-total-max' :
>> 'int',
>> '*iops-total-max-length' : 'int', '*iops-read' : 'int',
>> '*iops-read-max' : 'int', '*iops-read-max-length' : 'int',
>> '*iops-write' : 'int', '*iops-write-max' : 'int',
>
> And here it's iops-read-max-length. This breaks the block layer's old
> throttling API. Do we want the old API replaced for 2.11? I don't know
> which maintainer is responsible for this, so I CC'd armbru.
I do not think its going to break. It just exist we neither try to set
that or to query.
-Pradeep
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code
2017-09-18 16:20 ` Manos Pitsidianakis
@ 2017-09-20 8:57 ` Pradeep Jagadeesh
2017-09-22 11:31 ` Pradeep Jagadeesh
1 sibling, 0 replies; 25+ messages in thread
From: Pradeep Jagadeesh @ 2017-09-20 8:57 UTC (permalink / raw)
To: Manos Pitsidianakis, Pradeep Jagadeesh, eric blake, greg kurz,
qemu-devel, jani kokkonen, alberto garcia, Markus Armbruster
On 9/18/2017 6:20 PM, Manos Pitsidianakis wrote:
> On Thu, Sep 14, 2017 at 06:40:05AM -0400, Pradeep Jagadeesh wrote:
>> This patch factors out the duplicate throttle code that was still
>> present in block and fsdev devices.
>>
>> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
>> Reviewed-by: Alberto Garcia <berto@igalia.com>
>> Reviewed-by: Greg Kurz <groug@kaod.org>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> ---
>> blockdev.c | 44 +---------------------------------
>> fsdev/qemu-fsdev-throttle.c | 44 ++--------------------------------
>> include/qemu/throttle-options.h | 3 +++
>> include/qemu/throttle.h | 4 ++--
>> include/qemu/typedefs.h | 1 +
>> util/throttle.c | 52
>> +++++++++++++++++++++++++++++++++++++++++
>> 6 files changed, 61 insertions(+), 87 deletions(-)
>>
>> diff --git a/blockdev.c b/blockdev.c
>> index 56a6b24..9d33c25 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -387,49 +387,7 @@ static void
>> extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
>> }
>>
>> if (throttle_cfg) {
>> - throttle_config_init(throttle_cfg);
>> - throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
>> - qemu_opt_get_number(opts, "throttling.bps-total", 0);
>> - throttle_cfg->buckets[THROTTLE_BPS_READ].avg =
>> - qemu_opt_get_number(opts, "throttling.bps-read", 0);
>> - throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
>> - qemu_opt_get_number(opts, "throttling.bps-write", 0);
>> - throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
>> - qemu_opt_get_number(opts, "throttling.iops-total", 0);
>> - throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
>> - qemu_opt_get_number(opts, "throttling.iops-read", 0);
>> - throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
>> - qemu_opt_get_number(opts, "throttling.iops-write", 0);
>> -
>> - throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
>> - qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
>> - throttle_cfg->buckets[THROTTLE_BPS_READ].max =
>> - qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
>> - throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
>> - qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
>> - throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
>> - qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
>> - throttle_cfg->buckets[THROTTLE_OPS_READ].max =
>> - qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
>> - throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
>> - qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
>> -
>> - throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
>> - qemu_opt_get_number(opts,
>> "throttling.bps-total-max-length", 1);
>> - throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =
>> - qemu_opt_get_number(opts,
>> "throttling.bps-read-max-length", 1);
>> - throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
>> - qemu_opt_get_number(opts,
>> "throttling.bps-write-max-length", 1);
>> - throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
>> - qemu_opt_get_number(opts,
>> "throttling.iops-total-max-length", 1);
>> - throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
>> - qemu_opt_get_number(opts,
>> "throttling.iops-read-max-length", 1);
>> - throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
>> - qemu_opt_get_number(opts,
>> "throttling.iops-write-max-length", 1);
>> -
>> - throttle_cfg->op_size =
>> - qemu_opt_get_number(opts, "throttling.iops-size", 0);
>> -
>> + throttle_parse_options(throttle_cfg, opts);
>> if (!throttle_is_valid(throttle_cfg, errp)) {
>> return;
>> }
>> diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
>> index 49eebb5..0e6fb86 100644
>> --- a/fsdev/qemu-fsdev-throttle.c
>> +++ b/fsdev/qemu-fsdev-throttle.c
>> @@ -16,6 +16,7 @@
>> #include "qemu/error-report.h"
>> #include "qemu-fsdev-throttle.h"
>> #include "qemu/iov.h"
>> +#include "qemu/throttle-options.h"
>>
>> static void fsdev_throttle_read_timer_cb(void *opaque)
>> {
>> @@ -31,48 +32,7 @@ static void fsdev_throttle_write_timer_cb(void
>> *opaque)
>>
>> void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error
>> **errp)
>> {
>> - throttle_config_init(&fst->cfg);
>> - fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =
>> - qemu_opt_get_number(opts, "throttling.bps-total", 0);
>> - fst->cfg.buckets[THROTTLE_BPS_READ].avg =
>> - qemu_opt_get_number(opts, "throttling.bps-read", 0);
>> - fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =
>> - qemu_opt_get_number(opts, "throttling.bps-write", 0);
>> - fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =
>> - qemu_opt_get_number(opts, "throttling.iops-total", 0);
>> - fst->cfg.buckets[THROTTLE_OPS_READ].avg =
>> - qemu_opt_get_number(opts, "throttling.iops-read", 0);
>> - fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =
>> - qemu_opt_get_number(opts, "throttling.iops-write", 0);
>> -
>> - fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =
>> - qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
>> - fst->cfg.buckets[THROTTLE_BPS_READ].max =
>> - qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
>> - fst->cfg.buckets[THROTTLE_BPS_WRITE].max =
>> - qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
>> - fst->cfg.buckets[THROTTLE_OPS_TOTAL].max =
>> - qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
>> - fst->cfg.buckets[THROTTLE_OPS_READ].max =
>> - qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
>> - fst->cfg.buckets[THROTTLE_OPS_WRITE].max =
>> - qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
>> -
>> - fst->cfg.buckets[THROTTLE_BPS_TOTAL].burst_length =
>> - qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
>> - fst->cfg.buckets[THROTTLE_BPS_READ].burst_length =
>> - qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
>> - fst->cfg.buckets[THROTTLE_BPS_WRITE].burst_length =
>> - qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
>> - fst->cfg.buckets[THROTTLE_OPS_TOTAL].burst_length =
>> - qemu_opt_get_number(opts, "throttling.iops-total-max-length",
>> 1);
>> - fst->cfg.buckets[THROTTLE_OPS_READ].burst_length =
>> - qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
>> - fst->cfg.buckets[THROTTLE_OPS_WRITE].burst_length =
>> - qemu_opt_get_number(opts, "throttling.iops-write-max-length",
>> 1);
>> - fst->cfg.op_size =
>> - qemu_opt_get_number(opts, "throttling.iops-size", 0);
>> -
>> + throttle_parse_options(&fst->cfg, opts);
>> throttle_is_valid(&fst->cfg, errp);
>> }
>>
>> diff --git a/include/qemu/throttle-options.h
>> b/include/qemu/throttle-options.h
>> index 3528a8f..9709dcd 100644
>> --- a/include/qemu/throttle-options.h
>> +++ b/include/qemu/throttle-options.h
>> @@ -9,6 +9,7 @@
>> */
>> #ifndef THROTTLE_OPTIONS_H
>> #define THROTTLE_OPTIONS_H
>> +#include "typedefs.h"
>>
>> #define QEMU_OPT_IOPS_TOTAL "iops-total"
>> #define QEMU_OPT_IOPS_TOTAL_MAX "iops-total-max"
>> @@ -111,4 +112,6 @@
>> .help = "when limiting by iops max size of an I/O in bytes",\
>> }
>>
>> +void throttle_parse_options(ThrottleConfig *, QemuOpts *);
>> +
>> #endif
>> diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
>> index 8c93237..b6ebc6d 100644
>> --- a/include/qemu/throttle.h
>> +++ b/include/qemu/throttle.h
>> @@ -89,10 +89,10 @@ typedef struct LeakyBucket {
>> * However it allows to keep the code clean and the bucket field is
>> reset to
>> * zero at the right time.
>> */
>> -typedef struct ThrottleConfig {
>> +struct ThrottleConfig {
>> LeakyBucket buckets[BUCKETS_COUNT]; /* leaky buckets */
>> uint64_t op_size; /* size of an operation in bytes */
>> -} ThrottleConfig;
>> +};
>>
>> typedef struct ThrottleState {
>> ThrottleConfig cfg; /* configuration */
>> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
>> index 39bc835..90fe0f9 100644
>> --- a/include/qemu/typedefs.h
>> +++ b/include/qemu/typedefs.h
>> @@ -100,6 +100,7 @@ typedef struct uWireSlave uWireSlave;
>> typedef struct VirtIODevice VirtIODevice;
>> typedef struct Visitor Visitor;
>> typedef struct node_info NodeInfo;
>> +typedef struct ThrottleConfig ThrottleConfig;
>
> Is this really needed? Just include include/qemu/throttle.h wherever you
> need.
>
>> typedef void SaveStateHandler(QEMUFile *f, void *opaque);
>> typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);
>>
>> diff --git a/util/throttle.c b/util/throttle.c
>> index 06bf916..9ef28c4 100644
>> --- a/util/throttle.c
>> +++ b/util/throttle.c
>> @@ -27,6 +27,7 @@
>> #include "qemu/throttle.h"
>> #include "qemu/timer.h"
>> #include "block/aio.h"
>> +#include "qemu/throttle-options.h"
>>
>> /* This function make a bucket leak
>> *
>> @@ -635,3 +636,54 @@ void throttle_config_to_limits(ThrottleConfig
>> *cfg, ThrottleLimits *var)
>> var->has_iops_write_max_length = true;
>> var->has_iops_size = true;
>> }
>> +
>> +/* parse the throttle options
>> + *
>> + * @opts: qemu options
>> + * @throttle_cfg: throttle configuration
>> + */
>> +void throttle_parse_options(ThrottleConfig *throttle_cfg, QemuOpts
>> *opts)
>> +{
>> + throttle_config_init(throttle_cfg);
>> + throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
>> + qemu_opt_get_number(opts, "throttling.bps-total", 0);
>> + throttle_cfg->buckets[THROTTLE_BPS_READ].avg =
>> + qemu_opt_get_number(opts, "throttling.bps-read", 0);
>> + throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
>> + qemu_opt_get_number(opts, "throttling.bps-write", 0);
>> + throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
>> + qemu_opt_get_number(opts, "throttling.iops-total", 0);
>> + throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
>> + qemu_opt_get_number(opts, "throttling.iops-read", 0);
>> + throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
>> + qemu_opt_get_number(opts, "throttling.iops-write", 0);
>> +
>> + throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
>> + qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
>> + throttle_cfg->buckets[THROTTLE_BPS_READ].max =
>> + qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
>> + throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
>> + qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
>> + throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
>> + qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
>> + throttle_cfg->buckets[THROTTLE_OPS_READ].max =
>> + qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
>> + throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
>> + qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
>> +
>> + throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
>> + qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
>> + throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =
>> + qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
>> + throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
>> + qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
>> + throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
>> + qemu_opt_get_number(opts, "throttling.iops-total-max-length",
>> 1);
>> + throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
>> + qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
>> + throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
>> + qemu_opt_get_number(opts, "throttling.iops-write-max-length",
>> 1);
>> +
>> + throttle_cfg->op_size =
>> + qemu_opt_get_number(opts, "throttling.iops-size", 0);
>> +}
>
> You should reuse the #define's in include/qemu/throttle-options.h
> See throttle_extract_options() in this patch:
> http://patchwork.ozlabs.org/patch/803919/ Disregard the UINT_MAX checks,
> unsigned ints in LeakyBucket were replaced by uint64_t.
You mean something like below?
qemu_opt_get_number(opts, QEMU_OPT_IOPS_READ, 0);
instead
qemu_opt_get_number(opts, "throttling.bps-total", 0);
Regards,
Pradeep
> Don't forget to drop the reviews when you change a patch! The original
> reviews will probably not be valid anymore.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code
2017-09-18 16:20 ` Manos Pitsidianakis
2017-09-20 8:57 ` Pradeep Jagadeesh
@ 2017-09-22 11:31 ` Pradeep Jagadeesh
2017-09-23 10:33 ` Manos Pitsidianakis
1 sibling, 1 reply; 25+ messages in thread
From: Pradeep Jagadeesh @ 2017-09-22 11:31 UTC (permalink / raw)
To: Manos Pitsidianakis, Pradeep Jagadeesh, eric blake, greg kurz,
qemu-devel, jani kokkonen, alberto garcia, Markus Armbruster
On 9/18/2017 6:20 PM, Manos Pitsidianakis wrote:
> On Thu, Sep 14, 2017 at 06:40:05AM -0400, Pradeep Jagadeesh wrote:
>> This patch factors out the duplicate throttle code that was still
>> present in block and fsdev devices.
>>
>> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
>> Reviewed-by: Alberto Garcia <berto@igalia.com>
>> Reviewed-by: Greg Kurz <groug@kaod.org>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> ---
>> blockdev.c | 44 +---------------------------------
>> fsdev/qemu-fsdev-throttle.c | 44 ++--------------------------------
>> include/qemu/throttle-options.h | 3 +++
>> include/qemu/throttle.h | 4 ++--
>> include/qemu/typedefs.h | 1 +
>> util/throttle.c | 52
>> +++++++++++++++++++++++++++++++++++++++++
>> 6 files changed, 61 insertions(+), 87 deletions(-)
>>
>> diff --git a/blockdev.c b/blockdev.c
>> index 56a6b24..9d33c25 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -387,49 +387,7 @@ static void
>> extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
>> }
>>
>> if (throttle_cfg) {
>> - throttle_config_init(throttle_cfg);
>> - throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
>> - qemu_opt_get_number(opts, "throttling.bps-total", 0);
>> - throttle_cfg->buckets[THROTTLE_BPS_READ].avg =
>> - qemu_opt_get_number(opts, "throttling.bps-read", 0);
>> - throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
>> - qemu_opt_get_number(opts, "throttling.bps-write", 0);
>> - throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
>> - qemu_opt_get_number(opts, "throttling.iops-total", 0);
>> - throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
>> - qemu_opt_get_number(opts, "throttling.iops-read", 0);
>> - throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
>> - qemu_opt_get_number(opts, "throttling.iops-write", 0);
>> -
>> - throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
>> - qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
>> - throttle_cfg->buckets[THROTTLE_BPS_READ].max =
>> - qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
>> - throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
>> - qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
>> - throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
>> - qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
>> - throttle_cfg->buckets[THROTTLE_OPS_READ].max =
>> - qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
>> - throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
>> - qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
>> -
>> - throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
>> - qemu_opt_get_number(opts,
>> "throttling.bps-total-max-length", 1);
>> - throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =
>> - qemu_opt_get_number(opts,
>> "throttling.bps-read-max-length", 1);
>> - throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
>> - qemu_opt_get_number(opts,
>> "throttling.bps-write-max-length", 1);
>> - throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
>> - qemu_opt_get_number(opts,
>> "throttling.iops-total-max-length", 1);
>> - throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
>> - qemu_opt_get_number(opts,
>> "throttling.iops-read-max-length", 1);
>> - throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
>> - qemu_opt_get_number(opts,
>> "throttling.iops-write-max-length", 1);
>> -
>> - throttle_cfg->op_size =
>> - qemu_opt_get_number(opts, "throttling.iops-size", 0);
>> -
>> + throttle_parse_options(throttle_cfg, opts);
>> if (!throttle_is_valid(throttle_cfg, errp)) {
>> return;
>> }
>> diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
>> index 49eebb5..0e6fb86 100644
>> --- a/fsdev/qemu-fsdev-throttle.c
>> +++ b/fsdev/qemu-fsdev-throttle.c
>> @@ -16,6 +16,7 @@
>> #include "qemu/error-report.h"
>> #include "qemu-fsdev-throttle.h"
>> #include "qemu/iov.h"
>> +#include "qemu/throttle-options.h"
>>
>> static void fsdev_throttle_read_timer_cb(void *opaque)
>> {
>> @@ -31,48 +32,7 @@ static void fsdev_throttle_write_timer_cb(void
>> *opaque)
>>
>> void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error
>> **errp)
>> {
>> - throttle_config_init(&fst->cfg);
>> - fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =
>> - qemu_opt_get_number(opts, "throttling.bps-total", 0);
>> - fst->cfg.buckets[THROTTLE_BPS_READ].avg =
>> - qemu_opt_get_number(opts, "throttling.bps-read", 0);
>> - fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =
>> - qemu_opt_get_number(opts, "throttling.bps-write", 0);
>> - fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =
>> - qemu_opt_get_number(opts, "throttling.iops-total", 0);
>> - fst->cfg.buckets[THROTTLE_OPS_READ].avg =
>> - qemu_opt_get_number(opts, "throttling.iops-read", 0);
>> - fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =
>> - qemu_opt_get_number(opts, "throttling.iops-write", 0);
>> -
>> - fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =
>> - qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
>> - fst->cfg.buckets[THROTTLE_BPS_READ].max =
>> - qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
>> - fst->cfg.buckets[THROTTLE_BPS_WRITE].max =
>> - qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
>> - fst->cfg.buckets[THROTTLE_OPS_TOTAL].max =
>> - qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
>> - fst->cfg.buckets[THROTTLE_OPS_READ].max =
>> - qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
>> - fst->cfg.buckets[THROTTLE_OPS_WRITE].max =
>> - qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
>> -
>> - fst->cfg.buckets[THROTTLE_BPS_TOTAL].burst_length =
>> - qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
>> - fst->cfg.buckets[THROTTLE_BPS_READ].burst_length =
>> - qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
>> - fst->cfg.buckets[THROTTLE_BPS_WRITE].burst_length =
>> - qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
>> - fst->cfg.buckets[THROTTLE_OPS_TOTAL].burst_length =
>> - qemu_opt_get_number(opts, "throttling.iops-total-max-length",
>> 1);
>> - fst->cfg.buckets[THROTTLE_OPS_READ].burst_length =
>> - qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
>> - fst->cfg.buckets[THROTTLE_OPS_WRITE].burst_length =
>> - qemu_opt_get_number(opts, "throttling.iops-write-max-length",
>> 1);
>> - fst->cfg.op_size =
>> - qemu_opt_get_number(opts, "throttling.iops-size", 0);
>> -
>> + throttle_parse_options(&fst->cfg, opts);
>> throttle_is_valid(&fst->cfg, errp);
>> }
>>
>> diff --git a/include/qemu/throttle-options.h
>> b/include/qemu/throttle-options.h
>> index 3528a8f..9709dcd 100644
>> --- a/include/qemu/throttle-options.h
>> +++ b/include/qemu/throttle-options.h
>> @@ -9,6 +9,7 @@
>> */
>> #ifndef THROTTLE_OPTIONS_H
>> #define THROTTLE_OPTIONS_H
>> +#include "typedefs.h"
>>
>> #define QEMU_OPT_IOPS_TOTAL "iops-total"
>> #define QEMU_OPT_IOPS_TOTAL_MAX "iops-total-max"
>> @@ -111,4 +112,6 @@
>> .help = "when limiting by iops max size of an I/O in bytes",\
>> }
>>
>> +void throttle_parse_options(ThrottleConfig *, QemuOpts *);
>> +
>> #endif
>> diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
>> index 8c93237..b6ebc6d 100644
>> --- a/include/qemu/throttle.h
>> +++ b/include/qemu/throttle.h
>> @@ -89,10 +89,10 @@ typedef struct LeakyBucket {
>> * However it allows to keep the code clean and the bucket field is
>> reset to
>> * zero at the right time.
>> */
>> -typedef struct ThrottleConfig {
>> +struct ThrottleConfig {
>> LeakyBucket buckets[BUCKETS_COUNT]; /* leaky buckets */
>> uint64_t op_size; /* size of an operation in bytes */
>> -} ThrottleConfig;
>> +};
>>
>> typedef struct ThrottleState {
>> ThrottleConfig cfg; /* configuration */
>> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
>> index 39bc835..90fe0f9 100644
>> --- a/include/qemu/typedefs.h
>> +++ b/include/qemu/typedefs.h
>> @@ -100,6 +100,7 @@ typedef struct uWireSlave uWireSlave;
>> typedef struct VirtIODevice VirtIODevice;
>> typedef struct Visitor Visitor;
>> typedef struct node_info NodeInfo;
>> +typedef struct ThrottleConfig ThrottleConfig;
>
> Is this really needed? Just include include/qemu/throttle.h wherever you
> need.
>
>> typedef void SaveStateHandler(QEMUFile *f, void *opaque);
>> typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);
>>
>> diff --git a/util/throttle.c b/util/throttle.c
>> index 06bf916..9ef28c4 100644
>> --- a/util/throttle.c
>> +++ b/util/throttle.c
>> @@ -27,6 +27,7 @@
>> #include "qemu/throttle.h"
>> #include "qemu/timer.h"
>> #include "block/aio.h"
>> +#include "qemu/throttle-options.h"
>>
>> /* This function make a bucket leak
>> *
>> @@ -635,3 +636,54 @@ void throttle_config_to_limits(ThrottleConfig
>> *cfg, ThrottleLimits *var)
>> var->has_iops_write_max_length = true;
>> var->has_iops_size = true;
>> }
>> +
>> +/* parse the throttle options
>> + *
>> + * @opts: qemu options
>> + * @throttle_cfg: throttle configuration
>> + */
>> +void throttle_parse_options(ThrottleConfig *throttle_cfg, QemuOpts
>> *opts)
>> +{
>> + throttle_config_init(throttle_cfg);
>> + throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
>> + qemu_opt_get_number(opts, "throttling.bps-total", 0);
>> + throttle_cfg->buckets[THROTTLE_BPS_READ].avg =
>> + qemu_opt_get_number(opts, "throttling.bps-read", 0);
>> + throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
>> + qemu_opt_get_number(opts, "throttling.bps-write", 0);
>> + throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
>> + qemu_opt_get_number(opts, "throttling.iops-total", 0);
>> + throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
>> + qemu_opt_get_number(opts, "throttling.iops-read", 0);
>> + throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
>> + qemu_opt_get_number(opts, "throttling.iops-write", 0);
>> +
>> + throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
>> + qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
>> + throttle_cfg->buckets[THROTTLE_BPS_READ].max =
>> + qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
>> + throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
>> + qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
>> + throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
>> + qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
>> + throttle_cfg->buckets[THROTTLE_OPS_READ].max =
>> + qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
>> + throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
>> + qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
>> +
>> + throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
>> + qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
>> + throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =
>> + qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
>> + throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
>> + qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
>> + throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
>> + qemu_opt_get_number(opts, "throttling.iops-total-max-length",
>> 1);
>> + throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
>> + qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
>> + throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
>> + qemu_opt_get_number(opts, "throttling.iops-write-max-length",
>> 1);
>> +
>> + throttle_cfg->op_size =
>> + qemu_opt_get_number(opts, "throttling.iops-size", 0);
>> +}
>
> You should reuse the #define's in include/qemu/throttle-options.h
> See throttle_extract_options() in this patch:
> http://patchwork.ozlabs.org/patch/803919/ Disregard the UINT_MAX checks,
> unsigned ints in LeakyBucket were replaced by uint64_t.
>
I can not use the #define's because I use throttling.*.
In my last patch also we wanted to have it like that to align with the
block device throttle options.
If you see in blockdev.c, still there exists throttling.*
There may be change required every where, so would like to do it as part
of another patch.
-Pradeep
> Don't forget to drop the reviews when you change a patch! The original
> reviews will probably not be valid anymore.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code
2017-09-22 11:31 ` Pradeep Jagadeesh
@ 2017-09-23 10:33 ` Manos Pitsidianakis
2017-09-25 7:47 ` Pradeep Jagadeesh
0 siblings, 1 reply; 25+ messages in thread
From: Manos Pitsidianakis @ 2017-09-23 10:33 UTC (permalink / raw)
To: Pradeep Jagadeesh
Cc: Pradeep Jagadeesh, eric blake, greg kurz, qemu-devel,
jani kokkonen, alberto garcia, Markus Armbruster
[-- Attachment #1: Type: text/plain, Size: 13332 bytes --]
On Fri, Sep 22, 2017 at 01:31:58PM +0200, Pradeep Jagadeesh wrote:
>On 9/18/2017 6:20 PM, Manos Pitsidianakis wrote:
>>On Thu, Sep 14, 2017 at 06:40:05AM -0400, Pradeep Jagadeesh wrote:
>>>This patch factors out the duplicate throttle code that was still
>>>present in block and fsdev devices.
>>>
>>>Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
>>>Reviewed-by: Alberto Garcia <berto@igalia.com>
>>>Reviewed-by: Greg Kurz <groug@kaod.org>
>>>Reviewed-by: Eric Blake <eblake@redhat.com>
>>>---
>>>blockdev.c | 44 +---------------------------------
>>>fsdev/qemu-fsdev-throttle.c | 44 ++--------------------------------
>>>include/qemu/throttle-options.h | 3 +++
>>>include/qemu/throttle.h | 4 ++--
>>>include/qemu/typedefs.h | 1 +
>>>util/throttle.c | 52
>>>+++++++++++++++++++++++++++++++++++++++++
>>>6 files changed, 61 insertions(+), 87 deletions(-)
>>>
>>>diff --git a/blockdev.c b/blockdev.c
>>>index 56a6b24..9d33c25 100644
>>>--- a/blockdev.c
>>>+++ b/blockdev.c
>>>@@ -387,49 +387,7 @@ static void
>>>extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
>>> }
>>>
>>> if (throttle_cfg) {
>>>- throttle_config_init(throttle_cfg);
>>>- throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
>>>- qemu_opt_get_number(opts, "throttling.bps-total", 0);
>>>- throttle_cfg->buckets[THROTTLE_BPS_READ].avg =
>>>- qemu_opt_get_number(opts, "throttling.bps-read", 0);
>>>- throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
>>>- qemu_opt_get_number(opts, "throttling.bps-write", 0);
>>>- throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
>>>- qemu_opt_get_number(opts, "throttling.iops-total", 0);
>>>- throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
>>>- qemu_opt_get_number(opts, "throttling.iops-read", 0);
>>>- throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
>>>- qemu_opt_get_number(opts, "throttling.iops-write", 0);
>>>-
>>>- throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
>>>- qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
>>>- throttle_cfg->buckets[THROTTLE_BPS_READ].max =
>>>- qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
>>>- throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
>>>- qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
>>>- throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
>>>- qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
>>>- throttle_cfg->buckets[THROTTLE_OPS_READ].max =
>>>- qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
>>>- throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
>>>- qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
>>>-
>>>- throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
>>>- qemu_opt_get_number(opts,
>>>"throttling.bps-total-max-length", 1);
>>>- throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =
>>>- qemu_opt_get_number(opts,
>>>"throttling.bps-read-max-length", 1);
>>>- throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
>>>- qemu_opt_get_number(opts,
>>>"throttling.bps-write-max-length", 1);
>>>- throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
>>>- qemu_opt_get_number(opts,
>>>"throttling.iops-total-max-length", 1);
>>>- throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
>>>- qemu_opt_get_number(opts,
>>>"throttling.iops-read-max-length", 1);
>>>- throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
>>>- qemu_opt_get_number(opts,
>>>"throttling.iops-write-max-length", 1);
>>>-
>>>- throttle_cfg->op_size =
>>>- qemu_opt_get_number(opts, "throttling.iops-size", 0);
>>>-
>>>+ throttle_parse_options(throttle_cfg, opts);
>>> if (!throttle_is_valid(throttle_cfg, errp)) {
>>> return;
>>> }
>>>diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
>>>index 49eebb5..0e6fb86 100644
>>>--- a/fsdev/qemu-fsdev-throttle.c
>>>+++ b/fsdev/qemu-fsdev-throttle.c
>>>@@ -16,6 +16,7 @@
>>>#include "qemu/error-report.h"
>>>#include "qemu-fsdev-throttle.h"
>>>#include "qemu/iov.h"
>>>+#include "qemu/throttle-options.h"
>>>
>>>static void fsdev_throttle_read_timer_cb(void *opaque)
>>>{
>>>@@ -31,48 +32,7 @@ static void fsdev_throttle_write_timer_cb(void
>>>*opaque)
>>>
>>>void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error
>>>**errp)
>>>{
>>>- throttle_config_init(&fst->cfg);
>>>- fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =
>>>- qemu_opt_get_number(opts, "throttling.bps-total", 0);
>>>- fst->cfg.buckets[THROTTLE_BPS_READ].avg =
>>>- qemu_opt_get_number(opts, "throttling.bps-read", 0);
>>>- fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =
>>>- qemu_opt_get_number(opts, "throttling.bps-write", 0);
>>>- fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =
>>>- qemu_opt_get_number(opts, "throttling.iops-total", 0);
>>>- fst->cfg.buckets[THROTTLE_OPS_READ].avg =
>>>- qemu_opt_get_number(opts, "throttling.iops-read", 0);
>>>- fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =
>>>- qemu_opt_get_number(opts, "throttling.iops-write", 0);
>>>-
>>>- fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =
>>>- qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
>>>- fst->cfg.buckets[THROTTLE_BPS_READ].max =
>>>- qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
>>>- fst->cfg.buckets[THROTTLE_BPS_WRITE].max =
>>>- qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
>>>- fst->cfg.buckets[THROTTLE_OPS_TOTAL].max =
>>>- qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
>>>- fst->cfg.buckets[THROTTLE_OPS_READ].max =
>>>- qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
>>>- fst->cfg.buckets[THROTTLE_OPS_WRITE].max =
>>>- qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
>>>-
>>>- fst->cfg.buckets[THROTTLE_BPS_TOTAL].burst_length =
>>>- qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
>>>- fst->cfg.buckets[THROTTLE_BPS_READ].burst_length =
>>>- qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
>>>- fst->cfg.buckets[THROTTLE_BPS_WRITE].burst_length =
>>>- qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
>>>- fst->cfg.buckets[THROTTLE_OPS_TOTAL].burst_length =
>>>- qemu_opt_get_number(opts, "throttling.iops-total-max-length",
>>>1);
>>>- fst->cfg.buckets[THROTTLE_OPS_READ].burst_length =
>>>- qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
>>>- fst->cfg.buckets[THROTTLE_OPS_WRITE].burst_length =
>>>- qemu_opt_get_number(opts, "throttling.iops-write-max-length",
>>>1);
>>>- fst->cfg.op_size =
>>>- qemu_opt_get_number(opts, "throttling.iops-size", 0);
>>>-
>>>+ throttle_parse_options(&fst->cfg, opts);
>>> throttle_is_valid(&fst->cfg, errp);
>>>}
>>>
>>>diff --git a/include/qemu/throttle-options.h
>>>b/include/qemu/throttle-options.h
>>>index 3528a8f..9709dcd 100644
>>>--- a/include/qemu/throttle-options.h
>>>+++ b/include/qemu/throttle-options.h
>>>@@ -9,6 +9,7 @@
>>> */
>>>#ifndef THROTTLE_OPTIONS_H
>>>#define THROTTLE_OPTIONS_H
>>>+#include "typedefs.h"
>>>
>>>#define QEMU_OPT_IOPS_TOTAL "iops-total"
>>>#define QEMU_OPT_IOPS_TOTAL_MAX "iops-total-max"
>>>@@ -111,4 +112,6 @@
>>> .help = "when limiting by iops max size of an I/O in bytes",\
>>> }
>>>
>>>+void throttle_parse_options(ThrottleConfig *, QemuOpts *);
>>>+
>>>#endif
>>>diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
>>>index 8c93237..b6ebc6d 100644
>>>--- a/include/qemu/throttle.h
>>>+++ b/include/qemu/throttle.h
>>>@@ -89,10 +89,10 @@ typedef struct LeakyBucket {
>>> * However it allows to keep the code clean and the bucket field is
>>>reset to
>>> * zero at the right time.
>>> */
>>>-typedef struct ThrottleConfig {
>>>+struct ThrottleConfig {
>>> LeakyBucket buckets[BUCKETS_COUNT]; /* leaky buckets */
>>> uint64_t op_size; /* size of an operation in bytes */
>>>-} ThrottleConfig;
>>>+};
>>>
>>>typedef struct ThrottleState {
>>> ThrottleConfig cfg; /* configuration */
>>>diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
>>>index 39bc835..90fe0f9 100644
>>>--- a/include/qemu/typedefs.h
>>>+++ b/include/qemu/typedefs.h
>>>@@ -100,6 +100,7 @@ typedef struct uWireSlave uWireSlave;
>>>typedef struct VirtIODevice VirtIODevice;
>>>typedef struct Visitor Visitor;
>>>typedef struct node_info NodeInfo;
>>>+typedef struct ThrottleConfig ThrottleConfig;
>>
>>Is this really needed? Just include include/qemu/throttle.h wherever you
>>need.
>>
>>>typedef void SaveStateHandler(QEMUFile *f, void *opaque);
>>>typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);
>>>
>>>diff --git a/util/throttle.c b/util/throttle.c
>>>index 06bf916..9ef28c4 100644
>>>--- a/util/throttle.c
>>>+++ b/util/throttle.c
>>>@@ -27,6 +27,7 @@
>>>#include "qemu/throttle.h"
>>>#include "qemu/timer.h"
>>>#include "block/aio.h"
>>>+#include "qemu/throttle-options.h"
>>>
>>>/* This function make a bucket leak
>>> *
>>>@@ -635,3 +636,54 @@ void throttle_config_to_limits(ThrottleConfig
>>>*cfg, ThrottleLimits *var)
>>> var->has_iops_write_max_length = true;
>>> var->has_iops_size = true;
>>>}
>>>+
>>>+/* parse the throttle options
>>>+ *
>>>+ * @opts: qemu options
>>>+ * @throttle_cfg: throttle configuration
>>>+ */
>>>+void throttle_parse_options(ThrottleConfig *throttle_cfg, QemuOpts
>>>*opts)
>>>+{
>>>+ throttle_config_init(throttle_cfg);
>>>+ throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
>>>+ qemu_opt_get_number(opts, "throttling.bps-total", 0);
>>>+ throttle_cfg->buckets[THROTTLE_BPS_READ].avg =
>>>+ qemu_opt_get_number(opts, "throttling.bps-read", 0);
>>>+ throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
>>>+ qemu_opt_get_number(opts, "throttling.bps-write", 0);
>>>+ throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
>>>+ qemu_opt_get_number(opts, "throttling.iops-total", 0);
>>>+ throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
>>>+ qemu_opt_get_number(opts, "throttling.iops-read", 0);
>>>+ throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
>>>+ qemu_opt_get_number(opts, "throttling.iops-write", 0);
>>>+
>>>+ throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
>>>+ qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
>>>+ throttle_cfg->buckets[THROTTLE_BPS_READ].max =
>>>+ qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
>>>+ throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
>>>+ qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
>>>+ throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
>>>+ qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
>>>+ throttle_cfg->buckets[THROTTLE_OPS_READ].max =
>>>+ qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
>>>+ throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
>>>+ qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
>>>+
>>>+ throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
>>>+ qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
>>>+ throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =
>>>+ qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
>>>+ throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
>>>+ qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
>>>+ throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
>>>+ qemu_opt_get_number(opts, "throttling.iops-total-max-length",
>>>1);
>>>+ throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
>>>+ qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
>>>+ throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
>>>+ qemu_opt_get_number(opts, "throttling.iops-write-max-length",
>>>1);
>>>+
>>>+ throttle_cfg->op_size =
>>>+ qemu_opt_get_number(opts, "throttling.iops-size", 0);
>>>+}
>>
>>You should reuse the #define's in include/qemu/throttle-options.h
>>See throttle_extract_options() in this patch:
>>http://patchwork.ozlabs.org/patch/803919/ Disregard the UINT_MAX checks,
>>unsigned ints in LeakyBucket were replaced by uint64_t.
>>
>I can not use the #define's because I use throttling.*.
>In my last patch also we wanted to have it like that to align with the
>block device throttle options.
>If you see in blockdev.c, still there exists throttling.*
>
You can use them.
qemu_opt_get_number(opts, "throttling.iops-size", 0) becomes:
qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_SIZE, 0)
This is what I did in the patch I linked, except for redefining
THROTTLE_OPT_PREFIX to "limits.". I plan to send some patches for
blockdev.c code when the old legacy interface is replaced.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code
2017-09-23 10:33 ` Manos Pitsidianakis
@ 2017-09-25 7:47 ` Pradeep Jagadeesh
0 siblings, 0 replies; 25+ messages in thread
From: Pradeep Jagadeesh @ 2017-09-25 7:47 UTC (permalink / raw)
To: Manos Pitsidianakis, Pradeep Jagadeesh, eric blake, greg kurz,
qemu-devel, jani kokkonen, alberto garcia, Markus Armbruster
On 9/23/2017 12:33 PM, Manos Pitsidianakis wrote:
> On Fri, Sep 22, 2017 at 01:31:58PM +0200, Pradeep Jagadeesh wrote:
>> On 9/18/2017 6:20 PM, Manos Pitsidianakis wrote:
>>> On Thu, Sep 14, 2017 at 06:40:05AM -0400, Pradeep Jagadeesh wrote:
>>>> This patch factors out the duplicate throttle code that was still
>>>> present in block and fsdev devices.
>>>>
>>>> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
>>>> Reviewed-by: Alberto Garcia <berto@igalia.com>
>>>> Reviewed-by: Greg Kurz <groug@kaod.org>
>>>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>>> ---
>>>> blockdev.c | 44 +---------------------------------
>>>> fsdev/qemu-fsdev-throttle.c | 44 ++--------------------------------
>>>> include/qemu/throttle-options.h | 3 +++
>>>> include/qemu/throttle.h | 4 ++--
>>>> include/qemu/typedefs.h | 1 +
>>>> util/throttle.c | 52
>>>> +++++++++++++++++++++++++++++++++++++++++
>>>> 6 files changed, 61 insertions(+), 87 deletions(-)
>>>>
>>>> diff --git a/blockdev.c b/blockdev.c
>>>> index 56a6b24..9d33c25 100644
>>>> --- a/blockdev.c
>>>> +++ b/blockdev.c
>>>> @@ -387,49 +387,7 @@ static void
>>>> extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
>>>> }
>>>>
>>>> if (throttle_cfg) {
>>>> - throttle_config_init(throttle_cfg);
>>>> - throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
>>>> - qemu_opt_get_number(opts, "throttling.bps-total", 0);
>>>> - throttle_cfg->buckets[THROTTLE_BPS_READ].avg =
>>>> - qemu_opt_get_number(opts, "throttling.bps-read", 0);
>>>> - throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
>>>> - qemu_opt_get_number(opts, "throttling.bps-write", 0);
>>>> - throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
>>>> - qemu_opt_get_number(opts, "throttling.iops-total", 0);
>>>> - throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
>>>> - qemu_opt_get_number(opts, "throttling.iops-read", 0);
>>>> - throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
>>>> - qemu_opt_get_number(opts, "throttling.iops-write", 0);
>>>> -
>>>> - throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
>>>> - qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
>>>> - throttle_cfg->buckets[THROTTLE_BPS_READ].max =
>>>> - qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
>>>> - throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
>>>> - qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
>>>> - throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
>>>> - qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
>>>> - throttle_cfg->buckets[THROTTLE_OPS_READ].max =
>>>> - qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
>>>> - throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
>>>> - qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
>>>> -
>>>> - throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
>>>> - qemu_opt_get_number(opts,
>>>> "throttling.bps-total-max-length", 1);
>>>> - throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =
>>>> - qemu_opt_get_number(opts,
>>>> "throttling.bps-read-max-length", 1);
>>>> - throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
>>>> - qemu_opt_get_number(opts,
>>>> "throttling.bps-write-max-length", 1);
>>>> - throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
>>>> - qemu_opt_get_number(opts,
>>>> "throttling.iops-total-max-length", 1);
>>>> - throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
>>>> - qemu_opt_get_number(opts,
>>>> "throttling.iops-read-max-length", 1);
>>>> - throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
>>>> - qemu_opt_get_number(opts,
>>>> "throttling.iops-write-max-length", 1);
>>>> -
>>>> - throttle_cfg->op_size =
>>>> - qemu_opt_get_number(opts, "throttling.iops-size", 0);
>>>> -
>>>> + throttle_parse_options(throttle_cfg, opts);
>>>> if (!throttle_is_valid(throttle_cfg, errp)) {
>>>> return;
>>>> }
>>>> diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
>>>> index 49eebb5..0e6fb86 100644
>>>> --- a/fsdev/qemu-fsdev-throttle.c
>>>> +++ b/fsdev/qemu-fsdev-throttle.c
>>>> @@ -16,6 +16,7 @@
>>>> #include "qemu/error-report.h"
>>>> #include "qemu-fsdev-throttle.h"
>>>> #include "qemu/iov.h"
>>>> +#include "qemu/throttle-options.h"
>>>>
>>>> static void fsdev_throttle_read_timer_cb(void *opaque)
>>>> {
>>>> @@ -31,48 +32,7 @@ static void fsdev_throttle_write_timer_cb(void
>>>> *opaque)
>>>>
>>>> void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error
>>>> **errp)
>>>> {
>>>> - throttle_config_init(&fst->cfg);
>>>> - fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =
>>>> - qemu_opt_get_number(opts, "throttling.bps-total", 0);
>>>> - fst->cfg.buckets[THROTTLE_BPS_READ].avg =
>>>> - qemu_opt_get_number(opts, "throttling.bps-read", 0);
>>>> - fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =
>>>> - qemu_opt_get_number(opts, "throttling.bps-write", 0);
>>>> - fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =
>>>> - qemu_opt_get_number(opts, "throttling.iops-total", 0);
>>>> - fst->cfg.buckets[THROTTLE_OPS_READ].avg =
>>>> - qemu_opt_get_number(opts, "throttling.iops-read", 0);
>>>> - fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =
>>>> - qemu_opt_get_number(opts, "throttling.iops-write", 0);
>>>> -
>>>> - fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =
>>>> - qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
>>>> - fst->cfg.buckets[THROTTLE_BPS_READ].max =
>>>> - qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
>>>> - fst->cfg.buckets[THROTTLE_BPS_WRITE].max =
>>>> - qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
>>>> - fst->cfg.buckets[THROTTLE_OPS_TOTAL].max =
>>>> - qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
>>>> - fst->cfg.buckets[THROTTLE_OPS_READ].max =
>>>> - qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
>>>> - fst->cfg.buckets[THROTTLE_OPS_WRITE].max =
>>>> - qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
>>>> -
>>>> - fst->cfg.buckets[THROTTLE_BPS_TOTAL].burst_length =
>>>> - qemu_opt_get_number(opts,
>>>> "throttling.bps-total-max-length", 1);
>>>> - fst->cfg.buckets[THROTTLE_BPS_READ].burst_length =
>>>> - qemu_opt_get_number(opts, "throttling.bps-read-max-length",
>>>> 1);
>>>> - fst->cfg.buckets[THROTTLE_BPS_WRITE].burst_length =
>>>> - qemu_opt_get_number(opts,
>>>> "throttling.bps-write-max-length", 1);
>>>> - fst->cfg.buckets[THROTTLE_OPS_TOTAL].burst_length =
>>>> - qemu_opt_get_number(opts, "throttling.iops-total-max-length",
>>>> 1);
>>>> - fst->cfg.buckets[THROTTLE_OPS_READ].burst_length =
>>>> - qemu_opt_get_number(opts,
>>>> "throttling.iops-read-max-length", 1);
>>>> - fst->cfg.buckets[THROTTLE_OPS_WRITE].burst_length =
>>>> - qemu_opt_get_number(opts, "throttling.iops-write-max-length",
>>>> 1);
>>>> - fst->cfg.op_size =
>>>> - qemu_opt_get_number(opts, "throttling.iops-size", 0);
>>>> -
>>>> + throttle_parse_options(&fst->cfg, opts);
>>>> throttle_is_valid(&fst->cfg, errp);
>>>> }
>>>>
>>>> diff --git a/include/qemu/throttle-options.h
>>>> b/include/qemu/throttle-options.h
>>>> index 3528a8f..9709dcd 100644
>>>> --- a/include/qemu/throttle-options.h
>>>> +++ b/include/qemu/throttle-options.h
>>>> @@ -9,6 +9,7 @@
>>>> */
>>>> #ifndef THROTTLE_OPTIONS_H
>>>> #define THROTTLE_OPTIONS_H
>>>> +#include "typedefs.h"
>>>>
>>>> #define QEMU_OPT_IOPS_TOTAL "iops-total"
>>>> #define QEMU_OPT_IOPS_TOTAL_MAX "iops-total-max"
>>>> @@ -111,4 +112,6 @@
>>>> .help = "when limiting by iops max size of an I/O in
>>>> bytes",\
>>>> }
>>>>
>>>> +void throttle_parse_options(ThrottleConfig *, QemuOpts *);
>>>> +
>>>> #endif
>>>> diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
>>>> index 8c93237..b6ebc6d 100644
>>>> --- a/include/qemu/throttle.h
>>>> +++ b/include/qemu/throttle.h
>>>> @@ -89,10 +89,10 @@ typedef struct LeakyBucket {
>>>> * However it allows to keep the code clean and the bucket field is
>>>> reset to
>>>> * zero at the right time.
>>>> */
>>>> -typedef struct ThrottleConfig {
>>>> +struct ThrottleConfig {
>>>> LeakyBucket buckets[BUCKETS_COUNT]; /* leaky buckets */
>>>> uint64_t op_size; /* size of an operation in bytes */
>>>> -} ThrottleConfig;
>>>> +};
>>>>
>>>> typedef struct ThrottleState {
>>>> ThrottleConfig cfg; /* configuration */
>>>> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
>>>> index 39bc835..90fe0f9 100644
>>>> --- a/include/qemu/typedefs.h
>>>> +++ b/include/qemu/typedefs.h
>>>> @@ -100,6 +100,7 @@ typedef struct uWireSlave uWireSlave;
>>>> typedef struct VirtIODevice VirtIODevice;
>>>> typedef struct Visitor Visitor;
>>>> typedef struct node_info NodeInfo;
>>>> +typedef struct ThrottleConfig ThrottleConfig;
>>>
>>> Is this really needed? Just include include/qemu/throttle.h wherever you
>>> need.
>>>
>>>> typedef void SaveStateHandler(QEMUFile *f, void *opaque);
>>>> typedef int LoadStateHandler(QEMUFile *f, void *opaque, int
>>>> version_id);
>>>>
>>>> diff --git a/util/throttle.c b/util/throttle.c
>>>> index 06bf916..9ef28c4 100644
>>>> --- a/util/throttle.c
>>>> +++ b/util/throttle.c
>>>> @@ -27,6 +27,7 @@
>>>> #include "qemu/throttle.h"
>>>> #include "qemu/timer.h"
>>>> #include "block/aio.h"
>>>> +#include "qemu/throttle-options.h"
>>>>
>>>> /* This function make a bucket leak
>>>> *
>>>> @@ -635,3 +636,54 @@ void throttle_config_to_limits(ThrottleConfig
>>>> *cfg, ThrottleLimits *var)
>>>> var->has_iops_write_max_length = true;
>>>> var->has_iops_size = true;
>>>> }
>>>> +
>>>> +/* parse the throttle options
>>>> + *
>>>> + * @opts: qemu options
>>>> + * @throttle_cfg: throttle configuration
>>>> + */
>>>> +void throttle_parse_options(ThrottleConfig *throttle_cfg, QemuOpts
>>>> *opts)
>>>> +{
>>>> + throttle_config_init(throttle_cfg);
>>>> + throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
>>>> + qemu_opt_get_number(opts, "throttling.bps-total", 0);
>>>> + throttle_cfg->buckets[THROTTLE_BPS_READ].avg =
>>>> + qemu_opt_get_number(opts, "throttling.bps-read", 0);
>>>> + throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
>>>> + qemu_opt_get_number(opts, "throttling.bps-write", 0);
>>>> + throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
>>>> + qemu_opt_get_number(opts, "throttling.iops-total", 0);
>>>> + throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
>>>> + qemu_opt_get_number(opts, "throttling.iops-read", 0);
>>>> + throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
>>>> + qemu_opt_get_number(opts, "throttling.iops-write", 0);
>>>> +
>>>> + throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
>>>> + qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
>>>> + throttle_cfg->buckets[THROTTLE_BPS_READ].max =
>>>> + qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
>>>> + throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
>>>> + qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
>>>> + throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
>>>> + qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
>>>> + throttle_cfg->buckets[THROTTLE_OPS_READ].max =
>>>> + qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
>>>> + throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
>>>> + qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
>>>> +
>>>> + throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
>>>> + qemu_opt_get_number(opts,
>>>> "throttling.bps-total-max-length", 1);
>>>> + throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =
>>>> + qemu_opt_get_number(opts, "throttling.bps-read-max-length",
>>>> 1);
>>>> + throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
>>>> + qemu_opt_get_number(opts,
>>>> "throttling.bps-write-max-length", 1);
>>>> + throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
>>>> + qemu_opt_get_number(opts, "throttling.iops-total-max-length",
>>>> 1);
>>>> + throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
>>>> + qemu_opt_get_number(opts,
>>>> "throttling.iops-read-max-length", 1);
>>>> + throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
>>>> + qemu_opt_get_number(opts, "throttling.iops-write-max-length",
>>>> 1);
>>>> +
>>>> + throttle_cfg->op_size =
>>>> + qemu_opt_get_number(opts, "throttling.iops-size", 0);
>>>> +}
>>>
>>> You should reuse the #define's in include/qemu/throttle-options.h
>>> See throttle_extract_options() in this patch:
>>> http://patchwork.ozlabs.org/patch/803919/ Disregard the UINT_MAX checks,
>>> unsigned ints in LeakyBucket were replaced by uint64_t.
>>>
>> I can not use the #define's because I use throttling.*.
>> In my last patch also we wanted to have it like that to align with the
>> block device throttle options.
>> If you see in blockdev.c, still there exists throttling.*
>>
>
> You can use them.
>
> qemu_opt_get_number(opts, "throttling.iops-size", 0) becomes:
> qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_SIZE, 0)
> This is what I did in the patch I linked, except for redefining
> THROTTLE_OPT_PREFIX to "limits.". I plan to send some patches for
> blockdev.c code when the old legacy interface is replaced.
OK, I can do sendout a patch for blockdev.c also.
Regards,
Pradeep
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2017-09-25 7:49 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-14 10:40 [Qemu-devel] [PATCH v11 0/6] fsdev: qmp interface for io throttling Pradeep Jagadeesh
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code Pradeep Jagadeesh
2017-09-14 11:12 ` Greg Kurz
2017-09-18 16:20 ` Manos Pitsidianakis
2017-09-20 8:57 ` Pradeep Jagadeesh
2017-09-22 11:31 ` Pradeep Jagadeesh
2017-09-23 10:33 ` Manos Pitsidianakis
2017-09-25 7:47 ` Pradeep Jagadeesh
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 2/6] qmp: Use ThrottleLimits structure Pradeep Jagadeesh
2017-09-14 11:19 ` Greg Kurz
2017-09-14 11:23 ` Pradeep Jagadeesh
2017-09-18 16:04 ` Manos Pitsidianakis
2017-09-20 8:46 ` Pradeep Jagadeesh
2017-09-18 16:25 ` Manos Pitsidianakis
2017-09-19 8:07 ` Pradeep Jagadeesh
2017-09-18 17:10 ` Eric Blake
2017-09-19 10:06 ` Pradeep Jagadeesh
2017-09-19 13:10 ` Eric Blake
2017-09-19 13:32 ` Greg Kurz
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 3/6] qmp: factor out throttle code to reuse code Pradeep Jagadeesh
2017-09-14 11:22 ` Greg Kurz
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 4/6] hmp: create a throttle initialization function for code reuse Pradeep Jagadeesh
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 5/6] fsdev: QMP interface for throttling Pradeep Jagadeesh
2017-09-14 10:40 ` [Qemu-devel] [PATCH v11 6/6] fsdev: hmp " Pradeep Jagadeesh
2017-09-14 12:05 ` Alberto Garcia
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.