[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v8 5/6] fsdev: hmp interface for throttling
From: |
Alberto Garcia |
Subject: |
Re: [Qemu-devel] [PATCH v8 5/6] fsdev: hmp interface for throttling |
Date: |
Wed, 30 Aug 2017 11:38:43 +0200 |
User-agent: |
Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (i586-pc-linux-gnu) |
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
- Re: [Qemu-devel] [PATCH V8 3/6] throttle: move out function to reuse the code, (continued)
- [Qemu-devel] [PATCH V8 4/6] hmp: create a throttle initialization function for code reusability, Pradeep Jagadeesh, 2017/08/07
- [Qemu-devel] [PATCH V8 6/6] fsdev: hmp interface for throttling, Pradeep Jagadeesh, 2017/08/07
- [Qemu-devel] [PATCH V8 5/6] fsdev: QMP interface for throttling, Pradeep Jagadeesh, 2017/08/07
- [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling, Pradeep Jagadeesh, 2017/08/29
- [Qemu-devel] [PATCH v8 3/6] throttle: move out function to reuse the code, Pradeep Jagadeesh, 2017/08/29
- [Qemu-devel] [PATCH v8 5/6] fsdev: hmp interface for throttling, Pradeep Jagadeesh, 2017/08/29
- Re: [Qemu-devel] [PATCH v8 5/6] fsdev: hmp interface for throttling,
Alberto Garcia <=
- [Qemu-devel] [PATCH v8 6/6] fsdev: QMP interface for throttling, Pradeep Jagadeesh, 2017/08/29
- [Qemu-devel] [PATCH v8 1/6] throttle: factor out duplicate code, Pradeep Jagadeesh, 2017/08/29
- [Qemu-devel] [PATCH v8 2/6] qmp: Create IOThrottle structure, Pradeep Jagadeesh, 2017/08/29
- [Qemu-devel] [PATCH v8 4/6] hmp: create a throttle initialization function for code reusability, Pradeep Jagadeesh, 2017/08/29
- Re: [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling, Alberto Garcia, 2017/08/29
- Re: [Qemu-devel] [PATCH v8 0/6] fsdev: qmp interface for io throttling, Alberto Garcia, 2017/08/30