Re: [Qemu-devel] [PATCH 10/12] qga: add --dump-conf option

[Michael Roth]

Quoting Marc-André Lureau (2015-07-01 06:47:45)
> This new option allows to review the agent configuration,
> and ease the task of writing a configuration file.
> 
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>  qga/main.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/qga/main.c b/qga/main.c
> index bd87050..f6dbb3e 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -215,6 +215,7 @@ static void usage(const char *cmd)
>  #endif
>  "  -b, --blacklist   comma-separated list of RPCs to disable (no spaces, 
> \"?\"\n"
>  "                    to list available RPCs)\n"
> +"  -D, --dump-conf   dump the configuration and exit\n"
>  "  -h, --help        display this help and exit\n"
>  "\n"
>  "Report bugs to <address@hidden>\n"
> @@ -904,6 +905,21 @@ static void ga_print_cmd(QmpCommand *cmd, void *opaque)
>      printf("%s\n", qmp_command_name(cmd));
>  }
> 
> +static gchar *list_join(GList *list, const gchar separator)
> +{
> +    GString *str = g_string_new("");
> +
> +    while (list) {
> +        str = g_string_append(str, (gchar *)list->data);
> +        list = g_list_next(list);
> +        if (list) {
> +            str = g_string_append_c(str, separator);
> +        }
> +    }
> +
> +    return g_string_free(str, FALSE);
> +}
> +
>  static GList *split_list(gchar *str, const gchar separator)
>  {
>      GList *list = NULL;
> @@ -936,9 +952,28 @@ static char *state_dir;
>  static const char *service;
>  #endif
>  static GList *blacklist;
> -static int daemonize;
> +static int daemonize, dumpconf;
>  static GLogLevelFlags log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL;
> 
> +static void dump_config(void)
> +{
> +    gchar *bl = list_join(blacklist, ',');
> +
> +    printf("[general]\n");
> +    printf("daemonize = %d\n", daemonize);
> +    printf("pidfile = %s\n", pid_filepath);
> +    if (log_filepath) {
> +        printf("logfile = %s\n", log_filepath);
> +    }
> +    printf("verbose = %d\n", log_level == G_LOG_LEVEL_MASK);
> +    printf("method = %s\n", method);
> +    printf("path = %s\n", device_path);
> +    printf("statedir = %s\n", state_dir);
> +    printf("blacklist = %s\n", bl);

I think we're missing fsfreeze_hook option here.

To me it seems cleaner to actually create the GKeyFile from current
options, then let GLib do all the work of generation a config file
we can spit out (g_key_file_to_data() should do it i think).

That, paired with the idea of having a GAConfig structure to
encapulate all the config options, might warrant restructuring
things a bit so that we have a
gkeyfile_to_gaconfig()/gkeyfile_from_gaconfig() pair to use for
reading/dumping configs while keeping all the options in an
easily trackable place.

> +
> +    g_free(bl);
> +}
> +
>  static void option_parse(int argc, char **argv)
>  {
>      const char *sopt = "hVvdm:p:l:f:F::b:s:t:D";
> @@ -946,6 +981,7 @@ static void option_parse(int argc, char **argv)
>      const struct option lopt[] = {
>          { "help", 0, NULL, 'h' },
>          { "version", 0, NULL, 'V' },
> +        { "dump-conf", 0, NULL, 'D' },
>          { "logfile", 1, NULL, 'l' },
>          { "pidfile", 1, NULL, 'f' },
>  #ifdef CONFIG_FSFREEZE
> @@ -1031,6 +1067,9 @@ static void option_parse(int argc, char **argv)
>              }
>              break;
>  #endif
> +        case 'D':
> +            dumpconf = 1;
> +            break;
>          case 'h':
>              usage(argv[0]);
>              exit(EXIT_SUCCESS);
> @@ -1205,6 +1244,11 @@ int main(int argc, char **argv)
>          }
>      }
> 
> +    if (dumpconf) {
> +        dump_config();
> +        goto end;
> +    }
> +
>      s->log_level = log_level;
>      s->log_file = stderr;
>  #ifdef CONFIG_FSFREEZE
> -- 
> 2.4.3
>