All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v5 0/5] fsdev: qmp interface for io throttling
@ 2017-06-19 13:11 Pradeep Jagadeesh
  2017-06-19 13:11 ` [Qemu-devel] [PATCH v5 1/5] throttle: factor out duplicate code Pradeep Jagadeesh
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Pradeep Jagadeesh @ 2017-06-19 13:11 UTC (permalink / raw)
  To: eric blake, greg kurz
  Cc: Pradeep Jagadeesh, alberto garcia, 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 (5):
  throttle: factor out duplicate code
  qmp: Create IOThrottle structure
  qmp: refactor duplicate code
  fsdev: hmp interface for throttling
  fsdev: QMP interface for throttling

 Makefile                        |   3 +
 blockdev.c                      |  97 ++-------------------------------
 fsdev/qemu-fsdev-dummy.c        |  10 ++++
 fsdev/qemu-fsdev-throttle.c     | 118 ++++++++++++++++++++++++++--------------
 fsdev/qemu-fsdev-throttle.h     |  13 +++++
 fsdev/qemu-fsdev.c              |  37 +++++++++++++
 hmp-commands-info.hx            |  18 ++++++
 hmp-commands.hx                 |  19 +++++++
 hmp.c                           |  87 +++++++++++++++++++++++++++--
 hmp.h                           |   4 ++
 include/qemu/throttle-options.h |   7 +++
 monitor.c                       |   5 ++
 qapi-schema.json                |   3 +
 qapi/block-core.json            |  76 +-------------------------
 qapi/fsdev.json                 |  84 ++++++++++++++++++++++++++++
 qapi/iothrottle.json            |  88 ++++++++++++++++++++++++++++++
 qmp.c                           |  15 +++++
 util/throttle.c                 | 110 +++++++++++++++++++++++++++++++++++++
 18 files changed, 580 insertions(+), 214 deletions(-)
 create mode 100644 qapi/fsdev.json
 create mode 100644 qapi/iothrottle.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

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v5 1/5] throttle: factor out duplicate code
  2017-06-19 13:11 [Qemu-devel] [PATCH v5 0/5] fsdev: qmp interface for io throttling Pradeep Jagadeesh
@ 2017-06-19 13:11 ` Pradeep Jagadeesh
  2017-06-20 15:46   ` Greg Kurz
                     ` (2 more replies)
  2017-06-19 13:11 ` [Qemu-devel] [PATCH v5 2/5] qmp: Create IOThrottle structure Pradeep Jagadeesh
                   ` (3 subsequent siblings)
  4 siblings, 3 replies; 18+ messages in thread
From: Pradeep Jagadeesh @ 2017-06-19 13:11 UTC (permalink / raw)
  To: eric blake, greg kurz
  Cc: Pradeep Jagadeesh, alberto garcia, jani kokkonen, qemu-devel

This patch factor out the duplicate throttle code that was present in
block and fsdev devices.

Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
---
 blockdev.c                      | 44 +-----------------------------------
 fsdev/qemu-fsdev-throttle.c     | 43 +----------------------------------
 fsdev/qemu-fsdev-throttle.h     |  1 +
 include/qemu/throttle-options.h |  4 ++++
 util/throttle.c                 | 50 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 57 insertions(+), 85 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 6472548..5db9e5c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -386,49 +386,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 7ae4e86..da9c225 100644
--- a/fsdev/qemu-fsdev-throttle.c
+++ b/fsdev/qemu-fsdev-throttle.c
@@ -31,48 +31,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/fsdev/qemu-fsdev-throttle.h b/fsdev/qemu-fsdev-throttle.h
index e418643..c493e83 100644
--- a/fsdev/qemu-fsdev-throttle.h
+++ b/fsdev/qemu-fsdev-throttle.h
@@ -20,6 +20,7 @@
 #include "qemu/coroutine.h"
 #include "qapi/error.h"
 #include "qemu/throttle.h"
+#include "qemu/throttle-options.h"
 
 typedef struct FsThrottle {
     ThrottleState ts;
diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h
index 3133d1c..565553a 100644
--- a/include/qemu/throttle-options.h
+++ b/include/qemu/throttle-options.h
@@ -10,6 +10,8 @@
 #ifndef THROTTLE_OPTIONS_H
 #define THROTTLE_OPTIONS_H
 
+#include "qemu/throttle.h"
+
 #define THROTTLE_OPTS \
           { \
             .name = "throttling.iops-total",\
@@ -89,4 +91,6 @@
             .help = "when limiting by iops max size of an I/O in bytes",\
         }
 
+void throttle_parse_options(ThrottleConfig *, QemuOpts *);
+
 #endif
diff --git a/util/throttle.c b/util/throttle.c
index 3570ed2..ebe9dd0 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -514,3 +514,53 @@ void throttle_account(ThrottleState *ts, bool is_write, uint64_t size)
     }
 }
 
+/* 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] 18+ messages in thread

* [Qemu-devel] [PATCH v5 2/5] qmp: Create IOThrottle structure
  2017-06-19 13:11 [Qemu-devel] [PATCH v5 0/5] fsdev: qmp interface for io throttling Pradeep Jagadeesh
  2017-06-19 13:11 ` [Qemu-devel] [PATCH v5 1/5] throttle: factor out duplicate code Pradeep Jagadeesh
@ 2017-06-19 13:11 ` Pradeep Jagadeesh
  2017-06-20 15:51   ` Greg Kurz
  2017-06-19 13:11 ` [Qemu-devel] [PATCH v5 3/5] qmp: refactor duplicate code Pradeep Jagadeesh
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Pradeep Jagadeesh @ 2017-06-19 13:11 UTC (permalink / raw)
  To: eric blake, greg kurz
  Cc: Pradeep Jagadeesh, alberto garcia, jani kokkonen, qemu-devel

This patch enables 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: Greg Kurz <groug@kaod.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 qapi/block-core.json | 76 ++-------------------------------------------
 qapi/iothrottle.json | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 73 deletions(-)
 create mode 100644 qapi/iothrottle.json

diff --git a/qapi/block-core.json b/qapi/block-core.json
index f85c223..9320974 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -6,6 +6,7 @@
 
 # QAPI common definitions
 { 'include': 'common.json' }
+{ 'include': 'iothrottle.json' }
 
 ##
 # @SnapshotInfo:
@@ -1761,84 +1762,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': 'IOThrottle',
+  'data': { '*device': 'str', '*group': 'str' } }
 
 ##
 # @block-stream:
diff --git a/qapi/iothrottle.json b/qapi/iothrottle.json
new file mode 100644
index 0000000..0f067c3
--- /dev/null
+++ b/qapi/iothrottle.json
@@ -0,0 +1,88 @@
+# -*- Mode: Python -*-
+
+##
+# == QAPI IOThrottle definitions
+##
+
+##
+# @IOThrottle:
+#
+# A set of parameters describing IO throttling
+#
+# @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)
+#
+#
+# Since: 2.10
+##
+{ 'struct': 'IOThrottle',
+  'data': { '*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' } }
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v5 3/5] qmp: refactor duplicate code
  2017-06-19 13:11 [Qemu-devel] [PATCH v5 0/5] fsdev: qmp interface for io throttling Pradeep Jagadeesh
  2017-06-19 13:11 ` [Qemu-devel] [PATCH v5 1/5] throttle: factor out duplicate code Pradeep Jagadeesh
  2017-06-19 13:11 ` [Qemu-devel] [PATCH v5 2/5] qmp: Create IOThrottle structure Pradeep Jagadeesh
@ 2017-06-19 13:11 ` Pradeep Jagadeesh
  2017-06-20 16:05   ` Greg Kurz
  2017-06-19 13:11 ` [Qemu-devel] [PATCH v5 4/5] fsdev: hmp interface for throttling Pradeep Jagadeesh
  2017-06-19 13:11 ` [Qemu-devel] [PATCH v5 5/5] fsdev: QMP " Pradeep Jagadeesh
  4 siblings, 1 reply; 18+ messages in thread
From: Pradeep Jagadeesh @ 2017-06-19 13:11 UTC (permalink / raw)
  To: eric blake, greg kurz
  Cc: Pradeep Jagadeesh, alberto garcia, jani kokkonen, qemu-devel

This patch factor out the duplicate qmp throttle interface code
that was present in both block and fsdev device files.

Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
---
 blockdev.c                      | 53 +++---------------------------------
 hmp.c                           | 21 ++++++++++-----
 include/qemu/throttle-options.h |  3 +++
 util/throttle.c                 | 60 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 81 insertions(+), 56 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 5db9e5c..3d06e9e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2593,6 +2593,7 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp)
     BlockDriverState *bs;
     BlockBackend *blk;
     AioContext *aio_context;
+    IOThrottle *iothrottle;
 
     blk = qmp_get_blk(arg->has_device ? arg->device : NULL,
                       arg->has_id ? arg->id : NULL,
@@ -2610,56 +2611,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;
-    }
+    iothrottle = qapi_BlockIOThrottle_base(arg);
+    throttle_set_io_limits(&cfg, iothrottle);
 
     if (!throttle_is_valid(&cfg, errp)) {
         goto out;
diff --git a/hmp.c b/hmp.c
index 8c72c58..220d301 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1749,20 +1749,29 @@ void hmp_change(Monitor *mon, const QDict *qdict)
     hmp_handle_error(mon, &err);
 }
 
+static void hmp_initialize_io_throttle(IOThrottle *iot, const QDict *qdict)
+{
+    iot->has_id = true;
+    iot->id = (char *) qdict_get_str(qdict, "id");
+    iot->bps = qdict_get_int(qdict, "bps");
+    iot->bps_rd = qdict_get_int(qdict, "bps_rd");
+    iot->bps_wr = qdict_get_int(qdict, "bps_wr");
+    iot->iops = qdict_get_int(qdict, "iops");
+    iot->iops_rd = qdict_get_int(qdict, "iops_rd");
+    iot->iops_wr = qdict_get_int(qdict, "iops_wr");
+}
+
 void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict)
 {
     Error *err = NULL;
+    IOThrottle *iothrottle;
     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"),
     };
 
+    iothrottle = qapi_BlockIOThrottle_base(&throttle);
+    hmp_initialize_io_throttle(iothrottle, qdict);
     qmp_block_set_io_throttle(&throttle, &err);
     hmp_handle_error(mon, &err);
 }
diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h
index 565553a..e94ea39 100644
--- a/include/qemu/throttle-options.h
+++ b/include/qemu/throttle-options.h
@@ -11,6 +11,7 @@
 #define THROTTLE_OPTIONS_H
 
 #include "qemu/throttle.h"
+#include "qmp-commands.h"
 
 #define THROTTLE_OPTS \
           { \
@@ -93,4 +94,6 @@
 
 void throttle_parse_options(ThrottleConfig *, QemuOpts *);
 
+void throttle_set_io_limits(ThrottleConfig *, IOThrottle *);
+
 #endif
diff --git a/util/throttle.c b/util/throttle.c
index ebe9dd0..2cf9ec5 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
  *
@@ -564,3 +565,62 @@ void throttle_parse_options(ThrottleConfig *throttle_cfg, QemuOpts *opts)
     throttle_cfg->op_size =
         qemu_opt_get_number(opts, "throttling.iops-size", 0);
 }
+
+/* set the throttle limits
+ *
+ * @arg: iothrottle limits
+ * @cfg: throttle configuration
+ */
+void throttle_set_io_limits(ThrottleConfig *cfg, IOThrottle *arg)
+{
+    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;
+    }
+}
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Qemu-devel] [PATCH v5 4/5] fsdev: hmp interface for throttling
  2017-06-19 13:11 [Qemu-devel] [PATCH v5 0/5] fsdev: qmp interface for io throttling Pradeep Jagadeesh
                   ` (2 preceding siblings ...)
  2017-06-19 13:11 ` [Qemu-devel] [PATCH v5 3/5] qmp: refactor duplicate code Pradeep Jagadeesh
