Uh, I totally forgot about this series. My apologies...
Eduardo Habkost <ehabk...@redhat.com> writes:
> Change qemu_config_parse() to return the number of config groups
> in success and -EINVAL on error. This will allow callers of
> qemu_config_parse() to check if something was really loaded from
> the config file.
>
> All existing callers of qemu_config_parse() and
> qemu_read_config_file() only check if the return value was
> negative, so the change shouldn't affect them.
Two of them:
* read_config() maps negative value to -EINVAL. Callers:
- blkdebug_open() passes it on. As a .bdrv_file_open() method, it's
supposed to return -errno on failure. Good.
* qemu_read_config_file() maps non-zero value to -EINVAL. Callers:
- qemu_read_default_config_file() maps -EINVAL to zero. WTF?
- main() passes sterror(EINVAL) to error_report(). Good.
Also: qemu_config_parse() reports errors with error_report(). Let's
have another look at its callers:
* read_config() has an Error ** parameter. Bad. Care to convert the
sucker to Error?
* qemu_read_config_file() doesn't report errors. Callers:
- qemu_read_default_config_file() doesn't report errors. Its called
by main(), and ...
- main() reports with error_report(). Good.
>
> Signed-off-by: Eduardo Habkost <ehabk...@redhat.com>
> ---
> util/qemu-config.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/util/qemu-config.c b/util/qemu-config.c
> index 5527100a01..560c201bd3 100644
> --- a/util/qemu-config.c
> +++ b/util/qemu-config.c
> @@ -379,6 +379,7 @@ void qemu_config_write(FILE *fp)
> }
> }
>
> +/* Returns number of config groups on success, -errno on error */
> int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname)
> {
> char line[1024], group[64], id[64], arg[64], value[1024];
> @@ -386,7 +387,8 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists,
> const char *fname)
> QemuOptsList *list = NULL;
> Error *local_err = NULL;
> QemuOpts *opts = NULL;
> - int res = -1, lno = 0;
> + int res = -EINVAL, lno = 0;
> + int count = 0;
>
> loc_push_none(&loc);
> while (fgets(line, sizeof(line), fp) != NULL) {
> @@ -407,6 +409,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists,
> const char *fname)
> goto out;
> }
> opts = qemu_opts_create(list, id, 1, NULL);
> + count++;
> continue;
> }
> if (sscanf(line, "[%63[^]]]", group) == 1) {
> @@ -417,6 +420,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists,
> const char *fname)
> goto out;
> }
> opts = qemu_opts_create(list, NULL, 0, &error_abort);
> + count++;
> continue;
> }
> value[0] = '\0';
> @@ -441,7 +445,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists,
> const char *fname)
> error_report("error reading file");
> goto out;
> }
> - res = 0;
> + res = count;
> out:
> loc_pop(&loc);
> return res;
> @@ -458,12 +462,7 @@ int qemu_read_config_file(const char *filename)
>
> ret = qemu_config_parse(f, vm_config_groups, filename);
> fclose(f);
> -
> - if (ret == 0) {
> - return 0;
> - } else {
> - return -EINVAL;
> - }
> + return ret;
> }
>
> static void config_parse_qdict_section(QDict *options, QemuOptsList *opts,
I think this mapping to -EINVAL is also superfluous now:
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 6117ce5..fbefa9e 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -252,7 +252,6 @@ static int read_config(BDRVBlkdebugState *s, const char
*filename,
ret = qemu_config_parse(f, config_groups, filename);
if (ret < 0) {
error_setg(errp, "Could not parse blkdebug config file");
- ret = -EINVAL;
goto fail;
}
}
