qemu-devel
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH V1] throttle:Removed duplicate throtlle code fro

From: Pradeep Jagadeesh
Subject: Re: [Qemu-devel] [PATCH V1] throttle:Removed duplicate throtlle code from block and 9p files
Date: Mon, 23 Jan 2017 14:02:33 +0000 

-----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.
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!:)

> > - 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
> >  
>
reply via email to
[Prev in Thread] Current Thread [Next in Thread]