Hello, and sorry for being late reviewing these patches :)
On Tue 24 Jan 2017 10:24:07 AM CET, Pradeep Jagadeesh
<pradeepkiruv...@gmail.com> wrote:
> This patchset adds the io throttle support for the 9p-local driver.
> For now this functionality can be used only through qemu cli options.
> QMP interface and support to other 9p drivers need further extensions.
> To make it simple for other 9p drivers, the throttle code has been put
> in separate files.
Compared to the previous patch that I reviewed, you removed the
fsdev_throttle_cleanup() call from v9fs_device_unrealize_common(). Why
did you do that?
I miss the list of changes in the cover letter.
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -3520,6 +3520,10 @@ int v9fs_device_realize_common(V9fsState *s, Error
> **errp)
> error_setg(errp, "share path %s is not a directory", fse->path);
> goto out;
> }
> +
> + s->ctx.fst = &fse->fst;
> + fsdev_throttle_init(s->ctx.fst);
> +
> v9fs_path_free(&path);
>
> rc = 0;
> @@ -3528,6 +3532,7 @@ out:
> if (s->ops && s->ops->cleanup && s->ctx.private) {
> s->ops->cleanup(&s->ctx);
> }
> + fsdev_throttle_cleanup(s->ctx.fst);
> g_free(s->tag);
> g_free(s->ctx.fs_root);
> v9fs_path_free(&path);
You also added the fsdev_throttle_cleanup() here, but look at the code:
fsdev_throttle_init(s->ctx.fst);
v9fs_path_free(&path);
rc = 0;
out:
if (rc) {
/* ... */
fsdev_throttle_cleanup(s->ctx.fst);
/* ... */
}
If rc != 0 you don't need to do the clean up, because there was never an
initialization.
Berto
