[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V1] throttle:Removed duplicate throtlle code fro
|
From: |
Greg Kurz |
|
Subject: |
Re: [Qemu-devel] [PATCH V1] throttle:Removed duplicate throtlle code from block and 9p files |
|
Date: |
Mon, 23 Jan 2017 15:48:56 +0100 |
On Mon, 23 Jan 2017 14:02:33 +0000
Pradeep Jagadeesh <address@hidden> wrote:
> -----Original Message-----
> From: Greg Kurz [mailto:address@hidden
> Sent: Monday, January 23, 2017 11:28 AM
> To: Pradeep Jagadeesh
> Cc: Pradeep Jagadeesh; Alberto Garcia; address@hidden
> Subject: Re: [PATCH V1] throttle:Removed duplicate throtlle code from block
> and 9p files
>
> On Mon, 23 Jan 2017 10:58:19 +0100
> Pradeep Jagadeesh <address@hidden> wrote:
>
> > On 1/23/2017 10:47 AM, Greg Kurz wrote:
> > > On Mon, 23 Jan 2017 04:19:55 -0500
> > > Pradeep Jagadeesh <address@hidden> wrote:
> > >
> > >> This will allow other subsystems (i.e. fsdev) to implement
> > >> throttling without duplicating the command line options.
> > >>
> > >> Signed-off-by: Pradeep Jagadeesh <address@hidden>
> > >> ---
> > >
> > > This patch depends on your other patch "[PATCH V12] fsdev: add IO
> > > throttle support to fsdev devices", which I haven't reviewed yet
> > > BTW. Both patches should really be sent in a single series.
> > OK
> > >
> > > Please do the following:
> > > - fix the changelog in the other patch
> > > - write a cover letter to provide some context on this feature: why is it
> > > needed ? why the QMP part has been left aside ? what are the remaining
> > > efforts to make that feature fully implemented and useful ?
> > This cover letter should be part of the .patch file right?
>
> No. It is a separate mail with some introductory text to describe what the
> series tries to achieve and any other interesting details, such as usage
> examples, limitations, remaining work to be done... etc... It is usually
> referred to as PATCH 0/N.
>
> See https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg01255.html for a
> typical example.
>
> [Pradeep Jagadeesh] Ok thanks, for the clarification.
> So, now in new e-mail I do not need to send the whole .patch file right?
> Just the diff what it has in the beginning.
No patch in the cover letter. See below.
> I am asking this because, so far I used to sent through git send-email.
> This is my first patch please bare with my silly questions!:)
>
So when you have to send a patch series, you typically do the following, as
indicated in the git-send-email manual page:
$ git format-patch --cover-letter -M origin/master -o outgoing/
$ edit outgoing/0000-*
$ git send-email outgoing/*
The generated outgoing/0000-* file initially contains a *** BLURB HERE ***
line. This is where you should put the introductory text for the series.
> > > - post the cover+both patches with a v13 prefix
> > For both patches with V13?
> >
>
> Yes. The version is more a series attribute: V13 introduces a second patch to
> remove duplicate lines and fixes the changelog of the first patch.
>
> [Pradeep Jagadeesh] Ok
>
> -Pradeep
>
> > -Pradeep
> >
> >
> > >
> > > Cheers.
> > >
> > > --
> > > Greg
> > >
> > >> blockdev.c | 80 ++---------------------------------
> > >> fsdev/qemu-fsdev-opts.c | 80 ++---------------------------------
> > >> include/qemu/throttle-options.h | 93
> > >> +++++++++++++++++++++++++++++++++++++++++
> > >> 3 files changed, 101 insertions(+), 152 deletions(-) create mode
> > >> 100644 include/qemu/throttle-options.h
> > >>
> > >> There is some code duplication around the command line options.
> > >> This patch is a first proposal to reduce the duplication.
> > >>
> > >> diff --git a/blockdev.c b/blockdev.c index 245e1e1..1da6b7e 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);
> > >> @@ -4000,82 +4001,9 @@ QemuOptsList qemu_common_drive_opts = {
> > >> .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..13597a3 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..a2fb817
> > >> --- /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
> > >
> >
>