@ 2017-06-19 13:11 ` Pradeep Jagadeesh
  2017-06-19 13:11 ` [Qemu-devel] [PATCH v5 5/5] fsdev: QMP " Pradeep Jagadeesh
  4 siblings, 0 replies; 18+ messages in thread
From: Pradeep Jagadeesh @ 2017-06-19 13:11 UTC (permalink / raw)
  To: eric blake, greg kurz
  Cc: Pradeep Jagadeesh, alberto garcia, jani kokkonen, qemu-devel

This patch introduces hmp interfaces for the fsdev
throttle functionality

Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
---
 hmp-commands-info.hx | 18 ++++++++++++++
 hmp-commands.hx      | 19 +++++++++++++++
 hmp.c                | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 hmp.h                |  4 ++++
 4 files changed, 107 insertions(+)

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index ae16901..f23b627 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       = "query-fsdev-iothrottle",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show fsdev device throttle information",
+        .cmd        = hmp_fsdev_get_io_throttle,
+    },
+
+#endif
+
+STEXI
+@item info fsdev throttle
+@findex fsdevthrottleinfo
+Show fsdev device throttleinfo.
+ETEXI
+
     {
         .name       = "block-jobs",
         .args_type  = "",
diff --git a/hmp-commands.hx b/hmp-commands.hx
index e763606..c60fd7e 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1662,6 +1662,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 220d301..b1c698b 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1776,6 +1776,72 @@ 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;
+    IOThrottle throttle;
+
+    hmp_initialize_io_throttle(&throttle, qdict);
+    qmp_fsdev_set_io_throttle(&throttle, &err);
+    hmp_handle_error(mon, &err);
+}
+
+static void print_fsdev_throttle_config(Monitor *mon, IOThrottle *fscfg,
+                                       Error *err)
+{
+    if (fscfg->bps  || fscfg->bps_rd  || fscfg->bps_wr  ||
+        fscfg->iops || fscfg->iops_rd || fscfg->iops_wr)
+    {
+        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,
+                        fscfg->bps,
+                        fscfg->bps_rd,
+                        fscfg->bps_wr,
+                        fscfg->bps_max,
+                        fscfg->bps_rd_max,
+                        fscfg->bps_wr_max,
+                        fscfg->iops,
+                        fscfg->iops_rd,
+                        fscfg->iops_wr,
+                        fscfg->iops_max,
+                        fscfg->iops_rd_max,
+                        fscfg->iops_wr_max,
+                        fscfg->iops_size);
+   }
+   hmp_handle_error(mon, &err);
+}
+
+void hmp_fsdev_get_io_throttle(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+    IOThrottleList *fs9p_list, *info;
+    fs9p_list = qmp_query_fsdev_io_throttle(&err);
+
+    for (info = fs9p_list; info; info = info->next) {
+        if (info != fs9p_list) {
+            monitor_printf(mon, "\n");
+        }
+        print_fsdev_throttle_config(mon, info->value, err);
+        qapi_free_IOThrottle(info->value);
+    }
+    qapi_free_IOThrottleList(fs9p_list);
+}
+
+#endif
+
 void hmp_block_stream(Monitor *mon, const QDict *qdict)
 {
     Error *error = NULL;
diff --git a/hmp.h b/hmp.h
index d8b94ce..05e72f2 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_fsdev_get_io_throttle(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] 18+ messages in thread

* [Qemu-devel] [PATCH v5 5/5] fsdev: QMP interface for throttling
  2017-06-19 13:11 [Qemu-devel] [PATCH v5 0/5] fsdev: qmp interface for io throttling Pradeep Jagadeesh
                   ` (3 preceding siblings ...)
  2017-06-19 13:11 ` [Qemu-devel] [PATCH v5 4/5] fsdev: hmp interface for throttling Pradeep Jagadeesh
@ 2017-06-19 13:11 ` Pradeep Jagadeesh
  4 siblings, 0 replies; 18+ messages in thread
From: Pradeep Jagadeesh @ 2017-06-19 13:11 UTC (permalink / raw)
  To: eric blake, greg kurz
  Cc: Pradeep Jagadeesh, alberto garcia, jani kokkonen, qemu-devel

This patch enables 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>
---
 Makefile                    |  3 ++
 fsdev/qemu-fsdev-dummy.c    | 10 ++++++
 fsdev/qemu-fsdev-throttle.c | 75 ++++++++++++++++++++++++++++++++++++++++
 fsdev/qemu-fsdev-throttle.h | 12 +++++++
 fsdev/qemu-fsdev.c          | 37 ++++++++++++++++++++
 monitor.c                   |  5 +++
 qapi-schema.json            |  3 ++
 qapi/fsdev.json             | 84 +++++++++++++++++++++++++++++++++++++++++++++
 qmp.c                       | 15 ++++++++
 9 files changed, 244 insertions(+)
 create mode 100644 qapi/fsdev.json

diff --git a/Makefile b/Makefile
index c830d7a..996b1cf 100644
--- a/Makefile
+++ b/Makefile
@@ -414,6 +414,9 @@ qapi-modules = $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/qapi/common.json \
                $(SRC_PATH)/qapi/event.json $(SRC_PATH)/qapi/introspect.json \
                $(SRC_PATH)/qapi/crypto.json $(SRC_PATH)/qapi/rocker.json \
                $(SRC_PATH)/qapi/trace.json
+ifdef CONFIG_VIRTFS
+qapi-modules += $(SRC_PATH)/qapi/fsdev.json
+endif
 
 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..f33305d 100644
--- a/fsdev/qemu-fsdev-dummy.c
+++ b/fsdev/qemu-fsdev-dummy.c
@@ -19,3 +19,13 @@ int qemu_fsdev_add(QemuOpts *opts)
 {
     return 0;
 }
+
+void qmp_fsdev_set_io_throttle(IOThrottle *arg, Error **errp)
+{
+  return;
+}
+
+IOThrottleList *qmp_query_fsdev_io_throttle(Error **errp)
+{
+    abort();
+}
diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
index da9c225..4483533 100644
--- a/fsdev/qemu-fsdev-throttle.c
+++ b/fsdev/qemu-fsdev-throttle.c
@@ -29,6 +29,81 @@ static void fsdev_throttle_write_timer_cb(void *opaque)
     qemu_co_enter_next(&fst->throttled_reqs[true]);
 }
 
+void fsdev_set_io_throttle(IOThrottle *arg, FsThrottle *fst, Error **errp)
+{
+    ThrottleConfig cfg;
+
+    throttle_set_io_limits(&cfg, arg);
+
+    if (throttle_is_valid(&cfg, errp)) {
+        fst->cfg = cfg;
+        fsdev_throttle_init(fst);
+    }
+}
+
+void fsdev_get_io_throttle(FsThrottle *fst, IOThrottle **fs9pcfg,
+                           char *fsdevice, Error **errp)
+{
+
+    ThrottleConfig cfg = fst->cfg;
+    IOThrottle *fscfg = g_malloc0(sizeof(*fscfg));
+
+    fscfg->has_id = true;
+    fscfg->id = g_strdup(fsdevice);
+    fscfg->bps = cfg.buckets[THROTTLE_BPS_TOTAL].avg;
+    fscfg->bps_rd = cfg.buckets[THROTTLE_BPS_READ].avg;
+    fscfg->bps_wr = cfg.buckets[THROTTLE_BPS_WRITE].avg;
+
+    fscfg->iops = cfg.buckets[THROTTLE_OPS_TOTAL].avg;
+    fscfg->iops_rd = cfg.buckets[THROTTLE_OPS_READ].avg;
+    fscfg->iops_wr = cfg.buckets[THROTTLE_OPS_WRITE].avg;
+
+    fscfg->has_bps_max     = cfg.buckets[THROTTLE_BPS_TOTAL].max;
+    fscfg->bps_max         = cfg.buckets[THROTTLE_BPS_TOTAL].max;
+    fscfg->has_bps_rd_max  = cfg.buckets[THROTTLE_BPS_READ].max;
+    fscfg->bps_rd_max      = cfg.buckets[THROTTLE_BPS_READ].max;
+    fscfg->has_bps_wr_max  = cfg.buckets[THROTTLE_BPS_WRITE].max;
+    fscfg->bps_wr_max      = cfg.buckets[THROTTLE_BPS_WRITE].max;
+
+    fscfg->has_iops_max    = cfg.buckets[THROTTLE_OPS_TOTAL].max;
+    fscfg->iops_max        = cfg.buckets[THROTTLE_OPS_TOTAL].max;
+    fscfg->has_iops_rd_max = cfg.buckets[THROTTLE_OPS_READ].max;
+    fscfg->iops_rd_max     = cfg.buckets[THROTTLE_OPS_READ].max;
+    fscfg->has_iops_wr_max = cfg.buckets[THROTTLE_OPS_WRITE].max;
+    fscfg->iops_wr_max     = cfg.buckets[THROTTLE_OPS_WRITE].max;
+
+    fscfg->has_bps_max_length     = fscfg->has_bps_max;
+    fscfg->bps_max_length         =
+         cfg.buckets[THROTTLE_BPS_TOTAL].burst_length;
+    fscfg->has_bps_rd_max_length  = fscfg->has_bps_rd_max;
+    fscfg->bps_rd_max_length      =
+         cfg.buckets[THROTTLE_BPS_READ].burst_length;
+    fscfg->has_bps_wr_max_length  = fscfg->has_bps_wr_max;
+    fscfg->bps_wr_max_length      =
+         cfg.buckets[THROTTLE_BPS_WRITE].burst_length;
+
+    fscfg->has_iops_max_length    = fscfg->has_iops_max;
+    fscfg->iops_max_length        =
+         cfg.buckets[THROTTLE_OPS_TOTAL].burst_length;
+    fscfg->has_iops_rd_max_length = fscfg->has_iops_rd_max;
+    fscfg->iops_rd_max_length     =
+         cfg.buckets[THROTTLE_OPS_READ].burst_length;
+    fscfg->has_iops_wr_max_length = fscfg->has_iops_wr_max;
+    fscfg->iops_wr_max_length     =
+         cfg.buckets[THROTTLE_OPS_WRITE].burst_length;
+
+    fscfg->bps_max_length = cfg.buckets[THROTTLE_BPS_TOTAL].burst_length;
+    fscfg->bps_rd_max_length = cfg.buckets[THROTTLE_BPS_READ].burst_length;
+    fscfg->bps_wr_max_length = cfg.buckets[THROTTLE_BPS_WRITE].burst_length;
+    fscfg->iops_max_length = cfg.buckets[THROTTLE_OPS_TOTAL].burst_length;
+    fscfg->iops_rd_max_length = cfg.buckets[THROTTLE_OPS_READ].burst_length;
+    fscfg->iops_wr_max_length = cfg.buckets[THROTTLE_OPS_WRITE].burst_length;
+
+    fscfg->iops_size = cfg.op_size;
+
+    *fs9pcfg = fscfg;
+}
+
 void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error **errp)
 {
     throttle_parse_options(&fst->cfg, opts);
diff --git a/fsdev/qemu-fsdev-throttle.h b/fsdev/qemu-fsdev-throttle.h
index c493e83..a49b2e5 100644
--- a/fsdev/qemu-fsdev-throttle.h
+++ b/fsdev/qemu-fsdev-throttle.h
@@ -21,6 +21,12 @@
 #include "qapi/error.h"
 #include "qemu/throttle.h"
 #include "qemu/throttle-options.h"
+#include "qapi/qmp/qerror.h"
+#include "qapi/qmp/types.h"
+#include "qapi-visit.h"
+#include "qapi/qobject-output-visitor.h"
+#include "qapi/util.h"
+#include "qmp-commands.h"
 
 typedef struct FsThrottle {
     ThrottleState ts;
@@ -37,4 +43,10 @@ void coroutine_fn fsdev_co_throttle_request(FsThrottle *, bool ,
                                             struct iovec *, int);
 
 void fsdev_throttle_cleanup(FsThrottle *);
+
+void fsdev_set_io_throttle(IOThrottle *, FsThrottle *, Error **errp);
+
+void fsdev_get_io_throttle(FsThrottle *, IOThrottle **iothp,
+                           char *, Error **errp);
+
 #endif /* _FSDEV_THROTTLE_H */
diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c
index 266e442..a99e299 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,39 @@ FsDriverEntry *get_fsdev_fsentry(char *id)
     }
     return NULL;
 }
+
+void qmp_fsdev_set_io_throttle(IOThrottle *arg, Error **errp)
+{
+
+    FsDriverEntry *fse;
+
+    fse = get_fsdev_fsentry(arg->has_id ? arg->id : NULL);
+    if (!fse) {
+        return;
+    }
+
+    fsdev_set_io_throttle(arg, &fse->fst, errp);
+}
+
+IOThrottleList *qmp_query_fsdev_io_throttle(Error **errp)
+{
+    IOThrottleList *head = NULL, **p_next = &head;
+    struct FsDriverListEntry *fsle;
+    Error *local_err = NULL;
+
+    QTAILQ_FOREACH(fsle, &fsdriver_entries, next) {
+        IOThrottleList *fscfg = g_malloc0(sizeof(*fscfg));
+        fsdev_get_io_throttle(&fsle->fse.fst, &fscfg->value,
+                            fsle->fse.fsdev_id, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            g_free(fscfg);
+            qapi_free_IOThrottleList(head);
+            return NULL;
+        }
+
+        *p_next = fscfg;
+        p_next = &fscfg->next;
+    }
+    return head;
+}
diff --git a/monitor.c b/monitor.c
index fcf4fad..6b8c2cb 100644
--- a/monitor.c
+++ b/monitor.c
@@ -997,6 +997,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 4b50b65..dc676be 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -81,6 +81,9 @@
 # QAPI block definitions
 { 'include': 'qapi/block.json' }
 
+# QAPI fsdev definitions
+{ 'include': 'qapi/fsdev.json' }
+
 # QAPI event definitions
 { 'include': 'qapi/event.json' }
 
diff --git a/qapi/fsdev.json b/qapi/fsdev.json
new file mode 100644
index 0000000..eff1efe
--- /dev/null
+++ b/qapi/fsdev.json
@@ -0,0 +1,84 @@
+# -*- Mode: Python -*-
+
+##
+# == QAPI fsdev definitions
+##
+
+# QAPI common definitions
+{ 'include': 'iothrottle.json' }
+
+##
+# @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, DeviceNotFound
+#
+# Since: 2.10
+#
+# 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': 'IOThrottle' }
+##
+# @query-fsdev-io-throttle:
+#
+# Returns: a list of @IOThrottle describing io throttle values of each fsdev device
+#
+# Since: 2.10
+#
+# 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': [ 'IOThrottle' ] }
+
diff --git a/qmp.c b/qmp.c
index 7ee9bcf..7dc68a3 100644
--- a/qmp.c
+++ b/qmp.c
@@ -130,6 +130,21 @@ void qmp_cpu_add(int64_t id, Error **errp)
     }
 }
 
+#if defined(_WIN64) || defined(_WIN32)
+
+void qmp_fsdev_set_io_throttle(IOThrottle *arg, Error **errp)
+{
+  return;
+}
+
+IOThrottleList *qmp_query_fsdev_io_throttle(Error **errp)
+{
+    abort();
+}
+
+#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] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v5 1/5] throttle: factor out duplicate code
  2017-06-19 13:11 ` [Qemu-devel] [PATCH v5 1/5] throttle: factor out duplicate code Pradeep Jagadeesh
@ 2017-06-20 15:46   ` Greg Kurz
  2017-06-21 12:55   ` Alberto Garcia
  2017-06-22 14:38   ` Markus Armbruster
  2 siblings, 0 replies; 18+ messages in thread
From: Greg Kurz @ 2017-06-20 15:46 UTC (permalink / raw)
  To: Pradeep Jagadeesh
  Cc: eric blake, Pradeep Jagadeesh, alberto garcia, jani kokkonen,
	qemu-devel, Markus Armbruster

[-- Attachment #1: Type: text/plain, Size: 10642 bytes --]

On Mon, 19 Jun 2017 09:11:32 -0400
Pradeep Jagadeesh <pradeepkiruvale@gmail.com> wrote:

> This patch factor out the duplicate throttle code that was present in
> block and fsdev devices.
> 
> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
> ---

I guess this patch can be applied right away.

Cc'ing Markus (maintainer of blockdev.c)

>  blockdev.c                      | 44 +-----------------------------------
>  fsdev/qemu-fsdev-throttle.c     | 43 +----------------------------------
>  fsdev/qemu-fsdev-throttle.h     |  1 +
>  include/qemu/throttle-options.h |  4 ++++
>  util/throttle.c                 | 50 +++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 57 insertions(+), 85 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 6472548..5db9e5c 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -386,49 +386,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 7ae4e86..da9c225 100644
> --- a/fsdev/qemu-fsdev-throttle.c
> +++ b/fsdev/qemu-fsdev-throttle.c
> @@ -31,48 +31,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/fsdev/qemu-fsdev-throttle.h b/fsdev/qemu-fsdev-throttle.h
> index e418643..c493e83 100644
> --- a/fsdev/qemu-fsdev-throttle.h
> +++ b/fsdev/qemu-fsdev-throttle.h
> @@ -20,6 +20,7 @@
>  #include "qemu/coroutine.h"
>  #include "qapi/error.h"
>  #include "qemu/throttle.h"
> +#include "qemu/throttle-options.h"
>  
>  typedef struct FsThrottle {
>      ThrottleState ts;
> diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h
> index 3133d1c..565553a 100644
> --- a/include/qemu/throttle-options.h
> +++ b/include/qemu/throttle-options.h
> @@ -10,6 +10,8 @@
>  #ifndef THROTTLE_OPTIONS_H
>  #define THROTTLE_OPTIONS_H
>  
> +#include "qemu/throttle.h"
> +
>  #define THROTTLE_OPTS \
>            { \
>              .name = "throttling.iops-total",\
> @@ -89,4 +91,6 @@
>              .help = "when limiting by iops max size of an I/O in bytes",\
>          }
>  
> +void throttle_parse_options(ThrottleConfig *, QemuOpts *);
> +
>  #endif
> diff --git a/util/throttle.c b/util/throttle.c
> index 3570ed2..ebe9dd0 100644
> --- a/util/throttle.c
> +++ b/util/throttle.c
> @@ -514,3 +514,53 @@ void throttle_account(ThrottleState *ts, bool is_write, uint64_t size)
>      }
>  }
>  
> +/* 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: 181 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v5 2/5] qmp: Create IOThrottle structure
  2017-06-19 13:11 ` [Qemu-devel] [PATCH v5 2/5] qmp: Create IOThrottle structure Pradeep Jagadeesh
@ 2017-06-20 15:51   ` Greg Kurz
  0 siblings, 0 replies; 18+ messages in thread
From: Greg Kurz @ 2017-06-20 15:51 UTC (permalink / raw)
  To: Pradeep Jagadeesh
  Cc: eric blake, Pradeep Jagadeesh, alberto garcia, jani kokkonen, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 8651 bytes --]

On Mon, 19 Jun 2017 09:11:33 -0400
Pradeep Jagadeesh <pradeepkiruvale@gmail.com> wrote:

> This patch enables 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>
> 

No blank line here.

> Reviewed-by: Greg Kurz <groug@kaod.org>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>  qapi/block-core.json | 76 ++-------------------------------------------
>  qapi/iothrottle.json | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 91 insertions(+), 73 deletions(-)
>  create mode 100644 qapi/iothrottle.json
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index f85c223..9320974 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -6,6 +6,7 @@
>  
>  # QAPI common definitions
>  { 'include': 'common.json' }
> +{ 'include': 'iothrottle.json' }
>  
>  ##
>  # @SnapshotInfo:
> @@ -1761,84 +1762,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': 'IOThrottle',
> +  'data': { '*device': 'str', '*group': 'str' } }
>  
>  ##
>  # @block-stream:
> diff --git a/qapi/iothrottle.json b/qapi/iothrottle.json
> new file mode 100644
> index 0000000..0f067c3
> --- /dev/null
> +++ b/qapi/iothrottle.json
> @@ -0,0 +1,88 @@
> +# -*- Mode: Python -*-
> +
> +##
> +# == QAPI IOThrottle definitions
> +##
> +
> +##
> +# @IOThrottle:
> +#
> +# A set of parameters describing IO throttling
> +#
> +# @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)
> +#
> +#
> +# Since: 2.10
> +##
> +{ 'struct': 'IOThrottle',
> +  'data': { '*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' } }


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v5 3/5] qmp: refactor duplicate code
  2017-06-19 13:11 ` [Qemu-devel] [PATCH v5 3/5] qmp: refactor duplicate code Pradeep Jagadeesh
@ 2017-06-20 16:05   ` Greg Kurz
  2017-06-21  8:34     ` Pradeep Jagadeesh
  0 siblings, 1 reply; 18+ messages in thread
From: Greg Kurz @ 2017-06-20 16:05 UTC (permalink / raw)
  To: Pradeep Jagadeesh
  Cc: eric blake, Pradeep Jagadeesh, alberto garcia, jani kokkonen,
	qemu-devel, Markus Armbruster, Dr. David Alan Gilbert

[-- Attachment #1: Type: text/plain, Size: 8700 bytes --]

On Mon, 19 Jun 2017 09:11:34 -0400
Pradeep Jagadeesh <pradeepkiruvale@gmail.com> wrote:

> This patch factor out the duplicate qmp throttle interface code
> that was present in both block and fsdev device files.
> 

The text is obviously wrong. I don't see any duplicate code below.

It is more something like: let's factor out the code that will be used
by the existing QMP throttling API for block devices and the future
QMP throttling API for fs devices.

Please move the HMP part to another patch, as asked during v4 review.

Also, blockdev.c and hmp.c do have maintainers. You should Cc: them each
time because they know better than me and even if these patches are carried
through my tree, I won't do it without an ack from them.

Cc'ing Markus for blockdev.c and David for hmp.c.

> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
> ---
>  blockdev.c                      | 53 +++---------------------------------
>  hmp.c                           | 21 ++++++++++-----
>  include/qemu/throttle-options.h |  3 +++
>  util/throttle.c                 | 60 +++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 81 insertions(+), 56 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 5db9e5c..3d06e9e 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2593,6 +2593,7 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp)
>      BlockDriverState *bs;
>      BlockBackend *blk;
>      AioContext *aio_context;
> +    IOThrottle *iothrottle;
>  
>      blk = qmp_get_blk(arg->has_device ? arg->device : NULL,
>                        arg->has_id ? arg->id : NULL,
> @@ -2610,56 +2611,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;
> -    }
> +    iothrottle = qapi_BlockIOThrottle_base(arg);
> +    throttle_set_io_limits(&cfg, iothrottle);
>  
>      if (!throttle_is_valid(&cfg, errp)) {
>          goto out;
> diff --git a/hmp.c b/hmp.c
> index 8c72c58..220d301 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1749,20 +1749,29 @@ void hmp_change(Monitor *mon, const QDict *qdict)
>      hmp_handle_error(mon, &err);
>  }
>  
> +static void hmp_initialize_io_throttle(IOThrottle *iot, const QDict *qdict)
> +{
> +    iot->has_id = true;
> +    iot->id = (char *) qdict_get_str(qdict, "id");
> +    iot->bps = qdict_get_int(qdict, "bps");
> +    iot->bps_rd = qdict_get_int(qdict, "bps_rd");
> +    iot->bps_wr = qdict_get_int(qdict, "bps_wr");
> +    iot->iops = qdict_get_int(qdict, "iops");
> +    iot->iops_rd = qdict_get_int(qdict, "iops_rd");
> +    iot->iops_wr = qdict_get_int(qdict, "iops_wr");
> +}
> +
>  void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict)
>  {
>      Error *err = NULL;
> +    IOThrottle *iothrottle;
>      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"),
>      };
>  
> +    iothrottle = qapi_BlockIOThrottle_base(&throttle);
> +    hmp_initialize_io_throttle(iothrottle, qdict);
>      qmp_block_set_io_throttle(&throttle, &err);
>      hmp_handle_error(mon, &err);
>  }
> diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h
> index 565553a..e94ea39 100644
> --- a/include/qemu/throttle-options.h
> +++ b/include/qemu/throttle-options.h
> @@ -11,6 +11,7 @@
>  #define THROTTLE_OPTIONS_H
>  
>  #include "qemu/throttle.h"
> +#include "qmp-commands.h"
>  
>  #define THROTTLE_OPTS \
>            { \
> @@ -93,4 +94,6 @@
>  
>  void throttle_parse_options(ThrottleConfig *, QemuOpts *);
>  
> +void throttle_set_io_limits(ThrottleConfig *, IOThrottle *);
> +
>  #endif
> diff --git a/util/throttle.c b/util/throttle.c
> index ebe9dd0..2cf9ec5 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
>   *
> @@ -564,3 +565,62 @@ void throttle_parse_options(ThrottleConfig *throttle_cfg, QemuOpts *opts)
>      throttle_cfg->op_size =
>          qemu_opt_get_number(opts, "throttling.iops-size", 0);
>  }
> +
> +/* set the throttle limits
> + *
> + * @arg: iothrottle limits
> + * @cfg: throttle configuration
> + */
> +void throttle_set_io_limits(ThrottleConfig *cfg, IOThrottle *arg)
> +{
> +    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;
> +    }
> +}


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v5 3/5] qmp: refactor duplicate code
  2017-06-20 16:05   ` Greg Kurz
@ 2017-06-21  8:34     ` Pradeep Jagadeesh
  2017-06-21 10:00       ` Greg Kurz
  0 siblings, 1 reply; 18+ messages in thread
From: Pradeep Jagadeesh @ 2017-06-21  8:34 UTC (permalink / raw)
  To: Greg Kurz, Pradeep Jagadeesh
  Cc: eric blake, alberto garcia, jani kokkonen, qemu-devel,
	Markus Armbruster, Dr. David Alan Gilbert

On 6/20/2017 6:05 PM, Greg Kurz wrote:
> On Mon, 19 Jun 2017 09:11:34 -0400
> Pradeep Jagadeesh <pradeepkiruvale@gmail.com> wrote:
>
>> This patch factor out the duplicate qmp throttle interface code
>> that was present in both block and fsdev device files.
>>
>
> The text is obviously wrong. I don't see any duplicate code below.
OK, I will fix this.
>
> It is more something like: let's factor out the code that will be used
> by the existing QMP throttling API for block devices and the future
> QMP throttling API for fs devices.
>
> Please move the HMP part to another patch, as asked during v4 review.
I have moved the hmp patches for the fsdev into a separate patch. Do you 
want me to push this also into a separate patch?
>
> Also, blockdev.c and hmp.c do have maintainers. You should Cc: them each
> time because they know better than me and even if these patches are carried
> through my tree, I won't do it without an ack from them.

OK, I will add them next time on.

-Pradeep

> Cc'ing Markus for blockdev.c and David for hmp.c.
>
>> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
>> ---
>>  blockdev.c                      | 53 +++---------------------------------
>>  hmp.c                           | 21 ++++++++++-----
>>  include/qemu/throttle-options.h |  3 +++
>>  util/throttle.c                 | 60 +++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 81 insertions(+), 56 deletions(-)
>>
>> diff --git a/blockdev.c b/blockdev.c
>> index 5db9e5c..3d06e9e 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -2593,6 +2593,7 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp)
>>      BlockDriverState *bs;
>>      BlockBackend *blk;
>>      AioContext *aio_context;
>> +    IOThrottle *iothrottle;
>>
>>      blk = qmp_get_blk(arg->has_device ? arg->device : NULL,
>>                        arg->has_id ? arg->id : NULL,
>> @@ -2610,56 +2611,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;
>> -    }
>> +    iothrottle = qapi_BlockIOThrottle_base(arg);
>> +    throttle_set_io_limits(&cfg, iothrottle);
>>
>>      if (!throttle_is_valid(&cfg, errp)) {
>>          goto out;
>> diff --git a/hmp.c b/hmp.c
>> index 8c72c58..220d301 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -1749,20 +1749,29 @@ void hmp_change(Monitor *mon, const QDict *qdict)
>>      hmp_handle_error(mon, &err);
>>  }
>>
>> +static void hmp_initialize_io_throttle(IOThrottle *iot, const QDict *qdict)
>> +{
>> +    iot->has_id = true;
>> +    iot->id = (char *) qdict_get_str(qdict, "id");
>> +    iot->bps = qdict_get_int(qdict, "bps");
>> +    iot->bps_rd = qdict_get_int(qdict, "bps_rd");
>> +    iot->bps_wr = qdict_get_int(qdict, "bps_wr");
>> +    iot->iops = qdict_get_int(qdict, "iops");
>> +    iot->iops_rd = qdict_get_int(qdict, "iops_rd");
>> +    iot->iops_wr = qdict_get_int(qdict, "iops_wr");
>> +}
>> +
>>  void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict)
>>  {
>>      Error *err = NULL;
>> +    IOThrottle *iothrottle;
>>      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"),
>>      };
>>
>> +    iothrottle = qapi_BlockIOThrottle_base(&throttle);
>> +    hmp_initialize_io_throttle(iothrottle, qdict);
>>      qmp_block_set_io_throttle(&throttle, &err);
>>      hmp_handle_error(mon, &err);
>>  }
>> diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h
>> index 565553a..e94ea39 100644
>> --- a/include/qemu/throttle-options.h
>> +++ b/include/qemu/throttle-options.h
>> @@ -11,6 +11,7 @@
>>  #define THROTTLE_OPTIONS_H
>>
>>  #include "qemu/throttle.h"
>> +#include "qmp-commands.h"
>>
>>  #define THROTTLE_OPTS \
>>            { \
>> @@ -93,4 +94,6 @@
>>
>>  void throttle_parse_options(ThrottleConfig *, QemuOpts *);
>>
>> +void throttle_set_io_limits(ThrottleConfig *, IOThrottle *);
>> +
>>  #endif
>> diff --git a/util/throttle.c b/util/throttle.c
>> index ebe9dd0..2cf9ec5 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
>>   *
>> @@ -564,3 +565,62 @@ void throttle_parse_options(ThrottleConfig *throttle_cfg, QemuOpts *opts)
>>      throttle_cfg->op_size =
>>          qemu_opt_get_number(opts, "throttling.iops-size", 0);
>>  }
>> +
>> +/* set the throttle limits
>> + *
>> + * @arg: iothrottle limits
>> + * @cfg: throttle configuration
>> + */
>> +void throttle_set_io_limits(ThrottleConfig *cfg, IOThrottle *arg)
>> +{
>> +    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;
>> +    }
>> +}
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v5 3/5] qmp: refactor duplicate code
  2017-06-21  8:34     ` Pradeep Jagadeesh
