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
