diff mbox

[v5,1/5] throttle: factor out duplicate code

Message ID 1497877896-35700-2-git-send-email-pradeep.jagadeesh@huawei.com
State New
Headers show

Commit Message

Pradeep Jagadeesh June 19, 2017, 1:11 p.m. UTC
This patch factor out the duplicate throttle code that was present in
block and fsdev devices.

Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
---
 blockdev.c                      | 44 +-----------------------------------
 fsdev/qemu-fsdev-throttle.c     | 43 +----------------------------------
 fsdev/qemu-fsdev-throttle.h     |  1 +
 include/qemu/throttle-options.h |  4 ++++
 util/throttle.c                 | 50 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 57 insertions(+), 85 deletions(-)

Comments

Greg Kurz June 20, 2017, 3:46 p.m. UTC | #1
On Mon, 19 Jun 2017 09:11:32 -0400
Pradeep Jagadeesh <pradeepkiruvale@gmail.com> wrote:

> This patch factor out the duplicate throttle code that was present in
> block and fsdev devices.
> 
> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
> ---

I guess this patch can be applied right away.

Cc'ing Markus (maintainer of blockdev.c)

>  blockdev.c                      | 44 +-----------------------------------
>  fsdev/qemu-fsdev-throttle.c     | 43 +----------------------------------
>  fsdev/qemu-fsdev-throttle.h     |  1 +
>  include/qemu/throttle-options.h |  4 ++++
>  util/throttle.c                 | 50 +++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 57 insertions(+), 85 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 6472548..5db9e5c 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -386,49 +386,7 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
>      }
>  
>      if (throttle_cfg) {
> -        throttle_config_init(throttle_cfg);
> -        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
> -            qemu_opt_get_number(opts, "throttling.bps-total", 0);
> -        throttle_cfg->buckets[THROTTLE_BPS_READ].avg  =
> -            qemu_opt_get_number(opts, "throttling.bps-read", 0);
> -        throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
> -            qemu_opt_get_number(opts, "throttling.bps-write", 0);
> -        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
> -            qemu_opt_get_number(opts, "throttling.iops-total", 0);
> -        throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
> -            qemu_opt_get_number(opts, "throttling.iops-read", 0);
> -        throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
> -            qemu_opt_get_number(opts, "throttling.iops-write", 0);
> -
> -        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
> -            qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
> -        throttle_cfg->buckets[THROTTLE_BPS_READ].max  =
> -            qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
> -        throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
> -            qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
> -        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
> -            qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
> -        throttle_cfg->buckets[THROTTLE_OPS_READ].max =
> -            qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
> -        throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
> -            qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
> -
> -        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
> -            qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
> -        throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length  =
> -            qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
> -        throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
> -            qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
> -        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
> -            qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
> -        throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
> -            qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
> -        throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
> -            qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
> -
> -        throttle_cfg->op_size =
> -            qemu_opt_get_number(opts, "throttling.iops-size", 0);
> -
> +        throttle_parse_options(throttle_cfg, opts);
>          if (!throttle_is_valid(throttle_cfg, errp)) {
>              return;
>          }
> diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
> index 7ae4e86..da9c225 100644
> --- a/fsdev/qemu-fsdev-throttle.c
> +++ b/fsdev/qemu-fsdev-throttle.c
> @@ -31,48 +31,7 @@ static void fsdev_throttle_write_timer_cb(void *opaque)
>  
>  void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error **errp)
>  {
> -    throttle_config_init(&fst->cfg);
> -    fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =
> -        qemu_opt_get_number(opts, "throttling.bps-total", 0);
> -    fst->cfg.buckets[THROTTLE_BPS_READ].avg  =
> -        qemu_opt_get_number(opts, "throttling.bps-read", 0);
> -    fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =
> -        qemu_opt_get_number(opts, "throttling.bps-write", 0);
> -    fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =
> -        qemu_opt_get_number(opts, "throttling.iops-total", 0);
> -    fst->cfg.buckets[THROTTLE_OPS_READ].avg =
> -        qemu_opt_get_number(opts, "throttling.iops-read", 0);
> -    fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =
> -        qemu_opt_get_number(opts, "throttling.iops-write", 0);
> -
> -    fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =
> -        qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
> -    fst->cfg.buckets[THROTTLE_BPS_READ].max  =
> -        qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
> -    fst->cfg.buckets[THROTTLE_BPS_WRITE].max =
> -        qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
> -    fst->cfg.buckets[THROTTLE_OPS_TOTAL].max =
> -        qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
> -    fst->cfg.buckets[THROTTLE_OPS_READ].max =
> -        qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
> -    fst->cfg.buckets[THROTTLE_OPS_WRITE].max =
> -        qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
> -
> -    fst->cfg.buckets[THROTTLE_BPS_TOTAL].burst_length =
> -        qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
> -    fst->cfg.buckets[THROTTLE_BPS_READ].burst_length  =
> -        qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
> -    fst->cfg.buckets[THROTTLE_BPS_WRITE].burst_length =
> -        qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
> -    fst->cfg.buckets[THROTTLE_OPS_TOTAL].burst_length =
> -        qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
> -    fst->cfg.buckets[THROTTLE_OPS_READ].burst_length =
> -        qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
> -    fst->cfg.buckets[THROTTLE_OPS_WRITE].burst_length =
> -        qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
> -    fst->cfg.op_size =
> -        qemu_opt_get_number(opts, "throttling.iops-size", 0);
> -
> +    throttle_parse_options(&fst->cfg, opts);
>      throttle_is_valid(&fst->cfg, errp);
>  }
>  
> diff --git a/fsdev/qemu-fsdev-throttle.h b/fsdev/qemu-fsdev-throttle.h
> index e418643..c493e83 100644
> --- a/fsdev/qemu-fsdev-throttle.h
> +++ b/fsdev/qemu-fsdev-throttle.h
> @@ -20,6 +20,7 @@
>  #include "qemu/coroutine.h"
>  #include "qapi/error.h"
>  #include "qemu/throttle.h"
> +#include "qemu/throttle-options.h"
>  
>  typedef struct FsThrottle {
>      ThrottleState ts;
> diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h
> index 3133d1c..565553a 100644
> --- a/include/qemu/throttle-options.h
> +++ b/include/qemu/throttle-options.h
> @@ -10,6 +10,8 @@
>  #ifndef THROTTLE_OPTIONS_H
>  #define THROTTLE_OPTIONS_H
>  
> +#include "qemu/throttle.h"
> +
>  #define THROTTLE_OPTS \
>            { \
>              .name = "throttling.iops-total",\
> @@ -89,4 +91,6 @@
>              .help = "when limiting by iops max size of an I/O in bytes",\
>          }
>  
> +void throttle_parse_options(ThrottleConfig *, QemuOpts *);
> +
>  #endif
> diff --git a/util/throttle.c b/util/throttle.c
> index 3570ed2..ebe9dd0 100644
> --- a/util/throttle.c
> +++ b/util/throttle.c
> @@ -514,3 +514,53 @@ void throttle_account(ThrottleState *ts, bool is_write, uint64_t size)
>      }
>  }
>  
> +/* parse the throttle options
> + *
> + * @opts: qemu options
> + * @throttle_cfg: throttle configuration
> + */
> +void throttle_parse_options(ThrottleConfig *throttle_cfg, QemuOpts *opts)
> +{
> +    throttle_config_init(throttle_cfg);
> +    throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
> +        qemu_opt_get_number(opts, "throttling.bps-total", 0);
> +    throttle_cfg->buckets[THROTTLE_BPS_READ].avg  =
> +        qemu_opt_get_number(opts, "throttling.bps-read", 0);
> +    throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
> +        qemu_opt_get_number(opts, "throttling.bps-write", 0);
> +    throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
> +        qemu_opt_get_number(opts, "throttling.iops-total", 0);
> +    throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
> +        qemu_opt_get_number(opts, "throttling.iops-read", 0);
> +    throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
> +        qemu_opt_get_number(opts, "throttling.iops-write", 0);
> +
> +    throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
> +        qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
> +    throttle_cfg->buckets[THROTTLE_BPS_READ].max  =
> +        qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
> +    throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
> +        qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
> +    throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
> +        qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
> +    throttle_cfg->buckets[THROTTLE_OPS_READ].max =
> +        qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
> +    throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
> +        qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
> +
> +    throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
> +        qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
> +    throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length  =
> +        qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
> +    throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
> +        qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
> +    throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
> +        qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
> +    throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
> +        qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
> +    throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
> +        qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
> +
> +    throttle_cfg->op_size =
> +        qemu_opt_get_number(opts, "throttling.iops-size", 0);
> +}
Alberto Garcia June 21, 2017, 12:55 p.m. UTC | #2
On Mon 19 Jun 2017 03:11:32 PM CEST, Pradeep Jagadeesh wrote:
> This patch factor out the duplicate throttle code that was present in
> block and fsdev devices.
>
> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto
Greg Kurz June 21, 2017, 1:43 p.m. UTC | #3
On Wed, 21 Jun 2017 14:55:55 +0200
Alberto Garcia <berto@igalia.com> wrote:

