Message ID | 147981681351.30309.4854065801791462661.stgit@bahia.lab.toulouse-stg.fr.ibm.com |
---|---|
State | New |
Headers | show |
Hi, Your series seems to have some coding style problems. See output below for more information: Subject: [Qemu-devel] [RFC PATCH] throttle: move throttling cmdline options to a separate header file Type: series Message-id: 147981681351.30309.4854065801791462661.stgit@bahia.lab.toulouse-stg.fr.ibm.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 * [new tag] patchew/147981681351.30309.4854065801791462661.stgit@bahia.lab.toulouse-stg.fr.ibm.com -> patchew/147981681351.30309.4854065801791462661.stgit@bahia.lab.toulouse-stg.fr.ibm.com Switched to a new branch 'test' f5fd85c throttle: move throttling cmdline options to a separate header file === OUTPUT BEGIN === Checking PATCH 1/1: throttle: move throttling cmdline options to a separate header file... ERROR: Macros with multiple statements should be enclosed in a do - while loop #133: FILE: include/qemu/throttle-options.h:14: +#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, 188 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
On Tue 22 Nov 2016 01:49:51 PM CET, Greg Kurz wrote: > This will allow other subsystems (i.e. fsdev) to implement throttling > without duplicating the command line options. > > Signed-off-by: Greg Kurz <groug@kaod.org> Did you check if it's possible / easy to do defining a normal array instead of a macro and using qemu_opts_append() or something similar? Berto
On Tue, 22 Nov 2016 14:51:12 +0100 Alberto Garcia <berto@igalia.com> wrote: > On Tue 22 Nov 2016 01:49:51 PM CET, Greg Kurz wrote: > > > This will allow other subsystems (i.e. fsdev) to implement throttling > > without duplicating the command line options. > > > > Signed-off-by: Greg Kurz <groug@kaod.org> > > Did you check if it's possible / easy to do defining a normal array > instead of a macro and using qemu_opts_append() or something similar? > > Berto No I didn't due to lack of time but maybe Pradeep may have a look in this direction ? Cheers. -- Greg
On 22 November 2016 at 14:55, Greg Kurz <groug@kaod.org> wrote: > On Tue, 22 Nov 2016 14:51:12 +0100 > Alberto Garcia <berto@igalia.com> wrote: > > > On Tue 22 Nov 2016 01:49:51 PM CET, Greg Kurz wrote: > > > > > This will allow other subsystems (i.e. fsdev) to implement throttling > > > without duplicating the command line options. > > > > > > Signed-off-by: Greg Kurz <groug@kaod.org> > > > > Did you check if it's possible / easy to do defining a normal array > > instead of a macro and using qemu_opts_append() or something similar? > > > > Berto > > No I didn't due to lack of time but maybe Pradeep may have a look in this > direction ? > Only may be next year :) -Pradeep > > Cheers. > > -- > Greg >
On Tue, 22 Nov 2016 16:02:12 +0100 Pradeep Kiruvale <pradeepkiruvale@gmail.com> wrote: > On 22 November 2016 at 14:55, Greg Kurz <groug@kaod.org> wrote: > > > On Tue, 22 Nov 2016 14:51:12 +0100 > > Alberto Garcia <berto@igalia.com> wrote: > > > > > On Tue 22 Nov 2016 01:49:51 PM CET, Greg Kurz wrote: > > > > > > > This will allow other subsystems (i.e. fsdev) to implement throttling > > > > without duplicating the command line options. > > > > > > > > Signed-off-by: Greg Kurz <groug@kaod.org> > > > > > > Did you check if it's possible / easy to do defining a normal array > > > instead of a macro and using qemu_opts_append() or something similar? > > > > > > Berto > > > > No I didn't due to lack of time but maybe Pradeep may have a look in this > > direction ? > > > > Only may be next year :) > Ok, and also, it will be the occasion to add the throttling options to the legacy -virtfs command line option as well. Cheers. -- Greg > -Pradeep > > > > > > Cheers. > > > > -- > > Greg > >
On 23 November 2016 at 13:13, Greg Kurz <groug@kaod.org> wrote: > On Tue, 22 Nov 2016 16:02:12 +0100 > Pradeep Kiruvale <pradeepkiruvale@gmail.com> wrote: > > > On 22 November 2016 at 14:55, Greg Kurz <groug@kaod.org> wrote: > > > > > On Tue, 22 Nov 2016 14:51:12 +0100 > > > Alberto Garcia <berto@igalia.com> wrote: > > > > > > > On Tue 22 Nov 2016 01:49:51 PM CET, Greg Kurz wrote: > > > > > > > > > This will allow other subsystems (i.e. fsdev) to implement > throttling > > > > > without duplicating the command line options. > > > > > > > > > > Signed-off-by: Greg Kurz <groug@kaod.org> > > > > > > > > Did you check if it's possible / easy to do defining a normal array > > > > instead of a macro and using qemu_opts_append() or something similar? > > > > > > > > Berto > > > > > > No I didn't due to lack of time but maybe Pradeep may have a look in > this > > > direction ? > > > > > > > Only may be next year :) > > > > Ok, and also, it will be the occasion to add the throttling options to > the legacy -virtfs command line option as well. > Ok, also I think I have to add QMP interfaces for 9p case. -Pradeep > Cheers. > > -- > Greg > > > -Pradeep > > > > > > > > > > Cheers. > > > > > > -- > > > Greg > > > > >
On 22 November 2016 at 13:55, <no-reply@patchew.org> wrote: > Hi, > > Your series seems to have some coding style problems. See output below for > more information: > > Subject: [Qemu-devel] [RFC PATCH] throttle: move throttling cmdline > options to a separate header file > Type: series > Message-id: 147981681351.30309.4854065801791462661.stgit@ > bahia.lab.toulouse-stg.fr.ibm.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 > * [new tag] patchew/147981681351.30309.4854065801791462661.stgit@ > bahia.lab.toulouse-stg.fr.ibm.com -> patchew/147981681351.30309. > 4854065801791462661.stgit@bahia.lab.toulouse-stg.fr.ibm.com > Switched to a new branch 'test' > f5fd85c throttle: move throttling cmdline options to a separate header file > > === OUTPUT BEGIN === > Checking PATCH 1/1: throttle: move throttling cmdline options to a > separate header file... > ERROR: Macros with multiple statements should be enclosed in a do - while > loop > #133: FILE: include/qemu/throttle-options.h:14: > I did not understand how to fix this issue? I tried putting the do - while loop still did not work any idea, how to fix this? Regards, Pradeep +#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, 188 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
diff --git a/blockdev.c b/blockdev.c index 245e1e1d177a..59f3df700d23 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,11 @@ 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/include/qemu/throttle-options.h b/include/qemu/throttle-options.h new file mode 100644 index 000000000000..612f5d83bbd9 --- /dev/null +++ b/include/qemu/throttle-options.h @@ -0,0 +1,93 @@ +/* + * 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
This will allow other subsystems (i.e. fsdev) to implement throttling without duplicating the command line options. Signed-off-by: Greg Kurz <groug@kaod.org> --- Hi, Pradeep has been working on enabling throttling in 9p/fsdev. The latest version (v11) of the patch is: Message-Id: <1478854467-25061-1-git-send-email-pradeep.jagadeesh@huawei.com> There is some code duplication around the command line options. This patch is a first proposal to reduce the duplication. Some work should also be possible on the parsing side (see fsdev_throttle_parse_opts() in the patch above). Thanks for your comments. -- Greg blockdev.c | 83 +++-------------------------------- include/qemu/throttle-options.h | 93 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 77 deletions(-) create mode 100644 include/qemu/throttle-options.h