@ 2017-06-21 10:00       ` Greg Kurz
  2017-06-29 13:35         ` Pradeep Jagadeesh
  0 siblings, 1 reply; 18+ messages in thread
From: Greg Kurz @ 2017-06-21 10:00 UTC (permalink / raw)
  To: Pradeep Jagadeesh
  Cc: Pradeep Jagadeesh, eric blake, alberto garcia, jani kokkonen,
	qemu-devel, Markus Armbruster, Dr. David Alan Gilbert

[-- Attachment #1: Type: text/plain, Size: 9861 bytes --]

On Wed, 21 Jun 2017 10:34:42 +0200
Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com> wrote:

> On 6/20/2017 6:05 PM, Greg Kurz wrote:
> > On Mon, 19 Jun 2017 09:11:34 -0400
> > Pradeep Jagadeesh <pradeepkiruvale@gmail.com> wrote:
> >  
> >> This patch factor out the duplicate qmp throttle interface code
> >> that was present in both block and fsdev device files.
> >>  
> >
> > The text is obviously wrong. I don't see any duplicate code below.  
> OK, I will fix this.
> >
> > It is more something like: let's factor out the code that will be used
> > by the existing QMP throttling API for block devices and the future
> > QMP throttling API for fs devices.
> >
> > Please move the HMP part to another patch, as asked during v4 review.  
> I have moved the hmp patches for the fsdev into a separate patch. Do you 
> want me to push this also into a separate patch?

Yes. These changes aren't related and theoretically belong to separate
sub-maintainer trees.

> >
> > Also, blockdev.c and hmp.c do have maintainers. You should Cc: them each
> > time because they know better than me and even if these patches are carried
> > through my tree, I won't do it without an ack from them.  
> 
> OK, I will add them next time on.
> 
> -Pradeep
> 
> > Cc'ing Markus for blockdev.c and David for hmp.c.
> >  
> >> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
> >> ---
> >>  blockdev.c                      | 53 +++---------------------------------
> >>  hmp.c                           | 21 ++++++++++-----
> >>  include/qemu/throttle-options.h |  3 +++
> >>  util/throttle.c                 | 60 +++++++++++++++++++++++++++++++++++++++++
> >>  4 files changed, 81 insertions(+), 56 deletions(-)
> >>
> >> diff --git a/blockdev.c b/blockdev.c
> >> index 5db9e5c..3d06e9e 100644
> >> --- a/blockdev.c
> >> +++ b/blockdev.c
> >> @@ -2593,6 +2593,7 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp)
> >>      BlockDriverState *bs;
> >>      BlockBackend *blk;
> >>      AioContext *aio_context;
> >> +    IOThrottle *iothrottle;
> >>
> >>      blk = qmp_get_blk(arg->has_device ? arg->device : NULL,
> >>                        arg->has_id ? arg->id : NULL,
> >> @@ -2610,56 +2611,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;
> >> -    }
> >> +    iothrottle = qapi_BlockIOThrottle_base(arg);
> >> +    throttle_set_io_limits(&cfg, iothrottle);
> >>
> >>      if (!throttle_is_valid(&cfg, errp)) {
> >>          goto out;
> >> diff --git a/hmp.c b/hmp.c
> >> index 8c72c58..220d301 100644
> >> --- a/hmp.c
> >> +++ b/hmp.c
> >> @@ -1749,20 +1749,29 @@ void hmp_change(Monitor *mon, const QDict *qdict)
> >>      hmp_handle_error(mon, &err);
> >>  }
> >>
> >> +static void hmp_initialize_io_throttle(IOThrottle *iot, const QDict *qdict)
> >> +{
> >> +    iot->has_id = true;
> >> +    iot->id = (char *) qdict_get_str(qdict, "id");
> >> +    iot->bps = qdict_get_int(qdict, "bps");
> >> +    iot->bps_rd = qdict_get_int(qdict, "bps_rd");
> >> +    iot->bps_wr = qdict_get_int(qdict, "bps_wr");
> >> +    iot->iops = qdict_get_int(qdict, "iops");
> >> +    iot->iops_rd = qdict_get_int(qdict, "iops_rd");
> >> +    iot->iops_wr = qdict_get_int(qdict, "iops_wr");
> >> +}
> >> +
> >>  void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict)
> >>  {
> >>      Error *err = NULL;
> >> +    IOThrottle *iothrottle;
> >>      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"),
> >>      };
> >>
> >> +    iothrottle = qapi_BlockIOThrottle_base(&throttle);
> >> +    hmp_initialize_io_throttle(iothrottle, qdict);
> >>      qmp_block_set_io_throttle(&throttle, &err);
> >>      hmp_handle_error(mon, &err);
> >>  }
> >> diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h
> >> index 565553a..e94ea39 100644
> >> --- a/include/qemu/throttle-options.h
> >> +++ b/include/qemu/throttle-options.h
> >> @@ -11,6 +11,7 @@
> >>  #define THROTTLE_OPTIONS_H
> >>
> >>  #include "qemu/throttle.h"
> >> +#include "qmp-commands.h"
> >>
> >>  #define THROTTLE_OPTS \
> >>            { \
> >> @@ -93,4 +94,6 @@
> >>
> >>  void throttle_parse_options(ThrottleConfig *, QemuOpts *);
> >>
> >> +void throttle_set_io_limits(ThrottleConfig *, IOThrottle *);
> >> +
> >>  #endif
> >> diff --git a/util/throttle.c b/util/throttle.c
> >> index ebe9dd0..2cf9ec5 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
> >>   *
> >> @@ -564,3 +565,62 @@ void throttle_parse_options(ThrottleConfig *throttle_cfg, QemuOpts *opts)
> >>      throttle_cfg->op_size =
> >>          qemu_opt_get_number(opts, "throttling.iops-size", 0);
> >>  }
> >> +
> >> +/* set the throttle limits
> >> + *
> >> + * @arg: iothrottle limits
> >> + * @cfg: throttle configuration
> >> + */
> >> +void throttle_set_io_limits(ThrottleConfig *cfg, IOThrottle *arg)
> >> +{
> >> +    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;
> >> +    }
> >> +}  
> >  
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v5 1/5] throttle: factor out duplicate code
  2017-06-19 13:11 ` [Qemu-devel] [PATCH v5 1/5] throttle: factor out duplicate code Pradeep Jagadeesh
  2017-06-20 15:46   ` Greg Kurz