> On Mon 19 Jun 2017 03:11:32 PM CEST, Pradeep Jagadeesh wrote:
> > This patch factor out the duplicate throttle code that was present in
> > block and fsdev devices.
> >
> > Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>  
> 
> Reviewed-by: Alberto Garcia <berto@igalia.com>
> 

Thanks ! Is it okay if this goes through my 9P tree ?

> Berto

Cc'ing Markus, for the same question.
Alberto Garcia June 21, 2017, 2:21 p.m. UTC | #4
On Wed 21 Jun 2017 03:43:09 PM CEST, Greg Kurz wrote:
> On Wed, 21 Jun 2017 14:55:55 +0200
> Alberto Garcia <berto@igalia.com> wrote:
>
>> On Mon 19 Jun 2017 03:11:32 PM CEST, Pradeep Jagadeesh wrote:
>> > This patch factor out the duplicate throttle code that was present in
>> > block and fsdev devices.
>> >
>> > Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>  
>> 
>> Reviewed-by: Alberto Garcia <berto@igalia.com>
>
> Thanks ! Is it okay if this goes through my 9P tree ?

No problem from my side.

Berto
Markus Armbruster June 22, 2017, 2:38 p.m. UTC | #5
Pradeep Jagadeesh <pradeepkiruvale@gmail.com> writes:

