* [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices
@ 2017-01-23 15:50 Pradeep Jagadeesh
2017-01-23 15:50 ` [Qemu-devel] [PATCH 1/2 " Pradeep Jagadeesh
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Pradeep Jagadeesh @ 2017-01-23 15:50 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] 11+ messages in thread
* [Qemu-devel] [PATCH 1/2 V13] fsdev: add IO throttle support to fsdev devices
2017-01-23 15:50 [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices Pradeep Jagadeesh
@ 2017-01-23 15:50 ` Pradeep Jagadeesh
2017-01-23 15:50 ` [Qemu-devel] [PATCH 2/2 V13] throttle: factor out duplicate code Pradeep Jagadeesh
2017-01-23 16:03 ` [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices no-reply
2 siblings, 0 replies; 11+ messages in thread
From: Pradeep Jagadeesh @ 2017-01-23 15:50 UTC (permalink / raw)
To: Aneesh Kumar K.V, Greg Kurz; +Cc: Pradeep Jagadeesh, Alberto Garcia, qemu-devel
This patchset adds the throttle support for the 9p-local driver.
For now this functionality can be enabled only through qemu cli options.
QMP interface and support to other drivers need further extensions.
To make it simple for other 9p drivers, the throttle code has been put in
separate files.
---
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] 11+ messages in thread
* [Qemu-devel] [PATCH 2/2 V13] throttle: factor out duplicate code
2017-01-23 15:50 [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices Pradeep Jagadeesh
2017-01-23 15:50 ` [Qemu-devel] [PATCH 1/2 " Pradeep Jagadeesh
@ 2017-01-23 15:50 ` Pradeep Jagadeesh
2017-01-23 15:55 ` Eric Blake
2017-01-23 16:03 ` [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices no-reply
2 siblings, 1 reply; 11+ messages in thread
From: Pradeep Jagadeesh @ 2017-01-23 15:50 UTC (permalink / raw)
To: Greg Kurz
Cc: Pradeep Jagadeesh, Alberto Garcia, Stefan Hajnoczi, Eric Blake,
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.
---
blockdev.c | 81 ++----------------------------------
fsdev/qemu-fsdev-opts.c | 80 ++---------------------------------
include/qemu/throttle-options.h | 92 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 100 insertions(+), 153 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/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] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2 V13] throttle: factor out duplicate code
2017-01-23 15:50 ` [Qemu-devel] [PATCH 2/2 V13] throttle: factor out duplicate code Pradeep Jagadeesh
@ 2017-01-23 15:55 ` Eric Blake
2017-01-23 15:59 ` Pradeep Jagadeesh
0 siblings, 1 reply; 11+ messages in thread
From: Eric Blake @ 2017-01-23 15:55 UTC (permalink / raw)
To: Pradeep Jagadeesh, Greg Kurz
Cc: Pradeep Jagadeesh, Alberto Garcia, Stefan Hajnoczi, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 597 bytes --]
On 01/23/2017 09:50 AM, 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.
>
> ---
You forgot your S-o-b designation. Also, this is the second thread
you've sent with a title of v13; please remember to bump the version
each time you send a new version. 'git send-email -v14' will put in the
version string automatically (but with lower-case 'v').
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2 V13] throttle: factor out duplicate code
2017-01-23 15:55 ` Eric Blake
@ 2017-01-23 15:59 ` Pradeep Jagadeesh
0 siblings, 0 replies; 11+ messages in thread
From: Pradeep Jagadeesh @ 2017-01-23 15:59 UTC (permalink / raw)
To: Eric Blake, Pradeep Jagadeesh, Greg Kurz
Cc: Alberto Garcia, Stefan Hajnoczi, qemu-devel
On 1/23/2017 4:55 PM, Eric Blake wrote:
> On 01/23/2017 09:50 AM, 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.
>>
>> ---
>
> You forgot your S-o-b designation. Also, this is the second thread
> you've sent with a title of v13; please remember to bump the version
> each time you send a new version. 'git send-email -v14' will put in the
> version string automatically (but with lower-case 'v').
>
Hmm..Sure!
Thanks,
Pradeep
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices
2017-01-23 15:50 [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices Pradeep Jagadeesh
2017-01-23 15:50 ` [Qemu-devel] [PATCH 1/2 " Pradeep Jagadeesh
2017-01-23 15:50 ` [Qemu-devel] [PATCH 2/2 V13] throttle: factor out duplicate code Pradeep Jagadeesh
@ 2017-01-23 16:03 ` no-reply
2017-01-23 16:22 ` Greg Kurz
2 siblings, 1 reply; 11+ messages in thread
From: no-reply @ 2017-01-23 16:03 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 V13] fsdev: add IO throttle support to fsdev devices
Message-id: 1485186641-12220-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'
7686dc8 throttle: factor out duplicate code
7568614 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
#228: 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",\
+ }
ERROR: Missing Signed-off-by: line(s)
total: 2 errors, 0 warnings, 278 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] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices
2017-01-23 16:03 ` [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices no-reply
@ 2017-01-23 16:22 ` Greg Kurz
2017-01-23 16:30 ` Pradeep Jagadeesh
0 siblings, 1 reply; 11+ messages in thread
From: Greg Kurz @ 2017-01-23 16:22 UTC (permalink / raw)
To: pradeepkiruvale; +Cc: qemu-devel, famz, aneesh.kumar, berto, pradeep.jagadeesh
On Mon, 23 Jan 2017 08:03:18 -0800 (PST)
no-reply@patchew.org wrote:
> Hi,
>
> Your series seems to have some coding style problems. See output below for
> more information:
>
Pradeep,
One should usually take patchew's findings into account. See below.
> Type: series
> Subject: [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices
> Message-id: 1485186641-12220-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'
> 7686dc8 throttle: factor out duplicate code
> 7568614 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
I guess your patch is ok here: checkpatch.pl is simply not smart enough to
parse this.
> #228: 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",\
> + }
>
> ERROR: Missing Signed-off-by: line(s)
>
As Eric pointed out in another mail, all patches you send must have your
Signed-off-by line.
> total: 2 errors, 0 warnings, 278 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] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices
2017-01-23 16:22 ` Greg Kurz
@ 2017-01-23 16:30 ` Pradeep Jagadeesh
2017-01-23 16:34 ` Eric Blake
2017-01-23 16:42 ` Greg Kurz
0 siblings, 2 replies; 11+ messages in thread
From: Pradeep Jagadeesh @ 2017-01-23 16:30 UTC (permalink / raw)
To: Greg Kurz, pradeepkiruvale; +Cc: qemu-devel, famz, aneesh.kumar, berto
On 1/23/2017 5:22 PM, Greg Kurz wrote:
> On Mon, 23 Jan 2017 08:03:18 -0800 (PST)
> no-reply@patchew.org wrote:
>> Hi,
>>
>> Your series seems to have some coding style problems. See output below for
>> more information:
>>
>
Ya, I observed signoff issue. I have created next version files.
I will send them tomorrow.
But, I do not know how to solve the second warning.
i.e multi line macro issue.
-Pradeep
> Pradeep,
>
> One should usually take patchew's findings into account. See below.
>
>> Type: series
>> Subject: [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices
>> Message-id: 1485186641-12220-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'
>> 7686dc8 throttle: factor out duplicate code
>> 7568614 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
>
> I guess your patch is ok here: checkpatch.pl is simply not smart enough to
> parse this.
>
>> #228: 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",\
>> + }
>>
>> ERROR: Missing Signed-off-by: line(s)
>>
>
> As Eric pointed out in another mail, all patches you send must have your
> Signed-off-by line.
>
>> total: 2 errors, 0 warnings, 278 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] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices
2017-01-23 16:30 ` Pradeep Jagadeesh
@ 2017-01-23 16:34 ` Eric Blake
2017-01-23 16:41 ` Pradeep Jagadeesh
2017-01-23 16:42 ` Greg Kurz
1 sibling, 1 reply; 11+ messages in thread
From: Eric Blake @ 2017-01-23 16:34 UTC (permalink / raw)
To: Pradeep Jagadeesh, Greg Kurz, pradeepkiruvale
Cc: berto, famz, qemu-devel, aneesh.kumar
[-- Attachment #1: Type: text/plain, Size: 926 bytes --]
On 01/23/2017 10:30 AM, Pradeep Jagadeesh wrote:
> On 1/23/2017 5:22 PM, Greg Kurz wrote:
>> On Mon, 23 Jan 2017 08:03:18 -0800 (PST)
>> no-reply@patchew.org wrote:
>>> Hi,
>>>
>>> Your series seems to have some coding style problems. See output
>>> below for
>>> more information:
>>>
>>
> Ya, I observed signoff issue. I have created next version files.
> I will send them tomorrow.
>
> But, I do not know how to solve the second warning.
> i.e multi line macro issue.
I don't think there is anything to fix there; it is a false positive
from checkpatch.pl (unless you want to dive in to the perl script and
figure out how to relax it to allow that usage, as you are not the first
to have it). Documenting in the commit message that checkpatch has a
false-positive warning is acceptable.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices
2017-01-23 16:34 ` Eric Blake
@ 2017-01-23 16:41 ` Pradeep Jagadeesh
0 siblings, 0 replies; 11+ messages in thread
From: Pradeep Jagadeesh @ 2017-01-23 16:41 UTC (permalink / raw)
To: Eric Blake, Greg Kurz, pradeepkiruvale
Cc: berto, famz, qemu-devel, aneesh.kumar
On 1/23/2017 5:34 PM, Eric Blake wrote:
> On 01/23/2017 10:30 AM, Pradeep Jagadeesh wrote:
>> On 1/23/2017 5:22 PM, Greg Kurz wrote:
>>> On Mon, 23 Jan 2017 08:03:18 -0800 (PST)
>>> no-reply@patchew.org wrote:
>>>> Hi,
>>>>
>>>> Your series seems to have some coding style problems. See output
>>>> below for
>>>> more information:
>>>>
>>>
>> Ya, I observed signoff issue. I have created next version files.
>> I will send them tomorrow.
>>
>> But, I do not know how to solve the second warning.
>> i.e multi line macro issue.
>
> I don't think there is anything to fix there; it is a false positive
> from checkpatch.pl (unless you want to dive in to the perl script and
> figure out how to relax it to allow that usage, as you are not the first
> to have it). Documenting in the commit message that checkpatch has a
> false-positive warning is acceptable.
>
OK
Thanks,
Pradeep
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices
2017-01-23 16:30 ` Pradeep Jagadeesh
2017-01-23 16:34 ` Eric Blake
@ 2017-01-23 16:42 ` Greg Kurz
1 sibling, 0 replies; 11+ messages in thread
From: Greg Kurz @ 2017-01-23 16:42 UTC (permalink / raw)
To: Pradeep Jagadeesh; +Cc: pradeepkiruvale, qemu-devel, famz, aneesh.kumar, berto
On Mon, 23 Jan 2017 17:30:13 +0100
Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com> wrote:
> On 1/23/2017 5:22 PM, Greg Kurz wrote:
> > On Mon, 23 Jan 2017 08:03:18 -0800 (PST)
> > no-reply@patchew.org wrote:
> >> Hi,
> >>
> >> Your series seems to have some coding style problems. See output below for
> >> more information:
> >>
> >
> Ya, I observed signoff issue. I have created next version files.
> I will send them tomorrow.
>
> But, I do not know how to solve the second warning.
> i.e multi line macro issue.
>
As I had written in my mail:
> > I guess your patch is ok here: checkpatch.pl is simply not smart enough to
> > parse this.
You can ignore this warning.
Also, you had a positive review for patch 2/2:
https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg04637.html
You can add this line below your S-o-b.
> -Pradeep
>
> > Pradeep,
> >
> > One should usually take patchew's findings into account. See below.
> >
> >> Type: series
> >> Subject: [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices
> >> Message-id: 1485186641-12220-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'
> >> 7686dc8 throttle: factor out duplicate code
> >> 7568614 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
> >
> > I guess your patch is ok here: checkpatch.pl is simply not smart enough to
> > parse this.
> >
> >> #228: 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",\
> >> + }
> >>
> >> ERROR: Missing Signed-off-by: line(s)
> >>
> >
> > As Eric pointed out in another mail, all patches you send must have your
> > Signed-off-by line.
> >
> >> total: 2 errors, 0 warnings, 278 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] 11+ messages in thread
end of thread, other threads:[~2017-01-23 16:42 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-23 15:50 [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices Pradeep Jagadeesh
2017-01-23 15:50 ` [Qemu-devel] [PATCH 1/2 " Pradeep Jagadeesh
2017-01-23 15:50 ` [Qemu-devel] [PATCH 2/2 V13] throttle: factor out duplicate code Pradeep Jagadeesh
2017-01-23 15:55 ` Eric Blake
2017-01-23 15:59 ` Pradeep Jagadeesh
2017-01-23 16:03 ` [Qemu-devel] [PATCH 0/2 V13] fsdev: add IO throttle support to fsdev devices no-reply
2017-01-23 16:22 ` Greg Kurz
2017-01-23 16:30 ` Pradeep Jagadeesh
2017-01-23 16:34 ` Eric Blake
2017-01-23 16:41 ` Pradeep Jagadeesh
2017-01-23 16:42 ` Greg Kurz
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.