[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 5/6] fsdev: hmp interface for throttling
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH v6 5/6] fsdev: hmp interface for throttling |
Date: |
Fri, 30 Jun 2017 10:33:02 +0100 |
User-agent: |
Mutt/1.8.3 (2017-05-23) |
* Pradeep Jagadeesh (address@hidden) wrote:
> This patch introduces hmp interfaces for the fsdev
> devices.
>
> Signed-off-by: Pradeep Jagadeesh <address@hidden>
> ---
> hmp-commands-info.hx | 18 ++++++++++++++
> hmp-commands.hx | 19 +++++++++++++++
> hmp.c | 66
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
> hmp.h | 4 ++++
> 4 files changed, 107 insertions(+)
>
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index ae16901..f23b627 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -84,6 +84,24 @@ STEXI
> Show block device statistics.
> ETEXI
>
> +#if defined(CONFIG_VIRTFS)
> +
> + {
> + .name = "query-fsdev-iothrottle",
This will end up as:
info query-fsdev-iothrottle
so the query- is unneeded since it's already info.
> + .args_type = "",
> + .params = "",
> + .help = "show fsdev device throttle information",
> + .cmd = hmp_fsdev_get_io_throttle,
> + },
> +
> +#endif
> +
> +STEXI
> address@hidden info fsdev throttle
I think that's supposed to match the .name - i.e.
@item info query-fsdev-iothrottle
(or whatever it will become)
> address@hidden fsdevthrottleinfo
again I think that's supposed to match the .name - see the other entries
in that file.
> +Show fsdev device throttleinfo.
And we may as well make that text match the .help text.
> +ETEXI
> +
> {
> .name = "block-jobs",
> .args_type = "",
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index e763606..c60fd7e 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1662,6 +1662,25 @@ STEXI
> Change I/O throttle limits for a block drive to @var{bps} @var{bps_rd}
> @var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
> ETEXI
>
> +#if defined(CONFIG_VIRTFS)
> +
> + {
> + .name = "fsdev-set-io-throttle",
> + .args_type =
> "device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l",
> + .params = "device bps bps_rd bps_wr iops iops_rd iops_wr",
> + .help = "change I/O throttle limits for a fs devices",
> + .cmd = hmp_fsdev_set_io_throttle,
> + },
> +
> +#endif
> +
> +STEXI
> address@hidden fsdev_set_io_throttle @var{device} @var{bps} @var{bps_rd}
> @var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
> address@hidden fsdev_set_io_throttle
Again I think the @item and @findex have to match the .name - so I think
make the .name use _'s rather than -'s for HMP.
> +Change I/O throttle limits for a fs devices to @var{bps} @var{bps_rd}
> @var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
> +ETEXI
> +
> +
> {
> .name = "set_password",
> .args_type = "protocol:s,password:s,connected:s?",
> diff --git a/hmp.c b/hmp.c
> index 220d301..b1c698b 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1776,6 +1776,72 @@ void hmp_block_set_io_throttle(Monitor *mon, const
> QDict *qdict)
> hmp_handle_error(mon, &err);
> }
>
> +#ifdef CONFIG_VIRTFS
> +
> +void hmp_fsdev_set_io_throttle(Monitor *mon, const QDict *qdict)
> +{
> + Error *err = NULL;
> + IOThrottle throttle;
> +
> + hmp_initialize_io_throttle(&throttle, qdict);
> + qmp_fsdev_set_io_throttle(&throttle, &err);
> + hmp_handle_error(mon, &err);
> +}
> +
> +static void print_fsdev_throttle_config(Monitor *mon, IOThrottle *fscfg,
> + Error *err)
> +{
> + if (fscfg->bps || fscfg->bps_rd || fscfg->bps_wr ||
> + fscfg->iops || fscfg->iops_rd || fscfg->iops_wr)
> + {
> + monitor_printf(mon, "%s", fscfg->id);
> + monitor_printf(mon, " I/O throttling:"
> + " bps=%" PRId64
> + " bps_rd=%" PRId64 " bps_wr=%" PRId64
> + " bps_max=%" PRId64
> + " bps_rd_max=%" PRId64
> + " bps_wr_max=%" PRId64
> + " iops=%" PRId64 " iops_rd=%" PRId64
> + " iops_wr=%" PRId64
> + " iops_max=%" PRId64
> + " iops_rd_max=%" PRId64
> + " iops_wr_max=%" PRId64
> + " iops_size=%" PRId64,
> + fscfg->bps,
> + fscfg->bps_rd,
> + fscfg->bps_wr,
> + fscfg->bps_max,
> + fscfg->bps_rd_max,
> + fscfg->bps_wr_max,
> + fscfg->iops,
> + fscfg->iops_rd,
> + fscfg->iops_wr,
> + fscfg->iops_max,
> + fscfg->iops_rd_max,
> + fscfg->iops_wr_max,
> + fscfg->iops_size);
> + }
> + hmp_handle_error(mon, &err);
You've not got anything that can generate an error here have you?
> +}
> +
> +void hmp_fsdev_get_io_throttle(Monitor *mon, const QDict *qdict)
> +{
> + Error *err = NULL;
> + IOThrottleList *fs9p_list, *info;
> + fs9p_list = qmp_query_fsdev_io_throttle(&err);
> +
> + for (info = fs9p_list; info; info = info->next) {
> + if (info != fs9p_list) {
> + monitor_printf(mon, "\n");
> + }
> + print_fsdev_throttle_config(mon, info->value, err);
Since print_fsdev_throttle_config has an if() around it's print,
doesn't that mean you can end up with a few extra \n's ?
> + qapi_free_IOThrottle(info->value);
> + }
> + qapi_free_IOThrottleList(fs9p_list);
> +}
> +
> +#endif
> +
> void hmp_block_stream(Monitor *mon, const QDict *qdict)
> {
> Error *error = NULL;
> diff --git a/hmp.h b/hmp.h
> index d8b94ce..05e72f2 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -81,6 +81,10 @@ void hmp_set_password(Monitor *mon, const QDict *qdict);
> void hmp_expire_password(Monitor *mon, const QDict *qdict);
> void hmp_eject(Monitor *mon, const QDict *qdict);
> void hmp_change(Monitor *mon, const QDict *qdict);
> +#ifdef CONFIG_VIRTFS
> +void hmp_fsdev_set_io_throttle(Monitor *mon, const QDict *qdict);
> +void hmp_fsdev_get_io_throttle(Monitor *mon, const QDict *qdict);
> +#endif
> void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict);
> void hmp_block_stream(Monitor *mon, const QDict *qdict);
> void hmp_block_job_set_speed(Monitor *mon, const QDict *qdict);
> --
> 1.8.3.1
>
Dave
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
- [Qemu-devel] [PATCH v6 0/6] fsdev: qmp interface for io throttling, Pradeep Jagadeesh, 2017/06/29
- [Qemu-devel] [PATCH v6 2/6] qmp: Create IOThrottle structure, Pradeep Jagadeesh, 2017/06/29
- [Qemu-devel] [PATCH v6 1/6] throttle: factor out duplicate code, Pradeep Jagadeesh, 2017/06/29
- [Qemu-devel] [PATCH v6 4/6] hmp: create a throttle initialization function, Pradeep Jagadeesh, 2017/06/29
- [Qemu-devel] [PATCH v6 5/6] fsdev: hmp interface for throttling, Pradeep Jagadeesh, 2017/06/29
- Re: [Qemu-devel] [PATCH v6 5/6] fsdev: hmp interface for throttling,
Dr. David Alan Gilbert <=
- [Qemu-devel] [PATCH 3/6] throttle: move out function to reuse the code, Pradeep Jagadeesh, 2017/06/29
- [Qemu-devel] [PATCH v6 6/6] fsdev: QMP interface for throttling, Pradeep Jagadeesh, 2017/06/29