> This patch factor out the duplicate throttle code that was present in
> block and fsdev devices.
>
> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
> ---
>  blockdev.c                      | 44 +-----------------------------------
>  fsdev/qemu-fsdev-throttle.c     | 43 +----------------------------------
>  fsdev/qemu-fsdev-throttle.h     |  1 +
>  include/qemu/throttle-options.h |  4 ++++
>  util/throttle.c                 | 50 +++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 57 insertions(+), 85 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index 6472548..5db9e5c 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -386,49 +386,7 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
>      }
>  
>      if (throttle_cfg) {
> -        throttle_config_init(throttle_cfg);
> -        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
> -            qemu_opt_get_number(opts, "throttling.bps-total", 0);
> -        throttle_cfg->buckets[THROTTLE_BPS_READ].avg  =
> -            qemu_opt_get_number(opts, "throttling.bps-read", 0);
> -        throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
> -            qemu_opt_get_number(opts, "throttling.bps-write", 0);
> -        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
> -            qemu_opt_get_number(opts, "throttling.iops-total", 0);
> -        throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
> -            qemu_opt_get_number(opts, "throttling.iops-read", 0);
> -        throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
> -            qemu_opt_get_number(opts, "throttling.iops-write", 0);
> -
> -        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
> -            qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
> -        throttle_cfg->buckets[THROTTLE_BPS_READ].max  =
> -            qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
> -        throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
> -            qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
> -        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
> -            qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
> -        throttle_cfg->buckets[THROTTLE_OPS_READ].max =
> -            qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
> -        throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
> -            qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
> -
> -        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
> -            qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
> -        throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length  =
> -            qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
> -        throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
> -            qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
> -        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
> -            qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
> -        throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
> -            qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
> -        throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
> -            qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
> -
> -        throttle_cfg->op_size =
> -            qemu_opt_get_number(opts, "throttling.iops-size", 0);
> -
> +        throttle_parse_options(throttle_cfg, opts);
>          if (!throttle_is_valid(throttle_cfg, errp)) {
>              return;
>          }
> diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
> index 7ae4e86..da9c225 100644
> --- a/fsdev/qemu-fsdev-throttle.c
> +++ b/fsdev/qemu-fsdev-throttle.c
> @@ -31,48 +31,7 @@ static void fsdev_throttle_write_timer_cb(void *opaque)
>  
>  void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error **errp)
>  {
> -    throttle_config_init(&fst->cfg);
> -    fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =
> -        qemu_opt_get_number(opts, "throttling.bps-total", 0);
> -    fst->cfg.buckets[THROTTLE_BPS_READ].avg  =
> -        qemu_opt_get_number(opts, "throttling.bps-read", 0);
> -    fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =
> -        qemu_opt_get_number(opts, "throttling.bps-write", 0);
> -    fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =
> -        qemu_opt_get_number(opts, "throttling.iops-total", 0);
> -    fst->cfg.buckets[THROTTLE_OPS_READ].avg =
> -        qemu_opt_get_number(opts, "throttling.iops-read", 0);
> -    fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =
> -        qemu_opt_get_number(opts, "throttling.iops-write", 0);
> -
> -    fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =
> -        qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
> -    fst->cfg.buckets[THROTTLE_BPS_READ].max  =
> -        qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
> -    fst->cfg.buckets[THROTTLE_BPS_WRITE].max =
> -        qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
> -    fst->cfg.buckets[THROTTLE_OPS_TOTAL].max =
> -        qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
> -    fst->cfg.buckets[THROTTLE_OPS_READ].max =
> -        qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
> -    fst->cfg.buckets[THROTTLE_OPS_WRITE].max =
> -        qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
> -
> -    fst->cfg.buckets[THROTTLE_BPS_TOTAL].burst_length =
> -        qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
> -    fst->cfg.buckets[THROTTLE_BPS_READ].burst_length  =
> -        qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
> -    fst->cfg.buckets[THROTTLE_BPS_WRITE].burst_length =
> -        qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
> -    fst->cfg.buckets[THROTTLE_OPS_TOTAL].burst_length =
> -        qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
> -    fst->cfg.buckets[THROTTLE_OPS_READ].burst_length =
> -        qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
> -    fst->cfg.buckets[THROTTLE_OPS_WRITE].burst_length =
> -        qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
> -    fst->cfg.op_size =
> -        qemu_opt_get_number(opts, "throttling.iops-size", 0);
> -
> +    throttle_parse_options(&fst->cfg, opts);
>      throttle_is_valid(&fst->cfg, errp);
>  }
>  
> diff --git a/fsdev/qemu-fsdev-throttle.h b/fsdev/qemu-fsdev-throttle.h
> index e418643..c493e83 100644
> --- a/fsdev/qemu-fsdev-throttle.h
> +++ b/fsdev/qemu-fsdev-throttle.h
> @@ -20,6 +20,7 @@
>  #include "qemu/coroutine.h"
>  #include "qapi/error.h"
>  #include "qemu/throttle.h"
> +#include "qemu/throttle-options.h"

Why?

