All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling
@ 2017-08-29 14:23 Pradeep Jagadeesh
  2017-08-29 14:23 ` [Qemu-devel] [PATCH v8 1/6] throttle: factor out duplicate code Pradeep Jagadeesh
                   ` (7 more replies)
  0 siblings, 8 replies; 23+ messages in thread
From: Pradeep Jagadeesh @ 2017-08-29 14:23 UTC (permalink / raw)
  To: eric blake, greg kurz
  Cc: Pradeep Jagadeesh, alberto garcia, Markus Armbruster,
	Dr. David Alan Gilbert, jani kokkonen, qemu-devel

These patches provide the qmp interface, to query the io throttle 
status of the all fsdev devices that are present in a vm.
also, it provides an interface to set the io throttle parameters of a
fsdev to a required value. some of the patches also remove the duplicate
code that was present in block and fsdev files. 

Pradeep Jagadeesh (6):
  throttle: factor out duplicate code
  qmp: Create IOThrottle structure
  throttle: move out function to reuse the code
  hmp: create a throttle initialization function for code reusability
  fsdev: hmp interface for throttling
  fsdev: QMP interface for throttling

 Makefile                        |   4 ++
 blockdev.c                      |  97 ++------------------------------
 fsdev/qemu-fsdev-dummy.c        |  11 ++++
 fsdev/qemu-fsdev-throttle.c     | 120 ++++++++++++++++++++++++++--------------
 fsdev/qemu-fsdev-throttle.h     |   8 ++-
 fsdev/qemu-fsdev.c              |  38 +++++++++++++
 hmp-commands-info.hx            |  18 ++++++
 hmp-commands.hx                 |  19 +++++++
 hmp.c                           |  81 +++++++++++++++++++++++++--
 hmp.h                           |   4 ++
 include/qemu/throttle-options.h |   7 +++
 include/qemu/throttle.h         |   4 +-
 include/qemu/typedefs.h         |   1 +
 monitor.c                       |   5 ++
 qapi-schema.json                |   3 +
 qapi/block-core.json            |  34 +++++++++---
 qapi/fsdev.json                 |  81 +++++++++++++++++++++++++++
 qmp.c                           |  14 +++++
 util/throttle.c                 | 110 ++++++++++++++++++++++++++++++++++++
 19 files changed, 505 insertions(+), 154 deletions(-)
 create mode 100644 qapi/fsdev.json

v0 -> v1:
 Addressed comments from Eric Blake, Greg Kurz and Daniel P.Berrange
 Mainly renaming the functions and removing the redundant code.

v1 -> v2:
 Addressed comments from Eric Blake and Greg Kurz.
 As per the suggestion I split the patches into smaller patches.
 Removed some more duplicate code.

v2 -> v3:
 Addresssed comments from Alberto Garcia.
 Changed the comment from block to iothrottle in the iothrottle.json 
 Added the dummy functions in qemu-fsdev-dummy.c to address the compilation
 issues that were observed.

v3 -> v4:
 Addressed comments from Eric Blake and Greg Kurz
 Re-ordered the patches
 Added the dummy functions in qmp.c to address the cross compilation issues

v4 -> v5:
  Addressed comments from Eric Blake and Greg Kurz
  Split the fsdev qmp patch into hmp and qmp related patches
  Moved the common functionalities to throttle.c instead of creating
  a new file

v5 -> v6:
  Addressed comments from Greg Kurz and Markus Armbruster
  Split the commits to specific to hmp and throttle as suggested by Greg
  Moved ThrottleConfig typedef to qemu/typedefs.h
  Addressed compilation issue on FreeBSD by adding flags in qmp.c

v6 -> v7:
  Addressed comments from Albert Garcia and Dr. David Alan Gilbert
  Fixed the hmp-commands-info.hx and hmp-commands.hx as per Dr. David's
  comments.
  Fixed the bug with the hmp fsdev_set_io_throttle and info fsdev_iothrottle  

v7 -> v8:
  Addressed comments from Markus Armbruster and Eric Blake
  Removed the iothrottle.json and pushed the iothrottle struct to
  block-core.json
-- 
1.8.3.1

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

