From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34564) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZUfdx-0004S9-2w for qemu-devel@nongnu.org; Wed, 26 Aug 2015 14:41:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZUfdt-0002hu-Ko for qemu-devel@nongnu.org; Wed, 26 Aug 2015 14:41:53 -0400 Received: from mx2.parallels.com ([199.115.105.18]:49921) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZUfdt-0002hH-Cw for qemu-devel@nongnu.org; Wed, 26 Aug 2015 14:41:49 -0400 References: <1440583525-21632-1-git-send-email-marcandre.lureau@redhat.com> <1440583525-21632-11-git-send-email-marcandre.lureau@redhat.com> From: "Denis V. Lunev" Message-ID: <55DE0864.5010103@parallels.com> Date: Wed, 26 Aug 2015 21:41:40 +0300 MIME-Version: 1.0 In-Reply-To: <1440583525-21632-11-git-send-email-marcandre.lureau@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v3 10/12] qga: add an optionnal qemu-ga.conf system configuration List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: marcandre.lureau@redhat.com, qemu-devel@nongnu.org Cc: mdroth@linux.vnet.ibm.com On 08/26/2015 01:05 PM, marcandre.lureau@redhat.com wrote: > From: Marc-André Lureau > > 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 a > init service (instead of distro-specific init keys/files) > > Signed-off-by: Marc-André Lureau > Reviewed-by: Michael Roth > --- > qga/main.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 73 insertions(+), 7 deletions(-) > > diff --git a/qga/main.c b/qga/main.c > index 58f2fc7..9193043 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; > @@ -936,14 +937,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[] = { > @@ -1047,8 +1114,6 @@ static GAConfig *config_parse(int argc, char **argv) > exit(EXIT_FAILURE); > } > } > - > - return config; > } > > static void config_free(GAConfig *config) > @@ -1058,6 +1123,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 > @@ -1200,13 +1266,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); sequential calling of config_load/config_parse means memory leak in config_parse if the option is present in the file and overwritten during parsing.