>  
>  typedef struct FsThrottle {
>      ThrottleState ts;
> diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h
> index 3133d1c..565553a 100644
> --- a/include/qemu/throttle-options.h
> +++ b/include/qemu/throttle-options.h
> @@ -10,6 +10,8 @@
>  #ifndef THROTTLE_OPTIONS_H
>  #define THROTTLE_OPTIONS_H
>  
> +#include "qemu/throttle.h"
> +

Let's avoid this include: move the ThrottleConfig typedef to
qemu/typedefs.h.

>  #define THROTTLE_OPTS \
>            { \
>              .name = "throttling.iops-total",\
> @@ -89,4 +91,6 @@
>              .help = "when limiting by iops max size of an I/O in bytes",\
>          }
>  
> +void throttle_parse_options(ThrottleConfig *, QemuOpts *);
> +
>  #endif
> diff --git a/util/throttle.c b/util/throttle.c
> index 3570ed2..ebe9dd0 100644
> --- a/util/throttle.c
> +++ b/util/throttle.c
> @@ -514,3 +514,53 @@ void throttle_account(ThrottleState *ts, bool is_write, uint64_t size)
>      }
>  }
>  
> +/* parse the throttle options
> + *
> + * @opts: qemu options
> + * @throttle_cfg: throttle configuration
> + */
> +void throttle_parse_options(ThrottleConfig *throttle_cfg, QemuOpts *opts)
> +{
> +    throttle_config_init(throttle_cfg);
> +    throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
> +        qemu_opt_get_number(opts, "throttling.bps-total", 0);
> +    throttle_cfg->buckets[THROTTLE_BPS_READ].avg  =
> +        qemu_opt_get_number(opts, "throttling.bps-read", 0);
> +    throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
> +        qemu_opt_get_number(opts, "throttling.bps-write", 0);
> +    throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
> +        qemu_opt_get_number(opts, "throttling.iops-total", 0);
> +    throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
> +        qemu_opt_get_number(opts, "throttling.iops-read", 0);
> +    throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
> +        qemu_opt_get_number(opts, "throttling.iops-write", 0);
> +
> +    throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
> +        qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
> +    throttle_cfg->buckets[THROTTLE_BPS_READ].max  =
> +        qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
> +    throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
> +        qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
> +    throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
> +        qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
> +    throttle_cfg->buckets[THROTTLE_OPS_READ].max =
> +        qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
> +    throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
> +        qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
> +
> +    throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
> +        qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
> +    throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length  =
> +        qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
> +    throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
> +        qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
> +    throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
> +        qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
> +    throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
> +        qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
> +    throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
> +        qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
> +
> +    throttle_cfg->op_size =
> +        qemu_opt_get_number(opts, "throttling.iops-size", 0);
> +}

Looks good otherwise.
Greg Kurz June 22, 2017, 4:23 p.m. UTC | #6
On Thu, 22 Jun 2017 16:38:45 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Pradeep Jagadeesh <pradeepkiruvale@gmail.com> writes:
> 
> > This patch factor out the duplicate throttle code that was present in
> > block and fsdev devices.
> >
> > Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
> > ---
> >  blockdev.c                      | 44 +-----------------------------------
> >  fsdev/qemu-fsdev-throttle.c     | 43 +----------------------------------
> >  fsdev/qemu-fsdev-throttle.h     |  1 +
> >  include/qemu/throttle-options.h |  4 ++++
> >  util/throttle.c                 | 50 +++++++++++++++++++++++++++++++++++++++++
> >  5 files changed, 57 insertions(+), 85 deletions(-)
> >
> > diff --git a/blockdev.c b/blockdev.c
> > index 6472548..5db9e5c 100644
> > --- a/blockdev.c
> > +++ b/blockdev.c
> > @@ -386,49 +386,7 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
> >      }
> >  
> >      if (throttle_cfg) {
> > -        throttle_config_init(throttle_cfg);
> > -        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
> > -            qemu_opt_get_number(opts, "throttling.bps-total", 0);
> > -        throttle_cfg->buckets[THROTTLE_BPS_READ].avg  =
> > -            qemu_opt_get_number(opts, "throttling.bps-read", 0);
> > -        throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
> > -            qemu_opt_get_number(opts, "throttling.bps-write", 0);
> > -        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
> > -            qemu_opt_get_number(opts, "throttling.iops-total", 0);
> > -        throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
> > -            qemu_opt_get_number(opts, "throttling.iops-read", 0);
> > -        throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
> > -            qemu_opt_get_number(opts, "throttling.iops-write", 0);
> > -
> > -        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
> > -            qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
> > -        throttle_cfg->buckets[THROTTLE_BPS_READ].max  =
> > -            qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
> > -        throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
> > -            qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
> > -        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
> > -            qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
> > -        throttle_cfg->buckets[THROTTLE_OPS_READ].max =
> > -            qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
> > -        throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
> > -            qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
> > -
> > -        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
> > -            qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
> > -        throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length  =
> > -            qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
> > -        throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
> > -            qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
> > -        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
> > -            qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
> > -        throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
> > -            qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
> > -        throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
> > -            qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
> > -
> > -        throttle_cfg->op_size =
> > -            qemu_opt_get_number(opts, "throttling.iops-size", 0);
> > -
> > +        throttle_parse_options(throttle_cfg, opts);
> >          if (!throttle_is_valid(throttle_cfg, errp)) {
> >              return;
> >          }
> > diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
> > index 7ae4e86..da9c225 100644
> > --- a/fsdev/qemu-fsdev-throttle.c
> > +++ b/fsdev/qemu-fsdev-throttle.c
> > @@ -31,48 +31,7 @@ static void fsdev_throttle_write_timer_cb(void *opaque)
> >  
> >  void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error **errp)
> >  {
> > -    throttle_config_init(&fst->cfg);
> > -    fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =
> > -        qemu_opt_get_number(opts, "throttling.bps-total", 0);
> > -    fst->cfg.buckets[THROTTLE_BPS_READ].avg  =
> > -        qemu_opt_get_number(opts, "throttling.bps-read", 0);
> > -    fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =
> > -        qemu_opt_get_number(opts, "throttling.bps-write", 0);
> > -    fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =
> > -        qemu_opt_get_number(opts, "throttling.iops-total", 0);
> > -    fst->cfg.buckets[THROTTLE_OPS_READ].avg =
> > -        qemu_opt_get_number(opts, "throttling.iops-read", 0);
> > -    fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =
> > -        qemu_opt_get_number(opts, "throttling.iops-write", 0);
> > -
> > -    fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =
> > -        qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
> > -    fst->cfg.buckets[THROTTLE_BPS_READ].max  =
> > -        qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
> > -    fst->cfg.buckets[THROTTLE_BPS_WRITE].max =
> > -        qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
> > -    fst->cfg.buckets[THROTTLE_OPS_TOTAL].max =
> > -        qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
> > -    fst->cfg.buckets[THROTTLE_OPS_READ].max =
> > -        qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
> > -    fst->cfg.buckets[THROTTLE_OPS_WRITE].max =
> > -        qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
> > -
> > -    fst->cfg.buckets[THROTTLE_BPS_TOTAL].burst_length =
> > -        qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
> > -    fst->cfg.buckets[THROTTLE_BPS_READ].burst_length  =
> > -        qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
> > -    fst->cfg.buckets[THROTTLE_BPS_WRITE].burst_length =
> > -        qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
> > -    fst->cfg.buckets[THROTTLE_OPS_TOTAL].burst_length =
> > -        qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
> > -    fst->cfg.buckets[THROTTLE_OPS_READ].burst_length =
> > -        qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
> > -    fst->cfg.buckets[THROTTLE_OPS_WRITE].burst_length =
> > -        qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
> > -    fst->cfg.op_size =
> > -        qemu_opt_get_number(opts, "throttling.iops-size", 0);
> > -
> > +    throttle_parse_options(&fst->cfg, opts);
> >      throttle_is_valid(&fst->cfg, errp);
> >  }
> >  
> > diff --git a/fsdev/qemu-fsdev-throttle.h b/fsdev/qemu-fsdev-throttle.h
> > index e418643..c493e83 100644
> > --- a/fsdev/qemu-fsdev-throttle.h
> > +++ b/fsdev/qemu-fsdev-throttle.h
> > @@ -20,6 +20,7 @@
> >  #include "qemu/coroutine.h"
> >  #include "qapi/error.h"
> >  #include "qemu/throttle.h"
> > +#include "qemu/throttle-options.h"  
> 
> Why?
> 

