From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33536) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cal5W-0002hj-Bt for qemu-devel@nongnu.org; Mon, 06 Feb 2017 10:20:20 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cal5T-0004He-3v for qemu-devel@nongnu.org; Mon, 06 Feb 2017 10:20:18 -0500 Received: from lhrrgout.huawei.com ([194.213.3.17]:6006) by eggs.gnu.org with esmtps (TLS1.0:RSA_ARCFOUR_SHA1:16) (Exim 4.71) (envelope-from ) id 1cal5S-0004ES-KT for qemu-devel@nongnu.org; Mon, 06 Feb 2017 10:20:15 -0500 References: <1486123043-26493-1-git-send-email-pradeep.jagadeesh@huawei.com> <1486123043-26493-3-git-send-email-pradeep.jagadeesh@huawei.com> <20170206155843.27adcaed@bahia.lan> From: Pradeep Jagadeesh Message-ID: Date: Mon, 6 Feb 2017 16:19:50 +0100 MIME-Version: 1.0 In-Reply-To: <20170206155843.27adcaed@bahia.lan> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/2 v16] throttle: factor out duplicate code List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Greg Kurz , Pradeep Jagadeesh Cc: "Aneesh Kumar K.V" , Alberto Garcia , qemu-devel@nongnu.org On 2/6/2017 3:58 PM, Greg Kurz wrote: > On Fri, 3 Feb 2017 06:57:23 -0500 > 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 >> >> 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 >