[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2 v16] throttle: factor out duplicate code
|
From: |
Greg Kurz |
|
Subject: |
Re: [Qemu-devel] [PATCH 2/2 v16] throttle: factor out duplicate code |
|
Date: |
Mon, 6 Feb 2017 15:58:43 +0100 |
On Fri, 3 Feb 2017 06:57:23 -0500
Pradeep Jagadeesh <address@hidden> 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 <address@hidden>
>
> 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
- Re: [Qemu-devel] [PATCH 1/2 v16] fsdev: add IO throttle support to fsdev devices, (continued)
- [Qemu-devel] [PATCH 2/2 v16] throttle: factor out duplicate code, Pradeep Jagadeesh, 2017/02/03
- Re: [Qemu-devel] [PATCH 2/2 v16] throttle: factor out duplicate code, Alberto Garcia, 2017/02/03
- Re: [Qemu-devel] [PATCH 2/2 v16] throttle: factor out duplicate code, Pradeep Jagadeesh, 2017/02/03
- Re: [Qemu-devel] [PATCH 2/2 v16] throttle: factor out duplicate code, Alberto Garcia, 2017/02/03
- Re: [Qemu-devel] [PATCH 2/2 v16] throttle: factor out duplicate code, Pradeep Jagadeesh, 2017/02/03
- Re: [Qemu-devel] [PATCH 2/2 v16] throttle: factor out duplicate code, Alberto Garcia, 2017/02/03
- Re: [Qemu-devel] [PATCH 2/2 v16] throttle: factor out duplicate code, Pradeep Jagadeesh, 2017/02/03
Re: [Qemu-devel] [PATCH 2/2 v16] throttle: factor out duplicate code,
Greg Kurz <=
Re: [Qemu-devel] [PATCH 0/2 v16] fsdev: add IO throttle support to fsdev devices, no-reply, 2017/02/03