Yeah, looking at the previous hunk, I guess it should rather go to
fsdev/qemu-fsdev-throttle.c.

> >  
> >  typedef struct FsThrottle {
> >      ThrottleState ts;
> > diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h
> > index 3133d1c..565553a 100644
> > --- a/include/qemu/throttle-options.h
> > +++ b/include/qemu/throttle-options.h
> > @@ -10,6 +10,8 @@
> >  #ifndef THROTTLE_OPTIONS_H
> >  #define THROTTLE_OPTIONS_H
> >  
> > +#include "qemu/throttle.h"
> > +  
> 
> Let's avoid this include: move the ThrottleConfig typedef to
> qemu/typedefs.h.
> 
> >  #define THROTTLE_OPTS \
> >            { \
> >              .name = "throttling.iops-total",\
> > @@ -89,4 +91,6 @@
> >              .help = "when limiting by iops max size of an I/O in bytes",\
> >          }
> >  
> > +void throttle_parse_options(ThrottleConfig *, QemuOpts *);
> > +
> >  #endif
> > diff --git a/util/throttle.c b/util/throttle.c
> > index 3570ed2..ebe9dd0 100644
> > --- a/util/throttle.c
> > +++ b/util/throttle.c
> > @@ -514,3 +514,53 @@ void throttle_account(ThrottleState *ts, bool is_write, uint64_t size)
> >      }
> >  }
> >  
> > +/* parse the throttle options
> > + *
> > + * @opts: qemu options
> > + * @throttle_cfg: throttle configuration
> > + */
> > +void throttle_parse_options(ThrottleConfig *throttle_cfg, QemuOpts *opts)
> > +{
> > +    throttle_config_init(throttle_cfg);
> > +    throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
> > +        qemu_opt_get_number(opts, "throttling.bps-total", 0);
> > +    throttle_cfg->buckets[THROTTLE_BPS_READ].avg  =
> > +        qemu_opt_get_number(opts, "throttling.bps-read", 0);
> > +    throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
> > +        qemu_opt_get_number(opts, "throttling.bps-write", 0);
> > +    throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
> > +        qemu_opt_get_number(opts, "throttling.iops-total", 0);
> > +    throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
> > +        qemu_opt_get_number(opts, "throttling.iops-read", 0);
> > +    throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
> > +        qemu_opt_get_number(opts, "throttling.iops-write", 0);
> > +
> > +    throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
> > +        qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
> > +    throttle_cfg->buckets[THROTTLE_BPS_READ].max  =
> > +        qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
> > +    throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
> > +        qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
> > +    throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
> > +        qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
> > +    throttle_cfg->buckets[THROTTLE_OPS_READ].max =
> > +        qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
> > +    throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
> > +        qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
> > +
> > +    throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
> > +        qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
> > +    throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length  =
> > +        qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
> > +    throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
> > +        qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
> > +    throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
> > +        qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
> > +    throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
> > +        qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
> > +    throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
> > +        qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
> > +
> > +    throttle_cfg->op_size =
> > +        qemu_opt_get_number(opts, "throttling.iops-size", 0);
> > +}  
> 
> Looks good otherwise.
Pradeep Jagadeesh June 29, 2017, 9:50 a.m. UTC | #7
On 6/22/2017 4:38 PM, Markus Armbruster wrote:
> Pradeep Jagadeesh <pradeepkiruvale@gmail.com> writes:
>
>> This patch factor out the duplicate throttle code that was present in
>> block and fsdev devices.
>>
>> Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
>> ---
>>  blockdev.c                      | 44 +-----------------------------------
>>  fsdev/qemu-fsdev-throttle.c     | 43 +----------------------------------
>>  fsdev/qemu-fsdev-throttle.h     |  1 +
>>  include/qemu/throttle-options.h |  4 ++++
>>  util/throttle.c                 | 50 +++++++++++++++++++++++++++++++++++++++++
>>  5 files changed, 57 insertions(+), 85 deletions(-)
>>
>> diff --git a/blockdev.c b/blockdev.c
>> index 6472548..5db9e5c 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -386,49 +386,7 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
>>      }
>>
>>      if (throttle_cfg) {
>> -        throttle_config_init(throttle_cfg);
>> -        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
>> -            qemu_opt_get_number(opts, "throttling.bps-total", 0);
>> -        throttle_cfg->buckets[THROTTLE_BPS_READ].avg  =
>> -            qemu_opt_get_number(opts, "throttling.bps-read", 0);
>> -        throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
>> -            qemu_opt_get_number(opts, "throttling.bps-write", 0);
>> -        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
>> -            qemu_opt_get_number(opts, "throttling.iops-total", 0);
>> -        throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
>> -            qemu_opt_get_number(opts, "throttling.iops-read", 0);
>> -        throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
>> -            qemu_opt_get_number(opts, "throttling.iops-write", 0);
>> -
>> -        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
>> -            qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
>> -        throttle_cfg->buckets[THROTTLE_BPS_READ].max  =
>> -            qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
>> -        throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
>> -            qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
>> -        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
>> -            qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
>> -        throttle_cfg->buckets[THROTTLE_OPS_READ].max =
>> -            qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
>> -        throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
>> -            qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
>> -
>> -        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
>> -            qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
>> -        throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length  =
>> -            qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
>> -        throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
>> -            qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
>> -        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
>> -            qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
>> -        throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
>> -            qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
>> -        throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
>> -            qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
>> -
>> -        throttle_cfg->op_size =
>> -            qemu_opt_get_number(opts, "throttling.iops-size", 0);
>> -
>> +        throttle_parse_options(throttle_cfg, opts);
>>          if (!throttle_is_valid(throttle_cfg, errp)) {
>>              return;
>>          }
>> diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
>> index 7ae4e86..da9c225 100644
>> --- a/fsdev/qemu-fsdev-throttle.c
>> +++ b/fsdev/qemu-fsdev-throttle.c
>> @@ -31,48 +31,7 @@ static void fsdev_throttle_write_timer_cb(void *opaque)
>>
>>  void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error **errp)
>>  {
>> -    throttle_config_init(&fst->cfg);
>> -    fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =
>> -        qemu_opt_get_number(opts, "throttling.bps-total", 0);
>> -    fst->cfg.buckets[THROTTLE_BPS_READ].avg  =
>> -        qemu_opt_get_number(opts, "throttling.bps-read", 0);
>> -    fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =
>> -        qemu_opt_get_number(opts, "throttling.bps-write", 0);
>> -    fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =
>> -        qemu_opt_get_number(opts, "throttling.iops-total", 0);
>> -    fst->cfg.buckets[THROTTLE_OPS_READ].avg =
>> -        qemu_opt_get_number(opts, "throttling.iops-read", 0);
>> -    fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =
>> -        qemu_opt_get_number(opts, "throttling.iops-write", 0);
>> -
>> -    fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =
>> -        qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
>> -    fst->cfg.buckets[THROTTLE_BPS_READ].max  =
>> -        qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
>> -    fst->cfg.buckets[THROTTLE_BPS_WRITE].max =
>> -        qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
>> -    fst->cfg.buckets[THROTTLE_OPS_TOTAL].max =
>> -        qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
>> -    fst->cfg.buckets[THROTTLE_OPS_READ].max =
>> -        qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
>> -    fst->cfg.buckets[THROTTLE_OPS_WRITE].max =
>> -        qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
>> -
>> -    fst->cfg.buckets[THROTTLE_BPS_TOTAL].burst_length =
>> -        qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
>> -    fst->cfg.buckets[THROTTLE_BPS_READ].burst_length  =
>> -        qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
>> -    fst->cfg.buckets[THROTTLE_BPS_WRITE].burst_length =
>> -        qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
>> -    fst->cfg.buckets[THROTTLE_OPS_TOTAL].burst_length =
>> -        qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
>> -    fst->cfg.buckets[THROTTLE_OPS_READ].burst_length =
>> -        qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
>> -    fst->cfg.buckets[THROTTLE_OPS_WRITE].burst_length =
>> -        qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
>> -    fst->cfg.op_size =
>> -        qemu_opt_get_number(opts, "throttling.iops-size", 0);
>> -
>> +    throttle_parse_options(&fst->cfg, opts);
>>      throttle_is_valid(&fst->cfg, errp);
>>  }
>>
>> diff --git a/fsdev/qemu-fsdev-throttle.h b/fsdev/qemu-fsdev-throttle.h
>> index e418643..c493e83 100644
>> --- a/fsdev/qemu-fsdev-throttle.h
>> +++ b/fsdev/qemu-fsdev-throttle.h
>> @@ -20,6 +20,7 @@
>>  #include "qemu/coroutine.h"
>>  #include "qapi/error.h"
>>  #include "qemu/throttle.h"
>> +#include "qemu/throttle-options.h"
>
> Why?
>
Moved to qemu-fsdev-throttle.c
>>
>>  typedef struct FsThrottle {
>>      ThrottleState ts;
>> diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h
>> index 3133d1c..565553a 100644
>> --- a/include/qemu/throttle-options.h
>> +++ b/include/qemu/throttle-options.h
>> @@ -10,6 +10,8 @@
>>  #ifndef THROTTLE_OPTIONS_H
>>  #define THROTTLE_OPTIONS_H
>>
>> +#include "qemu/throttle.h"
>> +
>
> Let's avoid this include: move the ThrottleConfig typedef to
> qemu/typedefs.h.
OK
>
>>  #define THROTTLE_OPTS \
>>            { \
>>              .name = "throttling.iops-total",\
>> @@ -89,4 +91,6 @@
>>              .help = "when limiting by iops max size of an I/O in bytes",\
>>          }
>>
>> +void throttle_parse_options(ThrottleConfig *, QemuOpts *);
>> +
>>  #endif
>> diff --git a/util/throttle.c b/util/throttle.c
>> index 3570ed2..ebe9dd0 100644
>> --- a/util/throttle.c
>> +++ b/util/throttle.c
>> @@ -514,3 +514,53 @@ void throttle_account(ThrottleState *ts, bool is_write, uint64_t size)
>>      }
>>  }
>>
>> +/* parse the throttle options
>> + *
>> + * @opts: qemu options
>> + * @throttle_cfg: throttle configuration
>> + */
>> +void throttle_parse_options(ThrottleConfig *throttle_cfg, QemuOpts *opts)
>> +{
>> +    throttle_config_init(throttle_cfg);
>> +    throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
>> +        qemu_opt_get_number(opts, "throttling.bps-total", 0);
>> +    throttle_cfg->buckets[THROTTLE_BPS_READ].avg  =
>> +        qemu_opt_get_number(opts, "throttling.bps-read", 0);
>> +    throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
>> +        qemu_opt_get_number(opts, "throttling.bps-write", 0);
>> +    throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
>> +        qemu_opt_get_number(opts, "throttling.iops-total", 0);
>> +    throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
>> +        qemu_opt_get_number(opts, "throttling.iops-read", 0);
>> +    throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
>> +        qemu_opt_get_number(opts, "throttling.iops-write", 0);
>> +
>> +    throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
>> +        qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
>> +    throttle_cfg->buckets[THROTTLE_BPS_READ].max  =
>> +        qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
>> +    throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
>> +        qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
>> +    throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
>> +        qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
>> +    throttle_cfg->buckets[THROTTLE_OPS_READ].max =
>> +        qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
>> +    throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
>> +        qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
>> +
>> +    throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
>> +        qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
>> +    throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length  =
>> +        qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
>> +    throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
>> +        qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
>> +    throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
>> +        qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
>> +    throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
>> +        qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
>> +    throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
>> +        qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
>> +
>> +    throttle_cfg->op_size =
>> +        qemu_opt_get_number(opts, "throttling.iops-size", 0);
>> +}
>
> Looks good otherwise.
>
diff mbox

