[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 10/12] qga: add an optionnal qemu-ga.conf sys
From: |
Michael Roth |
Subject: |
Re: [Qemu-devel] [PATCH v2 10/12] qga: add an optionnal qemu-ga.conf system configuration |
Date: |
Tue, 25 Aug 2015 18:29:48 -0500 |
User-agent: |
alot/0.3.6 |
Quoting Marc-André Lureau (2015-08-25 18:18:16)
> Hi
>
> On Wed, Aug 26, 2015 at 1:09 AM, Michael Roth <address@hidden> wrote:
> > We should probably do this in init_dfl_pathnames() for consistency.
>
> QGA_FSFREEZE_HOOK_DEFAULT is also initialized here
>
> in init_dfl_pathnames(), it depends on the value of
> get_local_state_pathname() while here it's static.
Ahh, yah I missed that. It's fine where it's at then.
>
> >>
> >> 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 = 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);
> >
> > 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?
>
> all errors have a g_critical(), except for the case of file missing.
Argh, missed the '!'. Looks good then:
Reviewed-by: Michael Roth <address@hidden>
>
>
> --
> Marc-André Lureau
>
- Re: [Qemu-devel] [PATCH v2 04/12] qga: rename 'path' to 'channel_path', (continued)
- [Qemu-devel] [PATCH v2 06/12] qga: move option parsing to seperate function, marcandre . lureau, 2015/08/25
- [Qemu-devel] [PATCH v2 09/12] qga: free a bit more, marcandre . lureau, 2015/08/25
- [Qemu-devel] [PATCH v2 07/12] qga: fill default options in main(), marcandre . lureau, 2015/08/25
- [Qemu-devel] [PATCH v2 10/12] qga: add an optionnal qemu-ga.conf system configuration, marcandre . lureau, 2015/08/25
- Re: [Qemu-devel] [PATCH v2 10/12] qga: add an optionnal qemu-ga.conf system configuration, Markus Armbruster, 2015/08/26
- [Qemu-devel] [PATCH v2 08/12] qga: move agent run in a seperate function, marcandre . lureau, 2015/08/25
- [Qemu-devel] [PATCH v2 11/12] qga: add --dump-conf option, marcandre . lureau, 2015/08/25
- [Qemu-devel] [PATCH v2 12/12] qga: start a man page, marcandre . lureau, 2015/08/25