[{"id":1768515,"web_url":"http://patchwork.ozlabs.org/comment/1768515/","msgid":"<20170914131251.0881b755@bahia.lan>","list_archive_url":null,"date":"2017-09-14T11:12:51","subject":"Re: [Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code","submitter":{"id":69178,"url":"http://patchwork.ozlabs.org/api/people/69178/","name":"Greg Kurz","email":"groug@kaod.org"},"content":"On Thu, 14 Sep 2017 06:40:05 -0400\nPradeep Jagadeesh wrote:\n\n> This patch factors out the duplicate throttle code that was still\n> present in block and fsdev devices.\n> \n> Signed-off-by: Pradeep Jagadeesh \n> Reviewed-by: Alberto Garcia \n> Reviewed-by: Greg Kurz \n\nI see you took my remarks into account, except for the patch title\nwhich is still the very same as commit:\n\na2a7862ca9ab throttle: factor out duplicate code\n\n:-\\\n\n> Reviewed-by: Eric Blake \n> ---\n> blockdev.c | 44 +---------------------------------\n> fsdev/qemu-fsdev-throttle.c | 44 ++--------------------------------\n> include/qemu/throttle-options.h | 3 +++\n> include/qemu/throttle.h | 4 ++--\n> include/qemu/typedefs.h | 1 +\n> util/throttle.c | 52 +++++++++++++++++++++++++++++++++++++++++\n> 6 files changed, 61 insertions(+), 87 deletions(-)\n> \n> diff --git a/blockdev.c b/blockdev.c\n> index 56a6b24..9d33c25 100644\n> --- a/blockdev.c\n> +++ b/blockdev.c\n> @@ -387,49 +387,7 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,\n> }\n> \n> if (throttle_cfg) {\n> - throttle_config_init(throttle_cfg);\n> - throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =\n> - qemu_opt_get_number(opts, \"throttling.bps-total\", 0);\n> - throttle_cfg->buckets[THROTTLE_BPS_READ].avg =\n> - qemu_opt_get_number(opts, \"throttling.bps-read\", 0);\n> - throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =\n> - qemu_opt_get_number(opts, \"throttling.bps-write\", 0);\n> - throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =\n> - qemu_opt_get_number(opts, \"throttling.iops-total\", 0);\n> - throttle_cfg->buckets[THROTTLE_OPS_READ].avg =\n> - qemu_opt_get_number(opts, \"throttling.iops-read\", 0);\n> - throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =\n> - qemu_opt_get_number(opts, \"throttling.iops-write\", 0);\n> -\n> - throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =\n> - qemu_opt_get_number(opts, \"throttling.bps-total-max\", 0);\n> - throttle_cfg->buckets[THROTTLE_BPS_READ].max =\n> - qemu_opt_get_number(opts, \"throttling.bps-read-max\", 0);\n> - throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =\n> - qemu_opt_get_number(opts, \"throttling.bps-write-max\", 0);\n> - throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =\n> - qemu_opt_get_number(opts, \"throttling.iops-total-max\", 0);\n> - throttle_cfg->buckets[THROTTLE_OPS_READ].max =\n> - qemu_opt_get_number(opts, \"throttling.iops-read-max\", 0);\n> - throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =\n> - qemu_opt_get_number(opts, \"throttling.iops-write-max\", 0);\n> -\n> - throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =\n> - qemu_opt_get_number(opts, \"throttling.bps-total-max-length\", 1);\n> - throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =\n> - qemu_opt_get_number(opts, \"throttling.bps-read-max-length\", 1);\n> - throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =\n> - qemu_opt_get_number(opts, \"throttling.bps-write-max-length\", 1);\n> - throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =\n> - qemu_opt_get_number(opts, \"throttling.iops-total-max-length\", 1);\n> - throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =\n> - qemu_opt_get_number(opts, \"throttling.iops-read-max-length\", 1);\n> - throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =\n> - qemu_opt_get_number(opts, \"throttling.iops-write-max-length\", 1);\n> -\n> - throttle_cfg->op_size =\n> - qemu_opt_get_number(opts, \"throttling.iops-size\", 0);\n> -\n> + throttle_parse_options(throttle_cfg, opts);\n> if (!throttle_is_valid(throttle_cfg, errp)) {\n> return;\n> }\n> diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c\n> index 49eebb5..0e6fb86 100644\n> --- a/fsdev/qemu-fsdev-throttle.c\n> +++ b/fsdev/qemu-fsdev-throttle.c\n> @@ -16,6 +16,7 @@\n> #include \"qemu/error-report.h\"\n> #include \"qemu-fsdev-throttle.h\"\n> #include \"qemu/iov.h\"\n> +#include \"qemu/throttle-options.h\"\n> \n> static void fsdev_throttle_read_timer_cb(void *opaque)\n> {\n> @@ -31,48 +32,7 @@ static void fsdev_throttle_write_timer_cb(void *opaque)\n> \n> void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error **errp)\n> {\n> - throttle_config_init(&fst->cfg);\n> - fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =\n> - qemu_opt_get_number(opts, \"throttling.bps-total\", 0);\n> - fst->cfg.buckets[THROTTLE_BPS_READ].avg =\n> - qemu_opt_get_number(opts, \"throttling.bps-read\", 0);\n> - fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =\n> - qemu_opt_get_number(opts, \"throttling.bps-write\", 0);\n> - fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =\n> - qemu_opt_get_number(opts, \"throttling.iops-total\", 0);\n> - fst->cfg.buckets[THROTTLE_OPS_READ].avg =\n> - qemu_opt_get_number(opts, \"throttling.iops-read\", 0);\n> - fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =\n> - qemu_opt_get_number(opts, \"throttling.iops-write\", 0);\n> -\n> - fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =\n> - qemu_opt_get_number(opts, \"throttling.bps-total-max\", 0);\n> - fst->cfg.buckets[THROTTLE_BPS_READ].max =\n> - qemu_opt_get_number(opts, \"throttling.bps-read-max\", 0);\n> - fst->cfg.buckets[THROTTLE_BPS_WRITE].max =\n> - qemu_opt_get_number(opts, \"throttling.bps-write-max\", 0);\n> - fst->cfg.buckets[THROTTLE_OPS_TOTAL].max =\n> - qemu_opt_get_number(opts, \"throttling.iops-total-max\", 0);\n> - fst->cfg.buckets[THROTTLE_OPS_READ].max =\n> - qemu_opt_get_number(opts, \"throttling.iops-read-max\", 0);\n> - fst->cfg.buckets[THROTTLE_OPS_WRITE].max =\n> - qemu_opt_get_number(opts, \"throttling.iops-write-max\", 0);\n> -\n> - fst->cfg.buckets[THROTTLE_BPS_TOTAL].burst_length =\n> - qemu_opt_get_number(opts, \"throttling.bps-total-max-length\", 1);\n> - fst->cfg.buckets[THROTTLE_BPS_READ].burst_length =\n> - qemu_opt_get_number(opts, \"throttling.bps-read-max-length\", 1);\n> - fst->cfg.buckets[THROTTLE_BPS_WRITE].burst_length =\n> - qemu_opt_get_number(opts, \"throttling.bps-write-max-length\", 1);\n> - fst->cfg.buckets[THROTTLE_OPS_TOTAL].burst_length =\n> - qemu_opt_get_number(opts, \"throttling.iops-total-max-length\", 1);\n> - fst->cfg.buckets[THROTTLE_OPS_READ].burst_length =\n> - qemu_opt_get_number(opts, \"throttling.iops-read-max-length\", 1);\n> - fst->cfg.buckets[THROTTLE_OPS_WRITE].burst_length =\n> - qemu_opt_get_number(opts, \"throttling.iops-write-max-length\", 1);\n> - fst->cfg.op_size =\n> - qemu_opt_get_number(opts, \"throttling.iops-size\", 0);\n> -\n> + throttle_parse_options(&fst->cfg, opts);\n> throttle_is_valid(&fst->cfg, errp);\n> }\n> \n> diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h\n> index 3528a8f..9709dcd 100644\n> --- a/include/qemu/throttle-options.h\n> +++ b/include/qemu/throttle-options.h\n> @@ -9,6 +9,7 @@\n> */\n> #ifndef THROTTLE_OPTIONS_H\n> #define THROTTLE_OPTIONS_H\n> +#include \"typedefs.h\"\n> \n> #define QEMU_OPT_IOPS_TOTAL \"iops-total\"\n> #define QEMU_OPT_IOPS_TOTAL_MAX \"iops-total-max\"\n> @@ -111,4 +112,6 @@\n> .help = \"when limiting by iops max size of an I/O in bytes\",\\\n> }\n> \n> +void throttle_parse_options(ThrottleConfig *, QemuOpts *);\n> +\n> #endif\n> diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h\n> index 8c93237..b6ebc6d 100644\n> --- a/include/qemu/throttle.h\n> +++ b/include/qemu/throttle.h\n> @@ -89,10 +89,10 @@ typedef struct LeakyBucket {\n> * However it allows to keep the code clean and the bucket field is reset to\n> * zero at the right time.\n> */\n> -typedef struct ThrottleConfig {\n> +struct ThrottleConfig {\n> LeakyBucket buckets[BUCKETS_COUNT]; /* leaky buckets */\n> uint64_t op_size; /* size of an operation in bytes */\n> -} ThrottleConfig;\n> +};\n> \n> typedef struct ThrottleState {\n> ThrottleConfig cfg; /* configuration */\n> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h\n> index 39bc835..90fe0f9 100644\n> --- a/include/qemu/typedefs.h\n> +++ b/include/qemu/typedefs.h\n> @@ -100,6 +100,7 @@ typedef struct uWireSlave uWireSlave;\n> typedef struct VirtIODevice VirtIODevice;\n> typedef struct Visitor Visitor;\n> typedef struct node_info NodeInfo;\n> +typedef struct ThrottleConfig ThrottleConfig;\n> typedef void SaveStateHandler(QEMUFile *f, void *opaque);\n> typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);\n> \n> diff --git a/util/throttle.c b/util/throttle.c\n> index 06bf916..9ef28c4 100644\n> --- a/util/throttle.c\n> +++ b/util/throttle.c\n> @@ -27,6 +27,7 @@\n> #include \"qemu/throttle.h\"\n> #include \"qemu/timer.h\"\n> #include \"block/aio.h\"\n> +#include \"qemu/throttle-options.h\"\n> \n> /* This function make a bucket leak\n> *\n> @@ -635,3 +636,54 @@ void throttle_config_to_limits(ThrottleConfig *cfg, ThrottleLimits *var)\n> var->has_iops_write_max_length = true;\n> var->has_iops_size = true;\n> }\n> +\n> +/* parse the throttle options\n> + *\n> + * @opts: qemu options\n> + * @throttle_cfg: throttle configuration\n> + */\n> +void throttle_parse_options(ThrottleConfig *throttle_cfg, QemuOpts *opts)\n> +{\n> + throttle_config_init(throttle_cfg);\n> + throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =\n> + qemu_opt_get_number(opts, \"throttling.bps-total\", 0);\n> + throttle_cfg->buckets[THROTTLE_BPS_READ].avg =\n> + qemu_opt_get_number(opts, \"throttling.bps-read\", 0);\n> + throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =\n> + qemu_opt_get_number(opts, \"throttling.bps-write\", 0);\n> + throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =\n> + qemu_opt_get_number(opts, \"throttling.iops-total\", 0);\n> + throttle_cfg->buckets[THROTTLE_OPS_READ].avg =\n> + qemu_opt_get_number(opts, \"throttling.iops-read\", 0);\n> + throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =\n> + qemu_opt_get_number(opts, \"throttling.iops-write\", 0);\n> +\n> + throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =\n> + qemu_opt_get_number(opts, \"throttling.bps-total-max\", 0);\n> + throttle_cfg->buckets[THROTTLE_BPS_READ].max =\n> + qemu_opt_get_number(opts, \"throttling.bps-read-max\", 0);\n> + throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =\n> + qemu_opt_get_number(opts, \"throttling.bps-write-max\", 0);\n> + throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =\n> + qemu_opt_get_number(opts, \"throttling.iops-total-max\", 0);\n> + throttle_cfg->buckets[THROTTLE_OPS_READ].max =\n> + qemu_opt_get_number(opts, \"throttling.iops-read-max\", 0);\n> + throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =\n> + qemu_opt_get_number(opts, \"throttling.iops-write-max\", 0);\n> +\n> + throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =\n> + qemu_opt_get_number(opts, \"throttling.bps-total-max-length\", 1);\n> + throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =\n> + qemu_opt_get_number(opts, \"throttling.bps-read-max-length\", 1);\n> + throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =\n> + qemu_opt_get_number(opts, \"throttling.bps-write-max-length\", 1);\n> + throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =\n> + qemu_opt_get_number(opts, \"throttling.iops-total-max-length\", 1);\n> + throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =\n> + qemu_opt_get_number(opts, \"throttling.iops-read-max-length\", 1);\n> + throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =\n> + qemu_opt_get_number(opts, \"throttling.iops-write-max-length\", 1);\n> +\n> + throttle_cfg->op_size =\n> + qemu_opt_get_number(opts, \"throttling.iops-size\", 0);\n> +}","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xtG9w18XNz9s7v\n\tfor ;\n\tThu, 14 Sep 2017 21:13:38 +1000 (AEST)","from localhost ([::1]:46965 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dsS5O-000155-8H\n\tfor incoming@patchwork.ozlabs.org; Thu, 14 Sep 2017 07:13:34 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:40115)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dsS50-0000rL-8o\n\tfor qemu-devel@nongnu.org; Thu, 14 Sep 2017 07:13:12 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dsS4t-0007DZ-AC\n\tfor qemu-devel@nongnu.org; Thu, 14 Sep 2017 07:13:10 -0400","from 6.mo2.mail-out.ovh.net ([87.98.165.38]:33348)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1dsS4t-0007Bp-0c\n\tfor qemu-devel@nongnu.org; Thu, 14 Sep 2017 07:13:03 -0400","from player770.ha.ovh.net (b6.ovh.net [213.186.33.56])\n\tby mo2.mail-out.ovh.net (Postfix) with ESMTP id 98775ABF9E\n\tfor ; Thu, 14 Sep 2017 13:13:00 +0200 (CEST)","from bahia.lan (gar31-1-82-66-74-139.fbx.proxad.net [82.66.74.139])\n\t(Authenticated sender: groug@kaod.org)\n\tby player770.ha.ovh.net (Postfix) with ESMTPSA id E25413C00EF;\n\tThu, 14 Sep 2017 13:12:53 +0200 (CEST)"],"Date":"Thu, 14 Sep 2017 13:12:51 +0200","From":"Greg Kurz ","To":"Pradeep Jagadeesh ","Message-ID":"<20170914131251.0881b755@bahia.lan>","In-Reply-To":"<1505385610-35529-2-git-send-email-pradeep.jagadeesh@huawei.com>","References":"<1505385610-35529-1-git-send-email-pradeep.jagadeesh@huawei.com>\n\t<1505385610-35529-2-git-send-email-pradeep.jagadeesh@huawei.com>","X-Mailer":"Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-redhat-linux-gnu)","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha1;\n\tboundary=\"Sig_/2t04RV.kNT/ZyDmpOLnaVAU\";\n\tprotocol=\"application/pgp-signature\"","X-Ovh-Tracer-Id":"7222647904613341659","X-VR-SPAMSTATE":"OK","X-VR-SPAMSCORE":"-100","X-VR-SPAMCAUSE":"gggruggvucftvghtrhhoucdtuddrfeelledrgeeigdefkecutefuodetggdotefrodftvfcurfhrohhfihhlvgemucfqggfjpdevjffgvefmvefgnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"87.98.165.38","Subject":"Re: [Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"alberto garcia , qemu-devel@nongnu.org,\n\tMarkus Armbruster ,\n\tPradeep Jagadeesh ,\n\tjani kokkonen ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1770291,"web_url":"http://patchwork.ozlabs.org/comment/1770291/","msgid":"<20170918162033.ee7fuarojpkwp5wr@postretch>","list_archive_url":null,"date":"2017-09-18T16:20:33","subject":"Re: [Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code","submitter":{"id":71571,"url":"http://patchwork.ozlabs.org/api/people/71571/","name":"Manos Pitsidianakis","email":"el13635@mail.ntua.gr"},"content":"On Thu, Sep 14, 2017 at 06:40:05AM -0400, Pradeep Jagadeesh wrote:\n>This patch factors out the duplicate throttle code that was still\n>present in block and fsdev devices.\n>\n>Signed-off-by: Pradeep Jagadeesh \n>Reviewed-by: Alberto Garcia \n>Reviewed-by: Greg Kurz \n>Reviewed-by: Eric Blake \n>---\n> blockdev.c | 44 +---------------------------------\n> fsdev/qemu-fsdev-throttle.c | 44 ++--------------------------------\n> include/qemu/throttle-options.h | 3 +++\n> include/qemu/throttle.h | 4 ++--\n> include/qemu/typedefs.h | 1 +\n> util/throttle.c | 52 +++++++++++++++++++++++++++++++++++++++++\n> 6 files changed, 61 insertions(+), 87 deletions(-)\n>\n>diff --git a/blockdev.c b/blockdev.c\n>index 56a6b24..9d33c25 100644\n>--- a/blockdev.c\n>+++ b/blockdev.c\n>@@ -387,49 +387,7 @@ static void extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,\n> }\n>\n> if (throttle_cfg) {\n>- throttle_config_init(throttle_cfg);\n>- throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =\n>- qemu_opt_get_number(opts, \"throttling.bps-total\", 0);\n>- throttle_cfg->buckets[THROTTLE_BPS_READ].avg =\n>- qemu_opt_get_number(opts, \"throttling.bps-read\", 0);\n>- throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =\n>- qemu_opt_get_number(opts, \"throttling.bps-write\", 0);\n>- throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =\n>- qemu_opt_get_number(opts, \"throttling.iops-total\", 0);\n>- throttle_cfg->buckets[THROTTLE_OPS_READ].avg =\n>- qemu_opt_get_number(opts, \"throttling.iops-read\", 0);\n>- throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =\n>- qemu_opt_get_number(opts, \"throttling.iops-write\", 0);\n>-\n>- throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =\n>- qemu_opt_get_number(opts, \"throttling.bps-total-max\", 0);\n>- throttle_cfg->buckets[THROTTLE_BPS_READ].max =\n>- qemu_opt_get_number(opts, \"throttling.bps-read-max\", 0);\n>- throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =\n>- qemu_opt_get_number(opts, \"throttling.bps-write-max\", 0);\n>- throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =\n>- qemu_opt_get_number(opts, \"throttling.iops-total-max\", 0);\n>- throttle_cfg->buckets[THROTTLE_OPS_READ].max =\n>- qemu_opt_get_number(opts, \"throttling.iops-read-max\", 0);\n>- throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =\n>- qemu_opt_get_number(opts, \"throttling.iops-write-max\", 0);\n>-\n>- throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =\n>- qemu_opt_get_number(opts, \"throttling.bps-total-max-length\", 1);\n>- throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =\n>- qemu_opt_get_number(opts, \"throttling.bps-read-max-length\", 1);\n>- throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =\n>- qemu_opt_get_number(opts, \"throttling.bps-write-max-length\", 1);\n>- throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =\n>- qemu_opt_get_number(opts, \"throttling.iops-total-max-length\", 1);\n>- throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =\n>- qemu_opt_get_number(opts, \"throttling.iops-read-max-length\", 1);\n>- throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =\n>- qemu_opt_get_number(opts, \"throttling.iops-write-max-length\", 1);\n>-\n>- throttle_cfg->op_size =\n>- qemu_opt_get_number(opts, \"throttling.iops-size\", 0);\n>-\n>+ throttle_parse_options(throttle_cfg, opts);\n> if (!throttle_is_valid(throttle_cfg, errp)) {\n> return;\n> }\n>diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c\n>index 49eebb5..0e6fb86 100644\n>--- a/fsdev/qemu-fsdev-throttle.c\n>+++ b/fsdev/qemu-fsdev-throttle.c\n>@@ -16,6 +16,7 @@\n> #include \"qemu/error-report.h\"\n> #include \"qemu-fsdev-throttle.h\"\n> #include \"qemu/iov.h\"\n>+#include \"qemu/throttle-options.h\"\n>\n> static void fsdev_throttle_read_timer_cb(void *opaque)\n> {\n>@@ -31,48 +32,7 @@ static void fsdev_throttle_write_timer_cb(void *opaque)\n>\n> void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error **errp)\n> {\n>- throttle_config_init(&fst->cfg);\n>- fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =\n>- qemu_opt_get_number(opts, \"throttling.bps-total\", 0);\n>- fst->cfg.buckets[THROTTLE_BPS_READ].avg =\n>- qemu_opt_get_number(opts, \"throttling.bps-read\", 0);\n>- fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =\n>- qemu_opt_get_number(opts, \"throttling.bps-write\", 0);\n>- fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =\n>- qemu_opt_get_number(opts, \"throttling.iops-total\", 0);\n>- fst->cfg.buckets[THROTTLE_OPS_READ].avg =\n>- qemu_opt_get_number(opts, \"throttling.iops-read\", 0);\n>- fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =\n>- qemu_opt_get_number(opts, \"throttling.iops-write\", 0);\n>-\n>- fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =\n>- qemu_opt_get_number(opts, \"throttling.bps-total-max\", 0);\n>- fst->cfg.buckets[THROTTLE_BPS_READ].max =\n>- qemu_opt_get_number(opts, \"throttling.bps-read-max\", 0);\n>- fst->cfg.buckets[THROTTLE_BPS_WRITE].max =\n>- qemu_opt_get_number(opts, \"throttling.bps-write-max\", 0);\n>- fst->cfg.buckets[THROTTLE_OPS_TOTAL].max =\n>- qemu_opt_get_number(opts, \"throttling.iops-total-max\", 0);\n>- fst->cfg.buckets[THROTTLE_OPS_READ].max =\n>- qemu_opt_get_number(opts, \"throttling.iops-read-max\", 0);\n>- fst->cfg.buckets[THROTTLE_OPS_WRITE].max =\n>- qemu_opt_get_number(opts, \"throttling.iops-write-max\", 0);\n>-\n>- fst->cfg.buckets[THROTTLE_BPS_TOTAL].burst_length =\n>- qemu_opt_get_number(opts, \"throttling.bps-total-max-length\", 1);\n>- fst->cfg.buckets[THROTTLE_BPS_READ].burst_length =\n>- qemu_opt_get_number(opts, \"throttling.bps-read-max-length\", 1);\n>- fst->cfg.buckets[THROTTLE_BPS_WRITE].burst_length =\n>- qemu_opt_get_number(opts, \"throttling.bps-write-max-length\", 1);\n>- fst->cfg.buckets[THROTTLE_OPS_TOTAL].burst_length =\n>- qemu_opt_get_number(opts, \"throttling.iops-total-max-length\", 1);\n>- fst->cfg.buckets[THROTTLE_OPS_READ].burst_length =\n>- qemu_opt_get_number(opts, \"throttling.iops-read-max-length\", 1);\n>- fst->cfg.buckets[THROTTLE_OPS_WRITE].burst_length =\n>- qemu_opt_get_number(opts, \"throttling.iops-write-max-length\", 1);\n>- fst->cfg.op_size =\n>- qemu_opt_get_number(opts, \"throttling.iops-size\", 0);\n>-\n>+ throttle_parse_options(&fst->cfg, opts);\n> throttle_is_valid(&fst->cfg, errp);\n> }\n>\n>diff --git a/include/qemu/throttle-options.h b/include/qemu/throttle-options.h\n>index 3528a8f..9709dcd 100644\n>--- a/include/qemu/throttle-options.h\n>+++ b/include/qemu/throttle-options.h\n>@@ -9,6 +9,7 @@\n> */\n> #ifndef THROTTLE_OPTIONS_H\n> #define THROTTLE_OPTIONS_H\n>+#include \"typedefs.h\"\n>\n> #define QEMU_OPT_IOPS_TOTAL \"iops-total\"\n> #define QEMU_OPT_IOPS_TOTAL_MAX \"iops-total-max\"\n>@@ -111,4 +112,6 @@\n> .help = \"when limiting by iops max size of an I/O in bytes\",\\\n> }\n>\n>+void throttle_parse_options(ThrottleConfig *, QemuOpts *);\n>+\n> #endif\n>diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h\n>index 8c93237..b6ebc6d 100644\n>--- a/include/qemu/throttle.h\n>+++ b/include/qemu/throttle.h\n>@@ -89,10 +89,10 @@ typedef struct LeakyBucket {\n> * However it allows to keep the code clean and the bucket field is reset to\n> * zero at the right time.\n> */\n>-typedef struct ThrottleConfig {\n>+struct ThrottleConfig {\n> LeakyBucket buckets[BUCKETS_COUNT]; /* leaky buckets */\n> uint64_t op_size; /* size of an operation in bytes */\n>-} ThrottleConfig;\n>+};\n>\n> typedef struct ThrottleState {\n> ThrottleConfig cfg; /* configuration */\n>diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h\n>index 39bc835..90fe0f9 100644\n>--- a/include/qemu/typedefs.h\n>+++ b/include/qemu/typedefs.h\n>@@ -100,6 +100,7 @@ typedef struct uWireSlave uWireSlave;\n> typedef struct VirtIODevice VirtIODevice;\n> typedef struct Visitor Visitor;\n> typedef struct node_info NodeInfo;\n>+typedef struct ThrottleConfig ThrottleConfig;\n\nIs this really needed? Just include include/qemu/throttle.h wherever you \nneed.\n\n> typedef void SaveStateHandler(QEMUFile *f, void *opaque);\n> typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);\n>\n>diff --git a/util/throttle.c b/util/throttle.c\n>index 06bf916..9ef28c4 100644\n>--- a/util/throttle.c\n>+++ b/util/throttle.c\n>@@ -27,6 +27,7 @@\n> #include \"qemu/throttle.h\"\n> #include \"qemu/timer.h\"\n> #include \"block/aio.h\"\n>+#include \"qemu/throttle-options.h\"\n>\n> /* This function make a bucket leak\n> *\n>@@ -635,3 +636,54 @@ void throttle_config_to_limits(ThrottleConfig *cfg, ThrottleLimits *var)\n> var->has_iops_write_max_length = true;\n> var->has_iops_size = true;\n> }\n>+\n>+/* parse the throttle options\n>+ *\n>+ * @opts: qemu options\n>+ * @throttle_cfg: throttle configuration\n>+ */\n>+void throttle_parse_options(ThrottleConfig *throttle_cfg, QemuOpts *opts)\n>+{\n>+ throttle_config_init(throttle_cfg);\n>+ throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =\n>+ qemu_opt_get_number(opts, \"throttling.bps-total\", 0);\n>+ throttle_cfg->buckets[THROTTLE_BPS_READ].avg =\n>+ qemu_opt_get_number(opts, \"throttling.bps-read\", 0);\n>+ throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =\n>+ qemu_opt_get_number(opts, \"throttling.bps-write\", 0);\n>+ throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =\n>+ qemu_opt_get_number(opts, \"throttling.iops-total\", 0);\n>+ throttle_cfg->buckets[THROTTLE_OPS_READ].avg =\n>+ qemu_opt_get_number(opts, \"throttling.iops-read\", 0);\n>+ throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =\n>+ qemu_opt_get_number(opts, \"throttling.iops-write\", 0);\n>+\n>+ throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =\n>+ qemu_opt_get_number(opts, \"throttling.bps-total-max\", 0);\n>+ throttle_cfg->buckets[THROTTLE_BPS_READ].max =\n>+ qemu_opt_get_number(opts, \"throttling.bps-read-max\", 0);\n>+ throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =\n>+ qemu_opt_get_number(opts, \"throttling.bps-write-max\", 0);\n>+ throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =\n>+ qemu_opt_get_number(opts, \"throttling.iops-total-max\", 0);\n>+ throttle_cfg->buckets[THROTTLE_OPS_READ].max =\n>+ qemu_opt_get_number(opts, \"throttling.iops-read-max\", 0);\n>+ throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =\n>+ qemu_opt_get_number(opts, \"throttling.iops-write-max\", 0);\n>+\n>+ throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =\n>+ qemu_opt_get_number(opts, \"throttling.bps-total-max-length\", 1);\n>+ throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =\n>+ qemu_opt_get_number(opts, \"throttling.bps-read-max-length\", 1);\n>+ throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =\n>+ qemu_opt_get_number(opts, \"throttling.bps-write-max-length\", 1);\n>+ throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =\n>+ qemu_opt_get_number(opts, \"throttling.iops-total-max-length\", 1);\n>+ throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =\n>+ qemu_opt_get_number(opts, \"throttling.iops-read-max-length\", 1);\n>+ throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =\n>+ qemu_opt_get_number(opts, \"throttling.iops-write-max-length\", 1);\n>+\n>+ throttle_cfg->op_size =\n>+ qemu_opt_get_number(opts, \"throttling.iops-size\", 0);\n>+}\n\nYou should reuse the #define's in include/qemu/throttle-options.h\nSee throttle_extract_options() in this patch: \nhttp://patchwork.ozlabs.org/patch/803919/ Disregard the UINT_MAX checks, \nunsigned ints in LeakyBucket were replaced by uint64_t.\n\nDon't forget to drop the reviews when you change a patch! The original \nreviews will probably not be valid anymore.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xwrwt4xybz9s78\n\tfor ;\n\tTue, 19 Sep 2017 02:26:22 +1000 (AEST)","from localhost ([::1]:37707 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dtysG-0003kC-OG\n\tfor incoming@patchwork.ozlabs.org; Mon, 18 Sep 2017 12:26:20 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:45917)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dtynm-00005O-Ui\n\tfor qemu-devel@nongnu.org; Mon, 18 Sep 2017 12:21:45 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dtyni-0007iX-Ou\n\tfor qemu-devel@nongnu.org; Mon, 18 Sep 2017 12:21:42 -0400","from smtp1.ntua.gr ([2001:648:2000:de::183]:45009)\n\tby eggs.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dtyni-0007fm-7p\n\tfor qemu-devel@nongnu.org; Mon, 18 Sep 2017 12:21:38 -0400","from mail.ntua.gr (carp0.noc.ntua.gr [147.102.222.60])\n\t(authenticated bits=0)\n\tby smtp1.ntua.gr (8.15.2/8.15.2) with ESMTPSA id v8IGKXc1085929\n\t(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=NOT); Mon, 18 Sep 2017 19:20:33 +0300 (EEST)\n\t(envelope-from el13635@mail.ntua.gr)"],"X-Authentication-Warning":"smtp1.ntua.gr: Host carp0.noc.ntua.gr\n\t[147.102.222.60] claimed to be mail.ntua.gr","Date":"Mon, 18 Sep 2017 19:20:33 +0300","From":"Manos Pitsidianakis ","To":"Pradeep Jagadeesh ","Message-ID":"<20170918162033.ee7fuarojpkwp5wr@postretch>","Mail-Followup-To":"Manos Pitsidianakis ,\n\tPradeep Jagadeesh ,\n\teric blake , greg kurz ,\n\tqemu-devel@nongnu.org, jani kokkonen ,\n\talberto garcia ,\n\tPradeep Jagadeesh ,\n\tMarkus Armbruster ","References":"<1505385610-35529-1-git-send-email-pradeep.jagadeesh@huawei.com>\n\t<1505385610-35529-2-git-send-email-pradeep.jagadeesh@huawei.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha512;\n\tprotocol=\"application/pgp-signature\"; boundary=\"jhsjxyjhaxfjd4nl\"","Content-Disposition":"inline","In-Reply-To":"<1505385610-35529-2-git-send-email-pradeep.jagadeesh@huawei.com>","User-Agent":"NeoMutt/20170609-57-1e93be (1.8.3)","X-detected-operating-system":"by eggs.gnu.org: Genre and OS details not\n\trecognized.","X-Received-From":"2001:648:2000:de::183","Subject":"Re: [Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"alberto garcia , Markus Armbruster ,\n\tgreg kurz , qemu-devel@nongnu.org,\n\tPradeep Jagadeesh ,\n\tjani kokkonen ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1771935,"web_url":"http://patchwork.ozlabs.org/comment/1771935/","msgid":"<46fdb438-3425-c787-c1c3-75815db946fd@huawei.com>","list_archive_url":null,"date":"2017-09-20T08:57:06","subject":"Re: [Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code","submitter":{"id":69883,"url":"http://patchwork.ozlabs.org/api/people/69883/","name":"Pradeep Jagadeesh","email":"pradeep.jagadeesh@huawei.com"},"content":"On 9/18/2017 6:20 PM, Manos Pitsidianakis wrote:\n> On Thu, Sep 14, 2017 at 06:40:05AM -0400, Pradeep Jagadeesh wrote:\n>> This patch factors out the duplicate throttle code that was still\n>> present in block and fsdev devices.\n>>\n>> Signed-off-by: Pradeep Jagadeesh \n>> Reviewed-by: Alberto Garcia \n>> Reviewed-by: Greg Kurz \n>> Reviewed-by: Eric Blake \n>> ---\n>> blockdev.c | 44 +---------------------------------\n>> fsdev/qemu-fsdev-throttle.c | 44 ++--------------------------------\n>> include/qemu/throttle-options.h | 3 +++\n>> include/qemu/throttle.h | 4 ++--\n>> include/qemu/typedefs.h | 1 +\n>> util/throttle.c | 52\n>> +++++++++++++++++++++++++++++++++++++++++\n>> 6 files changed, 61 insertions(+), 87 deletions(-)\n>>\n>> diff --git a/blockdev.c b/blockdev.c\n>> index 56a6b24..9d33c25 100644\n>> --- a/blockdev.c\n>> +++ b/blockdev.c\n>> @@ -387,49 +387,7 @@ static void\n>> extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,\n>> }\n>>\n>> if (throttle_cfg) {\n>> - throttle_config_init(throttle_cfg);\n>> - throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =\n>> - qemu_opt_get_number(opts, \"throttling.bps-total\", 0);\n>> - throttle_cfg->buckets[THROTTLE_BPS_READ].avg =\n>> - qemu_opt_get_number(opts, \"throttling.bps-read\", 0);\n>> - throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =\n>> - qemu_opt_get_number(opts, \"throttling.bps-write\", 0);\n>> - throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =\n>> - qemu_opt_get_number(opts, \"throttling.iops-total\", 0);\n>> - throttle_cfg->buckets[THROTTLE_OPS_READ].avg =\n>> - qemu_opt_get_number(opts, \"throttling.iops-read\", 0);\n>> - throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =\n>> - qemu_opt_get_number(opts, \"throttling.iops-write\", 0);\n>> -\n>> - throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =\n>> - qemu_opt_get_number(opts, \"throttling.bps-total-max\", 0);\n>> - throttle_cfg->buckets[THROTTLE_BPS_READ].max =\n>> - qemu_opt_get_number(opts, \"throttling.bps-read-max\", 0);\n>> - throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =\n>> - qemu_opt_get_number(opts, \"throttling.bps-write-max\", 0);\n>> - throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =\n>> - qemu_opt_get_number(opts, \"throttling.iops-total-max\", 0);\n>> - throttle_cfg->buckets[THROTTLE_OPS_READ].max =\n>> - qemu_opt_get_number(opts, \"throttling.iops-read-max\", 0);\n>> - throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =\n>> - qemu_opt_get_number(opts, \"throttling.iops-write-max\", 0);\n>> -\n>> - throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =\n>> - qemu_opt_get_number(opts,\n>> \"throttling.bps-total-max-length\", 1);\n>> - throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =\n>> - qemu_opt_get_number(opts,\n>> \"throttling.bps-read-max-length\", 1);\n>> - throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =\n>> - qemu_opt_get_number(opts,\n>> \"throttling.bps-write-max-length\", 1);\n>> - throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =\n>> - qemu_opt_get_number(opts,\n>> \"throttling.iops-total-max-length\", 1);\n>> - throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =\n>> - qemu_opt_get_number(opts,\n>> \"throttling.iops-read-max-length\", 1);\n>> - throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =\n>> - qemu_opt_get_number(opts,\n>> \"throttling.iops-write-max-length\", 1);\n>> -\n>> - throttle_cfg->op_size =\n>> - qemu_opt_get_number(opts, \"throttling.iops-size\", 0);\n>> -\n>> + throttle_parse_options(throttle_cfg, opts);\n>> if (!throttle_is_valid(throttle_cfg, errp)) {\n>> return;\n>> }\n>> diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c\n>> index 49eebb5..0e6fb86 100644\n>> --- a/fsdev/qemu-fsdev-throttle.c\n>> +++ b/fsdev/qemu-fsdev-throttle.c\n>> @@ -16,6 +16,7 @@\n>> #include \"qemu/error-report.h\"\n>> #include \"qemu-fsdev-throttle.h\"\n>> #include \"qemu/iov.h\"\n>> +#include \"qemu/throttle-options.h\"\n>>\n>> static void fsdev_throttle_read_timer_cb(void *opaque)\n>> {\n>> @@ -31,48 +32,7 @@ static void fsdev_throttle_write_timer_cb(void\n>> *opaque)\n>>\n>> void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error\n>> **errp)\n>> {\n>> - throttle_config_init(&fst->cfg);\n>> - fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =\n>> - qemu_opt_get_number(opts, \"throttling.bps-total\", 0);\n>> - fst->cfg.buckets[THROTTLE_BPS_READ].avg =\n>> - qemu_opt_get_number(opts, \"throttling.bps-read\", 0);\n>> - fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =\n>> - qemu_opt_get_number(opts, \"throttling.bps-write\", 0);\n>> - fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =\n>> - qemu_opt_get_number(opts, \"throttling.iops-total\", 0);\n>> - fst->cfg.buckets[THROTTLE_OPS_READ].avg =\n>> - qemu_opt_get_number(opts, \"throttling.iops-read\", 0);\n>> - fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =\n>> - qemu_opt_get_number(opts, \"throttling.iops-write\", 0);\n>> -\n>> - fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =\n>> - qemu_opt_get_number(opts, \"throttling.bps-total-max\", 0);\n>> - fst->cfg.buckets[THROTTLE_BPS_READ].max =\n>> - qemu_opt_get_number(opts, \"throttling.bps-read-max\", 0);\n>> - fst->cfg.buckets[THROTTLE_BPS_WRITE].max =\n>> - qemu_opt_get_number(opts, \"throttling.bps-write-max\", 0);\n>> - fst->cfg.buckets[THROTTLE_OPS_TOTAL].max =\n>> - qemu_opt_get_number(opts, \"throttling.iops-total-max\", 0);\n>> - fst->cfg.buckets[THROTTLE_OPS_READ].max =\n>> - qemu_opt_get_number(opts, \"throttling.iops-read-max\", 0);\n>> - fst->cfg.buckets[THROTTLE_OPS_WRITE].max =\n>> - qemu_opt_get_number(opts, \"throttling.iops-write-max\", 0);\n>> -\n>> - fst->cfg.buckets[THROTTLE_BPS_TOTAL].burst_length =\n>> - qemu_opt_get_number(opts, \"throttling.bps-total-max-length\", 1);\n>> - fst->cfg.buckets[THROTTLE_BPS_READ].burst_length =\n>> - qemu_opt_get_number(opts, \"throttling.bps-read-max-length\", 1);\n>> - fst->cfg.buckets[THROTTLE_BPS_WRITE].burst_length =\n>> - qemu_opt_get_number(opts, \"throttling.bps-write-max-length\", 1);\n>> - fst->cfg.buckets[THROTTLE_OPS_TOTAL].burst_length =\n>> - qemu_opt_get_number(opts, \"throttling.iops-total-max-length\",\n>> 1);\n>> - fst->cfg.buckets[THROTTLE_OPS_READ].burst_length =\n>> - qemu_opt_get_number(opts, \"throttling.iops-read-max-length\", 1);\n>> - fst->cfg.buckets[THROTTLE_OPS_WRITE].burst_length =\n>> - qemu_opt_get_number(opts, \"throttling.iops-write-max-length\",\n>> 1);\n>> - fst->cfg.op_size =\n>> - qemu_opt_get_number(opts, \"throttling.iops-size\", 0);\n>> -\n>> + throttle_parse_options(&fst->cfg, opts);\n>> throttle_is_valid(&fst->cfg, errp);\n>> }\n>>\n>> diff --git a/include/qemu/throttle-options.h\n>> b/include/qemu/throttle-options.h\n>> index 3528a8f..9709dcd 100644\n>> --- a/include/qemu/throttle-options.h\n>> +++ b/include/qemu/throttle-options.h\n>> @@ -9,6 +9,7 @@\n>> */\n>> #ifndef THROTTLE_OPTIONS_H\n>> #define THROTTLE_OPTIONS_H\n>> +#include \"typedefs.h\"\n>>\n>> #define QEMU_OPT_IOPS_TOTAL \"iops-total\"\n>> #define QEMU_OPT_IOPS_TOTAL_MAX \"iops-total-max\"\n>> @@ -111,4 +112,6 @@\n>> .help = \"when limiting by iops max size of an I/O in bytes\",\\\n>> }\n>>\n>> +void throttle_parse_options(ThrottleConfig *, QemuOpts *);\n>> +\n>> #endif\n>> diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h\n>> index 8c93237..b6ebc6d 100644\n>> --- a/include/qemu/throttle.h\n>> +++ b/include/qemu/throttle.h\n>> @@ -89,10 +89,10 @@ typedef struct LeakyBucket {\n>> * However it allows to keep the code clean and the bucket field is\n>> reset to\n>> * zero at the right time.\n>> */\n>> -typedef struct ThrottleConfig {\n>> +struct ThrottleConfig {\n>> LeakyBucket buckets[BUCKETS_COUNT]; /* leaky buckets */\n>> uint64_t op_size; /* size of an operation in bytes */\n>> -} ThrottleConfig;\n>> +};\n>>\n>> typedef struct ThrottleState {\n>> ThrottleConfig cfg; /* configuration */\n>> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h\n>> index 39bc835..90fe0f9 100644\n>> --- a/include/qemu/typedefs.h\n>> +++ b/include/qemu/typedefs.h\n>> @@ -100,6 +100,7 @@ typedef struct uWireSlave uWireSlave;\n>> typedef struct VirtIODevice VirtIODevice;\n>> typedef struct Visitor Visitor;\n>> typedef struct node_info NodeInfo;\n>> +typedef struct ThrottleConfig ThrottleConfig;\n>\n> Is this really needed? Just include include/qemu/throttle.h wherever you\n> need.\n>\n>> typedef void SaveStateHandler(QEMUFile *f, void *opaque);\n>> typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);\n>>\n>> diff --git a/util/throttle.c b/util/throttle.c\n>> index 06bf916..9ef28c4 100644\n>> --- a/util/throttle.c\n>> +++ b/util/throttle.c\n>> @@ -27,6 +27,7 @@\n>> #include \"qemu/throttle.h\"\n>> #include \"qemu/timer.h\"\n>> #include \"block/aio.h\"\n>> +#include \"qemu/throttle-options.h\"\n>>\n>> /* This function make a bucket leak\n>> *\n>> @@ -635,3 +636,54 @@ void throttle_config_to_limits(ThrottleConfig\n>> *cfg, ThrottleLimits *var)\n>> var->has_iops_write_max_length = true;\n>> var->has_iops_size = true;\n>> }\n>> +\n>> +/* parse the throttle options\n>> + *\n>> + * @opts: qemu options\n>> + * @throttle_cfg: throttle configuration\n>> + */\n>> +void throttle_parse_options(ThrottleConfig *throttle_cfg, QemuOpts\n>> *opts)\n>> +{\n>> + throttle_config_init(throttle_cfg);\n>> + throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =\n>> + qemu_opt_get_number(opts, \"throttling.bps-total\", 0);\n>> + throttle_cfg->buckets[THROTTLE_BPS_READ].avg =\n>> + qemu_opt_get_number(opts, \"throttling.bps-read\", 0);\n>> + throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =\n>> + qemu_opt_get_number(opts, \"throttling.bps-write\", 0);\n>> + throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =\n>> + qemu_opt_get_number(opts, \"throttling.iops-total\", 0);\n>> + throttle_cfg->buckets[THROTTLE_OPS_READ].avg =\n>> + qemu_opt_get_number(opts, \"throttling.iops-read\", 0);\n>> + throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =\n>> + qemu_opt_get_number(opts, \"throttling.iops-write\", 0);\n>> +\n>> + throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =\n>> + qemu_opt_get_number(opts, \"throttling.bps-total-max\", 0);\n>> + throttle_cfg->buckets[THROTTLE_BPS_READ].max =\n>> + qemu_opt_get_number(opts, \"throttling.bps-read-max\", 0);\n>> + throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =\n>> + qemu_opt_get_number(opts, \"throttling.bps-write-max\", 0);\n>> + throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =\n>> + qemu_opt_get_number(opts, \"throttling.iops-total-max\", 0);\n>> + throttle_cfg->buckets[THROTTLE_OPS_READ].max =\n>> + qemu_opt_get_number(opts, \"throttling.iops-read-max\", 0);\n>> + throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =\n>> + qemu_opt_get_number(opts, \"throttling.iops-write-max\", 0);\n>> +\n>> + throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =\n>> + qemu_opt_get_number(opts, \"throttling.bps-total-max-length\", 1);\n>> + throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =\n>> + qemu_opt_get_number(opts, \"throttling.bps-read-max-length\", 1);\n>> + throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =\n>> + qemu_opt_get_number(opts, \"throttling.bps-write-max-length\", 1);\n>> + throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =\n>> + qemu_opt_get_number(opts, \"throttling.iops-total-max-length\",\n>> 1);\n>> + throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =\n>> + qemu_opt_get_number(opts, \"throttling.iops-read-max-length\", 1);\n>> + throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =\n>> + qemu_opt_get_number(opts, \"throttling.iops-write-max-length\",\n>> 1);\n>> +\n>> + throttle_cfg->op_size =\n>> + qemu_opt_get_number(opts, \"throttling.iops-size\", 0);\n>> +}\n>\n> You should reuse the #define's in include/qemu/throttle-options.h\n> See throttle_extract_options() in this patch:\n> http://patchwork.ozlabs.org/patch/803919/ Disregard the UINT_MAX checks,\n> unsigned ints in LeakyBucket were replaced by uint64_t.\nYou mean something like below?\n qemu_opt_get_number(opts, QEMU_OPT_IOPS_READ, 0);\ninstead\nqemu_opt_get_number(opts, \"throttling.bps-total\", 0);\n\nRegards,\nPradeep\n\n\n> Don't forget to drop the reviews when you change a patch! The original\n> reviews will probably not be valid anymore.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xy2X94DnWz9sP1\n\tfor ;\n\tThu, 21 Sep 2017 00:42:32 +1000 (AEST)","from localhost ([::1]:48546 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dugCr-0003KB-Uq\n\tfor incoming@patchwork.ozlabs.org; Wed, 20 Sep 2017 10:42:30 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:53442)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dufct-00032s-Cg\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 10:05:21 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dufcj-0001lB-Qg\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 10:05:19 -0400","from lhrrgout.huawei.com ([194.213.3.17]:24228)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_ARCFOUR_SHA1:16) (Exim 4.71)\n\t(envelope-from ) id 1dufci-0001i1-NQ\n\tfor qemu-devel@nongnu.org; Wed, 20 Sep 2017 10:05:09 -0400","from 172.18.7.190 (EHLO LHREML713-CAH.china.huawei.com)\n\t([172.18.7.190])\n\tby lhrrg02-dlp.huawei.com (MOS 4.3.7-GA FastPath queued)\n\twith ESMTP id DOY65699; Wed, 20 Sep 2017 08:57:16 +0000 (GMT)","from [127.0.0.1] (10.210.172.123) by LHREML713-CAH.china.huawei.com\n\t(10.201.108.36) with Microsoft SMTP Server id 14.3.301.0;\n\tWed, 20 Sep 2017 09:57:06 +0100"],"To":"Manos Pitsidianakis , Pradeep Jagadeesh\n\t, eric blake , greg kurz\n\t, , jani kokkonen\n\t, alberto garcia , \"Markus\n\tArmbruster\" ","References":"<1505385610-35529-1-git-send-email-pradeep.jagadeesh@huawei.com>\n\t<1505385610-35529-2-git-send-email-pradeep.jagadeesh@huawei.com>\n\t<20170918162033.ee7fuarojpkwp5wr@postretch>","From":"Pradeep Jagadeesh ","Message-ID":"<46fdb438-3425-c787-c1c3-75815db946fd@huawei.com>","Date":"Wed, 20 Sep 2017 10:57:06 +0200","User-Agent":"Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101\n\tThunderbird/45.8.0","MIME-Version":"1.0","In-Reply-To":"<20170918162033.ee7fuarojpkwp5wr@postretch>","Content-Type":"text/plain; charset=\"utf-8\"; format=flowed","Content-Transfer-Encoding":"7bit","X-Originating-IP":"[10.210.172.123]","X-CFilter-Loop":"Reflected","X-Mirapoint-Virus-RAPID-Raw":"score=unknown(0),\n\trefid=str=0001.0A020204.59C22D6D.008D, ss=1, re=0.000, recu=0.000,\n\treip=0.000, cl=1, cld=1, fgs=0, ip=0.0.0.0,\n\tso=2013-06-18 04:22:30, dmn=2013-03-21 17:37:32","X-Mirapoint-Loop-Id":"2bb84c5c4b9ee514ec198c646157bbed","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.4.x-2.6.x [generic]\n\t[fuzzy]","X-Received-From":"194.213.3.17","Subject":"Re: [Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1773505,"web_url":"http://patchwork.ozlabs.org/comment/1773505/","msgid":"<209dd16d-8d97-090d-35e9-b355c53c3448@huawei.com>","list_archive_url":null,"date":"2017-09-22T11:31:58","subject":"Re: [Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code","submitter":{"id":69883,"url":"http://patchwork.ozlabs.org/api/people/69883/","name":"Pradeep Jagadeesh","email":"pradeep.jagadeesh@huawei.com"},"content":"On 9/18/2017 6:20 PM, Manos Pitsidianakis wrote:\n> On Thu, Sep 14, 2017 at 06:40:05AM -0400, Pradeep Jagadeesh wrote:\n>> This patch factors out the duplicate throttle code that was still\n>> present in block and fsdev devices.\n>>\n>> Signed-off-by: Pradeep Jagadeesh \n>> Reviewed-by: Alberto Garcia \n>> Reviewed-by: Greg Kurz \n>> Reviewed-by: Eric Blake \n>> ---\n>> blockdev.c | 44 +---------------------------------\n>> fsdev/qemu-fsdev-throttle.c | 44 ++--------------------------------\n>> include/qemu/throttle-options.h | 3 +++\n>> include/qemu/throttle.h | 4 ++--\n>> include/qemu/typedefs.h | 1 +\n>> util/throttle.c | 52\n>> +++++++++++++++++++++++++++++++++++++++++\n>> 6 files changed, 61 insertions(+), 87 deletions(-)\n>>\n>> diff --git a/blockdev.c b/blockdev.c\n>> index 56a6b24..9d33c25 100644\n>> --- a/blockdev.c\n>> +++ b/blockdev.c\n>> @@ -387,49 +387,7 @@ static void\n>> extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,\n>> }\n>>\n>> if (throttle_cfg) {\n>> - throttle_config_init(throttle_cfg);\n>> - throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =\n>> - qemu_opt_get_number(opts, \"throttling.bps-total\", 0);\n>> - throttle_cfg->buckets[THROTTLE_BPS_READ].avg =\n>> - qemu_opt_get_number(opts, \"throttling.bps-read\", 0);\n>> - throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =\n>> - qemu_opt_get_number(opts, \"throttling.bps-write\", 0);\n>> - throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =\n>> - qemu_opt_get_number(opts, \"throttling.iops-total\", 0);\n>> - throttle_cfg->buckets[THROTTLE_OPS_READ].avg =\n>> - qemu_opt_get_number(opts, \"throttling.iops-read\", 0);\n>> - throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =\n>> - qemu_opt_get_number(opts, \"throttling.iops-write\", 0);\n>> -\n>> - throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =\n>> - qemu_opt_get_number(opts, \"throttling.bps-total-max\", 0);\n>> - throttle_cfg->buckets[THROTTLE_BPS_READ].max =\n>> - qemu_opt_get_number(opts, \"throttling.bps-read-max\", 0);\n>> - throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =\n>> - qemu_opt_get_number(opts, \"throttling.bps-write-max\", 0);\n>> - throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =\n>> - qemu_opt_get_number(opts, \"throttling.iops-total-max\", 0);\n>> - throttle_cfg->buckets[THROTTLE_OPS_READ].max =\n>> - qemu_opt_get_number(opts, \"throttling.iops-read-max\", 0);\n>> - throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =\n>> - qemu_opt_get_number(opts, \"throttling.iops-write-max\", 0);\n>> -\n>> - throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =\n>> - qemu_opt_get_number(opts,\n>> \"throttling.bps-total-max-length\", 1);\n>> - throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =\n>> - qemu_opt_get_number(opts,\n>> \"throttling.bps-read-max-length\", 1);\n>> - throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =\n>> - qemu_opt_get_number(opts,\n>> \"throttling.bps-write-max-length\", 1);\n>> - throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =\n>> - qemu_opt_get_number(opts,\n>> \"throttling.iops-total-max-length\", 1);\n>> - throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =\n>> - qemu_opt_get_number(opts,\n>> \"throttling.iops-read-max-length\", 1);\n>> - throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =\n>> - qemu_opt_get_number(opts,\n>> \"throttling.iops-write-max-length\", 1);\n>> -\n>> - throttle_cfg->op_size =\n>> - qemu_opt_get_number(opts, \"throttling.iops-size\", 0);\n>> -\n>> + throttle_parse_options(throttle_cfg, opts);\n>> if (!throttle_is_valid(throttle_cfg, errp)) {\n>> return;\n>> }\n>> diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c\n>> index 49eebb5..0e6fb86 100644\n>> --- a/fsdev/qemu-fsdev-throttle.c\n>> +++ b/fsdev/qemu-fsdev-throttle.c\n>> @@ -16,6 +16,7 @@\n>> #include \"qemu/error-report.h\"\n>> #include \"qemu-fsdev-throttle.h\"\n>> #include \"qemu/iov.h\"\n>> +#include \"qemu/throttle-options.h\"\n>>\n>> static void fsdev_throttle_read_timer_cb(void *opaque)\n>> {\n>> @@ -31,48 +32,7 @@ static void fsdev_throttle_write_timer_cb(void\n>> *opaque)\n>>\n>> void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error\n>> **errp)\n>> {\n>> - throttle_config_init(&fst->cfg);\n>> - fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =\n>> - qemu_opt_get_number(opts, \"throttling.bps-total\", 0);\n>> - fst->cfg.buckets[THROTTLE_BPS_READ].avg =\n>> - qemu_opt_get_number(opts, \"throttling.bps-read\", 0);\n>> - fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =\n>> - qemu_opt_get_number(opts, \"throttling.bps-write\", 0);\n>> - fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =\n>> - qemu_opt_get_number(opts, \"throttling.iops-total\", 0);\n>> - fst->cfg.buckets[THROTTLE_OPS_READ].avg =\n>> - qemu_opt_get_number(opts, \"throttling.iops-read\", 0);\n>> - fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =\n>> - qemu_opt_get_number(opts, \"throttling.iops-write\", 0);\n>> -\n>> - fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =\n>> - qemu_opt_get_number(opts, \"throttling.bps-total-max\", 0);\n>> - fst->cfg.buckets[THROTTLE_BPS_READ].max =\n>> - qemu_opt_get_number(opts, \"throttling.bps-read-max\", 0);\n>> - fst->cfg.buckets[THROTTLE_BPS_WRITE].max =\n>> - qemu_opt_get_number(opts, \"throttling.bps-write-max\", 0);\n>> - fst->cfg.buckets[THROTTLE_OPS_TOTAL].max =\n>> - qemu_opt_get_number(opts, \"throttling.iops-total-max\", 0);\n>> - fst->cfg.buckets[THROTTLE_OPS_READ].max =\n>> - qemu_opt_get_number(opts, \"throttling.iops-read-max\", 0);\n>> - fst->cfg.buckets[THROTTLE_OPS_WRITE].max =\n>> - qemu_opt_get_number(opts, \"throttling.iops-write-max\", 0);\n>> -\n>> - fst->cfg.buckets[THROTTLE_BPS_TOTAL].burst_length =\n>> - qemu_opt_get_number(opts, \"throttling.bps-total-max-length\", 1);\n>> - fst->cfg.buckets[THROTTLE_BPS_READ].burst_length =\n>> - qemu_opt_get_number(opts, \"throttling.bps-read-max-length\", 1);\n>> - fst->cfg.buckets[THROTTLE_BPS_WRITE].burst_length =\n>> - qemu_opt_get_number(opts, \"throttling.bps-write-max-length\", 1);\n>> - fst->cfg.buckets[THROTTLE_OPS_TOTAL].burst_length =\n>> - qemu_opt_get_number(opts, \"throttling.iops-total-max-length\",\n>> 1);\n>> - fst->cfg.buckets[THROTTLE_OPS_READ].burst_length =\n>> - qemu_opt_get_number(opts, \"throttling.iops-read-max-length\", 1);\n>> - fst->cfg.buckets[THROTTLE_OPS_WRITE].burst_length =\n>> - qemu_opt_get_number(opts, \"throttling.iops-write-max-length\",\n>> 1);\n>> - fst->cfg.op_size =\n>> - qemu_opt_get_number(opts, \"throttling.iops-size\", 0);\n>> -\n>> + throttle_parse_options(&fst->cfg, opts);\n>> throttle_is_valid(&fst->cfg, errp);\n>> }\n>>\n>> diff --git a/include/qemu/throttle-options.h\n>> b/include/qemu/throttle-options.h\n>> index 3528a8f..9709dcd 100644\n>> --- a/include/qemu/throttle-options.h\n>> +++ b/include/qemu/throttle-options.h\n>> @@ -9,6 +9,7 @@\n>> */\n>> #ifndef THROTTLE_OPTIONS_H\n>> #define THROTTLE_OPTIONS_H\n>> +#include \"typedefs.h\"\n>>\n>> #define QEMU_OPT_IOPS_TOTAL \"iops-total\"\n>> #define QEMU_OPT_IOPS_TOTAL_MAX \"iops-total-max\"\n>> @@ -111,4 +112,6 @@\n>> .help = \"when limiting by iops max size of an I/O in bytes\",\\\n>> }\n>>\n>> +void throttle_parse_options(ThrottleConfig *, QemuOpts *);\n>> +\n>> #endif\n>> diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h\n>> index 8c93237..b6ebc6d 100644\n>> --- a/include/qemu/throttle.h\n>> +++ b/include/qemu/throttle.h\n>> @@ -89,10 +89,10 @@ typedef struct LeakyBucket {\n>> * However it allows to keep the code clean and the bucket field is\n>> reset to\n>> * zero at the right time.\n>> */\n>> -typedef struct ThrottleConfig {\n>> +struct ThrottleConfig {\n>> LeakyBucket buckets[BUCKETS_COUNT]; /* leaky buckets */\n>> uint64_t op_size; /* size of an operation in bytes */\n>> -} ThrottleConfig;\n>> +};\n>>\n>> typedef struct ThrottleState {\n>> ThrottleConfig cfg; /* configuration */\n>> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h\n>> index 39bc835..90fe0f9 100644\n>> --- a/include/qemu/typedefs.h\n>> +++ b/include/qemu/typedefs.h\n>> @@ -100,6 +100,7 @@ typedef struct uWireSlave uWireSlave;\n>> typedef struct VirtIODevice VirtIODevice;\n>> typedef struct Visitor Visitor;\n>> typedef struct node_info NodeInfo;\n>> +typedef struct ThrottleConfig ThrottleConfig;\n>\n> Is this really needed? Just include include/qemu/throttle.h wherever you\n> need.\n>\n>> typedef void SaveStateHandler(QEMUFile *f, void *opaque);\n>> typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);\n>>\n>> diff --git a/util/throttle.c b/util/throttle.c\n>> index 06bf916..9ef28c4 100644\n>> --- a/util/throttle.c\n>> +++ b/util/throttle.c\n>> @@ -27,6 +27,7 @@\n>> #include \"qemu/throttle.h\"\n>> #include \"qemu/timer.h\"\n>> #include \"block/aio.h\"\n>> +#include \"qemu/throttle-options.h\"\n>>\n>> /* This function make a bucket leak\n>> *\n>> @@ -635,3 +636,54 @@ void throttle_config_to_limits(ThrottleConfig\n>> *cfg, ThrottleLimits *var)\n>> var->has_iops_write_max_length = true;\n>> var->has_iops_size = true;\n>> }\n>> +\n>> +/* parse the throttle options\n>> + *\n>> + * @opts: qemu options\n>> + * @throttle_cfg: throttle configuration\n>> + */\n>> +void throttle_parse_options(ThrottleConfig *throttle_cfg, QemuOpts\n>> *opts)\n>> +{\n>> + throttle_config_init(throttle_cfg);\n>> + throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =\n>> + qemu_opt_get_number(opts, \"throttling.bps-total\", 0);\n>> + throttle_cfg->buckets[THROTTLE_BPS_READ].avg =\n>> + qemu_opt_get_number(opts, \"throttling.bps-read\", 0);\n>> + throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =\n>> + qemu_opt_get_number(opts, \"throttling.bps-write\", 0);\n>> + throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =\n>> + qemu_opt_get_number(opts, \"throttling.iops-total\", 0);\n>> + throttle_cfg->buckets[THROTTLE_OPS_READ].avg =\n>> + qemu_opt_get_number(opts, \"throttling.iops-read\", 0);\n>> + throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =\n>> + qemu_opt_get_number(opts, \"throttling.iops-write\", 0);\n>> +\n>> + throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =\n>> + qemu_opt_get_number(opts, \"throttling.bps-total-max\", 0);\n>> + throttle_cfg->buckets[THROTTLE_BPS_READ].max =\n>> + qemu_opt_get_number(opts, \"throttling.bps-read-max\", 0);\n>> + throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =\n>> + qemu_opt_get_number(opts, \"throttling.bps-write-max\", 0);\n>> + throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =\n>> + qemu_opt_get_number(opts, \"throttling.iops-total-max\", 0);\n>> + throttle_cfg->buckets[THROTTLE_OPS_READ].max =\n>> + qemu_opt_get_number(opts, \"throttling.iops-read-max\", 0);\n>> + throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =\n>> + qemu_opt_get_number(opts, \"throttling.iops-write-max\", 0);\n>> +\n>> + throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =\n>> + qemu_opt_get_number(opts, \"throttling.bps-total-max-length\", 1);\n>> + throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =\n>> + qemu_opt_get_number(opts, \"throttling.bps-read-max-length\", 1);\n>> + throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =\n>> + qemu_opt_get_number(opts, \"throttling.bps-write-max-length\", 1);\n>> + throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =\n>> + qemu_opt_get_number(opts, \"throttling.iops-total-max-length\",\n>> 1);\n>> + throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =\n>> + qemu_opt_get_number(opts, \"throttling.iops-read-max-length\", 1);\n>> + throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =\n>> + qemu_opt_get_number(opts, \"throttling.iops-write-max-length\",\n>> 1);\n>> +\n>> + throttle_cfg->op_size =\n>> + qemu_opt_get_number(opts, \"throttling.iops-size\", 0);\n>> +}\n>\n> You should reuse the #define's in include/qemu/throttle-options.h\n> See throttle_extract_options() in this patch:\n> http://patchwork.ozlabs.org/patch/803919/ Disregard the UINT_MAX checks,\n> unsigned ints in LeakyBucket were replaced by uint64_t.\n>\nI can not use the #define's because I use throttling.*.\nIn my last patch also we wanted to have it like that to align with the \nblock device throttle options.\nIf you see in blockdev.c, still there exists throttling.*\n\nThere may be change required every where, so would like to do it as part \nof another patch.\n\n-Pradeep\n> Don't forget to drop the reviews when you change a patch! The original\n> reviews will probably not be valid anymore.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xzBFL3NTtz9sNw\n\tfor ;\n\tFri, 22 Sep 2017 21:33:41 +1000 (AEST)","from localhost ([::1]:58110 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dvMDC-0002ZT-OX\n\tfor incoming@patchwork.ozlabs.org; Fri, 22 Sep 2017 07:33:38 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:40244)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dvMCq-0002ZI-Cp\n\tfor qemu-devel@nongnu.org; Fri, 22 Sep 2017 07:33:18 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dvMCn-0003R7-3R\n\tfor qemu-devel@nongnu.org; Fri, 22 Sep 2017 07:33:16 -0400","from lhrrgout.huawei.com ([194.213.3.17]:24362)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_ARCFOUR_SHA1:16) (Exim 4.71)\n\t(envelope-from ) id 1dvMCm-0003PI-LJ\n\tfor qemu-devel@nongnu.org; Fri, 22 Sep 2017 07:33:13 -0400","from 172.18.7.190 (EHLO lhreml708-cah.china.huawei.com)\n\t([172.18.7.190])\n\tby lhrrg01-dlp.huawei.com (MOS 4.3.7-GA FastPath queued)\n\twith ESMTP id DVZ83210; Fri, 22 Sep 2017 11:32:10 +0000 (GMT)","from [127.0.0.1] (10.210.170.190) by lhreml708-cah.china.huawei.com\n\t(10.201.108.49) with Microsoft SMTP Server id 14.3.301.0;\n\tFri, 22 Sep 2017 12:31:58 +0100"],"To":"Manos Pitsidianakis , Pradeep Jagadeesh\n\t, eric blake , greg kurz\n\t, , jani kokkonen\n\t, alberto garcia , \"Markus\n\tArmbruster\" ","References":"<1505385610-35529-1-git-send-email-pradeep.jagadeesh@huawei.com>\n\t<1505385610-35529-2-git-send-email-pradeep.jagadeesh@huawei.com>\n\t<20170918162033.ee7fuarojpkwp5wr@postretch>","From":"Pradeep Jagadeesh ","Message-ID":"<209dd16d-8d97-090d-35e9-b355c53c3448@huawei.com>","Date":"Fri, 22 Sep 2017 13:31:58 +0200","User-Agent":"Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101\n\tThunderbird/45.8.0","MIME-Version":"1.0","In-Reply-To":"<20170918162033.ee7fuarojpkwp5wr@postretch>","Content-Type":"text/plain; charset=\"utf-8\"; format=flowed","Content-Transfer-Encoding":"7bit","X-Originating-IP":"[10.210.170.190]","X-CFilter-Loop":"Reflected","X-Mirapoint-Virus-RAPID-Raw":"score=unknown(0),\n\trefid=str=0001.0A0B0204.59C4F4BC.0089, ss=1, re=0.000, recu=0.000,\n\treip=0.000, cl=1, cld=1, fgs=0, ip=0.0.0.0,\n\tso=2013-06-18 04:22:30, dmn=2013-03-21 17:37:32","X-Mirapoint-Loop-Id":"407e43770d25182228020ccaee4f895c","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.4.x-2.6.x [generic]\n\t[fuzzy]","X-Received-From":"194.213.3.17","Subject":"Re: [Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1774012,"web_url":"http://patchwork.ozlabs.org/comment/1774012/","msgid":"<20170923103359.qzp5eyr54hjzzvie@postretch>","list_archive_url":null,"date":"2017-09-23T10:33:59","subject":"Re: [Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code","submitter":{"id":71571,"url":"http://patchwork.ozlabs.org/api/people/71571/","name":"Manos Pitsidianakis","email":"el13635@mail.ntua.gr"},"content":"On Fri, Sep 22, 2017 at 01:31:58PM +0200, Pradeep Jagadeesh wrote:\n>On 9/18/2017 6:20 PM, Manos Pitsidianakis wrote:\n>>On Thu, Sep 14, 2017 at 06:40:05AM -0400, Pradeep Jagadeesh wrote:\n>>>This patch factors out the duplicate throttle code that was still\n>>>present in block and fsdev devices.\n>>>\n>>>Signed-off-by: Pradeep Jagadeesh \n>>>Reviewed-by: Alberto Garcia \n>>>Reviewed-by: Greg Kurz \n>>>Reviewed-by: Eric Blake \n>>>---\n>>>blockdev.c | 44 +---------------------------------\n>>>fsdev/qemu-fsdev-throttle.c | 44 ++--------------------------------\n>>>include/qemu/throttle-options.h | 3 +++\n>>>include/qemu/throttle.h | 4 ++--\n>>>include/qemu/typedefs.h | 1 +\n>>>util/throttle.c | 52\n>>>+++++++++++++++++++++++++++++++++++++++++\n>>>6 files changed, 61 insertions(+), 87 deletions(-)\n>>>\n>>>diff --git a/blockdev.c b/blockdev.c\n>>>index 56a6b24..9d33c25 100644\n>>>--- a/blockdev.c\n>>>+++ b/blockdev.c\n>>>@@ -387,49 +387,7 @@ static void\n>>>extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,\n>>> }\n>>>\n>>> if (throttle_cfg) {\n>>>- throttle_config_init(throttle_cfg);\n>>>- throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =\n>>>- qemu_opt_get_number(opts, \"throttling.bps-total\", 0);\n>>>- throttle_cfg->buckets[THROTTLE_BPS_READ].avg =\n>>>- qemu_opt_get_number(opts, \"throttling.bps-read\", 0);\n>>>- throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =\n>>>- qemu_opt_get_number(opts, \"throttling.bps-write\", 0);\n>>>- throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =\n>>>- qemu_opt_get_number(opts, \"throttling.iops-total\", 0);\n>>>- throttle_cfg->buckets[THROTTLE_OPS_READ].avg =\n>>>- qemu_opt_get_number(opts, \"throttling.iops-read\", 0);\n>>>- throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =\n>>>- qemu_opt_get_number(opts, \"throttling.iops-write\", 0);\n>>>-\n>>>- throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =\n>>>- qemu_opt_get_number(opts, \"throttling.bps-total-max\", 0);\n>>>- throttle_cfg->buckets[THROTTLE_BPS_READ].max =\n>>>- qemu_opt_get_number(opts, \"throttling.bps-read-max\", 0);\n>>>- throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =\n>>>- qemu_opt_get_number(opts, \"throttling.bps-write-max\", 0);\n>>>- throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =\n>>>- qemu_opt_get_number(opts, \"throttling.iops-total-max\", 0);\n>>>- throttle_cfg->buckets[THROTTLE_OPS_READ].max =\n>>>- qemu_opt_get_number(opts, \"throttling.iops-read-max\", 0);\n>>>- throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =\n>>>- qemu_opt_get_number(opts, \"throttling.iops-write-max\", 0);\n>>>-\n>>>- throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =\n>>>- qemu_opt_get_number(opts,\n>>>\"throttling.bps-total-max-length\", 1);\n>>>- throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =\n>>>- qemu_opt_get_number(opts,\n>>>\"throttling.bps-read-max-length\", 1);\n>>>- throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =\n>>>- qemu_opt_get_number(opts,\n>>>\"throttling.bps-write-max-length\", 1);\n>>>- throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =\n>>>- qemu_opt_get_number(opts,\n>>>\"throttling.iops-total-max-length\", 1);\n>>>- throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =\n>>>- qemu_opt_get_number(opts,\n>>>\"throttling.iops-read-max-length\", 1);\n>>>- throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =\n>>>- qemu_opt_get_number(opts,\n>>>\"throttling.iops-write-max-length\", 1);\n>>>-\n>>>- throttle_cfg->op_size =\n>>>- qemu_opt_get_number(opts, \"throttling.iops-size\", 0);\n>>>-\n>>>+ throttle_parse_options(throttle_cfg, opts);\n>>> if (!throttle_is_valid(throttle_cfg, errp)) {\n>>> return;\n>>> }\n>>>diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c\n>>>index 49eebb5..0e6fb86 100644\n>>>--- a/fsdev/qemu-fsdev-throttle.c\n>>>+++ b/fsdev/qemu-fsdev-throttle.c\n>>>@@ -16,6 +16,7 @@\n>>>#include \"qemu/error-report.h\"\n>>>#include \"qemu-fsdev-throttle.h\"\n>>>#include \"qemu/iov.h\"\n>>>+#include \"qemu/throttle-options.h\"\n>>>\n>>>static void fsdev_throttle_read_timer_cb(void *opaque)\n>>>{\n>>>@@ -31,48 +32,7 @@ static void fsdev_throttle_write_timer_cb(void\n>>>*opaque)\n>>>\n>>>void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error\n>>>**errp)\n>>>{\n>>>- throttle_config_init(&fst->cfg);\n>>>- fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =\n>>>- qemu_opt_get_number(opts, \"throttling.bps-total\", 0);\n>>>- fst->cfg.buckets[THROTTLE_BPS_READ].avg =\n>>>- qemu_opt_get_number(opts, \"throttling.bps-read\", 0);\n>>>- fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =\n>>>- qemu_opt_get_number(opts, \"throttling.bps-write\", 0);\n>>>- fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =\n>>>- qemu_opt_get_number(opts, \"throttling.iops-total\", 0);\n>>>- fst->cfg.buckets[THROTTLE_OPS_READ].avg =\n>>>- qemu_opt_get_number(opts, \"throttling.iops-read\", 0);\n>>>- fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =\n>>>- qemu_opt_get_number(opts, \"throttling.iops-write\", 0);\n>>>-\n>>>- fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =\n>>>- qemu_opt_get_number(opts, \"throttling.bps-total-max\", 0);\n>>>- fst->cfg.buckets[THROTTLE_BPS_READ].max =\n>>>- qemu_opt_get_number(opts, \"throttling.bps-read-max\", 0);\n>>>- fst->cfg.buckets[THROTTLE_BPS_WRITE].max =\n>>>- qemu_opt_get_number(opts, \"throttling.bps-write-max\", 0);\n>>>- fst->cfg.buckets[THROTTLE_OPS_TOTAL].max =\n>>>- qemu_opt_get_number(opts, \"throttling.iops-total-max\", 0);\n>>>- fst->cfg.buckets[THROTTLE_OPS_READ].max =\n>>>- qemu_opt_get_number(opts, \"throttling.iops-read-max\", 0);\n>>>- fst->cfg.buckets[THROTTLE_OPS_WRITE].max =\n>>>- qemu_opt_get_number(opts, \"throttling.iops-write-max\", 0);\n>>>-\n>>>- fst->cfg.buckets[THROTTLE_BPS_TOTAL].burst_length =\n>>>- qemu_opt_get_number(opts, \"throttling.bps-total-max-length\", 1);\n>>>- fst->cfg.buckets[THROTTLE_BPS_READ].burst_length =\n>>>- qemu_opt_get_number(opts, \"throttling.bps-read-max-length\", 1);\n>>>- fst->cfg.buckets[THROTTLE_BPS_WRITE].burst_length =\n>>>- qemu_opt_get_number(opts, \"throttling.bps-write-max-length\", 1);\n>>>- fst->cfg.buckets[THROTTLE_OPS_TOTAL].burst_length =\n>>>- qemu_opt_get_number(opts, \"throttling.iops-total-max-length\",\n>>>1);\n>>>- fst->cfg.buckets[THROTTLE_OPS_READ].burst_length =\n>>>- qemu_opt_get_number(opts, \"throttling.iops-read-max-length\", 1);\n>>>- fst->cfg.buckets[THROTTLE_OPS_WRITE].burst_length =\n>>>- qemu_opt_get_number(opts, \"throttling.iops-write-max-length\",\n>>>1);\n>>>- fst->cfg.op_size =\n>>>- qemu_opt_get_number(opts, \"throttling.iops-size\", 0);\n>>>-\n>>>+ throttle_parse_options(&fst->cfg, opts);\n>>> throttle_is_valid(&fst->cfg, errp);\n>>>}\n>>>\n>>>diff --git a/include/qemu/throttle-options.h\n>>>b/include/qemu/throttle-options.h\n>>>index 3528a8f..9709dcd 100644\n>>>--- a/include/qemu/throttle-options.h\n>>>+++ b/include/qemu/throttle-options.h\n>>>@@ -9,6 +9,7 @@\n>>> */\n>>>#ifndef THROTTLE_OPTIONS_H\n>>>#define THROTTLE_OPTIONS_H\n>>>+#include \"typedefs.h\"\n>>>\n>>>#define QEMU_OPT_IOPS_TOTAL \"iops-total\"\n>>>#define QEMU_OPT_IOPS_TOTAL_MAX \"iops-total-max\"\n>>>@@ -111,4 +112,6 @@\n>>> .help = \"when limiting by iops max size of an I/O in bytes\",\\\n>>> }\n>>>\n>>>+void throttle_parse_options(ThrottleConfig *, QemuOpts *);\n>>>+\n>>>#endif\n>>>diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h\n>>>index 8c93237..b6ebc6d 100644\n>>>--- a/include/qemu/throttle.h\n>>>+++ b/include/qemu/throttle.h\n>>>@@ -89,10 +89,10 @@ typedef struct LeakyBucket {\n>>> * However it allows to keep the code clean and the bucket field is\n>>>reset to\n>>> * zero at the right time.\n>>> */\n>>>-typedef struct ThrottleConfig {\n>>>+struct ThrottleConfig {\n>>> LeakyBucket buckets[BUCKETS_COUNT]; /* leaky buckets */\n>>> uint64_t op_size; /* size of an operation in bytes */\n>>>-} ThrottleConfig;\n>>>+};\n>>>\n>>>typedef struct ThrottleState {\n>>> ThrottleConfig cfg; /* configuration */\n>>>diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h\n>>>index 39bc835..90fe0f9 100644\n>>>--- a/include/qemu/typedefs.h\n>>>+++ b/include/qemu/typedefs.h\n>>>@@ -100,6 +100,7 @@ typedef struct uWireSlave uWireSlave;\n>>>typedef struct VirtIODevice VirtIODevice;\n>>>typedef struct Visitor Visitor;\n>>>typedef struct node_info NodeInfo;\n>>>+typedef struct ThrottleConfig ThrottleConfig;\n>>\n>>Is this really needed? Just include include/qemu/throttle.h wherever you\n>>need.\n>>\n>>>typedef void SaveStateHandler(QEMUFile *f, void *opaque);\n>>>typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);\n>>>\n>>>diff --git a/util/throttle.c b/util/throttle.c\n>>>index 06bf916..9ef28c4 100644\n>>>--- a/util/throttle.c\n>>>+++ b/util/throttle.c\n>>>@@ -27,6 +27,7 @@\n>>>#include \"qemu/throttle.h\"\n>>>#include \"qemu/timer.h\"\n>>>#include \"block/aio.h\"\n>>>+#include \"qemu/throttle-options.h\"\n>>>\n>>>/* This function make a bucket leak\n>>> *\n>>>@@ -635,3 +636,54 @@ void throttle_config_to_limits(ThrottleConfig\n>>>*cfg, ThrottleLimits *var)\n>>> var->has_iops_write_max_length = true;\n>>> var->has_iops_size = true;\n>>>}\n>>>+\n>>>+/* parse the throttle options\n>>>+ *\n>>>+ * @opts: qemu options\n>>>+ * @throttle_cfg: throttle configuration\n>>>+ */\n>>>+void throttle_parse_options(ThrottleConfig *throttle_cfg, QemuOpts\n>>>*opts)\n>>>+{\n>>>+ throttle_config_init(throttle_cfg);\n>>>+ throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =\n>>>+ qemu_opt_get_number(opts, \"throttling.bps-total\", 0);\n>>>+ throttle_cfg->buckets[THROTTLE_BPS_READ].avg =\n>>>+ qemu_opt_get_number(opts, \"throttling.bps-read\", 0);\n>>>+ throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =\n>>>+ qemu_opt_get_number(opts, \"throttling.bps-write\", 0);\n>>>+ throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =\n>>>+ qemu_opt_get_number(opts, \"throttling.iops-total\", 0);\n>>>+ throttle_cfg->buckets[THROTTLE_OPS_READ].avg =\n>>>+ qemu_opt_get_number(opts, \"throttling.iops-read\", 0);\n>>>+ throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =\n>>>+ qemu_opt_get_number(opts, \"throttling.iops-write\", 0);\n>>>+\n>>>+ throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =\n>>>+ qemu_opt_get_number(opts, \"throttling.bps-total-max\", 0);\n>>>+ throttle_cfg->buckets[THROTTLE_BPS_READ].max =\n>>>+ qemu_opt_get_number(opts, \"throttling.bps-read-max\", 0);\n>>>+ throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =\n>>>+ qemu_opt_get_number(opts, \"throttling.bps-write-max\", 0);\n>>>+ throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =\n>>>+ qemu_opt_get_number(opts, \"throttling.iops-total-max\", 0);\n>>>+ throttle_cfg->buckets[THROTTLE_OPS_READ].max =\n>>>+ qemu_opt_get_number(opts, \"throttling.iops-read-max\", 0);\n>>>+ throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =\n>>>+ qemu_opt_get_number(opts, \"throttling.iops-write-max\", 0);\n>>>+\n>>>+ throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =\n>>>+ qemu_opt_get_number(opts, \"throttling.bps-total-max-length\", 1);\n>>>+ throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =\n>>>+ qemu_opt_get_number(opts, \"throttling.bps-read-max-length\", 1);\n>>>+ throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =\n>>>+ qemu_opt_get_number(opts, \"throttling.bps-write-max-length\", 1);\n>>>+ throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =\n>>>+ qemu_opt_get_number(opts, \"throttling.iops-total-max-length\",\n>>>1);\n>>>+ throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =\n>>>+ qemu_opt_get_number(opts, \"throttling.iops-read-max-length\", 1);\n>>>+ throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =\n>>>+ qemu_opt_get_number(opts, \"throttling.iops-write-max-length\",\n>>>1);\n>>>+\n>>>+ throttle_cfg->op_size =\n>>>+ qemu_opt_get_number(opts, \"throttling.iops-size\", 0);\n>>>+}\n>>\n>>You should reuse the #define's in include/qemu/throttle-options.h\n>>See throttle_extract_options() in this patch:\n>>http://patchwork.ozlabs.org/patch/803919/ Disregard the UINT_MAX checks,\n>>unsigned ints in LeakyBucket were replaced by uint64_t.\n>>\n>I can not use the #define's because I use throttling.*.\n>In my last patch also we wanted to have it like that to align with the \n>block device throttle options.\n>If you see in blockdev.c, still there exists throttling.*\n>\n\nYou can use them.\n\nqemu_opt_get_number(opts, \"throttling.iops-size\", 0) becomes:\nqemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_SIZE, 0) \n\nThis is what I did in the patch I linked, except for redefining \nTHROTTLE_OPT_PREFIX to \"limits.\". I plan to send some patches for \nblockdev.c code when the old legacy interface is replaced.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xzmwR0SRdz9tX5\n\tfor ;\n\tSat, 23 Sep 2017 20:36:02 +1000 (AEST)","from localhost ([::1]:34378 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dvhmc-0003ot-MD\n\tfor incoming@patchwork.ozlabs.org; Sat, 23 Sep 2017 06:35:38 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:50276)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dvhmD-0003nc-4Z\n\tfor qemu-devel@nongnu.org; Sat, 23 Sep 2017 06:35:14 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dvhm9-0000ik-0l\n\tfor qemu-devel@nongnu.org; Sat, 23 Sep 2017 06:35:13 -0400","from smtp1.ntua.gr ([2001:648:2000:de::183]:11257)\n\tby eggs.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dvhm8-0000cB-F4\n\tfor qemu-devel@nongnu.org; Sat, 23 Sep 2017 06:35:08 -0400","from mail.ntua.gr (carp0.noc.ntua.gr [147.102.222.60])\n\t(authenticated bits=0)\n\tby smtp1.ntua.gr (8.15.2/8.15.2) with ESMTPSA id v8NAY0AK050116\n\t(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=NOT); Sat, 23 Sep 2017 13:34:01 +0300 (EEST)\n\t(envelope-from el13635@mail.ntua.gr)"],"X-Authentication-Warning":"smtp1.ntua.gr: Host carp0.noc.ntua.gr\n\t[147.102.222.60] claimed to be mail.ntua.gr","Date":"Sat, 23 Sep 2017 13:33:59 +0300","From":"Manos Pitsidianakis ","To":"Pradeep Jagadeesh ","Message-ID":"<20170923103359.qzp5eyr54hjzzvie@postretch>","Mail-Followup-To":"Manos Pitsidianakis ,\n\tPradeep Jagadeesh ,\n\tPradeep Jagadeesh ,\n\teric blake , greg kurz ,\n\tqemu-devel@nongnu.org, jani kokkonen ,\n\talberto garcia ,\n\tMarkus Armbruster ","References":"<1505385610-35529-1-git-send-email-pradeep.jagadeesh@huawei.com>\n\t<1505385610-35529-2-git-send-email-pradeep.jagadeesh@huawei.com>\n\t<20170918162033.ee7fuarojpkwp5wr@postretch>\n\t<209dd16d-8d97-090d-35e9-b355c53c3448@huawei.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha512;\n\tprotocol=\"application/pgp-signature\"; boundary=\"u667n6m5fvgzudrn\"","Content-Disposition":"inline","In-Reply-To":"<209dd16d-8d97-090d-35e9-b355c53c3448@huawei.com>","User-Agent":"NeoMutt/20170609-57-1e93be (1.8.3)","X-detected-operating-system":"by eggs.gnu.org: Genre and OS details not\n\trecognized.","X-Received-From":"2001:648:2000:de::183","Subject":"Re: [Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"alberto garcia , Markus Armbruster ,\n\tgreg kurz ,\n\tPradeep Jagadeesh , \n\tqemu-devel@nongnu.org, jani kokkonen ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1774488,"web_url":"http://patchwork.ozlabs.org/comment/1774488/","msgid":"","list_archive_url":null,"date":"2017-09-25T07:47:37","subject":"Re: [Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code","submitter":{"id":69883,"url":"http://patchwork.ozlabs.org/api/people/69883/","name":"Pradeep Jagadeesh","email":"pradeep.jagadeesh@huawei.com"},"content":"On 9/23/2017 12:33 PM, Manos Pitsidianakis wrote:\n> On Fri, Sep 22, 2017 at 01:31:58PM +0200, Pradeep Jagadeesh wrote:\n>> On 9/18/2017 6:20 PM, Manos Pitsidianakis wrote:\n>>> On Thu, Sep 14, 2017 at 06:40:05AM -0400, Pradeep Jagadeesh wrote:\n>>>> This patch factors out the duplicate throttle code that was still\n>>>> present in block and fsdev devices.\n>>>>\n>>>> Signed-off-by: Pradeep Jagadeesh \n>>>> Reviewed-by: Alberto Garcia \n>>>> Reviewed-by: Greg Kurz \n>>>> Reviewed-by: Eric Blake \n>>>> ---\n>>>> blockdev.c | 44 +---------------------------------\n>>>> fsdev/qemu-fsdev-throttle.c | 44 ++--------------------------------\n>>>> include/qemu/throttle-options.h | 3 +++\n>>>> include/qemu/throttle.h | 4 ++--\n>>>> include/qemu/typedefs.h | 1 +\n>>>> util/throttle.c | 52\n>>>> +++++++++++++++++++++++++++++++++++++++++\n>>>> 6 files changed, 61 insertions(+), 87 deletions(-)\n>>>>\n>>>> diff --git a/blockdev.c b/blockdev.c\n>>>> index 56a6b24..9d33c25 100644\n>>>> --- a/blockdev.c\n>>>> +++ b/blockdev.c\n>>>> @@ -387,49 +387,7 @@ static void\n>>>> extract_common_blockdev_options(QemuOpts *opts, int *bdrv_flags,\n>>>> }\n>>>>\n>>>> if (throttle_cfg) {\n>>>> - throttle_config_init(throttle_cfg);\n>>>> - throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =\n>>>> - qemu_opt_get_number(opts, \"throttling.bps-total\", 0);\n>>>> - throttle_cfg->buckets[THROTTLE_BPS_READ].avg =\n>>>> - qemu_opt_get_number(opts, \"throttling.bps-read\", 0);\n>>>> - throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =\n>>>> - qemu_opt_get_number(opts, \"throttling.bps-write\", 0);\n>>>> - throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =\n>>>> - qemu_opt_get_number(opts, \"throttling.iops-total\", 0);\n>>>> - throttle_cfg->buckets[THROTTLE_OPS_READ].avg =\n>>>> - qemu_opt_get_number(opts, \"throttling.iops-read\", 0);\n>>>> - throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =\n>>>> - qemu_opt_get_number(opts, \"throttling.iops-write\", 0);\n>>>> -\n>>>> - throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =\n>>>> - qemu_opt_get_number(opts, \"throttling.bps-total-max\", 0);\n>>>> - throttle_cfg->buckets[THROTTLE_BPS_READ].max =\n>>>> - qemu_opt_get_number(opts, \"throttling.bps-read-max\", 0);\n>>>> - throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =\n>>>> - qemu_opt_get_number(opts, \"throttling.bps-write-max\", 0);\n>>>> - throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =\n>>>> - qemu_opt_get_number(opts, \"throttling.iops-total-max\", 0);\n>>>> - throttle_cfg->buckets[THROTTLE_OPS_READ].max =\n>>>> - qemu_opt_get_number(opts, \"throttling.iops-read-max\", 0);\n>>>> - throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =\n>>>> - qemu_opt_get_number(opts, \"throttling.iops-write-max\", 0);\n>>>> -\n>>>> - throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =\n>>>> - qemu_opt_get_number(opts,\n>>>> \"throttling.bps-total-max-length\", 1);\n>>>> - throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =\n>>>> - qemu_opt_get_number(opts,\n>>>> \"throttling.bps-read-max-length\", 1);\n>>>> - throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =\n>>>> - qemu_opt_get_number(opts,\n>>>> \"throttling.bps-write-max-length\", 1);\n>>>> - throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =\n>>>> - qemu_opt_get_number(opts,\n>>>> \"throttling.iops-total-max-length\", 1);\n>>>> - throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =\n>>>> - qemu_opt_get_number(opts,\n>>>> \"throttling.iops-read-max-length\", 1);\n>>>> - throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =\n>>>> - qemu_opt_get_number(opts,\n>>>> \"throttling.iops-write-max-length\", 1);\n>>>> -\n>>>> - throttle_cfg->op_size =\n>>>> - qemu_opt_get_number(opts, \"throttling.iops-size\", 0);\n>>>> -\n>>>> + throttle_parse_options(throttle_cfg, opts);\n>>>> if (!throttle_is_valid(throttle_cfg, errp)) {\n>>>> return;\n>>>> }\n>>>> diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c\n>>>> index 49eebb5..0e6fb86 100644\n>>>> --- a/fsdev/qemu-fsdev-throttle.c\n>>>> +++ b/fsdev/qemu-fsdev-throttle.c\n>>>> @@ -16,6 +16,7 @@\n>>>> #include \"qemu/error-report.h\"\n>>>> #include \"qemu-fsdev-throttle.h\"\n>>>> #include \"qemu/iov.h\"\n>>>> +#include \"qemu/throttle-options.h\"\n>>>>\n>>>> static void fsdev_throttle_read_timer_cb(void *opaque)\n>>>> {\n>>>> @@ -31,48 +32,7 @@ static void fsdev_throttle_write_timer_cb(void\n>>>> *opaque)\n>>>>\n>>>> void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error\n>>>> **errp)\n>>>> {\n>>>> - throttle_config_init(&fst->cfg);\n>>>> - fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =\n>>>> - qemu_opt_get_number(opts, \"throttling.bps-total\", 0);\n>>>> - fst->cfg.buckets[THROTTLE_BPS_READ].avg =\n>>>> - qemu_opt_get_number(opts, \"throttling.bps-read\", 0);\n>>>> - fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =\n>>>> - qemu_opt_get_number(opts, \"throttling.bps-write\", 0);\n>>>> - fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =\n>>>> - qemu_opt_get_number(opts, \"throttling.iops-total\", 0);\n>>>> - fst->cfg.buckets[THROTTLE_OPS_READ].avg =\n>>>> - qemu_opt_get_number(opts, \"throttling.iops-read\", 0);\n>>>> - fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =\n>>>> - qemu_opt_get_number(opts, \"throttling.iops-write\", 0);\n>>>> -\n>>>> - fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =\n>>>> - qemu_opt_get_number(opts, \"throttling.bps-total-max\", 0);\n>>>> - fst->cfg.buckets[THROTTLE_BPS_READ].max =\n>>>> - qemu_opt_get_number(opts, \"throttling.bps-read-max\", 0);\n>>>> - fst->cfg.buckets[THROTTLE_BPS_WRITE].max =\n>>>> - qemu_opt_get_number(opts, \"throttling.bps-write-max\", 0);\n>>>> - fst->cfg.buckets[THROTTLE_OPS_TOTAL].max =\n>>>> - qemu_opt_get_number(opts, \"throttling.iops-total-max\", 0);\n>>>> - fst->cfg.buckets[THROTTLE_OPS_READ].max =\n>>>> - qemu_opt_get_number(opts, \"throttling.iops-read-max\", 0);\n>>>> - fst->cfg.buckets[THROTTLE_OPS_WRITE].max =\n>>>> - qemu_opt_get_number(opts, \"throttling.iops-write-max\", 0);\n>>>> -\n>>>> - fst->cfg.buckets[THROTTLE_BPS_TOTAL].burst_length =\n>>>> - qemu_opt_get_number(opts,\n>>>> \"throttling.bps-total-max-length\", 1);\n>>>> - fst->cfg.buckets[THROTTLE_BPS_READ].burst_length =\n>>>> - qemu_opt_get_number(opts, \"throttling.bps-read-max-length\",\n>>>> 1);\n>>>> - fst->cfg.buckets[THROTTLE_BPS_WRITE].burst_length =\n>>>> - qemu_opt_get_number(opts,\n>>>> \"throttling.bps-write-max-length\", 1);\n>>>> - fst->cfg.buckets[THROTTLE_OPS_TOTAL].burst_length =\n>>>> - qemu_opt_get_number(opts, \"throttling.iops-total-max-length\",\n>>>> 1);\n>>>> - fst->cfg.buckets[THROTTLE_OPS_READ].burst_length =\n>>>> - qemu_opt_get_number(opts,\n>>>> \"throttling.iops-read-max-length\", 1);\n>>>> - fst->cfg.buckets[THROTTLE_OPS_WRITE].burst_length =\n>>>> - qemu_opt_get_number(opts, \"throttling.iops-write-max-length\",\n>>>> 1);\n>>>> - fst->cfg.op_size =\n>>>> - qemu_opt_get_number(opts, \"throttling.iops-size\", 0);\n>>>> -\n>>>> + throttle_parse_options(&fst->cfg, opts);\n>>>> throttle_is_valid(&fst->cfg, errp);\n>>>> }\n>>>>\n>>>> diff --git a/include/qemu/throttle-options.h\n>>>> b/include/qemu/throttle-options.h\n>>>> index 3528a8f..9709dcd 100644\n>>>> --- a/include/qemu/throttle-options.h\n>>>> +++ b/include/qemu/throttle-options.h\n>>>> @@ -9,6 +9,7 @@\n>>>> */\n>>>> #ifndef THROTTLE_OPTIONS_H\n>>>> #define THROTTLE_OPTIONS_H\n>>>> +#include \"typedefs.h\"\n>>>>\n>>>> #define QEMU_OPT_IOPS_TOTAL \"iops-total\"\n>>>> #define QEMU_OPT_IOPS_TOTAL_MAX \"iops-total-max\"\n>>>> @@ -111,4 +112,6 @@\n>>>> .help = \"when limiting by iops max size of an I/O in\n>>>> bytes\",\\\n>>>> }\n>>>>\n>>>> +void throttle_parse_options(ThrottleConfig *, QemuOpts *);\n>>>> +\n>>>> #endif\n>>>> diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h\n>>>> index 8c93237..b6ebc6d 100644\n>>>> --- a/include/qemu/throttle.h\n>>>> +++ b/include/qemu/throttle.h\n>>>> @@ -89,10 +89,10 @@ typedef struct LeakyBucket {\n>>>> * However it allows to keep the code clean and the bucket field is\n>>>> reset to\n>>>> * zero at the right time.\n>>>> */\n>>>> -typedef struct ThrottleConfig {\n>>>> +struct ThrottleConfig {\n>>>> LeakyBucket buckets[BUCKETS_COUNT]; /* leaky buckets */\n>>>> uint64_t op_size; /* size of an operation in bytes */\n>>>> -} ThrottleConfig;\n>>>> +};\n>>>>\n>>>> typedef struct ThrottleState {\n>>>> ThrottleConfig cfg; /* configuration */\n>>>> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h\n>>>> index 39bc835..90fe0f9 100644\n>>>> --- a/include/qemu/typedefs.h\n>>>> +++ b/include/qemu/typedefs.h\n>>>> @@ -100,6 +100,7 @@ typedef struct uWireSlave uWireSlave;\n>>>> typedef struct VirtIODevice VirtIODevice;\n>>>> typedef struct Visitor Visitor;\n>>>> typedef struct node_info NodeInfo;\n>>>> +typedef struct ThrottleConfig ThrottleConfig;\n>>>\n>>> Is this really needed? Just include include/qemu/throttle.h wherever you\n>>> need.\n>>>\n>>>> typedef void SaveStateHandler(QEMUFile *f, void *opaque);\n>>>> typedef int LoadStateHandler(QEMUFile *f, void *opaque, int\n>>>> version_id);\n>>>>\n>>>> diff --git a/util/throttle.c b/util/throttle.c\n>>>> index 06bf916..9ef28c4 100644\n>>>> --- a/util/throttle.c\n>>>> +++ b/util/throttle.c\n>>>> @@ -27,6 +27,7 @@\n>>>> #include \"qemu/throttle.h\"\n>>>> #include \"qemu/timer.h\"\n>>>> #include \"block/aio.h\"\n>>>> +#include \"qemu/throttle-options.h\"\n>>>>\n>>>> /* This function make a bucket leak\n>>>> *\n>>>> @@ -635,3 +636,54 @@ void throttle_config_to_limits(ThrottleConfig\n>>>> *cfg, ThrottleLimits *var)\n>>>> var->has_iops_write_max_length = true;\n>>>> var->has_iops_size = true;\n>>>> }\n>>>> +\n>>>> +/* parse the throttle options\n>>>> + *\n>>>> + * @opts: qemu options\n>>>> + * @throttle_cfg: throttle configuration\n>>>> + */\n>>>> +void throttle_parse_options(ThrottleConfig *throttle_cfg, QemuOpts\n>>>> *opts)\n>>>> +{\n>>>> + throttle_config_init(throttle_cfg);\n>>>> + throttle_cfg->buckets[THROTTLE_BPS_TOTAL].avg =\n>>>> + qemu_opt_get_number(opts, \"throttling.bps-total\", 0);\n>>>> + throttle_cfg->buckets[THROTTLE_BPS_READ].avg =\n>>>> + qemu_opt_get_number(opts, \"throttling.bps-read\", 0);\n>>>> + throttle_cfg->buckets[THROTTLE_BPS_WRITE].avg =\n>>>> + qemu_opt_get_number(opts, \"throttling.bps-write\", 0);\n>>>> + throttle_cfg->buckets[THROTTLE_OPS_TOTAL].avg =\n>>>> + qemu_opt_get_number(opts, \"throttling.iops-total\", 0);\n>>>> + throttle_cfg->buckets[THROTTLE_OPS_READ].avg =\n>>>> + qemu_opt_get_number(opts, \"throttling.iops-read\", 0);\n>>>> + throttle_cfg->buckets[THROTTLE_OPS_WRITE].avg =\n>>>> + qemu_opt_get_number(opts, \"throttling.iops-write\", 0);\n>>>> +\n>>>> + throttle_cfg->buckets[THROTTLE_BPS_TOTAL].max =\n>>>> + qemu_opt_get_number(opts, \"throttling.bps-total-max\", 0);\n>>>> + throttle_cfg->buckets[THROTTLE_BPS_READ].max =\n>>>> + qemu_opt_get_number(opts, \"throttling.bps-read-max\", 0);\n>>>> + throttle_cfg->buckets[THROTTLE_BPS_WRITE].max =\n>>>> + qemu_opt_get_number(opts, \"throttling.bps-write-max\", 0);\n>>>> + throttle_cfg->buckets[THROTTLE_OPS_TOTAL].max =\n>>>> + qemu_opt_get_number(opts, \"throttling.iops-total-max\", 0);\n>>>> + throttle_cfg->buckets[THROTTLE_OPS_READ].max =\n>>>> + qemu_opt_get_number(opts, \"throttling.iops-read-max\", 0);\n>>>> + throttle_cfg->buckets[THROTTLE_OPS_WRITE].max =\n>>>> + qemu_opt_get_number(opts, \"throttling.iops-write-max\", 0);\n>>>> +\n>>>> + throttle_cfg->buckets[THROTTLE_BPS_TOTAL].burst_length =\n>>>> + qemu_opt_get_number(opts,\n>>>> \"throttling.bps-total-max-length\", 1);\n>>>> + throttle_cfg->buckets[THROTTLE_BPS_READ].burst_length =\n>>>> + qemu_opt_get_number(opts, \"throttling.bps-read-max-length\",\n>>>> 1);\n>>>> + throttle_cfg->buckets[THROTTLE_BPS_WRITE].burst_length =\n>>>> + qemu_opt_get_number(opts,\n>>>> \"throttling.bps-write-max-length\", 1);\n>>>> + throttle_cfg->buckets[THROTTLE_OPS_TOTAL].burst_length =\n>>>> + qemu_opt_get_number(opts, \"throttling.iops-total-max-length\",\n>>>> 1);\n>>>> + throttle_cfg->buckets[THROTTLE_OPS_READ].burst_length =\n>>>> + qemu_opt_get_number(opts,\n>>>> \"throttling.iops-read-max-length\", 1);\n>>>> + throttle_cfg->buckets[THROTTLE_OPS_WRITE].burst_length =\n>>>> + qemu_opt_get_number(opts, \"throttling.iops-write-max-length\",\n>>>> 1);\n>>>> +\n>>>> + throttle_cfg->op_size =\n>>>> + qemu_opt_get_number(opts, \"throttling.iops-size\", 0);\n>>>> +}\n>>>\n>>> You should reuse the #define's in include/qemu/throttle-options.h\n>>> See throttle_extract_options() in this patch:\n>>> http://patchwork.ozlabs.org/patch/803919/ Disregard the UINT_MAX checks,\n>>> unsigned ints in LeakyBucket were replaced by uint64_t.\n>>>\n>> I can not use the #define's because I use throttling.*.\n>> In my last patch also we wanted to have it like that to align with the\n>> block device throttle options.\n>> If you see in blockdev.c, still there exists throttling.*\n>>\n>\n> You can use them.\n>\n> qemu_opt_get_number(opts, \"throttling.iops-size\", 0) becomes:\n> qemu_opt_get_number(opts, THROTTLE_OPT_PREFIX QEMU_OPT_IOPS_SIZE, 0)\n> This is what I did in the patch I linked, except for redefining\n> THROTTLE_OPT_PREFIX to \"limits.\". I plan to send some patches for\n> blockdev.c code when the old legacy interface is replaced.\nOK, I can do sendout a patch for blockdev.c also.\n\nRegards,\nPradeep","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3y0x7w52xbz9tXD\n\tfor ;\n\tMon, 25 Sep 2017 17:50:03 +1000 (AEST)","from localhost ([::1]:41141 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dwO9R-0001x8-09\n\tfor incoming@patchwork.ozlabs.org; Mon, 25 Sep 2017 03:50:01 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:39656)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dwO8z-0001wz-3y\n\tfor qemu-devel@nongnu.org; Mon, 25 Sep 2017 03:49:35 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dwO8v-0002bs-3Q\n\tfor qemu-devel@nongnu.org; Mon, 25 Sep 2017 03:49:33 -0400","from lhrrgout.huawei.com ([194.213.3.17]:24603)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_ARCFOUR_SHA1:16) (Exim 4.71)\n\t(envelope-from ) id 1dwO8u-0002ZC-Jy\n\tfor qemu-devel@nongnu.org; Mon, 25 Sep 2017 03:49:29 -0400","from 172.18.7.190 (EHLO LHREML712-CAH.china.huawei.com)\n\t([172.18.7.190])\n\tby lhrrg02-dlp.huawei.com (MOS 4.3.7-GA FastPath queued)\n\twith ESMTP id DPG07798; Mon, 25 Sep 2017 07:47:51 +0000 (GMT)","from [127.0.0.1] (10.210.171.45) by LHREML712-CAH.china.huawei.com\n\t(10.201.108.35) with Microsoft SMTP Server id 14.3.301.0;\n\tMon, 25 Sep 2017 08:47:43 +0100"],"To":"Manos Pitsidianakis , Pradeep Jagadeesh\n\t, eric blake , greg kurz\n\t, , jani kokkonen\n\t, alberto garcia , \"Markus\n\tArmbruster\" ","References":"<1505385610-35529-1-git-send-email-pradeep.jagadeesh@huawei.com>\n\t<1505385610-35529-2-git-send-email-pradeep.jagadeesh@huawei.com>\n\t<20170918162033.ee7fuarojpkwp5wr@postretch>\n\t<209dd16d-8d97-090d-35e9-b355c53c3448@huawei.com>\n\t<20170923103359.qzp5eyr54hjzzvie@postretch>","From":"Pradeep Jagadeesh ","Message-ID":"","Date":"Mon, 25 Sep 2017 09:47:37 +0200","User-Agent":"Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101\n\tThunderbird/45.8.0","MIME-Version":"1.0","In-Reply-To":"<20170923103359.qzp5eyr54hjzzvie@postretch>","Content-Type":"text/plain; charset=\"utf-8\"; format=flowed","Content-Transfer-Encoding":"7bit","X-Originating-IP":"[10.210.171.45]","X-CFilter-Loop":"Reflected","X-Mirapoint-Virus-RAPID-Raw":"score=unknown(0),\n\trefid=str=0001.0A090201.59C8B4A8.0084, ss=1, re=0.000, recu=0.000,\n\treip=0.000, cl=1, cld=1, fgs=0, ip=0.0.0.0,\n\tso=2013-06-18 04:22:30, dmn=2013-03-21 17:37:32","X-Mirapoint-Loop-Id":"2bb84c5c4b9ee514ec198c646157bbed","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.4.x-2.6.x [generic]\n\t[fuzzy]","X-Received-From":"194.213.3.17","Subject":"Re: [Qemu-devel] [PATCH v11 1/6] throttle: factor out duplicate code","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]