Patch

diff --git a/blockdev.c b/blockdev.c
index 6472548..5db9e5c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -386,49 +386,7 @@  static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,
     }
 
     if (throttle_cfg) {
-        throttle_config_init(throttle_cfg);
-        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
-            qemu_opt_get_number(opts, "throttling.bps-total", 0);
-        throttle_cfg->buckets[THROTTLE_BPS_READ].avg  =
-            qemu_opt_get_number(opts, "throttling.bps-read", 0);
-        throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
-            qemu_opt_get_number(opts, "throttling.bps-write", 0);
-        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
-            qemu_opt_get_number(opts, "throttling.iops-total", 0);
-        throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
-            qemu_opt_get_number(opts, "throttling.iops-read", 0);
-        throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
-            qemu_opt_get_number(opts, "throttling.iops-write", 0);
-
-        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
-            qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
-        throttle_cfg->buckets[THROTTLE_BPS_READ].max  =
-            qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
-        throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
-            qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
-        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
-            qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
-        throttle_cfg->buckets[THROTTLE_OPS_READ].max =
-            qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
-        throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
-            qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
-
-        throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
-            qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
-        throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length  =
-            qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
-        throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
-            qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
-        throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
-            qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
-        throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
-            qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
-        throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
-            qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
-
-        throttle_cfg->op_size =
-            qemu_opt_get_number(opts, "throttling.iops-size", 0);
-
+        throttle_parse_options(throttle_cfg, opts);
         if (!throttle_is_valid(throttle_cfg, errp)) {
             return;
         }
diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
index 7ae4e86..da9c225 100644
--- a/fsdev/qemu-fsdev-throttle.c
+++ b/fsdev/qemu-fsdev-throttle.c
@@ -31,48 +31,7 @@  static void fsdev_throttle_write_timer_cb(void *opaque)
 
 void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error **errp)
 {
-    throttle_config_init(&fst->cfg);
-    fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =
-        qemu_opt_get_number(opts, "throttling.bps-total", 0);
-    fst->cfg.buckets[THROTTLE_BPS_READ].avg  =
-        qemu_opt_get_number(opts, "throttling.bps-read", 0);
-    fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =
-        qemu_opt_get_number(opts, "throttling.bps-write", 0);
-    fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =
-        qemu_opt_get_number(opts, "throttling.iops-total", 0);
-    fst->cfg.buckets[THROTTLE_OPS_READ].avg =
-        qemu_opt_get_number(opts, "throttling.iops-read", 0);
-    fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =
-        qemu_opt_get_number(opts, "throttling.iops-write", 0);
-
-    fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =
-        qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
-    fst->cfg.buckets[THROTTLE_BPS_READ].max  =
-        qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
-    fst->cfg.buckets[THROTTLE_BPS_WRITE].max =
-        qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
-    fst->cfg.buckets[THROTTLE_OPS_TOTAL].max =
-        qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
-    fst->cfg.buckets[THROTTLE_OPS_READ].max =
-        qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
-    fst->cfg.buckets[THROTTLE_OPS_WRITE].max =
-        qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
-
-    fst->cfg.buckets[THROTTLE_BPS_TOTAL].burst_length =
-        qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
-    fst->cfg.buckets[THROTTLE_BPS_READ].burst_length  =
-        qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
-    fst->cfg.buckets[THROTTLE_BPS_WRITE].burst_length =
-        qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
-    fst->cfg.buckets[THROTTLE_OPS_TOTAL].burst_length =
-        qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
-    fst->cfg.buckets[THROTTLE_OPS_READ].burst_length =
-        qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
-    fst->cfg.buckets[THROTTLE_OPS_WRITE].burst_length =
-        qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
-    fst->cfg.op_size =
-        qemu_opt_get_number(opts, "throttling.iops-size", 0);
-
+    throttle_parse_options(&fst->cfg, opts);
     throttle_is_valid(&fst->cfg, errp);
 }
 