* [Qemu-devel] [PATCH v8 1/6] throttle: factor out duplicate code
  2017-08-29 14:23 [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling Pradeep Jagadeesh
@ 2017-08-29 14:23 ` Pradeep Jagadeesh
  2017-08-29 14:23 ` [Qemu-devel] [PATCH v8 2/6] qmp: Create IOThrottle structure Pradeep Jagadeesh
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 23+ messages in thread
From: Pradeep Jagadeesh @ 2017-08-29 14:23 UTC (permalink / raw)
  To: eric blake, greg kurz
  Cc: Pradeep Jagadeesh, alberto garcia, Markus Armbruster,
	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>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
 blockdev.c                      | 44 +----------------------------------
 fsdev/qemu-fsdev-throttle.c     | 44 ++---------------------------------
 include/qemu/throttle-options.h |  4 ++++
 include/qemu/throttle.h         |  4 ++--
 include/qemu/typedefs.h         |  1 +
 util/throttle.c                 | 51 +++++++++++++++++++++++++++++++++++++++++
 6 files changed, 61 insertions(+), 87 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 02cd69b..780ae58 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -388,49 +388,7 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
     }
 
     if (throttle_cfg) {
-        throttle_config_init(throttle_cfg);
-        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
-            qemu_opt_get_number(opts, "throttling.bps-total", 0);
-        throttle_cfg->buckets[THROTTLE_BPS_READ].avg  =
-            qemu_opt_get_number(opts, "throttling.bps-read", 0);
-        throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
-            qemu_opt_get_number(opts, "throttling.bps-write", 0);
-        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
-            qemu_opt_get_number(opts, "throttling.iops-total", 0);
-        throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
-            qemu_opt_get_number(opts, "throttling.iops-read", 0);
-        throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
-            qemu_opt_get_number(opts, "throttling.iops-write", 0);
-
-        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
-            qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
-        throttle_cfg->buckets[THROTTLE_BPS_READ].max  =
-            qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
-        throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
-            qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
-        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
-            qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
-        throttle_cfg->buckets[THROTTLE_OPS_READ].max =
-            qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
-        throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
-            qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
-
-        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
-            qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
-        throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length  =
-            qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
-        throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
-            qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
-        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
-            qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
-        throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
-            qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
-        throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
-            qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
-
-        throttle_cfg->op_size =
-            qemu_opt_get_number(opts, "throttling.iops-size", 0);
-
+        throttle_parse_options(throttle_cfg, opts);
         if (!throttle_is_valid(throttle_cfg, errp)) {
             return;
         }
diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
index 49eebb5..0e6fb86 100644
--- a/fsdev/qemu-fsdev-throttle.c
+++ b/fsdev/qemu-fsdev-throttle.c
@@ -16,6 +16,7 @@
 #include "qemu/error-report.h"
 #include "qemu-fsdev-throttle.h"
 #include "qemu/iov.h"
+#include "qemu/throttle-options.h"
 
 static void fsdev_throttle_read_timer_cb(void *opaque)
 {
@@ -31,48 +32,7 @@ static void fsdev_throttle_write_timer_cb(void *opaque)
 
 void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error **errp)
 {
-    throttle_config_init(&fst->cfg);
-    fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =
-        qemu_opt_get_number(opts, "throttling.bps-total", 0);
-    fst->cfg.buckets[THROTTLE_BPS_READ].avg  =
-        qemu_opt_get_number(opts, "throttling.bps-read", 0);
-    fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =
-        qemu_opt_get_number(opts, "throttling.bps-write", 0);
-    fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =
-        qemu_opt_get_number(opts, "throttling.iops-total", 0);
-    fst->cfg.buckets[THROTTLE_OPS_READ].avg =
-        qemu_opt_get_number(opts, "throttling.iops-read", 0);
-    fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =
-        qemu_opt_get_number(opts, "throttling.iops-write", 0);
-
-    fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =
-        qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
-    fst->cfg.buckets[THROTTLE_BPS_READ].max  =
-        qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
-    fst->cfg.buckets[THROTTLE_BPS_WRITE].max =
-        qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
-    fst->cfg.buckets[THROTTLE_OPS_TOTAL].max =
-        qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
-    fst->cfg.buckets[THROTTLE_OPS_READ].max =
-        qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
-    fst->cfg.buckets[THROTTLE_OPS_WRITE].max =
-        qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
-
-    fst->cfg.buckets[THROTTLE_BPS_TOTAL].burst_length =
-        qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
-    fst->cfg.buckets[THROTTLE_BPS_READ].burst_length  =
-        qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
-    fst->cfg.buckets[THROTTLE_BPS_WRITE].burst_length =
-        qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
-    fst->cfg.buckets[THROTTLE_OPS_TOTAL].burst_length =
-        qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
-    fst->cfg.buckets[THROTTLE_OPS_READ].burst_length =
-        qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
-    fst->cfg.buckets[THROTTLE_OPS_WRITE].burst_length =
-        qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
-    fst->cfg.op_size =
-        qemu_opt_get_number(opts, "throttling.iops-size", 0);
-
+    throttle_parse_options(&fst->cfg, opts);
     throttle_is_valid(&fst->cfg, errp);
 }
 
diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h
index 3133d1c..f63d38c 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 "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/include/qemu/throttle.h b/include/qemu/throttle.h
index d056008..75d930c 100644
--- a/include/qemu/throttle.h
+++ b/include/qemu/throttle.h
@@ -89,10 +89,10 @@ typedef struct LeakyBucket {
  * However it allows to keep the code clean and the bucket field is reset to
  * zero at the right time.
  */
-typedef struct ThrottleConfig {
+struct ThrottleConfig {
     LeakyBucket buckets[BUCKETS_COUNT]; /* leaky buckets */
     uint64_t op_size;         /* size of an operation in bytes */
-} ThrottleConfig;
+};
 
 typedef struct ThrottleState {
     ThrottleConfig cfg;       /* configuration */
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 39bc835..90fe0f9 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -100,6 +100,7 @@ typedef struct uWireSlave uWireSlave;
 typedef struct VirtIODevice VirtIODevice;
 typedef struct Visitor Visitor;
 typedef struct node_info NodeInfo;
+typedef struct ThrottleConfig ThrottleConfig;
 typedef void SaveStateHandler(QEMUFile *f, void *opaque);
 typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);
 
diff --git a/util/throttle.c b/util/throttle.c
index b2a52b8..95c2ecf 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
  *
@@ -502,3 +503,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] 23+ messages in thread

* [Qemu-devel] [PATCH v8 2/6] qmp: Create IOThrottle structure
  2017-08-29 14:23 [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling Pradeep Jagadeesh
  2017-08-29 14:23 ` [Qemu-devel] [PATCH v8 1/6] throttle: factor out duplicate code Pradeep Jagadeesh
@ 2017-08-29 14:23 ` Pradeep Jagadeesh
  2017-08-29 14:23 ` [Qemu-devel] [PATCH v8 3/6] throttle: move out function to reuse the code Pradeep Jagadeesh
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 23+ messages in thread
From: Pradeep Jagadeesh @ 2017-08-29 14:23 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>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
 qapi/block-core.json | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 833c602..95bbc5e 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1819,11 +1819,13 @@
   'data': 'BlockIOThrottle' }
 
 ##
-# @BlockIOThrottle:
-#
-# A set of parameters describing block throttling.
+# == QAPI IOThrottle definitions
+##
+
+##
+# @IOThrottle:
 #
-# @device: Block device name (deprecated, use @id instead)
+# A set of parameters describing IO throttling
 #
 # @id: The name or QOM path of the guest device (since: 2.8)
 #
@@ -1889,12 +1891,11 @@
 #
 # @iops_size: an I/O size in bytes (Since 1.7)
 #
-# @group: throttle group name (Since 2.4)
 #
-# Since: 1.1
+# Since: 2.11
 ##
-{ 'struct': 'BlockIOThrottle',
-  'data': { '*device': 'str', '*id': 'str', 'bps': 'int', 'bps_rd': 'int',
+{ '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',
@@ -1902,7 +1903,22 @@
             '*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' } }
+            '*iops_size': 'int' } }
+
+##
+# @BlockIOThrottle:
+#
+# A set of parameters describing block throttling.
+#
+# @device: Block device name (deprecated, use @id instead)
+#
+# @group: throttle group name (Since 2.4)
+#
+# Since: 1.1
+##
+{ 'struct': 'BlockIOThrottle',
+  'base': 'IOThrottle',
+  'data': { '*device': 'str', '*group': 'str' } }
 
 ##
 # @block-stream:
-- 
1.8.3.1

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

* [Qemu-devel] [PATCH v8 3/6] throttle: move out function to reuse the code
  2017-08-29 14:23 [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling Pradeep Jagadeesh
  2017-08-29 14:23 ` [Qemu-devel] [PATCH v8 1/6] throttle: factor out duplicate code Pradeep Jagadeesh
  2017-08-29 14:23 ` [Qemu-devel] [PATCH v8 2/6] qmp: Create IOThrottle structure Pradeep Jagadeesh
@ 2017-08-29 14:23 ` Pradeep Jagadeesh
  2017-08-29 14:23 ` [Qemu-devel] [PATCH v8 4/6] hmp: create a throttle initialization function for code reusability Pradeep Jagadeesh
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 23+ messages in thread
From: Pradeep Jagadeesh @ 2017-08-29 14:23 UTC (permalink / raw)
  To: eric blake, greg kurz
  Cc: Pradeep Jagadeesh, alberto garcia, Markus Armbruster,
	jani kokkonen, qemu-devel

This patch move out the throttle code to util/throttle.c to maximize
the reusability of the code.The same code is also used by fsdev.

Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
---
 blockdev.c                      | 53 +++---------------------------------
 include/qemu/throttle-options.h |  3 +++
 util/throttle.c                 | 59 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 50 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 780ae58..1caf2e0 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2571,6 +2571,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,
@@ -2588,56 +2589,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/include/qemu/throttle-options.h b/include/qemu/throttle-options.h
index f63d38c..a9deb8e 100644
--- a/include/qemu/throttle-options.h
+++ b/include/qemu/throttle-options.h
@@ -11,6 +11,7 @@
 #define THROTTLE_OPTIONS_H
 
 #include "typedefs.h"
+#include "qapi-types.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 95c2ecf..2d00532 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -553,3 +553,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] 23+ messages in thread

* [Qemu-devel] [PATCH v8 4/6] hmp: create a throttle initialization function for code reusability
  2017-08-29 14:23 [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling Pradeep Jagadeesh
                   ` (2 preceding siblings ...)
  2017-08-29 14:23 ` [Qemu-devel] [PATCH v8 3/6] throttle: move out function to reuse the code Pradeep Jagadeesh
@ 2017-08-29 14:23 ` Pradeep Jagadeesh
  2017-08-29 14:23 ` [Qemu-devel] [PATCH v8 5/6] fsdev: hmp interface for throttling Pradeep Jagadeesh
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 23+ messages in thread
From: Pradeep Jagadeesh @ 2017-08-29 14:23 UTC (permalink / raw)
  To: eric blake, greg kurz
  Cc: Pradeep Jagadeesh, alberto garcia, Dr. David Alan Gilbert,
	jani kokkonen, qemu-devel

This patch creates a throttle initialization function to maximize the
code reusability. The same code is also used by fsdev.

Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
---
 hmp.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/hmp.c b/hmp.c
index fd80dce..2dbfb80 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1758,20 +1758,27 @@ 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->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);
 }
-- 
1.8.3.1

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

* [Qemu-devel] [PATCH v8 5/6] fsdev: hmp interface for throttling
  2017-08-29 14:23 [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling Pradeep Jagadeesh
                   ` (3 preceding siblings ...)
  2017-08-29 14:23 ` [Qemu-devel] [PATCH v8 4/6] hmp: create a throttle initialization function for code reusability Pradeep Jagadeesh
@ 2017-08-29 14:23 ` Pradeep Jagadeesh
  2017-08-30  9:38   ` Alberto Garcia
  2017-08-29 14:23 ` [Qemu-devel] [PATCH v8 6/6] fsdev: QMP " Pradeep Jagadeesh
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 23+ messages in thread
From: Pradeep Jagadeesh @ 2017-08-29 14:23 UTC (permalink / raw)
  To: eric blake, greg kurz
  Cc: Pradeep Jagadeesh, alberto garcia, Dr. David Alan Gilbert,
	jani kokkonen, qemu-devel

This patch introduces hmp interfaces for the fsdev
devices.

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

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index d9df238..07ed338 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -84,6 +84,24 @@ STEXI
 Show block device statistics.
 ETEXI
 
+#if defined(CONFIG_VIRTFS)
+
+    {
+        .name       = "fsdev_iothrottle",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show fsdev iothrottle information",
+        .cmd        = hmp_info_fsdev_iothrottle,
+    },
+
+#endif
+
+STEXI
+@item info fsdev_iothrottle
+@findex fsdev_iothrottle
+Show fsdev device throttle info.
+ETEXI
+
     {
         .name       = "block-jobs",
         .args_type  = "",
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 1941e19..aef9f79 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1680,6 +1680,25 @@ STEXI
 Change I/O throttle limits for a block drive to @var{bps} @var{bps_rd} @var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
 ETEXI
 
+#if defined(CONFIG_VIRTFS)
+
+    {
+        .name       = "fsdev_set_io_throttle",
+        .args_type  = "device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l",
+        .params     = "device bps bps_rd bps_wr iops iops_rd iops_wr",
+        .help       = "change I/O throttle limits for a fs devices",
+        .cmd        = hmp_fsdev_set_io_throttle,
+    },
+
+#endif
+
+STEXI
+@item fsdev_set_io_throttle @var{device} @var{bps} @var{bps_rd} @var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
+@findex fsdev_set_io_throttle
+Change I/O throttle limits for a fs devices to @var{bps} @var{bps_rd} @var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
+ETEXI
+
+
     {
         .name       = "set_password",
         .args_type  = "protocol:s,password:s,connected:s?",
diff --git a/hmp.c b/hmp.c
index 2dbfb80..712c6a3 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1783,6 +1783,68 @@ 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 = {
+        .has_id = true,
+        .id = (char *) qdict_get_str(qdict, "device"),
+    };
+
+    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)
+{
+    monitor_printf(mon, "%s", fscfg->id);
+    monitor_printf(mon, "    I/O throttling:"
+                   " bps=%" PRId64
+                   " bps_rd=%" PRId64  " bps_wr=%" PRId64
+                   " bps_max=%" PRId64
+                   " bps_rd_max=%" PRId64
+                   " bps_wr_max=%" PRId64
+                   " iops=%" PRId64 " iops_rd=%" PRId64
+                   " iops_wr=%" PRId64
+                   " iops_max=%" PRId64
+                   " iops_rd_max=%" PRId64
+                   " iops_wr_max=%" PRId64
+                   " iops_size=%" PRId64
+                   "\n",
+                   fscfg->bps,
+                   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_info_fsdev_iothrottle(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+    IOThrottleList *fsdev_list, *info;
+    fsdev_list = qmp_query_fsdev_io_throttle(&err);
+
+    for (info = fsdev_list; info; info = info->next) {
+        print_fsdev_throttle_config(mon, info->value, err);
+    }
+    qapi_free_IOThrottleList(fsdev_list);
+}
+
+#endif
+
 void hmp_block_stream(Monitor *mon, const QDict *qdict)
 {
     Error *error = NULL;
diff --git a/hmp.h b/hmp.h
index 1ff4552..d700d7d 100644
--- a/hmp.h
+++ b/hmp.h
@@ -81,6 +81,10 @@ void hmp_set_password(Monitor *mon, const QDict *qdict);
 void hmp_expire_password(Monitor *mon, const QDict *qdict);
 void hmp_eject(Monitor *mon, const QDict *qdict);
 void hmp_change(Monitor *mon, const QDict *qdict);
+#ifdef CONFIG_VIRTFS
+void hmp_fsdev_set_io_throttle(Monitor *mon, const QDict *qdict);
+void hmp_info_fsdev_iothrottle(Monitor *mon, const QDict *qdict);
+#endif
 void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict);
 void hmp_block_stream(Monitor *mon, const QDict *qdict);
 void hmp_block_job_set_speed(Monitor *mon, const QDict *qdict);
-- 
1.8.3.1

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

* [Qemu-devel] [PATCH v8 6/6] fsdev: QMP interface for throttling
  2017-08-29 14:23 [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling Pradeep Jagadeesh
                   ` (4 preceding siblings ...)
  2017-08-29 14:23 ` [Qemu-devel] [PATCH v8 5/6] fsdev: hmp interface for throttling Pradeep Jagadeesh
@ 2017-08-29 14:23 ` Pradeep Jagadeesh
  2017-08-29 14:36 ` [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling Alberto Garcia
  2017-08-30 12:07 ` Alberto Garcia
  7 siblings, 0 replies; 23+ messages in thread
From: Pradeep Jagadeesh @ 2017-08-29 14:23 UTC (permalink / raw)
  To: eric blake, greg kurz
  Cc: Pradeep Jagadeesh, alberto garcia, Dr. David Alan Gilbert,
	jani kokkonen, qemu-devel

This patch introduces qmp interfaces for the fsdev
devices. This provides two interfaces one
for querying info of all the fsdev devices. The second one
to set the IO limits for the required fsdev device.

Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
---
 Makefile                    |  4 +++
 fsdev/qemu-fsdev-dummy.c    | 11 ++++++
 fsdev/qemu-fsdev-throttle.c | 76 ++++++++++++++++++++++++++++++++++++++++++
 fsdev/qemu-fsdev-throttle.h |  8 +++--
 fsdev/qemu-fsdev.c          | 38 +++++++++++++++++++++
 monitor.c                   |  5 +++
 qapi-schema.json            |  3 ++
 qapi/fsdev.json             | 81 +++++++++++++++++++++++++++++++++++++++++++++
 qmp.c                       | 14 ++++++++
 9 files changed, 238 insertions(+), 2 deletions(-)
 create mode 100644 qapi/fsdev.json

diff --git a/Makefile b/Makefile
index 81447b1..ec31ffa 100644
--- a/Makefile
+++ b/Makefile
@@ -414,6 +414,10 @@ qapi-modules = $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/qapi/common.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)
 	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \
diff --git a/fsdev/qemu-fsdev-dummy.c b/fsdev/qemu-fsdev-dummy.c
index 6dc0fbc..28c82d2 100644
--- a/fsdev/qemu-fsdev-dummy.c
+++ b/fsdev/qemu-fsdev-dummy.c
@@ -14,8 +14,19 @@
 #include "qemu-fsdev.h"
 #include "qemu/config-file.h"
 #include "qemu/module.h"
+#include "qmp-commands.h"
 
 int qemu_fsdev_add(QemuOpts *opts)
 {
     return 0;
 }
+
+void qmp_fsdev_set_io_throttle(IOThrottle *arg, Error **errp)
+{
+    return;
+}
+
+IOThrottleList *qmp_query_fsdev_io_throttle(Error **errp)
+{
+    return NULL;
+}
diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
index 0e6fb86..184ed4c 100644
--- a/fsdev/qemu-fsdev-throttle.c
+++ b/fsdev/qemu-fsdev-throttle.c
@@ -16,6 +16,7 @@
 #include "qemu/error-report.h"
 #include "qemu-fsdev-throttle.h"
 #include "qemu/iov.h"
+#include "qemu/main-loop.h"
 #include "qemu/throttle-options.h"
 
 static void fsdev_throttle_read_timer_cb(void *opaque)
@@ -30,6 +31,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 e418643..1db9125 100644
--- a/fsdev/qemu-fsdev-throttle.h
+++ b/fsdev/qemu-fsdev-throttle.h
@@ -15,8 +15,6 @@
 #ifndef _FSDEV_THROTTLE_H
 #define _FSDEV_THROTTLE_H
 
-#include "block/aio.h"
-#include "qemu/main-loop.h"
 #include "qemu/coroutine.h"
 #include "qapi/error.h"
 #include "qemu/throttle.h"
@@ -36,4 +34,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..63dc1b5 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,40 @@ 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) {
+        error_setg(errp, "Not a valid fsdev device");
+        return;
+    }
+
+    fsdev_set_io_throttle(arg, &fse->fst, errp);
+}
+
+IOThrottleList *qmp_query_fsdev_io_throttle(Error **errp)
+{
+    IOThrottleList *head = NULL, *p_next;
+    struct FsDriverListEntry *fsle;
+    Error *local_err = NULL;
+
+    QTAILQ_FOREACH(fsle, &fsdriver_entries, next) {
+        p_next = g_new0(IOThrottleList, 1);
+        fsdev_get_io_throttle(&fsle->fse.fst, &p_next->value,
+                              fsle->fse.fsdev_id, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            g_free(p_next);
+            qapi_free_IOThrottleList(head);
+            return NULL;
+        }
+
+        p_next->next = head;
+        head = p_next;
+    }
+    return head;
+}
diff --git a/monitor.c b/monitor.c
index e0f8801..eebda1c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -998,6 +998,11 @@ static void qmp_unregister_commands_hack(void)
     && !defined(TARGET_S390X)
     qmp_unregister_command(&qmp_commands, "query-cpu-definitions");
 #endif
+#ifndef CONFIG_VIRTFS
+    qmp_unregister_command(&qmp_commands, "fsdev-set-io-throttle");
+    qmp_unregister_command(&qmp_commands, "query-fsdev-io-throttle");
+#endif
+
 }
 
 void monitor_init_qmp_commands(void)
diff --git a/qapi-schema.json b/qapi-schema.json
index 802ea53..8cf8140 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..21d074d
--- /dev/null
+++ b/qapi/fsdev.json
@@ -0,0 +1,81 @@
+# -*- Mode: Python -*-
+
+##
+# == QAPI fsdev definitions
+##
+
+##
+# @fsdev-set-io-throttle:
+#
+# Change I/O limits for a 9p/fsdev device.
+#
+# I/O limits can be enabled by setting throttle value to non-zero number.
+#
+# I/O limits can be disabled by setting all throttle values to 0.
+#
+# Returns: Nothing on success
+#          If @device is not a valid fsdev device, GenericError
+#
+# Since: 2.11
+#
+# Example:
+#
+# -> { "execute": "fsdev-set-io-throttle",
+#      "arguments": { "id": "id0-1-0",
+#                     "bps": 1000000,
+#                     "bps_rd": 0,
+#                     "bps_wr": 0,
+#                     "iops": 0,
+#                     "iops_rd": 0,
+#                     "iops_wr": 0,
+#                     "bps_max": 8000000,
+#                     "bps_rd_max": 0,
+#                     "bps_wr_max": 0,
+#                     "iops_max": 0,
+#                     "iops_rd_max": 0,
+#                     "iops_wr_max": 0,
+#                     "bps_max_length": 60,
+#                     "iops_size": 0 } }
+# <- { "returns": {} }
+##
+{ 'command': 'fsdev-set-io-throttle', 'boxed': true,
+  'data': 'IOThrottle' }
+##
+# @query-fsdev-io-throttle:
+#
+# Returns: a list of @IOThrottle describing I/O throttle
+#          values of each fsdev device
+#
+# Since: 2.11
+#
+# Example:
+#
+# -> { "Execute": "query-fsdev-io-throttle" }
+# <- { "returns" : [
+#          {
+#              "id": "id0-hd0",
+#              "bps":1000000,
+#              "bps_rd":0,
+#              "bps_wr":0,
+#              "iops":1000000,
+#              "iops_rd":0,
+#              "iops_wr":0,
+#              "bps_max": 8000000,
+#              "bps_rd_max": 0,
+#              "bps_wr_max": 0,
+#              "iops_max": 0,
+#              "iops_rd_max": 0,
+#              "iops_wr_max": 0,
+#              "bps_max_length": 0,
+#              "bps_rd_max_length": 0,
+#              "bps_wr_max_length": 0,
+#              "iops_max_length": 0,
+#              "iops_rd_max_length": 0,
+#              "iops_wr_max_length": 0,
+#              "iops_size": 0
+#            }
+#          ]
+#      }
+#
+##
+{ 'command': 'query-fsdev-io-throttle', 'returns': [ 'IOThrottle' ] }
diff --git a/qmp.c b/qmp.c
index b86201e..eed91e5 100644
--- a/qmp.c
+++ b/qmp.c
@@ -130,6 +130,20 @@ void qmp_cpu_add(int64_t id, Error **errp)
     }
 }
 
+#if defined(_WIN64) || defined(_WIN32) || defined(__FreeBSD__)
+
+void qmp_fsdev_set_io_throttle(IOThrottle *arg, Error **errp)
+{
+    return;
+}
+
+IOThrottleList *qmp_query_fsdev_io_throttle(Error **errp)
+{
+    return NULL;
+}
+
+#endif
+
 #ifndef CONFIG_VNC
 /* If VNC support is enabled, the "true" query-vnc command is
    defined in the VNC subsystem */
-- 
1.8.3.1

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

* Re: [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling
  2017-08-29 14:23 [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling Pradeep Jagadeesh
                   ` (5 preceding siblings ...)
  2017-08-29 14:23 ` [Qemu-devel] [PATCH v8 6/6] fsdev: QMP " Pradeep Jagadeesh
@ 2017-08-29 14:36 ` Alberto Garcia
  2017-08-29 14:39   ` Pradeep Jagadeesh
  2017-08-30 12:07 ` Alberto Garcia
  7 siblings, 1 reply; 23+ messages in thread
From: Alberto Garcia @ 2017-08-29 14:36 UTC (permalink / raw)
  To: Pradeep Jagadeesh, eric blake, greg kurz
  Cc: Pradeep Jagadeesh, Markus Armbruster, Dr. David Alan Gilbert,
	jani kokkonen, qemu-devel

On Tue 29 Aug 2017 04:23:01 PM CEST, Pradeep Jagadeesh wrote:
> 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. 

Oops, I was just reviewing the previous version of this series, but it
looks like you just moved code around, so my comments still apply.

Anyway, this should be v9, shouldn't it? v8 was published weeks ago:

   https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg01043.html

Berto

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

* Re: [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling
  2017-08-29 14:36 ` [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling Alberto Garcia
@ 2017-08-29 14:39   ` Pradeep Jagadeesh
  0 siblings, 0 replies; 23+ messages in thread
From: Pradeep Jagadeesh @ 2017-08-29 14:39 UTC (permalink / raw)
  To: Alberto Garcia, Pradeep Jagadeesh, eric blake, greg kurz
  Cc: Markus Armbruster, Dr. David Alan Gilbert, jani kokkonen, qemu-devel

On 8/29/2017 4:36 PM, Alberto Garcia wrote:
> On Tue 29 Aug 2017 04:23:01 PM CEST, Pradeep Jagadeesh wrote:
>> 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.
>
> Oops, I was just reviewing the previous version of this series, but it
> looks like you just moved code around, so my comments still apply.
>
> Anyway, this should be v9, shouldn't it? v8 was published weeks ago:
>
>    https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg01043.html

Hmm, sorry it should be v9, I missed. I will resend the patches with 
newer version.

Regards,
Pradeep
>
> Berto
>

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

* Re: [Qemu-devel] [PATCH v8 5/6] fsdev: hmp interface for throttling
  2017-08-29 14:23 ` [Qemu-devel] [PATCH v8 5/6] fsdev: hmp interface for throttling Pradeep Jagadeesh
@ 2017-08-30  9:38   ` Alberto Garcia
  2017-09-04 12:22     ` Pradeep Jagadeesh
  0 siblings, 1 reply; 23+ messages in thread
From: Alberto Garcia @ 2017-08-30  9:38 UTC (permalink / raw)
  To: Pradeep Jagadeesh, eric blake, greg kurz
  Cc: Pradeep Jagadeesh, Dr. David Alan Gilbert, jani kokkonen, qemu-devel

On Tue 29 Aug 2017 04:23:06 PM CEST, Pradeep Jagadeesh wrote:

> +static void print_fsdev_throttle_config(Monitor *mon, IOThrottle *fscfg,
> +                                       Error *err)
> +{
> +    monitor_printf(mon, "%s", fscfg->id);
> +    monitor_printf(mon, "    I/O throttling:"
> +                   " bps=%" PRId64
> +                   " bps_rd=%" PRId64  " bps_wr=%" PRId64
> +                   " bps_max=%" PRId64
> +                   " bps_rd_max=%" PRId64
> +                   " bps_wr_max=%" PRId64
> +                   " iops=%" PRId64 " iops_rd=%" PRId64
> +                   " iops_wr=%" PRId64
> +                   " iops_max=%" PRId64
> +                   " iops_rd_max=%" PRId64
> +                   " iops_wr_max=%" PRId64
> +                   " iops_size=%" PRId64
> +                   "\n",
> +                   fscfg->bps,
> +                   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_info_fsdev_iothrottle(Monitor *mon, const QDict *qdict)
> +{
> +    Error *err = NULL;
> +    IOThrottleList *fsdev_list, *info;
> +    fsdev_list = qmp_query_fsdev_io_throttle(&err);
> +
> +    for (info = fsdev_list; info; info = info->next) {
> +        print_fsdev_throttle_config(mon, info->value, err);
> +    }
> +    qapi_free_IOThrottleList(fsdev_list);
> +}

I don't think what you're doing with the Error here is right:

   - You store the error returned by qmp_query_fsdev_io_throttle().
   - You report the error for _every_ fsdev in the list. That doesn't
     make much sense.
   - Furthermore, if there's an error then fsdev_list should be empty,
     so you're not reporting anything (and you're leaking the error).
   - Even if the list was not empty, hmp_handle_error() frees the error
     after reporting it. Therefore the second item in the list would
     try to report an error that has already been freed.

Berto

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

* Re: [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling
  2017-08-29 14:23 [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling Pradeep Jagadeesh
                   ` (6 preceding siblings ...)
  2017-08-29 14:36 ` [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling Alberto Garcia
@ 2017-08-30 12:07 ` Alberto Garcia
  2017-08-30 12:10   ` Pradeep Jagadeesh
  2017-08-30 14:28   ` Greg Kurz
  7 siblings, 2 replies; 23+ messages in thread
From: Alberto Garcia @ 2017-08-30 12:07 UTC (permalink / raw)
  To: Pradeep Jagadeesh, eric blake, greg kurz
  Cc: Pradeep Jagadeesh, Markus Armbruster, Dr. David Alan Gilbert,
	jani kokkonen, qemu-devel

On Tue 29 Aug 2017 04:23:01 PM CEST, Pradeep Jagadeesh wrote:
> These patches provide the qmp interface, to query the io throttle 
> status of the all fsdev devices that are present in a vm.

I'm trying to read from an 9p share that has limits set with hmp
fsdev_set_io_throttle and I'm having some problems.

For example if I'm reading a large file and I change the I/O limits
while the file is still being read then the guest process freezes.

Can you try to reproduce that scenario?

Berto

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

* Re: [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling
  2017-08-30 12:07 ` Alberto Garcia
@ 2017-08-30 12:10   ` Pradeep Jagadeesh
  2017-08-30 14:54     ` Alberto Garcia
  2017-08-30 14:28   ` Greg Kurz
  1 sibling, 1 reply; 23+ messages in thread
From: Pradeep Jagadeesh @ 2017-08-30 12:10 UTC (permalink / raw)
  To: Alberto Garcia, Pradeep Jagadeesh, eric blake, greg kurz
  Cc: Markus Armbruster, Dr. David Alan Gilbert, jani kokkonen, qemu-devel

On 8/30/2017 2:07 PM, Alberto Garcia wrote:
> On Tue 29 Aug 2017 04:23:01 PM CEST, Pradeep Jagadeesh wrote:
>> These patches provide the qmp interface, to query the io throttle
>> status of the all fsdev devices that are present in a vm.
>
> I'm trying to read from an 9p share that has limits set with hmp
> fsdev_set_io_throttle and I'm having some problems.
>
> For example if I'm reading a large file and I change the I/O limits
> while the file is still being read then the guest process freezes.
>
> Can you try to reproduce that scenario?
Thanks for letting me know about the issue.
OK, I will try to reproduce.have a look.

-Pradeep

>
> Berto
>

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

* Re: [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling
  2017-08-30 12:07 ` Alberto Garcia
  2017-08-30 12:10   ` Pradeep Jagadeesh
@ 2017-08-30 14:28   ` Greg Kurz
  1 sibling, 0 replies; 23+ messages in thread
From: Greg Kurz @ 2017-08-30 14:28 UTC (permalink / raw)
  To: Alberto Garcia
  Cc: Pradeep Jagadeesh, eric blake, Pradeep Jagadeesh,
	Markus Armbruster, Dr. David Alan Gilbert, jani kokkonen,
	qemu-devel

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

On Wed, 30 Aug 2017 14:07:59 +0200
Alberto Garcia <berto@igalia.com> wrote:

> On Tue 29 Aug 2017 04:23:01 PM CEST, Pradeep Jagadeesh wrote:
> > These patches provide the qmp interface, to query the io throttle 
> > status of the all fsdev devices that are present in a vm.  
> 
> I'm trying to read from an 9p share that has limits set with hmp
> fsdev_set_io_throttle and I'm having some problems.
> 
> For example if I'm reading a large file and I change the I/O limits
> while the file is still being read then the guest process freezes.
> 
> Can you try to reproduce that scenario?
> 
> Berto

Thanks Berto for the review and the testing ! I'm on vacation but I'll
jump in next week when I'm back and Pradeep has posted the next version.

Cheers,

--
Greg

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

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

* Re: [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling
  2017-08-30 12:10   ` Pradeep Jagadeesh
@ 2017-08-30 14:54     ` Alberto Garcia
  2017-08-30 15:07       ` Pradeep Jagadeesh
  0 siblings, 1 reply; 23+ messages in thread
From: Alberto Garcia @ 2017-08-30 14:54 UTC (permalink / raw)
  To: Pradeep Jagadeesh, Pradeep Jagadeesh, eric blake, greg kurz
  Cc: Markus Armbruster, Dr. David Alan Gilbert, jani kokkonen, qemu-devel

On Wed 30 Aug 2017 02:10:53 PM CEST, Pradeep Jagadeesh wrote:

>> I'm trying to read from an 9p share that has limits set with hmp
>> fsdev_set_io_throttle and I'm having some problems.

Here's one simple way to reproduce it:

1) Launch qemu with

   -fsdev local,security_model=none,id=fs0,path=/some/files
   -device virtio-9p-pci,fsdev=fs0,mount_tag=fs0

2) In the guest, mount the fs0 share in /mnt

3) Run this hmp command

   fsdev_set_io_throttle fs0 0 4096 0 0 0 0

4) In the guest, start reading some large file from the 9p share:

   dd if=/mnt/large_file of=/dev/null bs=4k iflag=direct status=progress

5) Check the progress, reading speed should be around 4k per second.

6) While dd is still running, change the I/O limits:

   fsdev_set_io_throttle fs0 0 8192 0 0 0 0

7) If you check the status of the dd command, reading should be faster
   now. Instead, it is stalled.

Berto

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

* Re: [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling
  2017-08-30 14:54     ` Alberto Garcia
@ 2017-08-30 15:07       ` Pradeep Jagadeesh
  2017-08-30 15:10         ` Alberto Garcia
  0 siblings, 1 reply; 23+ messages in thread
From: Pradeep Jagadeesh @ 2017-08-30 15:07 UTC (permalink / raw)
  To: Alberto Garcia, Pradeep Jagadeesh, eric blake, greg kurz
  Cc: Markus Armbruster, Dr. David Alan Gilbert, jani kokkonen, qemu-devel

On 8/30/2017 4:54 PM, Alberto Garcia wrote:
> On Wed 30 Aug 2017 02:10:53 PM CEST, Pradeep Jagadeesh wrote:
>
>>> I'm trying to read from an 9p share that has limits set with hmp
>>> fsdev_set_io_throttle and I'm having some problems.
>
> Here's one simple way to reproduce it:
>
> 1) Launch qemu with
>
>    -fsdev local,security_model=none,id=fs0,path=/some/files
>    -device virtio-9p-pci,fsdev=fs0,mount_tag=fs0
>
> 2) In the guest, mount the fs0 share in /mnt
>
> 3) Run this hmp command
>
>    fsdev_set_io_throttle fs0 0 4096 0 0 0 0
>
> 4) In the guest, start reading some large file from the 9p share:
>
>    dd if=/mnt/large_file of=/dev/null bs=4k iflag=direct status=progress
>
> 5) Check the progress, reading speed should be around 4k per second.
>
> 6) While dd is still running, change the I/O limits:
>
>    fsdev_set_io_throttle fs0 0 8192 0 0 0 0
>
> 7) If you check the status of the dd command, reading should be faster
>    now. Instead, it is stalled.

Thanks for the steps, I did reproduce the issue easily.
Looking into the code, may be we also need to try the same with the 
block devices.

-Pradeep
>
> Berto
>

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

* Re: [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling
  2017-08-30 15:07       ` Pradeep Jagadeesh
@ 2017-08-30 15:10         ` Alberto Garcia
  2017-08-30 15:12           ` Pradeep Jagadeesh
  0 siblings, 1 reply; 23+ messages in thread
From: Alberto Garcia @ 2017-08-30 15:10 UTC (permalink / raw)
  To: Pradeep Jagadeesh, Pradeep Jagadeesh, eric blake, greg kurz
  Cc: Markus Armbruster, Dr. David Alan Gilbert, jani kokkonen, qemu-devel

On Wed 30 Aug 2017 05:07:29 PM CEST, Pradeep Jagadeesh wrote:

> Thanks for the steps, I did reproduce the issue easily. Looking into
> the code, may be we also need to try the same with the block devices.

I did some tests and it was working fine, so I'd suspect of the fsdev
code first.

Berto

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

* Re: [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling
  2017-08-30 15:10         ` Alberto Garcia
@ 2017-08-30 15:12           ` Pradeep Jagadeesh
  2017-08-31 13:34             ` Alberto Garcia
  0 siblings, 1 reply; 23+ messages in thread
From: Pradeep Jagadeesh @ 2017-08-30 15:12 UTC (permalink / raw)
  To: Alberto Garcia, Pradeep Jagadeesh, eric blake, greg kurz
  Cc: Markus Armbruster, Dr. David Alan Gilbert, jani kokkonen, qemu-devel

On 8/30/2017 5:10 PM, Alberto Garcia wrote:
> On Wed 30 Aug 2017 05:07:29 PM CEST, Pradeep Jagadeesh wrote:
>
>> Thanks for the steps, I did reproduce the issue easily. Looking into
>> the code, may be we also need to try the same with the block devices.
>
> I did some tests and it was working fine, so I'd suspect of the fsdev
> code first.
>
OK, thanks for the clarification. I will look into fsdev code.

-Pradeep
> Berto
>

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

* Re: [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling
  2017-08-30 15:12           ` Pradeep Jagadeesh
@ 2017-08-31 13:34             ` Alberto Garcia
  2017-08-31 13:39               ` Pradeep Jagadeesh
  2017-09-01 12:44               ` Pradeep Jagadeesh
  0 siblings, 2 replies; 23+ messages in thread
From: Alberto Garcia @ 2017-08-31 13:34 UTC (permalink / raw)
  To: Pradeep Jagadeesh, Pradeep Jagadeesh, eric blake, greg kurz
  Cc: Markus Armbruster, Dr. David Alan Gilbert, jani kokkonen, qemu-devel

On Wed 30 Aug 2017 05:12:22 PM CEST, Pradeep Jagadeesh wrote:
> On 8/30/2017 5:10 PM, Alberto Garcia wrote:
>> On Wed 30 Aug 2017 05:07:29 PM CEST, Pradeep Jagadeesh wrote:
>>
>>> Thanks for the steps, I did reproduce the issue easily. Looking into
>>> the code, may be we also need to try the same with the block devices.
>>
>> I did some tests and it was working fine, so I'd suspect of the fsdev
>> code first.
>>
> OK, thanks for the clarification. I will look into fsdev code.

I just took a quick look at the code, the problem is almost certainly in
fsdev_set_io_throttle(): that doesn't simply update the config, it also
reinitializes the FsThrottle structure completely, creates new timers
and new throttled_reqs queues. If there were pending requests there
they're probably lost forever.

Take a look at blk_set_io_limits() and see how it is done for block
devices.

Berto

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

* Re: [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling
  2017-08-31 13:34             ` Alberto Garcia
@ 2017-08-31 13:39               ` Pradeep Jagadeesh
  2017-09-01 12:44               ` Pradeep Jagadeesh
  1 sibling, 0 replies; 23+ messages in thread
From: Pradeep Jagadeesh @ 2017-08-31 13:39 UTC (permalink / raw)
  To: Alberto Garcia, Pradeep Jagadeesh, eric blake, greg kurz
  Cc: Markus Armbruster, Dr. David Alan Gilbert, jani kokkonen, qemu-devel

On 8/31/2017 3:34 PM, Alberto Garcia wrote:
> On Wed 30 Aug 2017 05:12:22 PM CEST, Pradeep Jagadeesh wrote:
>> On 8/30/2017 5:10 PM, Alberto Garcia wrote:
>>> On Wed 30 Aug 2017 05:07:29 PM CEST, Pradeep Jagadeesh wrote:
>>>
>>>> Thanks for the steps, I did reproduce the issue easily. Looking into
>>>> the code, may be we also need to try the same with the block devices.
>>>
>>> I did some tests and it was working fine, so I'd suspect of the fsdev
>>> code first.
>>>
>> OK, thanks for the clarification. I will look into fsdev code.
>
> I just took a quick look at the code, the problem is almost certainly in
> fsdev_set_io_throttle(): that doesn't simply update the config, it also
> reinitializes the FsThrottle structure completely, creates new timers
> and new throttled_reqs queues. If there were pending requests there
> they're probably lost forever.
>
> Take a look at blk_set_io_limits() and see how it is done for block
> devices.
Yes, that is right. I had a look. Now I am figuring out how to 
initialize the timers without loosing the pending requests.
If I update the config when there is no IO going, it works fine.
When IO is going and try to update it hangs.

-Pradeep
>
> Berto
>

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

* Re: [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling
  2017-08-31 13:34             ` Alberto Garcia
  2017-08-31 13:39               ` Pradeep Jagadeesh
@ 2017-09-01 12:44               ` Pradeep Jagadeesh
  1 sibling, 0 replies; 23+ messages in thread
From: Pradeep Jagadeesh @ 2017-09-01 12:44 UTC (permalink / raw)
  To: Alberto Garcia, Pradeep Jagadeesh, eric blake, greg kurz
  Cc: Markus Armbruster, Dr. David Alan Gilbert, jani kokkonen, qemu-devel

On 8/31/2017 3:34 PM, Alberto Garcia wrote:
> On Wed 30 Aug 2017 05:12:22 PM CEST, Pradeep Jagadeesh wrote:
>> On 8/30/2017 5:10 PM, Alberto Garcia wrote:
>>> On Wed 30 Aug 2017 05:07:29 PM CEST, Pradeep Jagadeesh wrote:
>>>
>>>> Thanks for the steps, I did reproduce the issue easily. Looking into
>>>> the code, may be we also need to try the same with the block devices.
>>>
>>> I did some tests and it was working fine, so I'd suspect of the fsdev
>>> code first.
>>>
>> OK, thanks for the clarification. I will look into fsdev code.
>
> I just took a quick look at the code, the problem is almost certainly in
> fsdev_set_io_throttle(): that doesn't simply update the config, it also
> reinitializes the FsThrottle structure completely, creates new timers
> and new throttled_reqs queues. If there were pending requests there
> they're probably lost forever.
>
> Take a look at blk_set_io_limits() and see how it is done for block
> devices.
I fixed it. I am testing it. I was initializing the queues again.
But it just needs updation of throttle configuration.

-Pradeep
>
> Berto
>

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

* Re: [Qemu-devel] [PATCH v8 5/6] fsdev: hmp interface for throttling
  2017-08-30  9:38   ` Alberto Garcia
@ 2017-09-04 12:22     ` Pradeep Jagadeesh
  2017-09-04 13:05       ` Alberto Garcia
  0 siblings, 1 reply; 23+ messages in thread
From: Pradeep Jagadeesh @ 2017-09-04 12:22 UTC (permalink / raw)
  To: Alberto Garcia, Pradeep Jagadeesh, eric blake, greg kurz
  Cc: Dr. David Alan Gilbert, jani kokkonen, qemu-devel

On 8/30/2017 11:38 AM, Alberto Garcia wrote:
> On Tue 29 Aug 2017 04:23:06 PM CEST, Pradeep Jagadeesh wrote:
>
>> +static void print_fsdev_throttle_config(Monitor *mon, IOThrottle *fscfg,
>> +                                       Error *err)
>> +{
>> +    monitor_printf(mon, "%s", fscfg->id);
>> +    monitor_printf(mon, "    I/O throttling:"
>> +                   " bps=%" PRId64
>> +                   " bps_rd=%" PRId64  " bps_wr=%" PRId64
>> +                   " bps_max=%" PRId64
>> +                   " bps_rd_max=%" PRId64
>> +                   " bps_wr_max=%" PRId64
>> +                   " iops=%" PRId64 " iops_rd=%" PRId64
>> +                   " iops_wr=%" PRId64
>> +                   " iops_max=%" PRId64
>> +                   " iops_rd_max=%" PRId64
>> +                   " iops_wr_max=%" PRId64
>> +                   " iops_size=%" PRId64
>> +                   "\n",
>> +                   fscfg->bps,
>> +                   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_info_fsdev_iothrottle(Monitor *mon, const QDict *qdict)
>> +{
>> +    Error *err = NULL;
>> +    IOThrottleList *fsdev_list, *info;
>> +    fsdev_list = qmp_query_fsdev_io_throttle(&err);
>> +
>> +    for (info = fsdev_list; info; info = info->next) {
>> +        print_fsdev_throttle_config(mon, info->value, err);
>> +    }
>> +    qapi_free_IOThrottleList(fsdev_list);
>> +}
>
> I don't think what you're doing with the Error here is right:
>
>    - You store the error returned by qmp_query_fsdev_io_throttle().
>    - You report the error for _every_ fsdev in the list. That doesn't
>      make much sense.
>    - Furthermore, if there's an error then fsdev_list should be empty,
>      so you're not reporting anything (and you're leaking the error).
>    - Even if the list was not empty, hmp_handle_error() frees the error
>      after reporting it. Therefore the second item in the list would
>      try to report an error that has already been freed.
You mean something like below?

fsdev_list = qmp_query_fsdev_io_throttle(&err);
if (err) {
     error_report_err(err);
     return;
}

Regards,
Pradeep
>
> Berto
>

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

* Re: [Qemu-devel] [PATCH v8 5/6] fsdev: hmp interface for throttling
  2017-09-04 12:22     ` Pradeep Jagadeesh
@ 2017-09-04 13:05       ` Alberto Garcia
  0 siblings, 0 replies; 23+ messages in thread
From: Alberto Garcia @ 2017-09-04 13:05 UTC (permalink / raw)
  To: Pradeep Jagadeesh, Pradeep Jagadeesh, eric blake, greg kurz
  Cc: Dr. David Alan Gilbert, jani kokkonen, qemu-devel

On Mon 04 Sep 2017 02:22:40 PM CEST, Pradeep Jagadeesh wrote:
>>> +void hmp_info_fsdev_iothrottle(Monitor *mon, const QDict *qdict)
>>> +{
>>> +    Error *err = NULL;
>>> +    IOThrottleList *fsdev_list, *info;
>>> +    fsdev_list = qmp_query_fsdev_io_throttle(&err);
>>> +
>>> +    for (info = fsdev_list; info; info = info->next) {
>>> +        print_fsdev_throttle_config(mon, info->value, err);
>>> +    }
>>> +    qapi_free_IOThrottleList(fsdev_list);
>>> +}
>>
>> I don't think what you're doing with the Error here is right:
>>
>>    - You store the error returned by qmp_query_fsdev_io_throttle().
>>    - You report the error for _every_ fsdev in the list. That doesn't
>>      make much sense.
>>    - Furthermore, if there's an error then fsdev_list should be empty,
>>      so you're not reporting anything (and you're leaking the error).
>>    - Even if the list was not empty, hmp_handle_error() frees the error
>>      after reporting it. Therefore the second item in the list would
>>      try to report an error that has already been freed.
> You mean something like below?
>
> fsdev_list = qmp_query_fsdev_io_throttle(&err);
> if (err) {
>      error_report_err(err);
>      return;
> }

More or less, but there's hmp_handle_error() already, so you should use
it:

   void hmp_info_fsdev_iothrottle(Monitor *mon, const QDict *qdict)
   {
       Error *err = NULL;
       IOThrottleList *fsdev_list, *info;
       fsdev_list = qmp_query_fsdev_io_throttle(&err);

       for (info = fsdev_list; info; info = info->next) {
           print_fsdev_throttle_config(mon, info->value);
       }
       qapi_free_IOThrottleList(fsdev_list);

       hmp_handle_error(mon, &err);
   }

Anyway I just checked that fsdev_get_io_throttle() can never return an
Error, so why don't you remove the Error parameter from there
altogether?

You still need it in qmp_query_fsdev_io_throttle() because that's part
of the API, and hmp_info_fsdev_iothrottle() should have the code to
handle it like the one I just pasted, but fsdev_get_io_throttle() does
not need it, or does it?

Berto

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

* [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling
@ 2017-08-07 12:37 Pradeep Jagadeesh
  0 siblings, 0 replies; 23+ messages in thread
From: Pradeep Jagadeesh @ 2017-08-07 12:37 UTC (permalink / raw)
  To: eric blake, greg kurz
  Cc: Pradeep Jagadeesh, alberto garcia, Markus Armbruster,
	Dr. David Alan Gilbert, jani kokkonen, qemu-devel

These patches provide the qmp interface, to query the io throttle 
status of the all fsdev devices that are present in a vm.
also, it provides an interface to set the io throttle parameters of a
fsdev to a required value. some of the patches also remove the duplicate
code that was present in block and fsdev files. 

Pradeep Jagadeesh (6):
  throttle: factor out duplicate code
  qmp: Create IOThrottle structure
  throttle: move out function to reuse the code
  hmp: create a throttle initialization function for code reusability
  fsdev: QMP interface for throttling
  fsdev: hmp interface for throttling

 Makefile                        |   4 ++
 blockdev.c                      |  97 ++------------------------------
 fsdev/qemu-fsdev-dummy.c        |  11 ++++
 fsdev/qemu-fsdev-throttle.c     | 120 ++++++++++++++++++++++++++--------------
 fsdev/qemu-fsdev-throttle.h     |   8 ++-
 fsdev/qemu-fsdev.c              |  38 +++++++++++++
 hmp-commands-info.hx            |  18 ++++++
 hmp-commands.hx                 |  19 +++++++
 hmp.c                           |  81 +++++++++++++++++++++++++--
 hmp.h                           |   4 ++
 include/qemu/throttle-options.h |   7 +++
 include/qemu/throttle.h         |   4 +-
 include/qemu/typedefs.h         |   1 +
 monitor.c                       |   5 ++
 qapi-schema.json                |   3 +
 qapi/block-core.json            |  76 +------------------------
 qapi/fsdev.json                 |  84 ++++++++++++++++++++++++++++
 qapi/iothrottle.json            |  88 +++++++++++++++++++++++++++++
 qmp.c                           |  14 +++++
 util/throttle.c                 | 110 ++++++++++++++++++++++++++++++++++++
 20 files changed, 574 insertions(+), 218 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

v5 -> v6:
  Addressed comments from Greg Kurz and Markus Armbruster
  Split the commits to specific to hmp and throttle as suggested by Greg
  Moved ThrottleConfig typedef to qemu/typedefs.h
  Addressed compilation issue on FreeBSD by adding flags in qmp.c

v6 -> v7:
  Addressed comments from Albert Garcia and Dr. David Alan Gilbert
  Fixed the hmp-commands-info.hx and hmp-commands.hx as per Dr. David's
  comments.
  Fixed the bug with the hmp fsdev_set_io_throttle and info fsdev_iothrottle  

v7 -> v8:
  Addressed comments from Markus Armbruster and Eric Blake
  Removed unwanted headers from qmp-fsdev-throttle.h
-- 
1.8.3.1

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

end of thread, other threads:[~2017-09-04 13:10 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-29 14:23 [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling Pradeep Jagadeesh
2017-08-29 14:23 ` [Qemu-devel] [PATCH v8 1/6] throttle: factor out duplicate code Pradeep Jagadeesh
2017-08-29 14:23 ` [Qemu-devel] [PATCH v8 2/6] qmp: Create IOThrottle structure Pradeep Jagadeesh
2017-08-29 14:23 ` [Qemu-devel] [PATCH v8 3/6] throttle: move out function to reuse the code Pradeep Jagadeesh
2017-08-29 14:23 ` [Qemu-devel] [PATCH v8 4/6] hmp: create a throttle initialization function for code reusability Pradeep Jagadeesh
2017-08-29 14:23 ` [Qemu-devel] [PATCH v8 5/6] fsdev: hmp interface for throttling Pradeep Jagadeesh
2017-08-30  9:38   ` Alberto Garcia
2017-09-04 12:22     ` Pradeep Jagadeesh
2017-09-04 13:05       ` Alberto Garcia
2017-08-29 14:23 ` [Qemu-devel] [PATCH v8 6/6] fsdev: QMP " Pradeep Jagadeesh
2017-08-29 14:36 ` [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling Alberto Garcia
2017-08-29 14:39   ` Pradeep Jagadeesh
2017-08-30 12:07 ` Alberto Garcia
2017-08-30 12:10   ` Pradeep Jagadeesh
2017-08-30 14:54     ` Alberto Garcia
2017-08-30 15:07       ` Pradeep Jagadeesh
2017-08-30 15:10         ` Alberto Garcia
2017-08-30 15:12           ` Pradeep Jagadeesh
2017-08-31 13:34             ` Alberto Garcia
2017-08-31 13:39               ` Pradeep Jagadeesh
2017-09-01 12:44               ` Pradeep Jagadeesh
2017-08-30 14:28   ` Greg Kurz
  -- strict thread matches above, loose matches on Subject: below --
2017-08-07 12:37 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.