@ 2017-06-21 12:55   ` Alberto Garcia
  2017-06-21 13:43     ` Greg Kurz
  2017-06-22 14:38   ` Markus Armbruster
  2 siblings, 1 reply; 18+ messages in thread
From: Alberto Garcia @ 2017-06-21 12:55 UTC (permalink / raw)
  To: Pradeep Jagadeesh, eric blake, greg kurz
  Cc: Pradeep Jagadeesh, jani kokkonen, qemu-devel

On Mon 19 Jun 2017 03:11:32 PM CEST, Pradeep Jagadeesh wrote:
> This patch factor out the duplicate throttle code that was present in
> block and fsdev devices.
>
> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v5 1/5] throttle: factor out duplicate code
  2017-06-21 12:55   ` Alberto Garcia
@ 2017-06-21 13:43     ` Greg Kurz
  2017-06-21 14:21       ` Alberto Garcia
  0 siblings, 1 reply; 18+ messages in thread
From: Greg Kurz @ 2017-06-21 13:43 UTC (permalink / raw)
  To: Alberto Garcia
  Cc: Pradeep Jagadeesh, eric blake, Pradeep Jagadeesh, jani kokkonen,
	qemu-devel, Markus Armbruster

[-- Attachment #1: Type: text/plain, Size: 493 bytes --]

On Wed, 21 Jun 2017 14:55:55 +0200
Alberto Garcia <berto@igalia.com> wrote:

> On Mon 19 Jun 2017 03:11:32 PM CEST, Pradeep Jagadeesh wrote:
> > This patch factor out the duplicate throttle code that was present in
> > block and fsdev devices.
> >
> > Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>  
> 
> Reviewed-by: Alberto Garcia <berto@igalia.com>
> 

Thanks ! Is it okay if this goes through my 9P tree ?

> Berto

Cc'ing Markus, for the same question.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v5 1/5] throttle: factor out duplicate code
  2017-06-21 13:43     ` Greg Kurz
@ 2017-06-21 14:21       ` Alberto Garcia
  0 siblings, 0 replies; 18+ messages in thread
From: Alberto Garcia @ 2017-06-21 14:21 UTC (permalink / raw)
  To: Greg Kurz
  Cc: Pradeep Jagadeesh, eric blake, Pradeep Jagadeesh, jani kokkonen,
	qemu-devel, Markus Armbruster

On Wed 21 Jun 2017 03:43:09 PM CEST, Greg Kurz wrote:
> On Wed, 21 Jun 2017 14:55:55 +0200
> Alberto Garcia <berto@igalia.com> wrote:
>
>> On Mon 19 Jun 2017 03:11:32 PM CEST, Pradeep Jagadeesh wrote:
>> > This patch factor out the duplicate throttle code that was present in
>> > block and fsdev devices.
>> >
>> > Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>  
>> 
>> Reviewed-by: Alberto Garcia <berto@igalia.com>
>
> Thanks ! Is it okay if this goes through my 9P tree ?

No problem from my side.

Berto

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v5 1/5] throttle: factor out duplicate code
  2017-06-19 13:11 ` [Qemu-devel] [PATCH v5 1/5] throttle: factor out duplicate code Pradeep Jagadeesh
  2017-06-20 15:46   ` Greg Kurz
  2017-06-21 12:55   ` Alberto Garcia
@ 2017-06-22 14:38   ` Markus Armbruster
  2017-06-22 16:23     ` Greg Kurz
  2017-06-29  9:50     ` Pradeep Jagadeesh
  2 siblings, 2 replies; 18+ messages in thread