diff --git a/fsdev/qemu-fsdev-throttle.h b/fsdev/qemu-fsdev-throttle.h
index e418643..c493e83 100644
--- a/fsdev/qemu-fsdev-throttle.h
+++ b/fsdev/qemu-fsdev-throttle.h
@@ -20,6 +20,7 @@ 
 #include "qemu/coroutine.h"
 #include "qapi/error.h"
 #include "qemu/throttle.h"
+#include "qemu/throttle-options.h"
 
 typedef struct FsThrottle {
     ThrottleState ts;
diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h
index 3133d1c..565553a 100644
--- a/include/qemu/throttle-options.h
+++ b/include/qemu/throttle-options.h
@@ -10,6 +10,8 @@ 
 #ifndef THROTTLE_OPTIONS_H
 #define THROTTLE_OPTIONS_H
 
+#include "qemu/throttle.h"
+
 #define THROTTLE_OPTS \
           { \
             .name = "throttling.iops-total",\
@@ -89,4 +91,6 @@ 
             .help = "when limiting by iops max size of an I/O in bytes",\
         }
 
+void throttle_parse_options(ThrottleConfig *, QemuOpts *);
+
 #endif
diff --git a/util/throttle.c b/util/throttle.c
index 3570ed2..ebe9dd0 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -514,3 +514,53 @@  void throttle_account(ThrottleState *ts, bool is_write, uint64_t size)
     }
 }
 
