From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53681) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZUNLP-0008Um-D1 for qemu-devel@nongnu.org; Tue, 25 Aug 2015 19:09:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZUNLM-0000HY-6J for qemu-devel@nongnu.org; Tue, 25 Aug 2015 19:09:31 -0400 Received: from e37.co.us.ibm.com ([32.97.110.158]:60834) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZUNLM-0000HB-04 for qemu-devel@nongnu.org; Tue, 25 Aug 2015 19:09:28 -0400 Received: from /spool/local by e37.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 25 Aug 2015 17:09:27 -0600 Received: from b03cxnp08028.gho.boulder.ibm.com (b03cxnp08028.gho.boulder.ibm.com [9.17.130.20]) by d03dlp02.boulder.ibm.com (Postfix) with ESMTP id 283433E40047 for ; Tue, 25 Aug 2015 17:09:25 -0600 (MDT) Received: from d03av02.boulder.ibm.com (d03av02.boulder.ibm.com [9.17.195.168]) by b03cxnp08028.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t7PN8Pte43384954 for ; Tue, 25 Aug 2015 16:08:25 -0700 Received: from d03av02.boulder.ibm.com (localhost [127.0.0.1]) by d03av02.boulder.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t7PN9O4v013428 for ; Tue, 25 Aug 2015 17:09:24 -0600 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Michael Roth In-Reply-To: <1440540624-7998-11-git-send-email-marcandre.lureau@redhat.com> References: <1440540624-7998-1-git-send-email-marcandre.lureau@redhat.com> <1440540624-7998-11-git-send-email-marcandre.lureau@redhat.com> Message-ID: <20150825230918.11069.54295@loki> Date: Tue, 25 Aug 2015 18:09:18 -0500 Subject: Re: [Qemu-devel] [PATCH v2 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 Quoting marcandre.lureau@redhat.com (2015-08-25 17:10:22) > From: Marc-Andr=C3=A9 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=C3=A9 Lureau > --- > qga/main.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++= ------ > 1 file changed, 73 insertions(+), 7 deletions(-) > = > diff --git a/qga/main.c b/qga/main.c > index 58f2fc7..154975c 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" We should probably do this in init_dfl_pathnames() for consistency. > = > static struct { > const char *state_dir; > @@ -936,14 +937,80 @@ typedef struct GAConfig { > #ifdef _WIN32 > const char *service; > #endif > + gchar *bliststr; > GList *blacklist; > int daemonize; > GLogLevelFlags log_level; > } GAConfig; > = > -static GAConfig *config_parse(int argc, char **argv) > +static void config_load(GAConfig *config) > +{ > + GError *gerr =3D NULL; > + GKeyFile *keyfile; > + > + /* read system config */ > + keyfile =3D 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 =3D > + g_key_file_get_boolean(keyfile, "general", "daemon", &gerr); > + } > + if (g_key_file_has_key(keyfile, "general", "method", NULL)) { > + config->method =3D > + g_key_file_get_string(keyfile, "general", "method", &gerr); > + } > + if (g_key_file_has_key(keyfile, "general", "path", NULL)) { > + config->channel_path =3D > + g_key_file_get_string(keyfile, "general", "path", &gerr); > + } > + if (g_key_file_has_key(keyfile, "general", "logfile", NULL)) { > + config->log_filepath =3D > + g_key_file_get_string(keyfile, "general", "logfile", &gerr); > + } > + if (g_key_file_has_key(keyfile, "general", "pidfile", NULL)) { > + config->pid_filepath =3D > + 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 =3D > + g_key_file_get_string(keyfile, > + "general", "fsfreeze-hook", &gerr); > + } > +#endif > + if (g_key_file_has_key(keyfile, "general", "statedir", NULL)) { > + config->state_dir =3D > + 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 =3D G_LOG_LEVEL_MASK; > + } > + if (g_key_file_has_key(keyfile, "general", "blacklist", NULL)) { > + config->bliststr =3D > + g_key_file_get_string(keyfile, "general", "blacklist", &gerr= ); > + config->blacklist =3D g_list_concat(config->blacklist, > + split_list(config->bliststr, '= ,')); > + } > + > +end: > + if (keyfile) { > + g_key_file_free(keyfile); > + } > + if (gerr && > + !(gerr->domain =3D=3D G_FILE_ERROR && gerr->code =3D=3D G_FILE_E= RROR_NOENT)) { > + g_critical("error loading configuration from path: %s, %s", > + QGA_CONF_DEFAULT, gerr->message); > + exit(EXIT_FAILURE); > + } > + g_clear_error(&gerr); What about other file errors, like permission issues? Maybe just have config_load() return bool, and just spit out stringified gerr prior to return false? Looks good otherwise. > +} > + > +static void config_parse(GAConfig *config, int argc, char **argv) > { > - GAConfig *config =3D g_new0(GAConfig, 1); > const char *sopt =3D "hVvdm:p:l:f:F::b:s:t:D"; > int opt_ind =3D 0, ch; > const struct option lopt[] =3D { > @@ -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 =3D EXIT_SUCCESS; > GAState *s =3D g_new0(GAState, 1); > - GAConfig *config; > + GAConfig *config =3D g_new0(GAConfig, 1); > = > module_call_init(MODULE_INIT_QAPI); > = > init_dfl_pathnames(); > - > - config =3D config_parse(argc, argv); > + config_load(config); > + config_parse(config, argc, argv); > = > if (config->pid_filepath =3D=3D NULL) { > config->pid_filepath =3D g_strdup(dfl_pathnames.pidfile); > -- = > 2.4.3 >=20