This patch introduces hmp interfaces for the fsdev
devices.
Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
---
hmp-commands-info.hx | 18 +++++++++++++++
hmp-commands.hx | 19 ++++++++++++++++
hmp.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++
hmp.h | 4 ++++
4 files changed, 103 insertions(+)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index d9df238..07ed338 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 = "fsdev_iothrottle",
+ .args_type = "",
+ .params = "",
+ .help = "show fsdev iothrottle information",
+ .cmd = hmp_info_fsdev_iothrottle,
+ },
+
+#endif
+
+STEXI
+@item info fsdev_iothrottle
+@findex fsdev_iothrottle
+Show fsdev device throttle info.
+ETEXI
+
{
.name = "block-jobs",
.args_type = "",
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 1941e19..aef9f79 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1680,6 +1680,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
+@item fsdev_set_io_throttle @var{device} @var{bps} @var{bps_rd} @var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
+@findex fsdev_set_io_throttle
+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 2dbfb80..712c6a3 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1783,6 +1783,68 @@ 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 = {
+ .has_id = true,
+ .id = (char *) qdict_get_str(qdict, "device"),
+ };
+
+ 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)
+{
+ 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
+ "\n",
+ 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);
+}
+
+void hmp_info_fsdev_iothrottle(Monitor *mon, const QDict *qdict)
+{
+ Error *err = NULL;
+ IOThrottleList *fsdev_list, *info;
+ fsdev_list = qmp_query_fsdev_io_throttle(&err);
+
+ for (info = fsdev_list; info; info = info->next) {
+ print_fsdev_throttle_config(mon, info->value, err);
+ }
+ qapi_free_IOThrottleList(fsdev_list);
+}
+
+#endif
+
void hmp_block_stream(Monitor *mon, const QDict *qdict)
{
Error *error = NULL;
diff --git a/hmp.h b/hmp.h
index 1ff4552..d700d7d 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_info_fsdev_iothrottle(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
On Tue 29 Aug 2017 04:23:06 PM CEST, Pradeep Jagadeesh wrote:
> +static void print_fsdev_throttle_config(Monitor *mon, IOThrottle *fscfg,
> + Error *err)
> +{
> + 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
> + "\n",
> + 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);
> +}
> +
> +void hmp_info_fsdev_iothrottle(Monitor *mon, const QDict *qdict)
> +{
> + Error *err = NULL;
> + IOThrottleList *fsdev_list, *info;
> + fsdev_list = qmp_query_fsdev_io_throttle(&err);
> +
> + for (info = fsdev_list; info; info = info->next) {
> + print_fsdev_throttle_config(mon, info->value, err);
> + }
> + qapi_free_IOThrottleList(fsdev_list);
> +}
I don't think what you're doing with the Error here is right:
- You store the error returned by qmp_query_fsdev_io_throttle().
- You report the error for _every_ fsdev in the list. That doesn't
make much sense.
- Furthermore, if there's an error then fsdev_list should be empty,
so you're not reporting anything (and you're leaking the error).
- Even if the list was not empty, hmp_handle_error() frees the error
after reporting it. Therefore the second item in the list would
try to report an error that has already been freed.
Berto
On 8/30/2017 11:38 AM, Alberto Garcia wrote:
> On Tue 29 Aug 2017 04:23:06 PM CEST, Pradeep Jagadeesh wrote:
>
>> +static void print_fsdev_throttle_config(Monitor *mon, IOThrottle *fscfg,
>> + Error *err)
>> +{
>> + 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
>> + "\n",
>> + 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);
>> +}
>> +
>> +void hmp_info_fsdev_iothrottle(Monitor *mon, const QDict *qdict)
>> +{
>> + Error *err = NULL;
>> + IOThrottleList *fsdev_list, *info;
>> + fsdev_list = qmp_query_fsdev_io_throttle(&err);
>> +
>> + for (info = fsdev_list; info; info = info->next) {
>> + print_fsdev_throttle_config(mon, info->value, err);
>> + }
>> + qapi_free_IOThrottleList(fsdev_list);
>> +}
>
> I don't think what you're doing with the Error here is right:
>
> - You store the error returned by qmp_query_fsdev_io_throttle().
> - You report the error for _every_ fsdev in the list. That doesn't
> make much sense.
> - Furthermore, if there's an error then fsdev_list should be empty,
> so you're not reporting anything (and you're leaking the error).
> - Even if the list was not empty, hmp_handle_error() frees the error
> after reporting it. Therefore the second item in the list would
> try to report an error that has already been freed.
You mean something like below?
fsdev_list = qmp_query_fsdev_io_throttle(&err);
if (err) {
error_report_err(err);
return;
}
Regards,
Pradeep
>
> Berto
>
On Mon 04 Sep 2017 02:22:40 PM CEST, Pradeep Jagadeesh wrote:
>>> +void hmp_info_fsdev_iothrottle(Monitor *mon, const QDict *qdict)
>>> +{
>>> + Error *err = NULL;
>>> + IOThrottleList *fsdev_list, *info;
>>> + fsdev_list = qmp_query_fsdev_io_throttle(&err);
>>> +
>>> + for (info = fsdev_list; info; info = info->next) {
>>> + print_fsdev_throttle_config(mon, info->value, err);
>>> + }
>>> + qapi_free_IOThrottleList(fsdev_list);
>>> +}
>>
>> I don't think what you're doing with the Error here is right:
>>
>> - You store the error returned by qmp_query_fsdev_io_throttle().
>> - You report the error for _every_ fsdev in the list. That doesn't
>> make much sense.
>> - Furthermore, if there's an error then fsdev_list should be empty,
>> so you're not reporting anything (and you're leaking the error).
>> - Even if the list was not empty, hmp_handle_error() frees the error
>> after reporting it. Therefore the second item in the list would
>> try to report an error that has already been freed.
> You mean something like below?
>
> fsdev_list = qmp_query_fsdev_io_throttle(&err);
> if (err) {
> error_report_err(err);
> return;
> }
More or less, but there's hmp_handle_error() already, so you should use
it:
void hmp_info_fsdev_iothrottle(Monitor *mon, const QDict *qdict)
{
Error *err = NULL;
IOThrottleList *fsdev_list, *info;
fsdev_list = qmp_query_fsdev_io_throttle(&err);
for (info = fsdev_list; info; info = info->next) {
print_fsdev_throttle_config(mon, info->value);
}
qapi_free_IOThrottleList(fsdev_list);
hmp_handle_error(mon, &err);
}
Anyway I just checked that fsdev_get_io_throttle() can never return an
Error, so why don't you remove the Error parameter from there
altogether?
You still need it in qmp_query_fsdev_io_throttle() because that's part
of the API, and hmp_info_fsdev_iothrottle() should have the code to
handle it like the one I just pasted, but fsdev_get_io_throttle() does
not need it, or does it?
Berto
© 2016 - 2025 Red Hat, Inc.