From: Greg Kurz <groug@kaod.org>
To: Pradeep Jagadeesh <pradeepkiruvale@gmail.com>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>,
Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>,
Alberto Garcia <berto@igalia.com>,
qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 2/2 v16] throttle: factor out duplicate code
Date: Mon, 6 Feb 2017 15:58:43 +0100 [thread overview]
Message-ID: <20170206155843.27adcaed@bahia.lan> (raw)
In-Reply-To: <1486123043-26493-3-git-send-email-pradeep.jagadeesh@huawei.com>
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
next prev parent reply other threads:[~2017-02-06 14:58 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170206155843.27adcaed@bahia.lan \
--to=groug@kaod.org \
--cc=aneesh.kumar@linux.vnet.ibm.com \
--cc=berto@igalia.com \
--cc=pradeep.jagadeesh@huawei.com \
--cc=pradeepkiruvale@gmail.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.