+/* parse the throttle options
+ *
+ * @opts: qemu options
+ * @throttle_cfg: throttle configuration
+ */
+void throttle_parse_options(ThrottleConfig *throttle_cfg, QemuOpts *opts)
+{
+    throttle_config_init(throttle_cfg);
+    throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =
+        qemu_opt_get_number(opts, "throttling.bps-total", 0);
+    throttle_cfg->buckets[THROTTLE_BPS_READ].avg  =
+        qemu_opt_get_number(opts, "throttling.bps-read", 0);
+    throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =
+        qemu_opt_get_number(opts, "throttling.bps-write", 0);
+    throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =
+        qemu_opt_get_number(opts, "throttling.iops-total", 0);
+    throttle_cfg->buckets[THROTTLE_OPS_READ].avg =
+        qemu_opt_get_number(opts, "throttling.iops-read", 0);
+    throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =
+        qemu_opt_get_number(opts, "throttling.iops-write", 0);
+
+    throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =
+        qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
+    throttle_cfg->buckets[THROTTLE_BPS_READ].max  =
+        qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
+    throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =
+        qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
+    throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =
+        qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
+    throttle_cfg->buckets[THROTTLE_OPS_READ].max =
+        qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
+    throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =
+        qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
+
+    throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =
+        qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
+    throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length  =
+        qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
+    throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =
+        qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
+    throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =
+        qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
+    throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =
+        qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
+    throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =
+        qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
+
+    throttle_cfg->op_size =
+        qemu_opt_get_number(opts, "throttling.iops-size", 0);
+}