Re: [Qemu-devel] [PATCH 4/4] cpu model corrections/updates: add verbose config file handling

[Blue Swirl]

On Tue, Sep 7, 2010 at 12:31 PM, john cooper <address@hidden> wrote:
> Failure by qemu to open a default config file isn't cause to
> error exit -- it just quietly continues on.   After puzzling
> issues with otherwise opaque config file locations and
> startup handling numerous times, some help from qemu seemed
> justified.

Maybe there should be an error exit if the user specifies a config
file but there are problems with it?

> In the case of a "?" pseudo filename arg to -readconfig,
> verbose open of all config files will be enabled.  Normal
> handling of config files is otherwise unaffected by this
> option.

I think '?' is not very good name. Could we add flags to -readconfig,
like -readconfig verbose,nodefaultconfig,file='', to match other
options' syntax?

> Signed-off-by: john cooper <address@hidden>
> ---
>
> diff --git a/qemu-config.c b/qemu-config.c
> index 1efdbec..805fcc6 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -534,21 +534,31 @@ out:
>     return res;
>  }
>
> -int qemu_read_config_file(const char *filename)
> +/* attempt to open and parse config file, report problems if vflag
> + */
> +int qemu_read_config_file(const char *filename, int vflag)
>  {
>     FILE *f = fopen(filename, "r");
> -    int ret;
> +    int rv = 0;
> +    const char *err;
>
>     if (f == NULL) {
> -        return -errno;
> +        rv = -errno;
> +        err = "open";
>     }
> -
> -    ret = qemu_config_parse(f, vm_config_groups, filename);
> -    fclose(f);
> -
> -    if (ret == 0) {
> -        return 0;
> -    } else {
> -        return -EINVAL;
> +    else if (qemu_config_parse(f, vm_config_groups, filename) != 0) {
> +        rv = -EINVAL;
> +        err = "parse";
> +    }
> +    else if (vflag) {
> +        fprintf(stderr, "parsed config file %s\n", filename);
> +    }
> +    if (f) {
> +        fclose(f);
> +    }
> +    if (rv && vflag) {
> +        fprintf(stderr, "can't %s config file %s: %s\n",
> +                err, filename, strerror(-rv));

I'd just duplicate this for both open and parse.

>     }
> +    return rv;
>  }
> diff --git a/qemu-config.h b/qemu-config.h
> index 533a049..897fbce 100644
> --- a/qemu-config.h
> +++ b/qemu-config.h
> @@ -13,6 +13,6 @@ void qemu_add_globals(void);
>  void qemu_config_write(FILE *fp);
>  int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname);
>
> -int qemu_read_config_file(const char *filename);
> +int qemu_read_config_file(const char *filename, int vflag);
>
>  #endif /* QEMU_CONFIG_H */
> diff --git a/vl.c b/vl.c
> index fd491ba..d192b51 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1822,6 +1822,7 @@ int main(int argc, char **argv, char **envp)
>     const char *incoming = NULL;
>     int show_vnc_port = 0;
>     int defconfig = 1;
> +    int defconfig_verbose = 0;
>
>     atexit(qemu_run_exit_notifiers);
>     error_set_progname(argv[0]);
> @@ -1875,6 +1876,11 @@ int main(int argc, char **argv, char **envp)
>             case QEMU_OPTION_nodefconfig:
>                 defconfig=0;
>                 break;
> +            case QEMU_OPTION_readconfig:
> +                /* pseudo filename "?" enables verbose config file handling 
> */
> +                if (!strcmp(optarg, "?"))
> +                    defconfig_verbose = 1;
> +                break;
>             }
>         }
>     }
> @@ -1882,12 +1888,13 @@ int main(int argc, char **argv, char **envp)
>     if (defconfig) {
>         int ret;
>
> -        ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf");
> +        ret = qemu_read_config_file(CONFIG_QEMU_CONFDIR "/qemu.conf",
> +                                    defconfig_verbose);
>         if (ret < 0 && ret != -ENOENT) {
>             exit(1);
>         }
>
> -        ret = qemu_read_config_file(arch_config_name);
> +        ret = qemu_read_config_file(arch_config_name, defconfig_verbose);
>         if (ret < 0 && ret != -ENOENT) {
>             exit(1);
>         }
> @@ -2596,15 +2603,10 @@ int main(int argc, char **argv, char **envp)
>                 xen_mode = XEN_ATTACH;
>                 break;
>             case QEMU_OPTION_readconfig:
> -                {
> -                    int ret = qemu_read_config_file(optarg);
> -                    if (ret < 0) {
> -                        fprintf(stderr, "read config %s: %s\n", optarg,
> -                            strerror(-ret));
> +                if (strcmp(optarg, "?") &&
> +                    qemu_read_config_file(optarg, defconfig_verbose) < 0)
>                         exit(1);
> -                    }
> -                    break;
> -                }
> +                break;
>             case QEMU_OPTION_writeconfig:
>                 {
>                     FILE *fp;
> --
> address@hidden
>
>
>