On Mon 07 Nov 2016 04:45:55 PM CET, Pradeep Jagadeesh wrote:--- a/fsdev/qemu-fsdev-opts.c +++ b/fsdev/qemu-fsdev-opts.c @@ -37,8 +37,83 @@ static QemuOptsList qemu_fsdev_opts = { }, { .name = "sock_fd", .type = QEMU_OPT_NUMBER, + }, { + .name = "throttling.iops-total", + .type = QEMU_OPT_NUMBER, + .help = "limit total I/O operations per second", + },{Not very important, but since this version also needs to be corrected (as you'll see below) you can update this part to keep the sytle consisent: all other entries in this array have a space between the command and the opening brace ("}, {"). Most of your entries don't have that space ("},{")
OK
+void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error *err) +{ + fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg = + qemu_opt_get_number(opts, "throttling.bps-total", 0);/* ... */+ fst->cfg.op_size = + qemu_opt_get_number(opts, "throttling.iops-size", 0); + + if (!throttle_is_valid(&fst->cfg, &err)) { + return; + } +}That last 'if' is a bit odd :) but I won't object if you want to keep it. You can simply leave the throttle_is_valid() call and a comment that explains what it does ("Check that the config is valid and update errp otherwise").
I will better remove that. :)
+void fsdev_throttle_init(FsThrottle *fst) +{ + if (throttle_enabled(&fst->cfg)) { + throttle_init(&fst->ts);This block is indented with 8 whitespaces, not with 4 as it should be.
Hmm,I will fix it
--- a/hw/9pfs/9p-local.c +++ b/hw/9pfs/9p-local.c @@ -1209,6 +1209,7 @@ static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse) { const char *sec_model = qemu_opt_get(opts, "security_model"); const char *path = qemu_opt_get(opts, "path"); + Error *err = NULL; if (!sec_model) { error_report("Security model not specified, local fs needs security model"); @@ -1237,6 +1238,13 @@ static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse) error_report("fsdev: No path specified"); return -1; } + + fsdev_throttle_parse_opts(opts, &fse->fst, err); + if (err) { + error_reportf_err(err, "Throttle configuration is not valid: "); + return -1; + } +This is the important part that needs to be changed: 'err' here is NULL, so you're passing a NULL pointer to fsdev_throttle_parse_opts() and invalid throttling configurations are not being reported. You should pass &err instead, and fsdev_throttle_parse_opts() must receive Error **errp.
OK -Pradeep
Berto
