On 2/1/2017 4:08 PM, Alberto Garcia wrote:
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 actually missed while merging the code to newer version. Instead of
putting inside the unrealize it went some other function.
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.
Fixed it, it was because of merging issue as I answered above.
-Pradeep
Berto
