Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
On Fri, Jul 12, 2024 at 4:26 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:
> Allowing the user to set the QGA_CONF environment variable to change
> the default configuration file path is very unusual practice, made
> more obscure since this ability is not documented.
>
> This introduces the more normal '-c PATH' / '--config=PATH' command
> line argument approach. This requires that we parse the comamnd line
> twice, since we want the command line arguments to take priority over
> the configuration file settings in general.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> docs/interop/qemu-ga.rst | 5 +++++
> qga/main.c | 43 ++++++++++++++++++++++++++++++----------
> 2 files changed, 38 insertions(+), 10 deletions(-)
>
> diff --git a/docs/interop/qemu-ga.rst b/docs/interop/qemu-ga.rst
> index 72fb75a6f5..e42b370319 100644
> --- a/docs/interop/qemu-ga.rst
> +++ b/docs/interop/qemu-ga.rst
> @@ -33,6 +33,11 @@ Options
>
> .. program:: qemu-ga
>
> +.. option:: -c, --config=PATH
> +
> + Configuration file path (the default is |CONFDIR|\ ``/qemu-ga.conf``,
> + unless overriden by the QGA_CONF environment variable)
> +
> .. option:: -m, --method=METHOD
>
> Transport method: one of ``unix-listen``, ``virtio-serial``, or
> diff --git a/qga/main.c b/qga/main.c
> index 6ff022a85d..6ae911eb15 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -248,12 +248,16 @@ static void usage(const char *cmd)
> #ifdef CONFIG_FSFREEZE
> g_autofree char *fsfreeze_hook =
> get_relocated_path(QGA_FSFREEZE_HOOK_DEFAULT);
> #endif
> + g_autofree char *conf_path = get_relocated_path(QGA_CONF_DEFAULT);
>
> printf(
> "Usage: %s [-m <method> -p <path>] [<options>]\n"
> "QEMU Guest Agent " QEMU_FULL_VERSION "\n"
> QEMU_COPYRIGHT "\n"
> "\n"
> +" -c, --config=PATH configuration file path (default is\n"
> +" %s/qemu-ga.conf\n"
> +" unless overriden by the QGA_CONF environment
> variable)\n"
> " -m, --method transport method: one of unix-listen,
> virtio-serial,\n"
> " isa-serial, or vsock-listen (virtio-serial is the
> default)\n"
> " -p, --path device/socket path (the default for virtio-serial
> is:\n"
> @@ -294,8 +298,8 @@ QEMU_COPYRIGHT "\n"
> " plug/unplug, etc.)\n"
> " -h, --help display this help and exit\n"
> "\n"
> -QEMU_HELP_BOTTOM "\n"
> - , cmd, QGA_VIRTIO_PATH_DEFAULT, QGA_SERIAL_PATH_DEFAULT,
> +QEMU_HELP_BOTTOM "\n",
> + cmd, conf_path, QGA_VIRTIO_PATH_DEFAULT, QGA_SERIAL_PATH_DEFAULT,
> dfl_pathnames.pidfile,
> #ifdef CONFIG_FSFREEZE
> fsfreeze_hook,
> @@ -1018,15 +1022,14 @@ static GList *split_list(const gchar *str, const
> gchar *delim)
> return list;
> }
>
> -static void config_load(GAConfig *config)
> +static void config_load(GAConfig *config, const char *confpath, bool
> required)
> {
> GError *gerr = NULL;
> GKeyFile *keyfile;
> - g_autofree char *conf = g_strdup(g_getenv("QGA_CONF")) ?:
> get_relocated_path(QGA_CONF_DEFAULT);
>
> /* read system config */
> keyfile = g_key_file_new();
> - if (!g_key_file_load_from_file(keyfile, conf, 0, &gerr)) {
> + if (!g_key_file_load_from_file(keyfile, confpath, 0, &gerr)) {
> goto end;
> }
> if (g_key_file_has_key(keyfile, "general", "daemon", NULL)) {
> @@ -1092,10 +1095,10 @@ static void config_load(GAConfig *config)
>
> end:
> g_key_file_free(keyfile);
> - if (gerr &&
> - !(gerr->domain == G_FILE_ERROR && gerr->code ==
> G_FILE_ERROR_NOENT)) {
> + if (gerr && (required ||
> + !(gerr->domain == G_FILE_ERROR && gerr->code ==
> G_FILE_ERROR_NOENT))) {
> g_critical("error loading configuration from path: %s, %s",
> - conf, gerr->message);
> + confpath, gerr->message);
> exit(EXIT_FAILURE);
> }
> g_clear_error(&gerr);
> @@ -1167,12 +1170,13 @@ static void config_dump(GAConfig *config)
>
> static void config_parse(GAConfig *config, int argc, char **argv)
> {
> - const char *sopt = "hVvdm:p:l:f:F::b:a:s:t:Dr";
> + const char *sopt = "hVvdc:m:p:l:f:F::b:a:s:t:Dr";
> int opt_ind = 0, ch;
> bool block_rpcs = false, allow_rpcs = false;
> const struct option lopt[] = {
> { "help", 0, NULL, 'h' },
> { "version", 0, NULL, 'V' },
> + { "config", 1, NULL, 'c' },
> { "dump-conf", 0, NULL, 'D' },
> { "logfile", 1, NULL, 'l' },
> { "pidfile", 1, NULL, 'f' },
> @@ -1192,6 +1196,26 @@ static void config_parse(GAConfig *config, int
> argc, char **argv)
> { "retry-path", 0, NULL, 'r' },
> { NULL, 0, NULL, 0 }
> };
> + g_autofree char *confpath = g_strdup(g_getenv("QGA_CONF")) ?:
> + get_relocated_path(QGA_CONF_DEFAULT);
> + bool confrequired = false;
> +
> + while ((ch = getopt_long(argc, argv, sopt, lopt, NULL)) != -1) {
> + switch (ch) {
> + case 'c':
> + g_free(confpath);
> + confpath = g_strdup(optarg);
> + confrequired = true;
> + break;
> + default:
> + break;
> + }
> + }
> +
> + config_load(config, confpath, confrequired);
> +
> + /* Reset for second pass */
> + optind = 1;
>
> while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
> switch (ch) {
> @@ -1582,7 +1606,6 @@ int main(int argc, char **argv)
> qga_qmp_init_marshal(&ga_commands);
>
> init_dfl_pathnames();
> - config_load(config);
> config_parse(config, argc, argv);
>
> if (config->pid_filepath == NULL) {
> --
> 2.45.1
>
>