From: Markus Armbruster @ 2017-06-22 14:38 UTC (permalink / raw)
  To: Pradeep Jagadeesh
  Cc: eric blake, greg kurz, jani kokkonen, alberto garcia,
	Pradeep Jagadeesh, qemu-devel

Pradeep Jagadeesh <pradeepkiruvale@gmail.com> writes:

> This patch factor out the duplicate throttle code that was present in
> block and fsdev devices.
>
> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
> ---
>  blockdev.c                      | 44 +-----------------------------------
>  fsdev/qemu-fsdev-throttle.c     | 43 +----------------------------------
>  fsdev/qemu-fsdev-throttle.h     |  1 +
>  include/qemu/throttle-options.h |  4 ++++
>  util/throttle.c                 | 50 +++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 57 insertions(+), 85 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index 6472548..5db9e5c 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -386,49 +386,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 7ae4e86..da9c225 100644
> --- a/fsdev/qemu-fsdev-throttle.c
> +++ b/fsdev/qemu-fsdev-throttle.c
> @@ -31,48 +31,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/fsdev/qemu-fsdev-throttle.h b/fsdev/qemu-fsdev-throttle.h
> index e418643..c493e83 100644
> --- a/fsdev/qemu-fsdev-throttle.h
> +++ b/fsdev/qemu-fsdev-throttle.h
> @@ -20,6 +20,7 @@
>  #include "qemu/coroutine.h"
>  #include "qapi/error.h"
>  #include "qemu/throttle.h"
> +#include "qemu/throttle-options.h"

