* [Qemu-devel] [PATCH 0/2 v15] fsdev: add IO throttle support to fsdev devices
@ 2017-02-03 10:04 Pradeep Jagadeesh
2017-02-03 10:04 ` [Qemu-devel] [PATCH 1/2 " Pradeep Jagadeesh
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Pradeep Jagadeesh @ 2017-02-03 10:04 UTC (permalink / raw)
To: Aneesh Kumar K.V, Greg Kurz; +Cc: Pradeep Jagadeesh, Alberto Garcia, qemu-devel
This patch set adds the IO throttling functionality to fsdev/9p devices.
So far cgroups were used for throttling IO opertions on the fsdev/9p devices.
It is difficult to use cgroups for throttling because we have to set up
cgroups externally before we start the qemu process.
Qemu provides the throttling apis for implementing the throttling.
Block devices already make use of these APIs for throtting the IO operations.
So, we use the same APIs to enable the throttling functionality for
fsdevices.As of now the feature is enabled only on 9p-local driver.
This feature can be used as shown in the below example:
-fsdev local,id=sdb1,path=PATH_TO_DEVICE,security_model=none,writeout=immediate,
throttling.bps-read=4194304,throttling.bps-write=4194304 -device
virtio-9p-pci,fsdev=sdb1,mount_tag=sdb1
The main advantages are:
- Easy to use because the throttling options are part of qemu cli options
- Provides a uniform way of using throttling options across block and
fsdev/9p devices
- No need to setup cgroup to provide throttling functionality for the fsdev
devices.
- Removes the redundant throttling code that was present in block and fsdev
files
Missing features:
-QMP support
-Throttling support for other fsdev/9p drivers.
Thanks,
Pradeep
Pradeep Jagadeesh (2):
fsdev: add IO throttle support to fsdev devices
throttle: removed duplicate throtlle code from block and fsdev files
blockdev.c | 81 ++-------------------------
fsdev/Makefile.objs | 2 +-
fsdev/file-op-9p.h | 3 +
fsdev/qemu-fsdev-opts.c | 3 +
fsdev/qemu-fsdev-throttle.c | 118 ++++++++++++++++++++++++++++++++++++++++
fsdev/qemu-fsdev-throttle.h | 39 +++++++++++++
hw/9pfs/9p-local.c | 8 +++
hw/9pfs/9p.c | 5 ++
hw/9pfs/cofile.c | 2 +
include/qemu/throttle-options.h | 92 +++++++++++++++++++++++++++++++
10 files changed, 275 insertions(+), 78 deletions(-)
create mode 100644 fsdev/qemu-fsdev-throttle.c
create mode 100644 fsdev/qemu-fsdev-throttle.h
create mode 100644 include/qemu/throttle-options.h
--
1.8.3.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 1/2 v15] fsdev: add IO throttle support to fsdev devices
2017-02-03 10:04 [Qemu-devel] [PATCH 0/2 v15] fsdev: add IO throttle support to fsdev devices Pradeep Jagadeesh
@ 2017-02-03 10:04 ` Pradeep Jagadeesh
2017-02-03 10:10 ` Alberto Garcia
2017-02-03 10:04 ` [Qemu-devel] [PATCH 2/2 v15] throttle: factor out duplicate code Pradeep Jagadeesh
2017-02-03 10:50 ` [Qemu-devel] [PATCH 0/2 v15] fsdev: add IO throttle support to fsdev devices no-reply
2 siblings, 1 reply; 8+ messages in thread
From: Pradeep Jagadeesh @ 2017-02-03 10:04 UTC (permalink / raw)
To: Aneesh Kumar K.V, Greg Kurz; +Cc: Pradeep Jagadeesh, Alberto Garcia, qemu-devel
This patchset adds the io throttle support for the 9p-local driver.
For now this functionality can be used only through qemu cli options.
QMP interface and support to other 9p drivers need further extensions.
To make it simple for other 9p drivers, the throttle code has been put in
separate files.
Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
---
fsdev/Makefile.objs | 2 +-
fsdev/file-op-9p.h | 3 ++
fsdev/qemu-fsdev-opts.c | 77 ++++++++++++++++++++++++++++-
fsdev/qemu-fsdev-throttle.c | 118 ++++++++++++++++++++++++++++++++++++++++++++
fsdev/qemu-fsdev-throttle.h | 39 +++++++++++++++
hw/9pfs/9p-local.c | 8 +++
hw/9pfs/9p.c | 5 ++
hw/9pfs/cofile.c | 2 +
8 files changed, 252 insertions(+), 2 deletions(-)
create mode 100644 fsdev/qemu-fsdev-throttle.c
create mode 100644 fsdev/qemu-fsdev-throttle.h
diff --git a/fsdev/Makefile.objs b/fsdev/Makefile.objs
index 1b120a4..659df6e 100644
--- a/fsdev/Makefile.objs
+++ b/fsdev/Makefile.objs
@@ -5,7 +5,7 @@ common-obj-y = qemu-fsdev.o 9p-marshal.o 9p-iov-marshal.o
else
common-obj-y = qemu-fsdev-dummy.o
endif
-common-obj-y += qemu-fsdev-opts.o
+common-obj-y += qemu-fsdev-opts.o qemu-fsdev-throttle.o
# Toplevel always builds this; targets without virtio will put it in
# common-obj-y
diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
index a56dc84..0844a40 100644
--- a/fsdev/file-op-9p.h
+++ b/fsdev/file-op-9p.h
@@ -17,6 +17,7 @@
#include <dirent.h>
#include <utime.h>
#include <sys/vfs.h>
+#include "qemu-fsdev-throttle.h"
#define SM_LOCAL_MODE_BITS 0600
#define SM_LOCAL_DIR_MODE_BITS 0700
@@ -74,6 +75,7 @@ typedef struct FsDriverEntry {
char *path;
int export_flags;
FileOperations *ops;
+ FsThrottle fst;
} FsDriverEntry;
typedef struct FsContext
@@ -83,6 +85,7 @@ typedef struct FsContext
int export_flags;
struct xattr_operations **xops;
struct extended_ops exops;
+ FsThrottle *fst;
/* fs driver specific data */
void *private;
} FsContext;
diff --git a/fsdev/qemu-fsdev-opts.c b/fsdev/qemu-fsdev-opts.c
index 1dd8c7a..385423f0 100644
--- a/fsdev/qemu-fsdev-opts.c
+++ b/fsdev/qemu-fsdev-opts.c
@@ -37,8 +37,83 @@ static QemuOptsList qemu_fsdev_opts = {
}, {
.name = "sock_fd",
.type = QEMU_OPT_NUMBER,
+ }, {
+ .name = "throttling.iops-total",
+ .type = QEMU_OPT_NUMBER,
+ .help = "limit total I/O operations per second",
+ }, {
+ .name = "throttling.iops-read",
+ .type = QEMU_OPT_NUMBER,
+ .help = "limit read operations per second",
+ }, {
+ .name = "throttling.iops-write",
+ .type = QEMU_OPT_NUMBER,
+ .help = "limit write operations per second",
+ }, {
+ .name = "throttling.bps-total",
+ .type = QEMU_OPT_NUMBER,
+ .help = "limit total bytes per second",
+ }, {
+ .name = "throttling.bps-read",
+ .type = QEMU_OPT_NUMBER,
+ .help = "limit read bytes per second",
+ }, {
+ .name = "throttling.bps-write",
+ .type = QEMU_OPT_NUMBER,
+ .help = "limit write bytes per second",
+ }, {
+ .name = "throttling.iops-total-max",
+ .type = QEMU_OPT_NUMBER,
+ .help = "I/O operations burst",
+ }, {
+ .name = "throttling.iops-read-max",
+ .type = QEMU_OPT_NUMBER,
+ .help = "I/O operations read burst",
+ }, {
+ .name = "throttling.iops-write-max",
+ .type = QEMU_OPT_NUMBER,
+ .help = "I/O operations write burst",
+ }, {
+ .name = "throttling.bps-total-max",
+ .type = QEMU_OPT_NUMBER,
+ .help = "total bytes burst",
+ }, {
+ .name = "throttling.bps-read-max",
+ .type = QEMU_OPT_NUMBER,
+ .help = "total bytes read burst",
+ }, {
+ .name = "throttling.bps-write-max",
+ .type = QEMU_OPT_NUMBER,
+ .help = "total bytes write burst",
+ }, {
+ .name = "throttling.iops-total-max-length",
+ .type = QEMU_OPT_NUMBER,
+ .help = "length of the iops-total-max burst period, in seconds",
+ }, {
+ .name = "throttling.iops-read-max-length",
+ .type = QEMU_OPT_NUMBER,
+ .help = "length of the iops-read-max burst period, in seconds",
+ }, {
+ .name = "throttling.iops-write-max-length",
+ .type = QEMU_OPT_NUMBER,
+ .help = "length of the iops-write-max burst period, in seconds",
+ }, {
+ .name = "throttling.bps-total-max-length",
+ .type = QEMU_OPT_NUMBER,
+ .help = "length of the bps-total-max burst period, in seconds",
+ }, {
+ .name = "throttling.bps-read-max-length",
+ .type = QEMU_OPT_NUMBER,
+ .help = "length of the bps-read-max burst period, in seconds",
+ }, {
+ .name = "throttling.bps-write-max-length",
+ .type = QEMU_OPT_NUMBER,
+ .help = "length of the bps-write-max burst period, in seconds",
+ }, {
+ .name = "throttling.iops-size",
+ .type = QEMU_OPT_NUMBER,
+ .help = "when limiting by iops max size of an I/O in bytes",
},
-
{ /*End of list */ }
},
};
diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
new file mode 100644
index 0000000..feb9af3
--- /dev/null
+++ b/fsdev/qemu-fsdev-throttle.c
@@ -0,0 +1,118 @@
+/*
+ * Fsdev Throttle
+ *
+ * Copyright (C) 2016 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.
+ *
+ * See the COPYING file in the top-level directory for details.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "qemu-fsdev-throttle.h"
+#include "qemu/iov.h"
+
+static void fsdev_throttle_read_timer_cb(void *opaque)
+{
+ FsThrottle *fst = opaque;
+ qemu_co_enter_next(&fst->throttled_reqs[false]);
+}
+
+static void fsdev_throttle_write_timer_cb(void *opaque)
+{
+ FsThrottle *fst = opaque;
+ qemu_co_enter_next(&fst->throttled_reqs[true]);
+}
+
+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_is_valid(&fst->cfg, errp);
+}
+
+void fsdev_throttle_init(FsThrottle *fst)
+{
+ if (throttle_enabled(&fst->cfg)) {
+ throttle_init(&fst->ts);
+ throttle_timers_init(&fst->tt,
+ qemu_get_aio_context(),
+ QEMU_CLOCK_REALTIME,
+ fsdev_throttle_read_timer_cb,
+ fsdev_throttle_write_timer_cb,
+ fst);
+ throttle_config(&fst->ts, &fst->tt, &fst->cfg);
+ qemu_co_queue_init(&fst->throttled_reqs[0]);
+ qemu_co_queue_init(&fst->throttled_reqs[1]);
+ }
+}
+
+void coroutine_fn fsdev_co_throttle_request(FsThrottle *fst, bool is_write,
+ struct iovec *iov, int iovcnt)
+{
+ if (throttle_enabled(&fst->cfg)) {
+ if (throttle_schedule_timer(&fst->ts, &fst->tt, is_write) ||
+ !qemu_co_queue_empty(&fst->throttled_reqs[is_write])) {
+ qemu_co_queue_wait(&fst->throttled_reqs[is_write]);
+ }
+
+ throttle_account(&fst->ts, is_write, iov_size(iov, iovcnt));
+
+ if (!qemu_co_queue_empty(&fst->throttled_reqs[is_write]) &&
+ !throttle_schedule_timer(&fst->ts, &fst->tt, is_write)) {
+ qemu_co_queue_next(&fst->throttled_reqs[is_write]);
+ }
+ }
+}
+
+void fsdev_throttle_cleanup(FsThrottle *fst)
+{
+ if (throttle_enabled(&fst->cfg)) {
+ throttle_timers_destroy(&fst->tt);
+ }
+}
diff --git a/fsdev/qemu-fsdev-throttle.h b/fsdev/qemu-fsdev-throttle.h
new file mode 100644
index 0000000..e418643
--- /dev/null
+++ b/fsdev/qemu-fsdev-throttle.h
@@ -0,0 +1,39 @@
+/*
+ * Fsdev Throttle
+ *
+ * Copyright (C) 2016 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.
+ *
+ * See the COPYING file in the top-level directory for details.
+ *
+ */
+
+#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"
+
+typedef struct FsThrottle {
+ ThrottleState ts;
+ ThrottleTimers tt;
+ ThrottleConfig cfg;
+ CoQueue throttled_reqs[2];
+} FsThrottle;
+
+void fsdev_throttle_parse_opts(QemuOpts *, FsThrottle *, Error **);
+
+void fsdev_throttle_init(FsThrottle *);
+
+void coroutine_fn fsdev_co_throttle_request(FsThrottle *, bool ,
+ struct iovec *, int);
+
+void fsdev_throttle_cleanup(FsThrottle *);
+#endif /* _FSDEV_THROTTLE_H */
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 845675e..828348d 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -1209,6 +1209,7 @@ static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse)
{
const char *sec_model = qemu_opt_get(opts, "security_model");
const char *path = qemu_opt_get(opts, "path");
+ Error *err = NULL;
if (!sec_model) {
error_report("Security model not specified, local fs needs security model");
@@ -1237,6 +1238,13 @@ static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse)
error_report("fsdev: No path specified");
return -1;
}
+
+ fsdev_throttle_parse_opts(opts, &fse->fst, &err);
+ if (err) {
+ error_reportf_err(err, "Throttle configuration is not valid: ");
+ return -1;
+ }
+
fse->path = g_strdup(path);
return 0;
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index fa58877..920eb05 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -3520,6 +3520,10 @@ int v9fs_device_realize_common(V9fsState *s, Error **errp)
error_setg(errp, "share path %s is not a directory", fse->path);
goto out;
}
+
+ s->ctx.fst = &fse->fst;
+ fsdev_throttle_init(s->ctx.fst);
+
v9fs_path_free(&path);
rc = 0;
@@ -3528,6 +3532,7 @@ out:
if (s->ops && s->ops->cleanup && s->ctx.private) {
s->ops->cleanup(&s->ctx);
}
+ fsdev_throttle_cleanup(s->ctx.fst);
g_free(s->tag);
g_free(s->ctx.fs_root);
v9fs_path_free(&path);
diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
index 120e267..88791bc 100644
--- a/hw/9pfs/cofile.c
+++ b/hw/9pfs/cofile.c
@@ -247,6 +247,7 @@ int coroutine_fn v9fs_co_pwritev(V9fsPDU *pdu, V9fsFidState *fidp,
if (v9fs_request_cancelled(pdu)) {
return -EINTR;
}
+ fsdev_co_throttle_request(s->ctx.fst, true, iov, iovcnt);
v9fs_co_run_in_worker(
{
err = s->ops->pwritev(&s->ctx, &fidp->fs, iov, iovcnt, offset);
@@ -266,6 +267,7 @@ int coroutine_fn v9fs_co_preadv(V9fsPDU *pdu, V9fsFidState *fidp,
if (v9fs_request_cancelled(pdu)) {
return -EINTR;
}
+ fsdev_co_throttle_request(s->ctx.fst, false, iov, iovcnt);
v9fs_co_run_in_worker(
{
err = s->ops->preadv(&s->ctx, &fidp->fs, iov, iovcnt, offset);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 2/2 v15] throttle: factor out duplicate code
2017-02-03 10:04 [Qemu-devel] [PATCH 0/2 v15] fsdev: add IO throttle support to fsdev devices Pradeep Jagadeesh
2017-02-03 10:04 ` [Qemu-devel] [PATCH 1/2 " Pradeep Jagadeesh
@ 2017-02-03 10:04 ` Pradeep Jagadeesh
2017-02-03 10:38 ` Alberto Garcia
2017-02-03 10:50 ` [Qemu-devel] [PATCH 0/2 v15] fsdev: add IO throttle support to fsdev devices no-reply
2 siblings, 1 reply; 8+ messages in thread
From: Pradeep Jagadeesh @ 2017-02-03 10:04 UTC (permalink / raw)
To: Aneesh Kumar K.V, Greg Kurz; +Cc: Pradeep Jagadeesh, Alberto Garcia, qemu-devel
This patch removes the redundant throttle code that was present in
block and fsdev device files. Now the common code is moved
to a single file.
Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg04637.html
---
blockdev.c | 81 ++----------------------------------
fsdev/qemu-fsdev-opts.c | 80 ++---------------------------------
hw/9pfs/9p.c | 2 +-
include/qemu/throttle-options.h | 92 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 101 insertions(+), 154 deletions(-)
create mode 100644 include/qemu/throttle-options.h
diff --git a/blockdev.c b/blockdev.c
index 245e1e1..9320c8a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -52,6 +52,7 @@
#include "sysemu/arch_init.h"
#include "qemu/cutils.h"
#include "qemu/help_option.h"
+#include "qemu/throttle-options.h"
static QTAILQ_HEAD(, BlockDriverState) monitor_bdrv_states =
QTAILQ_HEAD_INITIALIZER(monitor_bdrv_states);
@@ -3999,83 +4000,9 @@ QemuOptsList qemu_common_drive_opts = {
.name = BDRV_OPT_READ_ONLY,
.type = QEMU_OPT_BOOL,
.help = "open drive file as read-only",
- },{
- .name = "throttling.iops-total",
- .type = QEMU_OPT_NUMBER,
- .help = "limit total I/O operations per second",
- },{
- .name = "throttling.iops-read",
- .type = QEMU_OPT_NUMBER,
- .help = "limit read operations per second",
- },{
- .name = "throttling.iops-write",
- .type = QEMU_OPT_NUMBER,
- .help = "limit write operations per second",
- },{
- .name = "throttling.bps-total",
- .type = QEMU_OPT_NUMBER,
- .help = "limit total bytes per second",
- },{
- .name = "throttling.bps-read",
- .type = QEMU_OPT_NUMBER,
- .help = "limit read bytes per second",
- },{
- .name = "throttling.bps-write",
- .type = QEMU_OPT_NUMBER,
- .help = "limit write bytes per second",
- },{
- .name = "throttling.iops-total-max",
- .type = QEMU_OPT_NUMBER,
- .help = "I/O operations burst",
- },{
- .name = "throttling.iops-read-max",
- .type = QEMU_OPT_NUMBER,
- .help = "I/O operations read burst",
- },{
- .name = "throttling.iops-write-max",
- .type = QEMU_OPT_NUMBER,
- .help = "I/O operations write burst",
- },{
- .name = "throttling.bps-total-max",
- .type = QEMU_OPT_NUMBER,
- .help = "total bytes burst",
- },{
- .name = "throttling.bps-read-max",
- .type = QEMU_OPT_NUMBER,
- .help = "total bytes read burst",
- },{
- .name = "throttling.bps-write-max",
- .type = QEMU_OPT_NUMBER,
- .help = "total bytes write burst",
- },{
- .name = "throttling.iops-total-max-length",
- .type = QEMU_OPT_NUMBER,
- .help = "length of the iops-total-max burst period, in seconds",
- },{
- .name = "throttling.iops-read-max-length",
- .type = QEMU_OPT_NUMBER,
- .help = "length of the iops-read-max burst period, in seconds",
- },{
- .name = "throttling.iops-write-max-length",
- .type = QEMU_OPT_NUMBER,
- .help = "length of the iops-write-max burst period, in seconds",
- },{
- .name = "throttling.bps-total-max-length",
- .type = QEMU_OPT_NUMBER,
- .help = "length of the bps-total-max burst period, in seconds",
- },{
- .name = "throttling.bps-read-max-length",
- .type = QEMU_OPT_NUMBER,
- .help = "length of the bps-read-max burst period, in seconds",
- },{
- .name = "throttling.bps-write-max-length",
- .type = QEMU_OPT_NUMBER,
- .help = "length of the bps-write-max burst period, in seconds",
- },{
- .name = "throttling.iops-size",
- .type = QEMU_OPT_NUMBER,
- .help = "when limiting by iops max size of an I/O in bytes",
- },{
+ },
+ THROTTLE_OPTS,
+ {
.name = "throttling.group",
.type = QEMU_OPT_STRING,
.help = "name of the block throttling group",
diff --git a/fsdev/qemu-fsdev-opts.c b/fsdev/qemu-fsdev-opts.c
index 385423f0..bf57130 100644
--- a/fsdev/qemu-fsdev-opts.c
+++ b/fsdev/qemu-fsdev-opts.c
@@ -9,6 +9,7 @@
#include "qemu/config-file.h"
#include "qemu/option.h"
#include "qemu/module.h"
+#include "qemu/throttle-options.h"
static QemuOptsList qemu_fsdev_opts = {
.name = "fsdev",
@@ -37,83 +38,10 @@ static QemuOptsList qemu_fsdev_opts = {
}, {
.name = "sock_fd",
.type = QEMU_OPT_NUMBER,
- }, {
- .name = "throttling.iops-total",
- .type = QEMU_OPT_NUMBER,
- .help = "limit total I/O operations per second",
- }, {
- .name = "throttling.iops-read",
- .type = QEMU_OPT_NUMBER,
- .help = "limit read operations per second",
- }, {
- .name = "throttling.iops-write",
- .type = QEMU_OPT_NUMBER,
- .help = "limit write operations per second",
- }, {
- .name = "throttling.bps-total",
- .type = QEMU_OPT_NUMBER,
- .help = "limit total bytes per second",
- }, {
- .name = "throttling.bps-read",
- .type = QEMU_OPT_NUMBER,
- .help = "limit read bytes per second",
- }, {
- .name = "throttling.bps-write",
- .type = QEMU_OPT_NUMBER,
- .help = "limit write bytes per second",
- }, {
- .name = "throttling.iops-total-max",
- .type = QEMU_OPT_NUMBER,
- .help = "I/O operations burst",
- }, {
- .name = "throttling.iops-read-max",
- .type = QEMU_OPT_NUMBER,
- .help = "I/O operations read burst",
- }, {
- .name = "throttling.iops-write-max",
- .type = QEMU_OPT_NUMBER,
- .help = "I/O operations write burst",
- }, {
- .name = "throttling.bps-total-max",
- .type = QEMU_OPT_NUMBER,
- .help = "total bytes burst",
- }, {
- .name = "throttling.bps-read-max",
- .type = QEMU_OPT_NUMBER,
- .help = "total bytes read burst",
- }, {
- .name = "throttling.bps-write-max",
- .type = QEMU_OPT_NUMBER,
- .help = "total bytes write burst",
- }, {
- .name = "throttling.iops-total-max-length",
- .type = QEMU_OPT_NUMBER,
- .help = "length of the iops-total-max burst period, in seconds",
- }, {
- .name = "throttling.iops-read-max-length",
- .type = QEMU_OPT_NUMBER,
- .help = "length of the iops-read-max burst period, in seconds",
- }, {
- .name = "throttling.iops-write-max-length",
- .type = QEMU_OPT_NUMBER,
- .help = "length of the iops-write-max burst period, in seconds",
- }, {
- .name = "throttling.bps-total-max-length",
- .type = QEMU_OPT_NUMBER,
- .help = "length of the bps-total-max burst period, in seconds",
- }, {
- .name = "throttling.bps-read-max-length",
- .type = QEMU_OPT_NUMBER,
- .help = "length of the bps-read-max burst period, in seconds",
- }, {
- .name = "throttling.bps-write-max-length",
- .type = QEMU_OPT_NUMBER,
- .help = "length of the bps-write-max burst period, in seconds",
- }, {
- .name = "throttling.iops-size",
- .type = QEMU_OPT_NUMBER,
- .help = "when limiting by iops max size of an I/O in bytes",
},
+
+ THROTTLE_OPTS,
+
{ /*End of list */ }
},
};
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 920eb05..22a6a99 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -3532,7 +3532,6 @@ out:
if (s->ops && s->ops->cleanup && s->ctx.private) {
s->ops->cleanup(&s->ctx);
}
- fsdev_throttle_cleanup(s->ctx.fst);
g_free(s->tag);
g_free(s->ctx.fs_root);
v9fs_path_free(&path);
@@ -3545,6 +3544,7 @@ void v9fs_device_unrealize_common(V9fsState *s, Error **errp)
if (s->ops->cleanup) {
s->ops->cleanup(&s->ctx);
}
+ fsdev_throttle_cleanup(s->ctx.fst);
g_free(s->tag);
g_free(s->ctx.fs_root);
}
diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h
new file mode 100644
index 0000000..3133d1c
--- /dev/null
+++ b/include/qemu/throttle-options.h
@@ -0,0 +1,92 @@
+/*
+ * QEMU throttling command line options
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.
+ *
+ * See the COPYING file in the top-level directory for details.
+ *
+ */
+#ifndef THROTTLE_OPTIONS_H
+#define THROTTLE_OPTIONS_H
+
+#define THROTTLE_OPTS \
+ { \
+ .name = "throttling.iops-total",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "limit total I/O operations per second",\
+ },{ \
+ .name = "throttling.iops-read",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "limit read operations per second",\
+ },{ \
+ .name = "throttling.iops-write",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "limit write operations per second",\
+ },{ \
+ .name = "throttling.bps-total",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "limit total bytes per second",\
+ },{ \
+ .name = "throttling.bps-read",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "limit read bytes per second",\
+ },{ \
+ .name = "throttling.bps-write",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "limit write bytes per second",\
+ },{ \
+ .name = "throttling.iops-total-max",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "I/O operations burst",\
+ },{ \
+ .name = "throttling.iops-read-max",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "I/O operations read burst",\
+ },{ \
+ .name = "throttling.iops-write-max",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "I/O operations write burst",\
+ },{ \
+ .name = "throttling.bps-total-max",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "total bytes burst",\
+ },{ \
+ .name = "throttling.bps-read-max",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "total bytes read burst",\
+ },{ \
+ .name = "throttling.bps-write-max",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "total bytes write burst",\
+ },{ \
+ .name = "throttling.iops-total-max-length",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "length of the iops-total-max burst period, in seconds",\
+ },{ \
+ .name = "throttling.iops-read-max-length",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "length of the iops-read-max burst period, in seconds",\
+ },{ \
+ .name = "throttling.iops-write-max-length",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "length of the iops-write-max burst period, in seconds",\
+ },{ \
+ .name = "throttling.bps-total-max-length",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "length of the bps-total-max burst period, in seconds",\
+ },{ \
+ .name = "throttling.bps-read-max-length",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "length of the bps-read-max burst period, in seconds",\
+ },{ \
+ .name = "throttling.bps-write-max-length",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "length of the bps-write-max burst period, in seconds",\
+ },{ \
+ .name = "throttling.iops-size",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "when limiting by iops max size of an I/O in bytes",\
+ }
+
+#endif
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2 v15] fsdev: add IO throttle support to fsdev devices
2017-02-03 10:04 ` [Qemu-devel] [PATCH 1/2 " Pradeep Jagadeesh
@ 2017-02-03 10:10 ` Alberto Garcia
2017-02-03 10:33 ` Pradeep Jagadeesh
0 siblings, 1 reply; 8+ messages in thread
From: Alberto Garcia @ 2017-02-03 10:10 UTC (permalink / raw)
To: Pradeep Jagadeesh, Aneesh Kumar K.V, Greg Kurz
Cc: Pradeep Jagadeesh, qemu-devel
On Fri 03 Feb 2017 11:04:57 AM CET, Pradeep Jagadeesh wrote:
> This patchset adds the io throttle support for the 9p-local driver.
> For now this functionality can be used only through qemu cli options.
> QMP interface and support to other 9p drivers need further extensions.
> To make it simple for other 9p drivers, the throttle code has been put in
> separate files.
>
> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
I don't see any difference between this patch and the one in v14...
> +
> + s->ctx.fst = &fse->fst;
> + fsdev_throttle_init(s->ctx.fst);
> +
> v9fs_path_free(&path);
>
> rc = 0;
> @@ -3528,6 +3532,7 @@ out:
> if (s->ops && s->ops->cleanup && s->ctx.private) {
> s->ops->cleanup(&s->ctx);
> }
> + fsdev_throttle_cleanup(s->ctx.fst);
> g_free(s->tag);
> g_free(s->ctx.fs_root);
> v9fs_path_free(&path);
Berto
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2 v15] fsdev: add IO throttle support to fsdev devices
2017-02-03 10:10 ` Alberto Garcia
@ 2017-02-03 10:33 ` Pradeep Jagadeesh
0 siblings, 0 replies; 8+ messages in thread
From: Pradeep Jagadeesh @ 2017-02-03 10:33 UTC (permalink / raw)
To: Alberto Garcia, Pradeep Jagadeesh, Aneesh Kumar K.V, Greg Kurz; +Cc: qemu-devel
On 2/3/2017 11:10 AM, Alberto Garcia wrote:
> On Fri 03 Feb 2017 11:04:57 AM CET, Pradeep Jagadeesh wrote:
>> This patchset adds the io throttle support for the 9p-local driver.
>> For now this functionality can be used only through qemu cli options.
>> QMP interface and support to other 9p drivers need further extensions.
>> To make it simple for other 9p drivers, the throttle code has been put in
>> separate files.
>>
>> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
>
> I don't see any difference between this patch and the one in v14...
I moved the fsdev_throttle_cleanup() inside the unrealize() function
You want me to clean up even here ?
out:
if (rc) {
/* ... */
fsdev_throttle_cleanup(s->ctx.fst);
/* ... */
}
I thought one place is enough.
Or I did not understand your comment :)
-Pradeep
>
>> +
>> + s->ctx.fst = &fse->fst;
>> + fsdev_throttle_init(s->ctx.fst);
>> +
>> v9fs_path_free(&path);
>>
>> rc = 0;
>> @@ -3528,6 +3532,7 @@ out:
>> if (s->ops && s->ops->cleanup && s->ctx.private) {
>> s->ops->cleanup(&s->ctx);
>> }
>> + fsdev_throttle_cleanup(s->ctx.fst);
>> g_free(s->tag);
>> g_free(s->ctx.fs_root);
>> v9fs_path_free(&path);
>
> Berto
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2 v15] throttle: factor out duplicate code
2017-02-03 10:04 ` [Qemu-devel] [PATCH 2/2 v15] throttle: factor out duplicate code Pradeep Jagadeesh
@ 2017-02-03 10:38 ` Alberto Garcia
2017-02-03 11:58 ` Pradeep Jagadeesh
0 siblings, 1 reply; 8+ messages in thread
From: Alberto Garcia @ 2017-02-03 10:38 UTC (permalink / raw)
To: Pradeep Jagadeesh, Aneesh Kumar K.V, Greg Kurz
Cc: Pradeep Jagadeesh, qemu-devel
On Fri 03 Feb 2017 11:04:58 AM CET, Pradeep Jagadeesh wrote:
> This patch removes the redundant throttle code that was present in
> block and fsdev device files. Now the common code is moved to a single
> file.
Here it says that this patch moves common code to a separate file,
however:
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 920eb05..22a6a99 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -3532,7 +3532,6 @@ out:
> if (s->ops && s->ops->cleanup && s->ctx.private) {
> s->ops->cleanup(&s->ctx);
> }
> - fsdev_throttle_cleanup(s->ctx.fst);
> g_free(s->tag);
> g_free(s->ctx.fs_root);
> v9fs_path_free(&path);
> @@ -3545,6 +3544,7 @@ void v9fs_device_unrealize_common(V9fsState *s, Error **errp)
> if (s->ops->cleanup) {
> s->ops->cleanup(&s->ctx);
> }
> + fsdev_throttle_cleanup(s->ctx.fst);
> g_free(s->tag);
> g_free(s->ctx.fs_root);
> }
This doesn't belong here. In patch 1 you add a fsdev_throttle_cleanup()
in v9fs_device_realize_common(), and in patch 2 you remove it from there
and put it in v9fs_device_unrealize_common().
It looks like you wanted to fix patch 1 but added the fix to patch 2
instead.
Berto
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2 v15] fsdev: add IO throttle support to fsdev devices
2017-02-03 10:04 [Qemu-devel] [PATCH 0/2 v15] fsdev: add IO throttle support to fsdev devices Pradeep Jagadeesh
2017-02-03 10:04 ` [Qemu-devel] [PATCH 1/2 " Pradeep Jagadeesh
2017-02-03 10:04 ` [Qemu-devel] [PATCH 2/2 v15] throttle: factor out duplicate code Pradeep Jagadeesh
@ 2017-02-03 10:50 ` no-reply
2 siblings, 0 replies; 8+ messages in thread
From: no-reply @ 2017-02-03 10:50 UTC (permalink / raw)
To: pradeepkiruvale
Cc: famz, aneesh.kumar, groug, berto, pradeep.jagadeesh, qemu-devel
Hi,
Your series seems to have some coding style problems. See output below for
more information:
Type: series
Subject: [Qemu-devel] [PATCH 0/2 v15] fsdev: add IO throttle support to fsdev devices
Message-id: 1486116298-25046-1-git-send-email-pradeep.jagadeesh@huawei.com
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
d987747 throttle: factor out duplicate code
f1dd13e fsdev: add IO throttle support to fsdev devices
=== OUTPUT BEGIN ===
Checking PATCH 1/2: fsdev: add IO throttle support to fsdev devices...
Checking PATCH 2/2: throttle: factor out duplicate code...
ERROR: Macros with multiple statements should be enclosed in a do - while loop
#251: FILE: include/qemu/throttle-options.h:13:
+#define THROTTLE_OPTS \
+ { \
+ .name = "throttling.iops-total",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "limit total I/O operations per second",\
+ },{ \
+ .name = "throttling.iops-read",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "limit read operations per second",\
+ },{ \
+ .name = "throttling.iops-write",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "limit write operations per second",\
+ },{ \
+ .name = "throttling.bps-total",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "limit total bytes per second",\
+ },{ \
+ .name = "throttling.bps-read",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "limit read bytes per second",\
+ },{ \
+ .name = "throttling.bps-write",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "limit write bytes per second",\
+ },{ \
+ .name = "throttling.iops-total-max",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "I/O operations burst",\
+ },{ \
+ .name = "throttling.iops-read-max",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "I/O operations read burst",\
+ },{ \
+ .name = "throttling.iops-write-max",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "I/O operations write burst",\
+ },{ \
+ .name = "throttling.bps-total-max",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "total bytes burst",\
+ },{ \
+ .name = "throttling.bps-read-max",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "total bytes read burst",\
+ },{ \
+ .name = "throttling.bps-write-max",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "total bytes write burst",\
+ },{ \
+ .name = "throttling.iops-total-max-length",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "length of the iops-total-max burst period, in seconds",\
+ },{ \
+ .name = "throttling.iops-read-max-length",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "length of the iops-read-max burst period, in seconds",\
+ },{ \
+ .name = "throttling.iops-write-max-length",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "length of the iops-write-max burst period, in seconds",\
+ },{ \
+ .name = "throttling.bps-total-max-length",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "length of the bps-total-max burst period, in seconds",\
+ },{ \
+ .name = "throttling.bps-read-max-length",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "length of the bps-read-max burst period, in seconds",\
+ },{ \
+ .name = "throttling.bps-write-max-length",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "length of the bps-write-max burst period, in seconds",\
+ },{ \
+ .name = "throttling.iops-size",\
+ .type = QEMU_OPT_NUMBER,\
+ .help = "when limiting by iops max size of an I/O in bytes",\
+ }
total: 1 errors, 0 warnings, 292 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2 v15] throttle: factor out duplicate code
2017-02-03 10:38 ` Alberto Garcia
@ 2017-02-03 11:58 ` Pradeep Jagadeesh
0 siblings, 0 replies; 8+ messages in thread
From: Pradeep Jagadeesh @ 2017-02-03 11:58 UTC (permalink / raw)
To: Alberto Garcia, Pradeep Jagadeesh, Aneesh Kumar K.V, Greg Kurz; +Cc: qemu-devel
On 2/3/2017 11:38 AM, Alberto Garcia wrote:
> On Fri 03 Feb 2017 11:04:58 AM CET, Pradeep Jagadeesh wrote:
>
>> This patch removes the redundant throttle code that was present in
>> block and fsdev device files. Now the common code is moved to a single
>> file.
>
> Here it says that this patch moves common code to a separate file,
> however:
Hmm, sorry. I just messed it up while merging.
Regards,
Pradeep
>
>> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
>> index 920eb05..22a6a99 100644
>> --- a/hw/9pfs/9p.c
>> +++ b/hw/9pfs/9p.c
>> @@ -3532,7 +3532,6 @@ out:
>> if (s->ops && s->ops->cleanup && s->ctx.private) {
>> s->ops->cleanup(&s->ctx);
>> }
>> - fsdev_throttle_cleanup(s->ctx.fst);
>> g_free(s->tag);
>> g_free(s->ctx.fs_root);
>> v9fs_path_free(&path);
>> @@ -3545,6 +3544,7 @@ void v9fs_device_unrealize_common(V9fsState *s, Error **errp)
>> if (s->ops->cleanup) {
>> s->ops->cleanup(&s->ctx);
>> }
>> + fsdev_throttle_cleanup(s->ctx.fst);
>> g_free(s->tag);
>> g_free(s->ctx.fs_root);
>> }
>
> This doesn't belong here. In patch 1 you add a fsdev_throttle_cleanup()
> in v9fs_device_realize_common(), and in patch 2 you remove it from there
> and put it in v9fs_device_unrealize_common().
>
> It looks like you wanted to fix patch 1 but added the fix to patch 2
> instead.
>
> Berto
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-02-03 11:59 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-03 10:04 [Qemu-devel] [PATCH 0/2 v15] fsdev: add IO throttle support to fsdev devices Pradeep Jagadeesh
2017-02-03 10:04 ` [Qemu-devel] [PATCH 1/2 " Pradeep Jagadeesh
2017-02-03 10:10 ` Alberto Garcia
2017-02-03 10:33 ` Pradeep Jagadeesh
2017-02-03 10:04 ` [Qemu-devel] [PATCH 2/2 v15] throttle: factor out duplicate code Pradeep Jagadeesh
2017-02-03 10:38 ` Alberto Garcia
2017-02-03 11:58 ` Pradeep Jagadeesh
2017-02-03 10:50 ` [Qemu-devel] [PATCH 0/2 v15] fsdev: add IO throttle support to fsdev devices no-reply
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).