diff mbox

[v4,11/13] qga: add an optional qemu-ga.conf system configuration

Message ID 1440632099-5169-12-git-send-email-marcandre.lureau@redhat.com
State New
Headers show

Commit Message

Marc-André Lureau Aug. 26, 2015, 11:34 p.m. UTC
From: Marc-André Lureau <marcandre.lureau@redhat.com>

Learn to configure the agent with a system configuration.

This may simplify command-line handling, especially when the blacklist
is long.

Among the other benefits, this may standardize the configuration of an
init service (instead of distro-specific init keys/files)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qga/main.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 79 insertions(+), 7 deletions(-)

Comments

Denis V. Lunev Aug. 27, 2015, 10:02 a.m. UTC | #1
On 08/27/2015 02:34 AM, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Learn to configure the agent with a system configuration.
>
> This may simplify command-line handling, especially when the blacklist
> is long.
>
> Among the other benefits, this may standardize the configuration of an
> init service (instead of distro-specific init keys/files)
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>   qga/main.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
>   1 file changed, 79 insertions(+), 7 deletions(-)
>
> diff --git a/qga/main.c b/qga/main.c
> index 710dd47..881a92b 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -56,6 +56,7 @@
>   #define QGA_FSFREEZE_HOOK_DEFAULT CONFIG_QEMU_CONFDIR "/fsfreeze-hook"
>   #endif
>   #define QGA_SENTINEL_BYTE 0xFF
> +#define QGA_CONF_DEFAULT CONFIG_QEMU_CONFDIR G_DIR_SEPARATOR_S "qemu-ga.conf"
>   
>   static struct {
>       const char *state_dir;
> @@ -931,14 +932,80 @@ typedef struct GAConfig {
>   #ifdef _WIN32
>       const char *service;
>   #endif
> +    gchar *bliststr; /* blacklist may point to this string */
>       GList *blacklist;
>       int daemonize;
>       GLogLevelFlags log_level;
>   } GAConfig;
>   
> -static GAConfig *config_parse(int argc, char **argv)
> +static void config_load(GAConfig *config)
> +{
> +    GError *gerr = NULL;
> +    GKeyFile *keyfile;
> +
> +    /* read system config */
> +    keyfile = g_key_file_new();
> +    if (!g_key_file_load_from_file(keyfile, QGA_CONF_DEFAULT, 0, &gerr)) {
> +        goto end;
> +    }
> +    if (g_key_file_has_key(keyfile, "general", "daemon", NULL)) {
> +        config->daemonize =
> +            g_key_file_get_boolean(keyfile, "general", "daemon", &gerr);
> +    }
> +    if (g_key_file_has_key(keyfile, "general", "method", NULL)) {
> +        config->method =
> +            g_key_file_get_string(keyfile, "general", "method", &gerr);
> +    }
> +    if (g_key_file_has_key(keyfile, "general", "path", NULL)) {
> +        config->channel_path =
> +            g_key_file_get_string(keyfile, "general", "path", &gerr);
> +    }
> +    if (g_key_file_has_key(keyfile, "general", "logfile", NULL)) {
> +        config->log_filepath =
> +            g_key_file_get_string(keyfile, "general", "logfile", &gerr);
> +    }
> +    if (g_key_file_has_key(keyfile, "general", "pidfile", NULL)) {
> +        config->pid_filepath =
> +            g_key_file_get_string(keyfile, "general", "pidfile", &gerr);
> +    }
> +#ifdef CONFIG_FSFREEZE
> +    if (g_key_file_has_key(keyfile, "general", "fsfreeze-hook", NULL)) {
> +        config->fsfreeze_hook =
> +            g_key_file_get_string(keyfile,
> +                                  "general", "fsfreeze-hook", &gerr);
> +    }
> +#endif
> +    if (g_key_file_has_key(keyfile, "general", "statedir", NULL)) {
> +        config->state_dir =
> +            g_key_file_get_string(keyfile, "general", "statedir", &gerr);
> +    }
> +    if (g_key_file_has_key(keyfile, "general", "verbose", NULL) &&
> +        g_key_file_get_boolean(keyfile, "general", "verbose", &gerr)) {
> +        /* enable all log levels */
> +        config->log_level = G_LOG_LEVEL_MASK;
> +    }
> +    if (g_key_file_has_key(keyfile, "general", "blacklist", NULL)) {
> +        config->bliststr =
> +            g_key_file_get_string(keyfile, "general", "blacklist", &gerr);
> +        config->blacklist = g_list_concat(config->blacklist,
> +                                          split_list(config->bliststr, ","));
> +    }
> +
> +end:
> +    if (keyfile) {
> +        g_key_file_free(keyfile);
> +    }
> +    if (gerr &&
> +        !(gerr->domain == G_FILE_ERROR && gerr->code == G_FILE_ERROR_NOENT)) {
> +        g_critical("error loading configuration from path: %s, %s",
> +                   QGA_CONF_DEFAULT, gerr->message);
> +        exit(EXIT_FAILURE);
> +    }
> +    g_clear_error(&gerr);
> +}
> +
> +static void config_parse(GAConfig *config, int argc, char **argv)
>   {
> -    GAConfig *config = g_new0(GAConfig, 1);
>       const char *sopt = "hVvdm:p:l:f:F::b:s:t:D";
>       int opt_ind = 0, ch;
>       const struct option lopt[] = {
> @@ -966,23 +1033,29 @@ static GAConfig *config_parse(int argc, char **argv)
>       while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
>           switch (ch) {
>           case 'm':
> +            g_free(config->method);
>               config->method = g_strdup(optarg);
>               break;
>           case 'p':
> +            g_free(config->channel_path);
>               config->channel_path = g_strdup(optarg);
>               break;
>           case 'l':
> +            g_free(config->log_filepath);
>               config->log_filepath = g_strdup(optarg);
>               break;
>           case 'f':
> +            g_free(config->pid_filepath);
>               config->pid_filepath = g_strdup(optarg);
>               break;
>   #ifdef CONFIG_FSFREEZE
>           case 'F':
> +            g_free(config->fsfreeze_hook);
>               config->fsfreeze_hook = g_strdup(optarg ?: QGA_FSFREEZE_HOOK_DEFAULT);
>               break;
>   #endif
>           case 't':
> +            g_free(config->state_dir);
>               config->state_dir = g_strdup(optarg);
>               break;
>           case 'v':
> @@ -1042,8 +1115,6 @@ static GAConfig *config_parse(int argc, char **argv)
>               exit(EXIT_FAILURE);
>           }
>       }
> -
> -    return config;
>   }
>   
>   static void config_free(GAConfig *config)
> @@ -1053,6 +1124,7 @@ static void config_free(GAConfig *config)
>       g_free(config->pid_filepath);
>       g_free(config->state_dir);
>       g_free(config->channel_path);
> +    g_free(config->bliststr);
>   #ifdef CONFIG_FSFREEZE
>       g_free(config->fsfreeze_hook);
>   #endif
> @@ -1195,13 +1267,13 @@ int main(int argc, char **argv)
>   {
>       int ret = EXIT_SUCCESS;
>       GAState *s = g_new0(GAState, 1);
> -    GAConfig *config;
> +    GAConfig *config = g_new0(GAConfig, 1);
>   
>       module_call_init(MODULE_INIT_QAPI);
>   
>       init_dfl_pathnames();
> -
> -    config = config_parse(argc, argv);
> +    config_load(config);
> +    config_parse(config, argc, argv);
>   
>       if (config->pid_filepath == NULL) {
>           config->pid_filepath = g_strdup(dfl_pathnames.pidfile);
Reviewed-by: Denis V. Lunev <den@openvz.org>

except same note as in patch 12 about key file.
diff mbox

Patch

diff --git a/qga/main.c b/qga/main.c
index 710dd47..881a92b 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -56,6 +56,7 @@ 
 #define QGA_FSFREEZE_HOOK_DEFAULT CONFIG_QEMU_CONFDIR "/fsfreeze-hook"
 #endif
 #define QGA_SENTINEL_BYTE 0xFF
+#define QGA_CONF_DEFAULT CONFIG_QEMU_CONFDIR G_DIR_SEPARATOR_S "qemu-ga.conf"
 
 static struct {
     const char *state_dir;
@@ -931,14 +932,80 @@  typedef struct GAConfig {
 #ifdef _WIN32
     const char *service;
 #endif
+    gchar *bliststr; /* blacklist may point to this string */
     GList *blacklist;
     int daemonize;
     GLogLevelFlags log_level;
 } GAConfig;
 
-static GAConfig *config_parse(int argc, char **argv)
+static void config_load(GAConfig *config)
+{
+    GError *gerr = NULL;
+    GKeyFile *keyfile;
+
+    /* read system config */
+    keyfile = g_key_file_new();
+    if (!g_key_file_load_from_file(keyfile, QGA_CONF_DEFAULT, 0, &gerr)) {
+        goto end;
+    }
+    if (g_key_file_has_key(keyfile, "general", "daemon", NULL)) {
+        config->daemonize =
+            g_key_file_get_boolean(keyfile, "general", "daemon", &gerr);
+    }
+    if (g_key_file_has_key(keyfile, "general", "method", NULL)) {
+        config->method =
+            g_key_file_get_string(keyfile, "general", "method", &gerr);
+    }
+    if (g_key_file_has_key(keyfile, "general", "path", NULL)) {
+        config->channel_path =
+            g_key_file_get_string(keyfile, "general", "path", &gerr);
+    }
+    if (g_key_file_has_key(keyfile, "general", "logfile", NULL)) {
+        config->log_filepath =
+            g_key_file_get_string(keyfile, "general", "logfile", &gerr);
+    }
+    if (g_key_file_has_key(keyfile, "general", "pidfile", NULL)) {
+        config->pid_filepath =
+            g_key_file_get_string(keyfile, "general", "pidfile", &gerr);
+    }
+#ifdef CONFIG_FSFREEZE
+    if (g_key_file_has_key(keyfile, "general", "fsfreeze-hook", NULL)) {
+        config->fsfreeze_hook =
+            g_key_file_get_string(keyfile,
+                                  "general", "fsfreeze-hook", &gerr);
+    }
+#endif
+    if (g_key_file_has_key(keyfile, "general", "statedir", NULL)) {
+        config->state_dir =
+            g_key_file_get_string(keyfile, "general", "statedir", &gerr);
+    }
+    if (g_key_file_has_key(keyfile, "general", "verbose", NULL) &&
+        g_key_file_get_boolean(keyfile, "general", "verbose", &gerr)) {
+        /* enable all log levels */
+        config->log_level = G_LOG_LEVEL_MASK;
+    }
+    if (g_key_file_has_key(keyfile, "general", "blacklist", NULL)) {
+        config->bliststr =
+            g_key_file_get_string(keyfile, "general", "blacklist", &gerr);
+        config->blacklist = g_list_concat(config->blacklist,
+                                          split_list(config->bliststr, ","));
+    }
+
+end:
+    if (keyfile) {
+        g_key_file_free(keyfile);
+    }
+    if (gerr &&
+        !(gerr->domain == G_FILE_ERROR && gerr->code == G_FILE_ERROR_NOENT)) {
+        g_critical("error loading configuration from path: %s, %s",
+                   QGA_CONF_DEFAULT, gerr->message);
+        exit(EXIT_FAILURE);
+    }
+    g_clear_error(&gerr);
+}
+
+static void config_parse(GAConfig *config, int argc, char **argv)
 {
-    GAConfig *config = g_new0(GAConfig, 1);
     const char *sopt = "hVvdm:p:l:f:F::b:s:t:D";
     int opt_ind = 0, ch;
     const struct option lopt[] = {
@@ -966,23 +1033,29 @@  static GAConfig *config_parse(int argc, char **argv)
     while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
         switch (ch) {
         case 'm':
+            g_free(config->method);
             config->method = g_strdup(optarg);
             break;
         case 'p':
+            g_free(config->channel_path);
             config->channel_path = g_strdup(optarg);
             break;
         case 'l':
+            g_free(config->log_filepath);
             config->log_filepath = g_strdup(optarg);
             break;
         case 'f':
+            g_free(config->pid_filepath);
             config->pid_filepath = g_strdup(optarg);
             break;
 #ifdef CONFIG_FSFREEZE
         case 'F':
+            g_free(config->fsfreeze_hook);
             config->fsfreeze_hook = g_strdup(optarg ?: QGA_FSFREEZE_HOOK_DEFAULT);
             break;
 #endif
         case 't':
+            g_free(config->state_dir);
             config->state_dir = g_strdup(optarg);
             break;
         case 'v':
@@ -1042,8 +1115,6 @@  static GAConfig *config_parse(int argc, char **argv)
             exit(EXIT_FAILURE);
         }
     }
-
-    return config;
 }
 
 static void config_free(GAConfig *config)
@@ -1053,6 +1124,7 @@  static void config_free(GAConfig *config)
     g_free(config->pid_filepath);
     g_free(config->state_dir);
     g_free(config->channel_path);
+    g_free(config->bliststr);
 #ifdef CONFIG_FSFREEZE
     g_free(config->fsfreeze_hook);
 #endif
@@ -1195,13 +1267,13 @@  int main(int argc, char **argv)
 {
     int ret = EXIT_SUCCESS;
     GAState *s = g_new0(GAState, 1);
-    GAConfig *config;
+    GAConfig *config = g_new0(GAConfig, 1);
 
     module_call_init(MODULE_INIT_QAPI);
 
     init_dfl_pathnames();
-
-    config = config_parse(argc, argv);
+    config_load(config);
+    config_parse(config, argc, argv);
 
     if (config->pid_filepath == NULL) {
         config->pid_filepath = g_strdup(dfl_pathnames.pidfile);