* [Qemu-devel] [PATCH 0/2 v16] fsdev: add IO throttle support to fsdev devices
@ 2017-02-03 11:57 Pradeep Jagadeesh
2017-02-03 11:57 ` [Qemu-devel] [PATCH 1/2 " Pradeep Jagadeesh
` (2 more replies)
0 siblings, 3 replies; 25+ messages in thread
From: Pradeep Jagadeesh @ 2017-02-03 11:57 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: factor out duplicate code
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] 25+ messages in thread
* [Qemu-devel] [PATCH 1/2 v16] fsdev: add IO throttle support to fsdev devices
2017-02-03 11:57 [Qemu-devel] [PATCH 0/2 v16] fsdev: add IO throttle support to fsdev devices Pradeep Jagadeesh
@ 2017-02-03 11:57 ` Pradeep Jagadeesh
2017-02-03 12:21 ` Alberto Garcia
2017-02-06 19:36 ` Eric Blake
2017-02-03 11:57 ` [Qemu-devel] [PATCH 2/2 v16] throttle: factor out duplicate code Pradeep Jagadeesh
2017-02-03 12:02 ` [Qemu-devel] [PATCH 0/2 v16] fsdev: add IO throttle support to fsdev devices no-reply
2 siblings, 2 replies; 25+ messages in thread
From: Pradeep Jagadeesh @ 2017-02-03 11:57 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.
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..22a6a99 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;
@@ -3540,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/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] 25+ messages in thread
* [Qemu-devel] [PATCH 2/2 v16] throttle: factor out duplicate code
2017-02-03 11:57 [Qemu-devel] [PATCH 0/2 v16] fsdev: add IO throttle support to fsdev devices Pradeep Jagadeesh
2017-02-03 11:57 ` [Qemu-devel] [PATCH 1/2 " Pradeep Jagadeesh
@ 2017-02-03 11:57 ` Pradeep Jagadeesh
2017-02-03 12:22 ` Alberto Garcia
2017-02-06 14:58 ` Greg Kurz
2017-02-03 12:02 ` [Qemu-devel] [PATCH 0/2 v16] fsdev: add IO throttle support to fsdev devices no-reply
2 siblings, 2 replies; 25+ messages in thread
From: Pradeep Jagadeesh @ 2017-02-03 11:57 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 ++---------------------------------
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] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2 v16] fsdev: add IO throttle support to fsdev devices
2017-02-03 11:57 [Qemu-devel] [PATCH 0/2 v16] fsdev: add IO throttle support to fsdev devices Pradeep Jagadeesh
2017-02-03 11:57 ` [Qemu-devel] [PATCH 1/2 " Pradeep Jagadeesh
2017-02-03 11:57 ` [Qemu-devel] [PATCH 2/2 v16] throttle: factor out duplicate code Pradeep Jagadeesh
@ 2017-02-03 12:02 ` no-reply
2 siblings, 0 replies; 25+ messages in thread
From: no-reply @ 2017-02-03 12:02 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 v16] fsdev: add IO throttle support to fsdev devices
Message-id: 1486123043-26493-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
From https://github.com/patchew-project/qemu
- [tag update] patchew/1486120416-11566-1-git-send-email-amit.shah@redhat.com -> patchew/1486120416-11566-1-git-send-email-amit.shah@redhat.com
- [tag update] patchew/1486120433-11628-1-git-send-email-amit.shah@redhat.com -> patchew/1486120433-11628-1-git-send-email-amit.shah@redhat.com
* [new tag] patchew/1486123043-26493-1-git-send-email-pradeep.jagadeesh@huawei.com -> patchew/1486123043-26493-1-git-send-email-pradeep.jagadeesh@huawei.com
- [tag update] patchew/20170201150553.9381-1-alex.bennee@linaro.org -> patchew/20170201150553.9381-1-alex.bennee@linaro.org
Switched to a new branch 'test'
ea942ba throttle: factor out duplicate code
b4896f1 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
#231: 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, 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] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2 v16] fsdev: add IO throttle support to fsdev devices
2017-02-03 11:57 ` [Qemu-devel] [PATCH 1/2 " Pradeep Jagadeesh
@ 2017-02-03 12:21 ` Alberto Garcia
2017-02-06 19:36 ` Eric Blake
1 sibling, 0 replies; 25+ messages in thread
From: Alberto Garcia @ 2017-02-03 12:21 UTC (permalink / raw)
To: Pradeep Jagadeesh, Aneesh Kumar K.V, Greg Kurz
Cc: Pradeep Jagadeesh, qemu-devel
On Fri 03 Feb 2017 12:57:22 PM CET, Pradeep Jagadeesh wrote:
> 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.
>
> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Berto
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2 v16] throttle: factor out duplicate code
2017-02-03 11:57 ` [Qemu-devel] [PATCH 2/2 v16] throttle: factor out duplicate code Pradeep Jagadeesh
@ 2017-02-03 12:22 ` Alberto Garcia
2017-02-03 12:27 ` Pradeep Jagadeesh
2017-02-06 14:58 ` Greg Kurz
1 sibling, 1 reply; 25+ messages in thread
From: Alberto Garcia @ 2017-02-03 12:22 UTC (permalink / raw)
To: Pradeep Jagadeesh, Aneesh Kumar K.V, Greg Kurz
Cc: Pradeep Jagadeesh, qemu-devel
On Fri 03 Feb 2017 12:57:23 PM 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.
>
> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
> https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg04637.html
This line doesn't belong in the commit message.
Berto
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2 v16] throttle: factor out duplicate code
2017-02-03 12:22 ` Alberto Garcia
@ 2017-02-03 12:27 ` Pradeep Jagadeesh
2017-02-03 12:44 ` Alberto Garcia
0 siblings, 1 reply; 25+ messages in thread
From: Pradeep Jagadeesh @ 2017-02-03 12:27 UTC (permalink / raw)
To: Alberto Garcia, Pradeep Jagadeesh, Aneesh Kumar K.V, Greg Kurz; +Cc: qemu-devel
On 2/3/2017 1:22 PM, Alberto Garcia wrote:
> On Fri 03 Feb 2017 12:57:23 PM 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.
>>
>> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
>
> Reviewed-by: Alberto Garcia <berto@igalia.com>
>
>> https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg04637.html
>
> This line doesn't belong in the commit message.
>
There is one false positive error in the second patch. So Greg asked me
to put this line after SoB. Just to say that I had a positive review
comment.
Regards.
Pradeep
> Berto
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2 v16] throttle: factor out duplicate code
2017-02-03 12:27 ` Pradeep Jagadeesh
@ 2017-02-03 12:44 ` Alberto Garcia
2017-02-03 12:46 ` Pradeep Jagadeesh
0 siblings, 1 reply; 25+ messages in thread
From: Alberto Garcia @ 2017-02-03 12:44 UTC (permalink / raw)
To: Pradeep Jagadeesh, Pradeep Jagadeesh, Aneesh Kumar K.V, Greg Kurz
Cc: qemu-devel
On Fri 03 Feb 2017 01:27:25 PM CET, Pradeep Jagadeesh wrote:
>>> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
>>
>> Reviewed-by: Alberto Garcia <berto@igalia.com>
>>
>>> https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg04637.html
>>
>> This line doesn't belong in the commit message.
>
> There is one false positive error in the second patch. So Greg asked
> me to put this line after SoB. Just to say that I had a positive
> review comment.
The false positive is not related to what Greg said :-) What you have to
put is not that URL but the Reviewed-by: line with Stefan's name and
e-mail.
Berto
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2 v16] throttle: factor out duplicate code
2017-02-03 12:44 ` Alberto Garcia
@ 2017-02-03 12:46 ` Pradeep Jagadeesh
2017-02-03 12:53 ` Alberto Garcia
0 siblings, 1 reply; 25+ messages in thread
From: Pradeep Jagadeesh @ 2017-02-03 12:46 UTC (permalink / raw)
To: Alberto Garcia, Pradeep Jagadeesh, Aneesh Kumar K.V, Greg Kurz; +Cc: qemu-devel
On 2/3/2017 1:44 PM, Alberto Garcia wrote:
> On Fri 03 Feb 2017 01:27:25 PM CET, Pradeep Jagadeesh wrote:
>
>>>> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
>>>
>>> Reviewed-by: Alberto Garcia <berto@igalia.com>
>>>
>>>> https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg04637.html
>>>
>>> This line doesn't belong in the commit message.
>>
>> There is one false positive error in the second patch. So Greg asked
>> me to put this line after SoB. Just to say that I had a positive
>> review comment.
>
> The false positive is not related to what Greg said :-) What you have to
> put is not that URL but the Reviewed-by: line with Stefan's name and
> e-mail.
Haha OK. I will change it and send new patch set again.
-Pradeep
>
> Berto
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2 v16] throttle: factor out duplicate code
2017-02-03 12:46 ` Pradeep Jagadeesh
@ 2017-02-03 12:53 ` Alberto Garcia
2017-02-03 12:57 ` Pradeep Jagadeesh
0 siblings, 1 reply; 25+ messages in thread
From: Alberto Garcia @ 2017-02-03 12:53 UTC (permalink / raw)
To: Pradeep Jagadeesh, Pradeep Jagadeesh, Aneesh Kumar K.V, Greg Kurz
Cc: qemu-devel
On Fri 03 Feb 2017 01:46:31 PM CET, Pradeep Jagadeesh wrote:
>> The false positive is not related to what Greg said :-) What you have
>> to put is not that URL but the Reviewed-by: line with Stefan's name
>> and e-mail.
>
> Haha OK. I will change it and send new patch set again.
I don't know if it's necessary now, but if you need to send the patch
again you have to keep your Signed-off-by line and my and Stefan's
Reviewed-by lines.
Berto
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2 v16] throttle: factor out duplicate code
2017-02-03 12:53 ` Alberto Garcia
@ 2017-02-03 12:57 ` Pradeep Jagadeesh
0 siblings, 0 replies; 25+ messages in thread
From: Pradeep Jagadeesh @ 2017-02-03 12:57 UTC (permalink / raw)
To: Alberto Garcia, Pradeep Jagadeesh, Aneesh Kumar K.V, Greg Kurz; +Cc: qemu-devel
On 2/3/2017 1:53 PM, Alberto Garcia wrote:
> On Fri 03 Feb 2017 01:46:31 PM CET, Pradeep Jagadeesh wrote:
>>> The false positive is not related to what Greg said :-) What you have
>>> to put is not that URL but the Reviewed-by: line with Stefan's name
>>> and e-mail.
>>
>> Haha OK. I will change it and send new patch set again.
>
> I don't know if it's necessary now, but if you need to send the patch
> again you have to keep your Signed-off-by line and my and Stefan's
> Reviewed-by lines.
OK sure!. If it is required, then I will add Reviewed by Line.
Regards,
Pradeep
>
> Berto
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2 v16] throttle: factor out duplicate code
2017-02-03 11:57 ` [Qemu-devel] [PATCH 2/2 v16] throttle: factor out duplicate code Pradeep Jagadeesh
2017-02-03 12:22 ` Alberto Garcia
@ 2017-02-06 14:58 ` Greg Kurz
2017-02-06 15:19 ` Pradeep Jagadeesh
1 sibling, 1 reply; 25+ messages in thread
From: Greg Kurz @ 2017-02-06 14:58 UTC (permalink / raw)
To: Pradeep Jagadeesh
Cc: Aneesh Kumar K.V, Pradeep Jagadeesh, Alberto Garcia, qemu-devel
On Fri, 3 Feb 2017 06:57:23 -0500
Pradeep Jagadeesh <pradeepkiruvale@gmail.com> 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.
>
> 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 ++---------------------------------
> 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,
> + {
Indent nit ^^. Also this could be surrounded by some empty lines as you
did in fsdev/qemu-fsdev-opts.c below.
I wouldn't ask you to send a v17 for this though and I've updated your
patch instead. I've also added Stefan's and Berto's Reviewed-by, and
pushed the whole series to my 9p-next tree at:
https://github.com/gkurz/qemu/commits/9p-next
Cheers.
--
Greg
> .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
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2 v16] throttle: factor out duplicate code
2017-02-06 14:58 ` Greg Kurz
@ 2017-02-06 15:19 ` Pradeep Jagadeesh
0 siblings, 0 replies; 25+ messages in thread
From: Pradeep Jagadeesh @ 2017-02-06 15:19 UTC (permalink / raw)
To: Greg Kurz, Pradeep Jagadeesh; +Cc: Aneesh Kumar K.V, Alberto Garcia, qemu-devel
On 2/6/2017 3:58 PM, Greg Kurz wrote:
> On Fri, 3 Feb 2017 06:57:23 -0500
> Pradeep Jagadeesh <pradeepkiruvale@gmail.com> 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.
>>
>> 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 ++---------------------------------
>> 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,
>> + {
>
> Indent nit ^^. Also this could be surrounded by some empty lines as you
> did in fsdev/qemu-fsdev-opts.c below.
>
> I wouldn't ask you to send a v17 for this though and I've updated your
> patch instead. I've also added Stefan's and Berto's Reviewed-by, and
> pushed the whole series to my 9p-next tree at:
>
> https://github.com/gkurz/qemu/commits/9p-next
Thanks Greg for updating the patch!.
-Pradeep
>
> Cheers.
>
> --
> Greg
>
>> .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
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2 v16] fsdev: add IO throttle support to fsdev devices
2017-02-03 11:57 ` [Qemu-devel] [PATCH 1/2 " Pradeep Jagadeesh
2017-02-03 12:21 ` Alberto Garcia
@ 2017-02-06 19:36 ` Eric Blake
2017-02-06 23:15 ` Greg Kurz
` (2 more replies)
1 sibling, 3 replies; 25+ messages in thread
From: Eric Blake @ 2017-02-06 19:36 UTC (permalink / raw)
To: Pradeep Jagadeesh, Aneesh Kumar K.V, Greg Kurz
Cc: Alberto Garcia, Pradeep Jagadeesh, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 768 bytes --]
On 02/03/2017 05:57 AM, Pradeep Jagadeesh wrote:
> 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.
This part is a bit scary - if 2.9 is released with just the cli option
and not the QMP interface, then how does someone like libvirt introspect
whether the feature is available for use?
Please make sure we don't reach 2.9 with only a half-baked feature;
whether that means finishing the QMP work or temporarily disabling the
cli additions until a later release can finish the work.
--
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] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2 v16] fsdev: add IO throttle support to fsdev devices
2017-02-06 19:36 ` Eric Blake
@ 2017-02-06 23:15 ` Greg Kurz
2017-02-07 10:32 ` Greg Kurz
2017-02-07 6:18 ` Fam Zheng
2017-02-07 10:34 ` Pradeep Jagadeesh
2 siblings, 1 reply; 25+ messages in thread
From: Greg Kurz @ 2017-02-06 23:15 UTC (permalink / raw)
To: Eric Blake
Cc: Pradeep Jagadeesh, Aneesh Kumar K.V, Alberto Garcia,
Pradeep Jagadeesh, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1076 bytes --]
On Mon, 6 Feb 2017 13:36:43 -0600
Eric Blake <eblake@redhat.com> wrote:
> On 02/03/2017 05:57 AM, Pradeep Jagadeesh wrote:
> > 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.
>
> This part is a bit scary - if 2.9 is released with just the cli option
> and not the QMP interface, then how does someone like libvirt introspect
> whether the feature is available for use?
>
I'm not aware of anything related to fsdev in QMP... and libvirt seems to
only parse the output of -help to guess fsdev capabilities. And indeed,
qemu-options.hx doesn't expose this new feature.
> Please make sure we don't reach 2.9 with only a half-baked feature;
> whether that means finishing the QMP work or temporarily disabling the
> cli additions until a later release can finish the work.
>
Would this be ok to add the missing bits in qemu-options.hx or do you
expect more ?
Cheers.
--
Greg
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2 v16] fsdev: add IO throttle support to fsdev devices
2017-02-06 19:36 ` Eric Blake
2017-02-06 23:15 ` Greg Kurz
@ 2017-02-07 6:18 ` Fam Zheng
2017-02-07 10:34 ` Pradeep Jagadeesh
2 siblings, 0 replies; 25+ messages in thread
From: Fam Zheng @ 2017-02-07 6:18 UTC (permalink / raw)
To: Eric Blake
Cc: Pradeep Jagadeesh, Aneesh Kumar K.V, Greg Kurz, Alberto Garcia,
Pradeep Jagadeesh, qemu-devel
On Mon, 02/06 13:36, Eric Blake wrote:
> On 02/03/2017 05:57 AM, Pradeep Jagadeesh wrote:
> > 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.
>
> This part is a bit scary - if 2.9 is released with just the cli option
> and not the QMP interface, then how does someone like libvirt introspect
> whether the feature is available for use?
Not that I want to make an unconstructive point, but just for understanding
this: if libvirt does introspect via QMP, is it technically okay to release CLI
in 2.9 and QMP in 2.10? libvirt can just ignore the possible existence of CLI
options if QMP is not there, no?
Fam
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2 v16] fsdev: add IO throttle support to fsdev devices
2017-02-06 23:15 ` Greg Kurz
@ 2017-02-07 10:32 ` Greg Kurz
2017-02-07 15:56 ` Eric Blake
0 siblings, 1 reply; 25+ messages in thread
From: Greg Kurz @ 2017-02-07 10:32 UTC (permalink / raw)
To: Eric Blake
Cc: Pradeep Jagadeesh, Aneesh Kumar K.V, Alberto Garcia,
Pradeep Jagadeesh, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1358 bytes --]
On Tue, 7 Feb 2017 00:15:33 +0100
Greg Kurz <groug@kaod.org> wrote:
> On Mon, 6 Feb 2017 13:36:43 -0600
> Eric Blake <eblake@redhat.com> wrote:
>
> > On 02/03/2017 05:57 AM, Pradeep Jagadeesh wrote:
> > > 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.
> >
> > This part is a bit scary - if 2.9 is released with just the cli option
> > and not the QMP interface, then how does someone like libvirt introspect
> > whether the feature is available for use?
> >
>
> I'm not aware of anything related to fsdev in QMP... and libvirt seems to
> only parse the output of -help to guess fsdev capabilities.
Oops, reading some more libvirt code I now see that libvirt doesn't parse
-help anymore with QEMU >= 1.2.0... sorry for the noise :)
> And indeed,
> qemu-options.hx doesn't expose this new feature.
>
> > Please make sure we don't reach 2.9 with only a half-baked feature;
> > whether that means finishing the QMP work or temporarily disabling the
> > cli additions until a later release can finish the work.
> >
>
> Would this be ok to add the missing bits in qemu-options.hx or do you
> expect more ?
>
> Cheers.
>
> --
> Greg
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2 v16] fsdev: add IO throttle support to fsdev devices
2017-02-06 19:36 ` Eric Blake
2017-02-06 23:15 ` Greg Kurz
2017-02-07 6:18 ` Fam Zheng
@ 2017-02-07 10:34 ` Pradeep Jagadeesh
2 siblings, 0 replies; 25+ messages in thread
From: Pradeep Jagadeesh @ 2017-02-07 10:34 UTC (permalink / raw)
To: Eric Blake, Pradeep Jagadeesh, Aneesh Kumar K.V, Greg Kurz
Cc: Alberto Garcia, qemu-devel
On 2/6/2017 8:36 PM, Eric Blake wrote:
> On 02/03/2017 05:57 AM, Pradeep Jagadeesh wrote:
>> 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.
>
> This part is a bit scary - if 2.9 is released with just the cli option
> and not the QMP interface, then how does someone like libvirt introspect
> whether the feature is available for use?
I do have plans to extend the qmp interface for this feature.
Already started looking into it.Do not know, I can push it for 2.9.
Because I am busy with some other work.
>
> Please make sure we don't reach 2.9 with only a half-baked feature;
> whether that means finishing the QMP work or temporarily disabling the
> cli additions until a later release can finish the work.
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2 v16] fsdev: add IO throttle support to fsdev devices
2017-02-07 10:32 ` Greg Kurz
@ 2017-02-07 15:56 ` Eric Blake
2017-02-07 16:29 ` Greg Kurz
0 siblings, 1 reply; 25+ messages in thread
From: Eric Blake @ 2017-02-07 15:56 UTC (permalink / raw)
To: Greg Kurz
Cc: Pradeep Jagadeesh, Aneesh Kumar K.V, Alberto Garcia,
Pradeep Jagadeesh, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1341 bytes --]
On 02/07/2017 04:32 AM, Greg Kurz wrote:
>>
>> I'm not aware of anything related to fsdev in QMP... and libvirt seems to
>> only parse the output of -help to guess fsdev capabilities.
>
> Oops, reading some more libvirt code I now see that libvirt doesn't parse
> -help anymore with QEMU >= 1.2.0... sorry for the noise :)
>
>> And indeed,
>> qemu-options.hx doesn't expose this new feature.
>>
>>> Please make sure we don't reach 2.9 with only a half-baked feature;
>>> whether that means finishing the QMP work or temporarily disabling the
>>> cli additions until a later release can finish the work.
>>>
>>
>> Would this be ok to add the missing bits in qemu-options.hx or do you
>> expect more ?
If it cannot be probed via QMP, then libvirt will most likely assume
that it does not exist. I guess we're okay having command line only in
2.9 if you can't get QMP working, because libvirt will just never drive
the feature until 2.10 when QMP is available; but then we risk the
command line subtly changing and breaking someone else that was using
the command line without QMP. Maybe the safest approach is to just use
the 'x-' prefix to the command line portion, until the feature is complete.
--
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] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2 v16] fsdev: add IO throttle support to fsdev devices
2017-02-07 15:56 ` Eric Blake
@ 2017-02-07 16:29 ` Greg Kurz
2017-02-14 13:21 ` Stefan Hajnoczi
0 siblings, 1 reply; 25+ messages in thread
From: Greg Kurz @ 2017-02-07 16:29 UTC (permalink / raw)
To: Eric Blake
Cc: Pradeep Jagadeesh, Aneesh Kumar K.V, Alberto Garcia,
Pradeep Jagadeesh, qemu-devel, Stefan Hajnoczi
[-- Attachment #1: Type: text/plain, Size: 1917 bytes --]
Cc'ing Stefan who reviewed patch 2/2.
On Tue, 7 Feb 2017 09:56:08 -0600
Eric Blake <eblake@redhat.com> wrote:
> On 02/07/2017 04:32 AM, Greg Kurz wrote:
> >>
> >> I'm not aware of anything related to fsdev in QMP... and libvirt seems to
> >> only parse the output of -help to guess fsdev capabilities.
> >
> > Oops, reading some more libvirt code I now see that libvirt doesn't parse
> > -help anymore with QEMU >= 1.2.0... sorry for the noise :)
> >
> >> And indeed,
> >> qemu-options.hx doesn't expose this new feature.
> >>
> >>> Please make sure we don't reach 2.9 with only a half-baked feature;
> >>> whether that means finishing the QMP work or temporarily disabling the
> >>> cli additions until a later release can finish the work.
> >>>
> >>
> >> Would this be ok to add the missing bits in qemu-options.hx or do you
> >> expect more ?
>
> If it cannot be probed via QMP, then libvirt will most likely assume
> that it does not exist. I guess we're okay having command line only in
> 2.9 if you can't get QMP working, because libvirt will just never drive
> the feature until 2.10 when QMP is available; but then we risk the
> command line subtly changing and breaking someone else that was using
> the command line without QMP. Maybe the safest approach is to just use
> the 'x-' prefix to the command line portion, until the feature is complete.
>
The semantics here are exactly the same as for block devices. The
command line options added to -fsdev are the very same already used
by -drive for years.
Patch 2/2 in this series even factors them out to a common header file
to be used by fsdev and blockdev. I really don't expect any modification
at all on the command line (nor the other people who reviewed that patch
obviously)... are you suggesting that we should put 2/2 on hold and
use the 'x-' prefix anyway ?
Cheers.
--
Greg
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2 v16] fsdev: add IO throttle support to fsdev devices
2017-02-07 16:29 ` Greg Kurz
@ 2017-02-14 13:21 ` Stefan Hajnoczi
2017-02-22 13:41 ` Greg Kurz
0 siblings, 1 reply; 25+ messages in thread
From: Stefan Hajnoczi @ 2017-02-14 13:21 UTC (permalink / raw)
To: Greg Kurz
Cc: Eric Blake, Pradeep Jagadeesh, Aneesh Kumar K.V, Alberto Garcia,
Pradeep Jagadeesh, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2137 bytes --]
On Tue, Feb 07, 2017 at 05:29:33PM +0100, Greg Kurz wrote:
> Cc'ing Stefan who reviewed patch 2/2.
>
> On Tue, 7 Feb 2017 09:56:08 -0600
> Eric Blake <eblake@redhat.com> wrote:
>
> > On 02/07/2017 04:32 AM, Greg Kurz wrote:
> > >>
> > >> I'm not aware of anything related to fsdev in QMP... and libvirt seems to
> > >> only parse the output of -help to guess fsdev capabilities.
> > >
> > > Oops, reading some more libvirt code I now see that libvirt doesn't parse
> > > -help anymore with QEMU >= 1.2.0... sorry for the noise :)
> > >
> > >> And indeed,
> > >> qemu-options.hx doesn't expose this new feature.
> > >>
> > >>> Please make sure we don't reach 2.9 with only a half-baked feature;
> > >>> whether that means finishing the QMP work or temporarily disabling the
> > >>> cli additions until a later release can finish the work.
> > >>>
> > >>
> > >> Would this be ok to add the missing bits in qemu-options.hx or do you
> > >> expect more ?
> >
> > If it cannot be probed via QMP, then libvirt will most likely assume
> > that it does not exist. I guess we're okay having command line only in
> > 2.9 if you can't get QMP working, because libvirt will just never drive
> > the feature until 2.10 when QMP is available; but then we risk the
> > command line subtly changing and breaking someone else that was using
> > the command line without QMP. Maybe the safest approach is to just use
> > the 'x-' prefix to the command line portion, until the feature is complete.
> >
>
> The semantics here are exactly the same as for block devices. The
> command line options added to -fsdev are the very same already used
> by -drive for years.
>
> Patch 2/2 in this series even factors them out to a common header file
> to be used by fsdev and blockdev. I really don't expect any modification
> at all on the command line (nor the other people who reviewed that patch
> obviously)... are you suggesting that we should put 2/2 on hold and
> use the 'x-' prefix anyway ?
I see these parameter names as stable. There is little risk that they
would change.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2 v16] fsdev: add IO throttle support to fsdev devices
2017-02-14 13:21 ` Stefan Hajnoczi
@ 2017-02-22 13:41 ` Greg Kurz
2017-02-22 16:01 ` Eric Blake
0 siblings, 1 reply; 25+ messages in thread
From: Greg Kurz @ 2017-02-22 13:41 UTC (permalink / raw)
To: Eric Blake
Cc: Stefan Hajnoczi, Pradeep Jagadeesh, Aneesh Kumar K.V,
Alberto Garcia, Pradeep Jagadeesh, qemu-devel, Fam Zheng
[-- Attachment #1: Type: text/plain, Size: 2593 bytes --]
Eric,
I fully understand your concern about the missing QMP bits, but given the other
comments people made on this series, I'd like to move forward and merge it for
2.9, without the 'x-' prefixed options. Is it okay with you ?
Cheers.
--
Greg
On Tue, 14 Feb 2017 13:21:22 +0000
Stefan Hajnoczi <stefanha@redhat.com> wrote:
> On Tue, Feb 07, 2017 at 05:29:33PM +0100, Greg Kurz wrote:
> > Cc'ing Stefan who reviewed patch 2/2.
> >
> > On Tue, 7 Feb 2017 09:56:08 -0600
> > Eric Blake <eblake@redhat.com> wrote:
> >
> > > On 02/07/2017 04:32 AM, Greg Kurz wrote:
> > > >>
> > > >> I'm not aware of anything related to fsdev in QMP... and libvirt seems to
> > > >> only parse the output of -help to guess fsdev capabilities.
> > > >
> > > > Oops, reading some more libvirt code I now see that libvirt doesn't parse
> > > > -help anymore with QEMU >= 1.2.0... sorry for the noise :)
> > > >
> > > >> And indeed,
> > > >> qemu-options.hx doesn't expose this new feature.
> > > >>
> > > >>> Please make sure we don't reach 2.9 with only a half-baked feature;
> > > >>> whether that means finishing the QMP work or temporarily disabling the
> > > >>> cli additions until a later release can finish the work.
> > > >>>
> > > >>
> > > >> Would this be ok to add the missing bits in qemu-options.hx or do you
> > > >> expect more ?
> > >
> > > If it cannot be probed via QMP, then libvirt will most likely assume
> > > that it does not exist. I guess we're okay having command line only in
> > > 2.9 if you can't get QMP working, because libvirt will just never drive
> > > the feature until 2.10 when QMP is available; but then we risk the
> > > command line subtly changing and breaking someone else that was using
> > > the command line without QMP. Maybe the safest approach is to just use
> > > the 'x-' prefix to the command line portion, until the feature is complete.
> > >
> >
> > The semantics here are exactly the same as for block devices. The
> > command line options added to -fsdev are the very same already used
> > by -drive for years.
> >
> > Patch 2/2 in this series even factors them out to a common header file
> > to be used by fsdev and blockdev. I really don't expect any modification
> > at all on the command line (nor the other people who reviewed that patch
> > obviously)... are you suggesting that we should put 2/2 on hold and
> > use the 'x-' prefix anyway ?
>
> I see these parameter names as stable. There is little risk that they
> would change.
>
> Stefan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2 v16] fsdev: add IO throttle support to fsdev devices
2017-02-22 13:41 ` Greg Kurz
@ 2017-02-22 16:01 ` Eric Blake
2017-02-22 16:59 ` Greg Kurz
0 siblings, 1 reply; 25+ messages in thread
From: Eric Blake @ 2017-02-22 16:01 UTC (permalink / raw)
To: Greg Kurz
Cc: Stefan Hajnoczi, Pradeep Jagadeesh, Aneesh Kumar K.V,
Alberto Garcia, Pradeep Jagadeesh, qemu-devel, Fam Zheng
[-- Attachment #1: Type: text/plain, Size: 1952 bytes --]
On 02/22/2017 07:41 AM, Greg Kurz wrote:
> Eric,
>
> I fully understand your concern about the missing QMP bits, but given the other
> comments people made on this series, I'd like to move forward and merge it for
> 2.9, without the 'x-' prefixed options. Is it okay with you ?
Yes, I think we'll be okay. Libvirt will just be unable to recognize the
feature until the QMP bits are in place.
>>>> If it cannot be probed via QMP, then libvirt will most likely assume
>>>> that it does not exist. I guess we're okay having command line only in
>>>> 2.9 if you can't get QMP working, because libvirt will just never drive
>>>> the feature until 2.10 when QMP is available; but then we risk the
>>>> command line subtly changing and breaking someone else that was using
>>>> the command line without QMP. Maybe the safest approach is to just use
>>>> the 'x-' prefix to the command line portion, until the feature is complete.
>>>>
>>>
>>> The semantics here are exactly the same as for block devices. The
>>> command line options added to -fsdev are the very same already used
>>> by -drive for years.
>>>
>>> Patch 2/2 in this series even factors them out to a common header file
>>> to be used by fsdev and blockdev. I really don't expect any modification
>>> at all on the command line (nor the other people who reviewed that patch
>>> obviously)... are you suggesting that we should put 2/2 on hold and
>>> use the 'x-' prefix anyway ?
>>
>> I see these parameter names as stable. There is little risk that they
>> would change.
We just need to be sure that when we do add QMP, that it doesn't use
names that differ from the command line (blockdev is an example where we
weren't careful in 2.8, and had to rename things in 2.9 to be
consistent, and I want to avoid repeating that mistake).
--
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] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2 v16] fsdev: add IO throttle support to fsdev devices
2017-02-22 16:01 ` Eric Blake
@ 2017-02-22 16:59 ` Greg Kurz
2017-02-22 17:04 ` Eric Blake
0 siblings, 1 reply; 25+ messages in thread
From: Greg Kurz @ 2017-02-22 16:59 UTC (permalink / raw)
To: Eric Blake
Cc: Stefan Hajnoczi, Pradeep Jagadeesh, Aneesh Kumar K.V,
Alberto Garcia, Pradeep Jagadeesh, qemu-devel, Fam Zheng
[-- Attachment #1: Type: text/plain, Size: 2149 bytes --]
On Wed, 22 Feb 2017 10:01:09 -0600
Eric Blake <eblake@redhat.com> wrote:
> On 02/22/2017 07:41 AM, Greg Kurz wrote:
> > Eric,
> >
> > I fully understand your concern about the missing QMP bits, but given the other
> > comments people made on this series, I'd like to move forward and merge it for
> > 2.9, without the 'x-' prefixed options. Is it okay with you ?
>
> Yes, I think we'll be okay. Libvirt will just be unable to recognize the
> feature until the QMP bits are in place.
>
Ok. I'll work this out with Pradeep for 2.10.
>
> >>>> If it cannot be probed via QMP, then libvirt will most likely assume
> >>>> that it does not exist. I guess we're okay having command line only in
> >>>> 2.9 if you can't get QMP working, because libvirt will just never drive
> >>>> the feature until 2.10 when QMP is available; but then we risk the
> >>>> command line subtly changing and breaking someone else that was using
> >>>> the command line without QMP. Maybe the safest approach is to just use
> >>>> the 'x-' prefix to the command line portion, until the feature is complete.
> >>>>
> >>>
> >>> The semantics here are exactly the same as for block devices. The
> >>> command line options added to -fsdev are the very same already used
> >>> by -drive for years.
> >>>
> >>> Patch 2/2 in this series even factors them out to a common header file
> >>> to be used by fsdev and blockdev. I really don't expect any modification
> >>> at all on the command line (nor the other people who reviewed that patch
> >>> obviously)... are you suggesting that we should put 2/2 on hold and
> >>> use the 'x-' prefix anyway ?
> >>
> >> I see these parameter names as stable. There is little risk that they
> >> would change.
>
> We just need to be sure that when we do add QMP, that it doesn't use
> names that differ from the command line (blockdev is an example where we
> weren't careful in 2.8, and had to rename things in 2.9 to be
> consistent, and I want to avoid repeating that mistake).
>
Can you provide some pointers on what had to be fixed in blockdev ?
Thanks.
--
Greg
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2 v16] fsdev: add IO throttle support to fsdev devices
2017-02-22 16:59 ` Greg Kurz
@ 2017-02-22 17:04 ` Eric Blake
0 siblings, 0 replies; 25+ messages in thread
From: Eric Blake @ 2017-02-22 17:04 UTC (permalink / raw)
To: Greg Kurz
Cc: Stefan Hajnoczi, Pradeep Jagadeesh, Aneesh Kumar K.V,
Alberto Garcia, Pradeep Jagadeesh, qemu-devel, Fam Zheng
[-- Attachment #1: Type: text/plain, Size: 730 bytes --]
On 02/22/2017 10:59 AM, Greg Kurz wrote:
>>>> I see these parameter names as stable. There is little risk that they
>>>> would change.
>>
>> We just need to be sure that when we do add QMP, that it doesn't use
>> names that differ from the command line (blockdev is an example where we
>> weren't careful in 2.8, and had to rename things in 2.9 to be
>> consistent, and I want to avoid repeating that mistake).
>>
>
> Can you provide some pointers on what had to be fixed in blockdev ?
See for example commits f67409a, 4230e5d, 1a417e4 showing several cases
where we had to fix up inconsistencies.
--
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] 25+ messages in thread
end of thread, other threads:[~2017-02-22 17:04 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-03 11:57 [Qemu-devel] [PATCH 0/2 v16] fsdev: add IO throttle support to fsdev devices Pradeep Jagadeesh
2017-02-03 11:57 ` [Qemu-devel] [PATCH 1/2 " Pradeep Jagadeesh
2017-02-03 12:21 ` Alberto Garcia
2017-02-06 19:36 ` Eric Blake
2017-02-06 23:15 ` Greg Kurz
2017-02-07 10:32 ` Greg Kurz
2017-02-07 15:56 ` Eric Blake
2017-02-07 16:29 ` Greg Kurz
2017-02-14 13:21 ` Stefan Hajnoczi
2017-02-22 13:41 ` Greg Kurz
2017-02-22 16:01 ` Eric Blake
2017-02-22 16:59 ` Greg Kurz
2017-02-22 17:04 ` Eric Blake
2017-02-07 6:18 ` Fam Zheng
2017-02-07 10:34 ` Pradeep Jagadeesh
2017-02-03 11:57 ` [Qemu-devel] [PATCH 2/2 v16] throttle: factor out duplicate code Pradeep Jagadeesh
2017-02-03 12:22 ` Alberto Garcia
2017-02-03 12:27 ` Pradeep Jagadeesh
2017-02-03 12:44 ` Alberto Garcia
2017-02-03 12:46 ` Pradeep Jagadeesh
2017-02-03 12:53 ` Alberto Garcia
2017-02-03 12:57 ` Pradeep Jagadeesh
2017-02-06 14:58 ` Greg Kurz
2017-02-06 15:19 ` Pradeep Jagadeesh
2017-02-03 12:02 ` [Qemu-devel] [PATCH 0/2 v16] fsdev: add IO throttle support to fsdev devices no-reply
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.