Why?

>  
>  typedef struct FsThrottle {
>      ThrottleState ts;
> diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h
> index 3133d1c..565553a 100644
> --- a/include/qemu/throttle-options.h
> +++ b/include/qemu/throttle-options.h
> @@ -10,6 +10,8 @@
>  #ifndef THROTTLE_OPTIONS_H
>  #define THROTTLE_OPTIONS_H
>  
> +#include "qemu/throttle.h"
> +

Let's avoid this include: move the ThrottleConfig typedef to
qemu/typedefs.h.

>  #define THROTTLE_OPTS \
>            { \
>              .name = "throttling.iops-total",\
> @@ -89,4 +91,6 @@
>              .help = "when limiting by iops max size of an I/O in bytes",\
>          }
>  
> +void throttle_parse_options(ThrottleConfig *, QemuOpts *);
> +
>  #endif
> diff --git a/util/throttle.c b/util/throttle.c
> index 3570ed2..ebe9dd0 100644
> --- a/util/throttle.c
> +++ b/util/throttle.c
> @@ -514,3 +514,53 @@ void throttle_account(ThrottleState *ts, bool is_write, uint64_t size)
>      }
>  }
>  
> +/* 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);
> +}

Looks good otherwise.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v5 1/5] throttle: factor out duplicate code
  2017-06-22 14:38   ` Markus Armbruster
@ 2017-06-22 16:23     ` Greg Kurz
  2017-06-29  9:50     ` Pradeep Jagadeesh
  1 sibling, 0 replies; 18+ messages in thread
From: Greg Kurz @ 2017-06-22 16:23 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Pradeep Jagadeesh, eric blake, jani kokkonen, alberto garcia,
	Pradeep Jagadeesh, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 11265 bytes --]

On Thu, 22 Jun 2017 16:38:45 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Pradeep Jagadeesh <pradeepkiruvale@gmail.com> writes:
> 
> > This patch factor out the duplicate throttle code that was present in
> > block and fsdev devices.
> >
> > Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
> > ---
> >  blockdev.c                      | 44 +-----------------------------------
> >  fsdev/qemu-fsdev-throttle.c     | 43 +----------------------------------
> >  fsdev/qemu-fsdev-throttle.h     |  1 +
> >  include/qemu/throttle-options.h |  4 ++++
> >  util/throttle.c                 | 50 +++++++++++++++++++++++++++++++++++++++++
> >  5 files changed, 57 insertions(+), 85 deletions(-)
> >
> > diff --git a/blockdev.c b/blockdev.c
> > index 6472548..5db9e5c 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -386,49 +386,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 7ae4e86..da9c225 100644
> > --- a/fsdev/qemu-fsdev-throttle.c
> > +++ b/fsdev/qemu-fsdev-throttle.c
> > @@ -31,48 +31,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/fsdev/qemu-fsdev-throttle.h b/fsdev/qemu-fsdev-throttle.h
> > index e418643..c493e83 100644
> > --- a/fsdev/qemu-fsdev-throttle.h
> > +++ b/fsdev/qemu-fsdev-throttle.h
> > @@ -20,6 +20,7 @@
> >  #include "qemu/coroutine.h"
> >  #include "qapi/error.h"
> >  #include "qemu/throttle.h"
> > +#include "qemu/throttle-options.h"  
> 
> Why?
> 

Yeah, looking at the previous hunk, I guess it should rather go to
fsdev/qemu-fsdev-throttle.c.

> >  
> >  typedef struct FsThrottle {
> >      ThrottleState ts;
> > diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h
> > index 3133d1c..565553a 100644
> > --- a/include/qemu/throttle-options.h
> > +++ b/include/qemu/throttle-options.h
> > @@ -10,6 +10,8 @@
> >  #ifndef THROTTLE_OPTIONS_H
> >  #define THROTTLE_OPTIONS_H
> >  
> > +#include "qemu/throttle.h"
> > +  
> 
> Let's avoid this include: move the ThrottleConfig typedef to
> qemu/typedefs.h.
> 
> >  #define THROTTLE_OPTS \
> >            { \
> >              .name = "throttling.iops-total",\
> > @@ -89,4 +91,6 @@
> >              .help = "when limiting by iops max size of an I/O in bytes",\
> >          }
> >  
> > +void throttle_parse_options(ThrottleConfig *, QemuOpts *);
> > +
> >  #endif
> > diff --git a/util/throttle.c b/util/throttle.c
> > index 3570ed2..ebe9dd0 100644
> > --- a/util/throttle.c
> > +++ b/util/throttle.c
> > @@ -514,3 +514,53 @@ void throttle_account(ThrottleState *ts, bool is_write, uint64_t size)
> >      }
> >  }
> >  
> > +/* 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);
> > +}  
> 
> Looks good otherwise.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v5 1/5] throttle: factor out duplicate code
  2017-06-22 14:38   ` Markus Armbruster
  2017-06-22 16:23     ` Greg Kurz
@ 2017-06-29  9:50     ` Pradeep Jagadeesh
  1 sibling, 0 replies; 18+ messages in thread
From: Pradeep Jagadeesh @ 2017-06-29  9:50 UTC (permalink / raw)
  To: Markus Armbruster, Pradeep Jagadeesh
  Cc: eric blake, greg kurz, jani kokkonen, alberto garcia, qemu-devel

On 6/22/2017 4:38 PM, Markus Armbruster wrote:
> Pradeep Jagadeesh <pradeepkiruvale@gmail.com> writes:
>
>> This patch factor out the duplicate throttle code that was present in
>> block and fsdev devices.
>>
>> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
>> ---
>>  blockdev.c                      | 44 +-----------------------------------
>>  fsdev/qemu-fsdev-throttle.c     | 43 +----------------------------------
>>  fsdev/qemu-fsdev-throttle.h     |  1 +
>>  include/qemu/throttle-options.h |  4 ++++
>>  util/throttle.c                 | 50 +++++++++++++++++++++++++++++++++++++++++
>>  5 files changed, 57 insertions(+), 85 deletions(-)
>>
>> diff --git a/blockdev.c b/blockdev.c
>> index 6472548..5db9e5c 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -386,49 +386,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 7ae4e86..da9c225 100644
>> --- a/fsdev/qemu-fsdev-throttle.c
>> +++ b/fsdev/qemu-fsdev-throttle.c
>> @@ -31,48 +31,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/fsdev/qemu-fsdev-throttle.h b/fsdev/qemu-fsdev-throttle.h
>> index e418643..c493e83 100644
>> --- a/fsdev/qemu-fsdev-throttle.h
>> +++ b/fsdev/qemu-fsdev-throttle.h
>> @@ -20,6 +20,7 @@
>>  #include "qemu/coroutine.h"
>>  #include "qapi/error.h"
>>  #include "qemu/throttle.h"
>> +#include "qemu/throttle-options.h"
>
> Why?
>
Moved to qemu-fsdev-throttle.c
>>
>>  typedef struct FsThrottle {
>>      ThrottleState ts;
>> diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h
>> index 3133d1c..565553a 100644
>> --- a/include/qemu/throttle-options.h
>> +++ b/include/qemu/throttle-options.h
>> @@ -10,6 +10,8 @@
>>  #ifndef THROTTLE_OPTIONS_H
>>  #define THROTTLE_OPTIONS_H
>>
>> +#include "qemu/throttle.h"
>> +
>
> Let's avoid this include: move the ThrottleConfig typedef to
> qemu/typedefs.h.
OK
>
>>  #define THROTTLE_OPTS \
>>            { \
>>              .name = "throttling.iops-total",\
>> @@ -89,4 +91,6 @@
>>              .help = "when limiting by iops max size of an I/O in bytes",\
>>          }
>>
>> +void throttle_parse_options(ThrottleConfig *, QemuOpts *);
>> +
>>  #endif
>> diff --git a/util/throttle.c b/util/throttle.c
>> index 3570ed2..ebe9dd0 100644
>> --- a/util/throttle.c
>> +++ b/util/throttle.c
>> @@ -514,3 +514,53 @@ void throttle_account(ThrottleState *ts, bool is_write, uint64_t size)
>>      }
>>  }
>>
>> +/* 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);
>> +}
>
> Looks good otherwise.
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Qemu-devel] [PATCH v5 3/5] qmp: refactor duplicate code
  2017-06-21 10:00       ` Greg Kurz
@ 2017-06-29 13:35         ` Pradeep Jagadeesh
  0 siblings, 0 replies; 18+ messages in thread
From: Pradeep Jagadeesh @ 2017-06-29 13:35 UTC (permalink / raw)
  To: Greg Kurz
  Cc: Pradeep Jagadeesh, eric blake, alberto garcia, jani kokkonen,
	qemu-devel, Markus Armbruster, Dr. David Alan Gilbert

On 6/21/2017 12:00 PM, Greg Kurz wrote:
> On Wed, 21 Jun 2017 10:34:42 +0200
> Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com> wrote:
>
>> On 6/20/2017 6:05 PM, Greg Kurz wrote:
>>> On Mon, 19 Jun 2017 09:11:34 -0400
>>> Pradeep Jagadeesh <pradeepkiruvale@gmail.com> wrote:
>>>
>>>> This patch factor out the duplicate qmp throttle interface code
>>>> that was present in both block and fsdev device files.
>>>>
>>>
>>> The text is obviously wrong. I don't see any duplicate code below.
>> OK, I will fix this.
>>>
>>> It is more something like: let's factor out the code that will be used
>>> by the existing QMP throttling API for block devices and the future
>>> QMP throttling API for fs devices.
>>>
>>> Please move the HMP part to another patch, as asked during v4 review.
>> I have moved the hmp patches for the fsdev into a separate patch. Do you
>> want me to push this also into a separate patch?
>
> Yes. These changes aren't related and theoretically belong to separate
> sub-maintainer trees.
OK, I will split the commit and make separate patches.

-Pradeep
>
>>>
>>> Also, blockdev.c and hmp.c do have maintainers. You should Cc: them each
>>> time because they know better than me and even if these patches are carried
>>> through my tree, I won't do it without an ack from them.
>>
>> OK, I will add them next time on.
>>
>> -Pradeep
>>
>>> Cc'ing Markus for blockdev.c and David for hmp.c.
>>>
>>>> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
>>>> ---
>>>>  blockdev.c                      | 53 +++---------------------------------
>>>>  hmp.c                           | 21 ++++++++++-----
>>>>  include/qemu/throttle-options.h |  3 +++
>>>>  util/throttle.c                 | 60 +++++++++++++++++++++++++++++++++++++++++
>>>>  4 files changed, 81 insertions(+), 56 deletions(-)
>>>>
>>>> diff --git a/blockdev.c b/blockdev.c
>>>> index 5db9e5c..3d06e9e 100644
>>>> --- a/blockdev.c
>>>> +++ b/blockdev.c
>>>> @@ -2593,6 +2593,7 @@ void qmp_block_set_io_throttle(BlockIOThrottle *arg, Error **errp)
>>>>      BlockDriverState *bs;
>>>>      BlockBackend *blk;
>>>>      AioContext *aio_context;
>>>> +    IOThrottle *iothrottle;
>>>>
>>>>      blk = qmp_get_blk(arg->has_device ? arg->device : NULL,
>>>>                        arg->has_id ? arg->id : NULL,
>>>> @@ -2610,56 +2611,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;
>>>> -    }
>>>> +    iothrottle = qapi_BlockIOThrottle_base(arg);
>>>> +    throttle_set_io_limits(&cfg, iothrottle);
>>>>
>>>>      if (!throttle_is_valid(&cfg, errp)) {
>>>>          goto out;
>>>> diff --git a/hmp.c b/hmp.c
>>>> index 8c72c58..220d301 100644
>>>> --- a/hmp.c
>>>> +++ b/hmp.c
>>>> @@ -1749,20 +1749,29 @@ void hmp_change(Monitor *mon, const QDict *qdict)
>>>>      hmp_handle_error(mon, &err);
>>>>  }
>>>>
>>>> +static void hmp_initialize_io_throttle(IOThrottle *iot, const QDict *qdict)
>>>> +{
>>>> +    iot->has_id = true;
>>>> +    iot->id = (char *) qdict_get_str(qdict, "id");
>>>> +    iot->bps = qdict_get_int(qdict, "bps");
>>>> +    iot->bps_rd = qdict_get_int(qdict, "bps_rd");
>>>> +    iot->bps_wr = qdict_get_int(qdict, "bps_wr");
>>>> +    iot->iops = qdict_get_int(qdict, "iops");
>>>> +    iot->iops_rd = qdict_get_int(qdict, "iops_rd");
>>>> +    iot->iops_wr = qdict_get_int(qdict, "iops_wr");
>>>> +}
>>>> +
>>>>  void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict)
>>>>  {
>>>>      Error *err = NULL;
>>>> +    IOThrottle *iothrottle;
>>>>      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"),
>>>>      };
>>>>
>>>> +    iothrottle = qapi_BlockIOThrottle_base(&throttle);
>>>> +    hmp_initialize_io_throttle(iothrottle, qdict);
>>>>      qmp_block_set_io_throttle(&throttle, &err);
>>>>      hmp_handle_error(mon, &err);
>>>>  }
>>>> diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h
>>>> index 565553a..e94ea39 100644
>>>> --- a/include/qemu/throttle-options.h
>>>> +++ b/include/qemu/throttle-options.h
>>>> @@ -11,6 +11,7 @@
>>>>  #define THROTTLE_OPTIONS_H
>>>>
>>>>  #include "qemu/throttle.h"
>>>> +#include "qmp-commands.h"
>>>>
>>>>  #define THROTTLE_OPTS \
>>>>            { \
>>>> @@ -93,4 +94,6 @@
>>>>
>>>>  void throttle_parse_options(ThrottleConfig *, QemuOpts *);
>>>>
>>>> +void throttle_set_io_limits(ThrottleConfig *, IOThrottle *);
>>>> +
>>>>  #endif
>>>> diff --git a/util/throttle.c b/util/throttle.c
>>>> index ebe9dd0..2cf9ec5 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
>>>>   *
>>>> @@ -564,3 +565,62 @@ void throttle_parse_options(ThrottleConfig *throttle_cfg, QemuOpts *opts)
>>>>      throttle_cfg->op_size =
>>>>          qemu_opt_get_number(opts, "throttling.iops-size", 0);
>>>>  }
>>>> +
>>>> +/* set the throttle limits
>>>> + *
>>>> + * @arg: iothrottle limits
>>>> + * @cfg: throttle configuration
>>>> + */
>>>> +void throttle_set_io_limits(ThrottleConfig *cfg, IOThrottle *arg)
>>>> +{
>>>> +    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;
>>>> +    }
>>>> +}
>>>
>>
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2017-06-29 13:36 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-19 13:11 [Qemu-devel] [PATCH v5 0/5] fsdev: qmp interface for io throttling Pradeep Jagadeesh
2017-06-19 13:11 ` [Qemu-devel] [PATCH v5 1/5] throttle: factor out duplicate code Pradeep Jagadeesh
2017-06-20 15:46   ` Greg Kurz
2017-06-21 12:55   ` Alberto Garcia
2017-06-21 13:43     ` Greg Kurz
2017-06-21 14:21       ` Alberto Garcia
2017-06-22 14:38   ` Markus Armbruster
2017-06-22 16:23     ` Greg Kurz
2017-06-29  9:50     ` Pradeep Jagadeesh
2017-06-19 13:11 ` [Qemu-devel] [PATCH v5 2/5] qmp: Create IOThrottle structure Pradeep Jagadeesh
2017-06-20 15:51   ` Greg Kurz
2017-06-19 13:11 ` [Qemu-devel] [PATCH v5 3/5] qmp: refactor duplicate code Pradeep Jagadeesh
2017-06-20 16:05   ` Greg Kurz
2017-06-21  8:34     ` Pradeep Jagadeesh
2017-06-21 10:00       ` Greg Kurz
2017-06-29 13:35         ` Pradeep Jagadeesh
2017-06-19 13:11 ` [Qemu-devel] [PATCH v5 4/5] fsdev: hmp interface for throttling Pradeep Jagadeesh
2017-06-19 13:11 ` [Qemu-devel] [PATCH v5 5/5] fsdev: QMP " Pradeep Jagadeesh

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.