[PATCH 0/3] qga: Add new option --allow-rpcs

[PATCH 0/3] qga: Add new option --allow-rpcs

[Konstantin Kostiuk]

The allow-rpcs option accepts a comma-separated list of RPCs to
enable. This option is opposite to --block-rpcs. Using --block-rpcs
and --allow-rpcs at the same time is not allowed.

resolves: https://gitlab.com/qemu-project/qemu/-/issues/1505

Konstantin Kostiuk (3):
  qga: Rename ga_disable_not_allowed -> ga_disable_not_allowed_freeze
  qga: Add new option --allow-rpcs
  qga: Add tests for --allow-rpcs option

 docs/interop/qemu-ga.rst |  5 +++
 qga/main.c               | 91 ++++++++++++++++++++++++++++++++++++----
 tests/unit/test-qga.c    | 31 ++++++++++++++
 3 files changed, 118 insertions(+), 9 deletions(-)

--
2.34.1

[PATCH 1/3] qga: Rename ga_disable_not_allowed -> ga_disable_not_allowed_freeze

[Konstantin Kostiuk]

Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com>
---
 qga/main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index 2b992a55b3..121ff7a748 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -395,7 +395,7 @@ static gint ga_strcmp(gconstpointer str1, gconstpointer str2)
 }
 
 /* disable commands that aren't safe for fsfreeze */
-static void ga_disable_not_allowed(const QmpCommand *cmd, void *opaque)
+static void ga_disable_not_allowed_freeze(const QmpCommand *cmd, void *opaque)
 {
     bool allowed = false;
     int i = 0;
@@ -459,7 +459,7 @@ void ga_set_frozen(GAState *s)
         return;
     }
     /* disable all forbidden (for frozen state) commands */
-    qmp_for_each_command(&ga_commands, ga_disable_not_allowed, NULL);
+    qmp_for_each_command(&ga_commands, ga_disable_not_allowed_freeze, NULL);
     g_warning("disabling logging due to filesystem freeze");
     ga_disable_logging(s);
     s->frozen = true;
@@ -1350,7 +1350,7 @@ static GAState *initialize_agent(GAConfig *config, int socket_activation)
             s->deferred_options.log_filepath = config->log_filepath;
         }
         ga_disable_logging(s);
-        qmp_for_each_command(&ga_commands, ga_disable_not_allowed, NULL);
+        qmp_for_each_command(&ga_commands, ga_disable_not_allowed_freeze, NULL);
     } else {
         if (config->daemonize) {
             become_daemon(config->pid_filepath);
-- 
2.34.1

Re: [PATCH 1/3] qga: Rename ga_disable_not_allowed -> ga_disable_not_allowed_freeze

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1772 bytes --]

On Thu, Jul 6, 2023 at 12:31 PM Konstantin Kostiuk <kkostiuk@redhat.com>
wrote:

> Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  qga/main.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/qga/main.c b/qga/main.c
> index 2b992a55b3..121ff7a748 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -395,7 +395,7 @@ static gint ga_strcmp(gconstpointer str1,
> gconstpointer str2)
>  }
>
>  /* disable commands that aren't safe for fsfreeze */
> -static void ga_disable_not_allowed(const QmpCommand *cmd, void *opaque)
> +static void ga_disable_not_allowed_freeze(const QmpCommand *cmd, void
> *opaque)
>  {
>      bool allowed = false;
>      int i = 0;
> @@ -459,7 +459,7 @@ void ga_set_frozen(GAState *s)
>          return;
>      }
>      /* disable all forbidden (for frozen state) commands */
> -    qmp_for_each_command(&ga_commands, ga_disable_not_allowed, NULL);
> +    qmp_for_each_command(&ga_commands, ga_disable_not_allowed_freeze,
> NULL);
>      g_warning("disabling logging due to filesystem freeze");
>      ga_disable_logging(s);
>      s->frozen = true;
> @@ -1350,7 +1350,7 @@ static GAState *initialize_agent(GAConfig *config,
> int socket_activation)
>              s->deferred_options.log_filepath = config->log_filepath;
>          }
>          ga_disable_logging(s);
> -        qmp_for_each_command(&ga_commands, ga_disable_not_allowed, NULL);
> +        qmp_for_each_command(&ga_commands, ga_disable_not_allowed_freeze,
> NULL);
>      } else {
>          if (config->daemonize) {
>              become_daemon(config->pid_filepath);
> --
> 2.34.1
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 2657 bytes --]

[PATCH 2/3] qga: Add new option --allow-rpcs

[Konstantin Kostiuk]

The allow-rpcs option accepts a comma-separated list of RPCs to
enable. This option is opposite to --block-rpcs. Using --block-rpcs
and --allow-rpcs at the same time is not allowed.

resolves: https://gitlab.com/qemu-project/qemu/-/issues/1505

Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com>
---
 docs/interop/qemu-ga.rst |  5 +++
 qga/main.c               | 85 +++++++++++++++++++++++++++++++++++++---
 2 files changed, 84 insertions(+), 6 deletions(-)

diff --git a/docs/interop/qemu-ga.rst b/docs/interop/qemu-ga.rst
index a9183802d1..461c5a35ee 100644
--- a/docs/interop/qemu-ga.rst
+++ b/docs/interop/qemu-ga.rst
@@ -84,6 +84,11 @@ Options
   Comma-separated list of RPCs to disable (no spaces, use ``help`` to
   list available RPCs).
 
+.. option:: -a, --allow-rpcs=LIST
+
+  Comma-separated list of RPCs to enable (no spaces, use ``help`` to
+  list available RPCs).
+
 .. option:: -D, --dump-conf
 
   Dump the configuration in a format compatible with ``qemu-ga.conf``
diff --git a/qga/main.c b/qga/main.c
index 121ff7a748..0f95fa87c0 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -87,6 +87,7 @@ struct GAState {
     bool delimit_response;
     bool frozen;
     GList *blockedrpcs;
+    GList *allowedrpcs;
     char *state_filepath_isfrozen;
     struct {
         const char *log_filepath;
@@ -261,6 +262,8 @@ QEMU_COPYRIGHT "\n"
 #endif
 "  -b, --block-rpcs  comma-separated list of RPCs to disable (no spaces,\n"
 "                    use \"help\" to list available RPCs)\n"
+"  -a, --allow-rpcs  comma-separated list of RPCs to enable (no spaces,\n"
+"                    use \"help\" to list available RPCs)\n"
 "  -D, --dump-conf   dump a qemu-ga config file based on current config\n"
 "                    options / command-line parameters to stdout\n"
 "  -r, --retry-path  attempt re-opening path if it's unavailable or closed\n"
@@ -416,16 +419,38 @@ static void ga_disable_not_allowed_freeze(const QmpCommand *cmd, void *opaque)
 /* [re-]enable all commands, except those explicitly blocked by user */
 static void ga_enable_non_blocked(const QmpCommand *cmd, void *opaque)
 {
-    GList *blockedrpcs = opaque;
+    GAState *s = opaque;
+    GList *blockedrpcs = s->blockedrpcs;
+    GList *allowedrpcs = s->allowedrpcs;
     const char *name = qmp_command_name(cmd);
 
-    if (g_list_find_custom(blockedrpcs, name, ga_strcmp) == NULL &&
-        !qmp_command_is_enabled(cmd)) {
+    if (g_list_find_custom(blockedrpcs, name, ga_strcmp) == NULL) {
+        if (qmp_command_is_enabled(cmd)) {
+            return;
+        }
+
+        if (allowedrpcs &&
+            g_list_find_custom(allowedrpcs, name, ga_strcmp) == NULL) {
+            return;
+        }
+
         g_debug("enabling command: %s", name);
         qmp_enable_command(&ga_commands, name);
     }
 }
 
+/* disable commands that aren't allowed */
+static void ga_disable_not_allowed(const QmpCommand *cmd, void *opaque)
+{
+    GList *allowedrpcs = opaque;
+    const char *name = qmp_command_name(cmd);
+
+    if (g_list_find_custom(allowedrpcs, name, ga_strcmp) == NULL) {
+        g_debug("disabling command: %s", name);
+        qmp_disable_command(&ga_commands, name, "the command is not allowed");
+    }
+}
+
 static bool ga_create_file(const char *path)
 {
     int fd = open(path, O_CREAT | O_WRONLY, S_IWUSR | S_IRUSR);
@@ -497,8 +522,8 @@ void ga_unset_frozen(GAState *s)
         s->deferred_options.pid_filepath = NULL;
     }
 
-    /* enable all disabled, non-blocked commands */
-    qmp_for_each_command(&ga_commands, ga_enable_non_blocked, s->blockedrpcs);
+    /* enable all disabled, non-blocked and allowed commands */
+    qmp_for_each_command(&ga_commands, ga_enable_non_blocked, s);
     s->frozen = false;
     if (!ga_delete_file(s->state_filepath_isfrozen)) {
         g_warning("unable to delete %s, fsfreeze may not function properly",
@@ -984,7 +1009,9 @@ struct GAConfig {
     const char *service;
 #endif
     gchar *bliststr; /* blockedrpcs may point to this string */
+    gchar *aliststr; /* allowedrpcs may point to this string */
     GList *blockedrpcs;
+    GList *allowedrpcs;
     int daemonize;
     GLogLevelFlags log_level;
     int dumpconf;
@@ -1055,6 +1082,19 @@ static void config_load(GAConfig *config)
         config->blockedrpcs = g_list_concat(config->blockedrpcs,
                                           split_list(config->bliststr, ","));
     }
+    if (g_key_file_has_key(keyfile, "general", "allow-rpcs", NULL)) {
+        config->aliststr =
+            g_key_file_get_string(keyfile, "general", "allow-rpcs", &gerr);
+        config->allowedrpcs = g_list_concat(config->allowedrpcs,
+                                          split_list(config->aliststr, ","));
+    }
+
+    if (g_key_file_has_key(keyfile, "general", blockrpcs_key, NULL) &&
+        g_key_file_has_key(keyfile, "general", "allow-rpcs", NULL)) {
+        g_critical("wrong config, using --block-rpcs and --allow-rpcs at the"
+                   " same time is not allowed");
+        exit(EXIT_FAILURE);
+    }
 
 end:
     g_key_file_free(keyfile);
@@ -1115,6 +1155,9 @@ static void config_dump(GAConfig *config)
     tmp = list_join(config->blockedrpcs, ',');
     g_key_file_set_string(keyfile, "general", "block-rpcs", tmp);
     g_free(tmp);
+    tmp = list_join(config->allowedrpcs, ',');
+    g_key_file_set_string(keyfile, "general", "allow-rpcs", tmp);
+    g_free(tmp);
 
     tmp = g_key_file_to_data(keyfile, NULL, &error);
     if (error) {
@@ -1130,8 +1173,9 @@ 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:s:t:Dr";
+    const char *sopt = "hVvdm: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' },
@@ -1147,6 +1191,7 @@ static void config_parse(GAConfig *config, int argc, char **argv)
         { "daemonize", 0, NULL, 'd' },
         { "block-rpcs", 1, NULL, 'b' },
         { "blacklist", 1, NULL, 'b' },  /* deprecated alias for 'block-rpcs' */
+        { "allow-rpcs", 1, NULL, 'a' },
 #ifdef _WIN32
         { "service", 1, NULL, 's' },
 #endif
@@ -1206,6 +1251,17 @@ static void config_parse(GAConfig *config, int argc, char **argv)
             }
             config->blockedrpcs = g_list_concat(config->blockedrpcs,
                                                 split_list(optarg, ","));
+            block_rpcs = true;
+            break;
+        }
+        case 'a': {
+            if (is_help_option(optarg)) {
+                qmp_for_each_command(&ga_commands, ga_print_cmd, NULL);
+                exit(EXIT_SUCCESS);
+            }
+            config->allowedrpcs = g_list_concat(config->allowedrpcs,
+                                                split_list(optarg, ","));
+            allow_rpcs = true;
             break;
         }
 #ifdef _WIN32
@@ -1246,6 +1302,12 @@ static void config_parse(GAConfig *config, int argc, char **argv)
             exit(EXIT_FAILURE);
         }
     }
+
+    if (block_rpcs && allow_rpcs) {
+        g_critical("wrong commandline, using --block-rpcs and --allow-rpcs at the"
+                   " same time is not allowed");
+        exit(EXIT_FAILURE);
+    }
 }
 
 static void config_free(GAConfig *config)
@@ -1256,10 +1318,12 @@ static void config_free(GAConfig *config)
     g_free(config->state_dir);
     g_free(config->channel_path);
     g_free(config->bliststr);
+    g_free(config->aliststr);
 #ifdef CONFIG_FSFREEZE
     g_free(config->fsfreeze_hook);
 #endif
     g_list_free_full(config->blockedrpcs, g_free);
+    g_list_free_full(config->allowedrpcs, g_free);
     g_free(config);
 }
 
@@ -1374,6 +1438,15 @@ static GAState *initialize_agent(GAConfig *config, int socket_activation)
         return NULL;
     }
 
+    if (config->allowedrpcs) {
+        qmp_for_each_command(&ga_commands, ga_disable_not_allowed, config->allowedrpcs);
+        s->allowedrpcs = config->allowedrpcs;
+    }
+
+    /*
+     * Some commands can be blocked due to system limitation.
+     * Initialize blockedrpcs list even if allowedrpcs specified.
+     */
     config->blockedrpcs = ga_command_init_blockedrpcs(config->blockedrpcs);
     if (config->blockedrpcs) {
         GList *l = config->blockedrpcs;
-- 
2.34.1

Re: [PATCH 2/3] qga: Add new option --allow-rpcs

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 9403 bytes --]

On Thu, Jul 6, 2023 at 12:32 PM Konstantin Kostiuk <kkostiuk@redhat.com>
wrote:

> The allow-rpcs option accepts a comma-separated list of RPCs to
> enable. This option is opposite to --block-rpcs. Using --block-rpcs
> and --allow-rpcs at the same time is not allowed.
>
> resolves: https://gitlab.com/qemu-project/qemu/-/issues/1505
>
> Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com>
>




> ---
>  docs/interop/qemu-ga.rst |  5 +++
>  qga/main.c               | 85 +++++++++++++++++++++++++++++++++++++---
>  2 files changed, 84 insertions(+), 6 deletions(-)
>
> diff --git a/docs/interop/qemu-ga.rst b/docs/interop/qemu-ga.rst
> index a9183802d1..461c5a35ee 100644
> --- a/docs/interop/qemu-ga.rst
> +++ b/docs/interop/qemu-ga.rst
> @@ -84,6 +84,11 @@ Options
>    Comma-separated list of RPCs to disable (no spaces, use ``help`` to
>    list available RPCs).
>
> +.. option:: -a, --allow-rpcs=LIST
> +
> +  Comma-separated list of RPCs to enable (no spaces, use ``help`` to
> +  list available RPCs).
> +
>  .. option:: -D, --dump-conf
>
>    Dump the configuration in a format compatible with ``qemu-ga.conf``
> diff --git a/qga/main.c b/qga/main.c
> index 121ff7a748..0f95fa87c0 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -87,6 +87,7 @@ struct GAState {
>      bool delimit_response;
>      bool frozen;
>      GList *blockedrpcs;
> +    GList *allowedrpcs;
>      char *state_filepath_isfrozen;
>      struct {
>          const char *log_filepath;
> @@ -261,6 +262,8 @@ QEMU_COPYRIGHT "\n"
>  #endif
>  "  -b, --block-rpcs  comma-separated list of RPCs to disable (no
> spaces,\n"
>  "                    use \"help\" to list available RPCs)\n"
> +"  -a, --allow-rpcs  comma-separated list of RPCs to enable (no spaces,\n"
> +"                    use \"help\" to list available RPCs)\n"
>  "  -D, --dump-conf   dump a qemu-ga config file based on current config\n"
>  "                    options / command-line parameters to stdout\n"
>  "  -r, --retry-path  attempt re-opening path if it's unavailable or
> closed\n"
> @@ -416,16 +419,38 @@ static void ga_disable_not_allowed_freeze(const
> QmpCommand *cmd, void *opaque)
>  /* [re-]enable all commands, except those explicitly blocked by user */
>  static void ga_enable_non_blocked(const QmpCommand *cmd, void *opaque)
>  {
> -    GList *blockedrpcs = opaque;
> +    GAState *s = opaque;
> +    GList *blockedrpcs = s->blockedrpcs;
> +    GList *allowedrpcs = s->allowedrpcs;
>      const char *name = qmp_command_name(cmd);
>
> -    if (g_list_find_custom(blockedrpcs, name, ga_strcmp) == NULL &&
> -        !qmp_command_is_enabled(cmd)) {
> +    if (g_list_find_custom(blockedrpcs, name, ga_strcmp) == NULL) {
> +        if (qmp_command_is_enabled(cmd)) {
> +            return;
> +        }
> +
> +        if (allowedrpcs &&
> +            g_list_find_custom(allowedrpcs, name, ga_strcmp) == NULL) {
> +            return;
> +        }
> +
>          g_debug("enabling command: %s", name);
>          qmp_enable_command(&ga_commands, name);
>      }
>  }
>
> +/* disable commands that aren't allowed */
> +static void ga_disable_not_allowed(const QmpCommand *cmd, void *opaque)
> +{
> +    GList *allowedrpcs = opaque;
> +    const char *name = qmp_command_name(cmd);
> +
> +    if (g_list_find_custom(allowedrpcs, name, ga_strcmp) == NULL) {
> +        g_debug("disabling command: %s", name);
> +        qmp_disable_command(&ga_commands, name, "the command is not
> allowed");
> +    }
> +}
> +
>  static bool ga_create_file(const char *path)
>  {
>      int fd = open(path, O_CREAT | O_WRONLY, S_IWUSR | S_IRUSR);
> @@ -497,8 +522,8 @@ void ga_unset_frozen(GAState *s)
>          s->deferred_options.pid_filepath = NULL;
>      }
>
> -    /* enable all disabled, non-blocked commands */
> -    qmp_for_each_command(&ga_commands, ga_enable_non_blocked,
> s->blockedrpcs);
> +    /* enable all disabled, non-blocked and allowed commands */
> +    qmp_for_each_command(&ga_commands, ga_enable_non_blocked, s);
>      s->frozen = false;
>      if (!ga_delete_file(s->state_filepath_isfrozen)) {
>          g_warning("unable to delete %s, fsfreeze may not function
> properly",
> @@ -984,7 +1009,9 @@ struct GAConfig {
>      const char *service;
>  #endif
>      gchar *bliststr; /* blockedrpcs may point to this string */
> +    gchar *aliststr; /* allowedrpcs may point to this string */
>      GList *blockedrpcs;
> +    GList *allowedrpcs;
>      int daemonize;
>      GLogLevelFlags log_level;
>      int dumpconf;
> @@ -1055,6 +1082,19 @@ static void config_load(GAConfig *config)
>          config->blockedrpcs = g_list_concat(config->blockedrpcs,
>                                            split_list(config->bliststr,
> ","));
>      }
> +    if (g_key_file_has_key(keyfile, "general", "allow-rpcs", NULL)) {
> +        config->aliststr =
> +            g_key_file_get_string(keyfile, "general", "allow-rpcs",
> &gerr);
> +        config->allowedrpcs = g_list_concat(config->allowedrpcs,
> +                                          split_list(config->aliststr,
> ","));
> +    }
> +
> +    if (g_key_file_has_key(keyfile, "general", blockrpcs_key, NULL) &&
> +        g_key_file_has_key(keyfile, "general", "allow-rpcs", NULL)) {
> +        g_critical("wrong config, using --block-rpcs and --allow-rpcs at
> the"
> +                   " same time is not allowed");
>


It's not CLI usage here, it's the configuration file/keys. Please tweak the
warning.

Otherwise, lgtm
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> +        exit(EXIT_FAILURE);
> +    }
>

>  end:
>      g_key_file_free(keyfile);
> @@ -1115,6 +1155,9 @@ static void config_dump(GAConfig *config)
>      tmp = list_join(config->blockedrpcs, ',');
>      g_key_file_set_string(keyfile, "general", "block-rpcs", tmp);
>      g_free(tmp);
> +    tmp = list_join(config->allowedrpcs, ',');
> +    g_key_file_set_string(keyfile, "general", "allow-rpcs", tmp);
> +    g_free(tmp);
>
>      tmp = g_key_file_to_data(keyfile, NULL, &error);
>      if (error) {
> @@ -1130,8 +1173,9 @@ 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:s:t:Dr";
> +    const char *sopt = "hVvdm: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' },
> @@ -1147,6 +1191,7 @@ static void config_parse(GAConfig *config, int argc,
> char **argv)
>          { "daemonize", 0, NULL, 'd' },
>          { "block-rpcs", 1, NULL, 'b' },
>          { "blacklist", 1, NULL, 'b' },  /* deprecated alias for
> 'block-rpcs' */
> +        { "allow-rpcs", 1, NULL, 'a' },
>  #ifdef _WIN32
>          { "service", 1, NULL, 's' },
>  #endif
> @@ -1206,6 +1251,17 @@ static void config_parse(GAConfig *config, int
> argc, char **argv)
>              }
>              config->blockedrpcs = g_list_concat(config->blockedrpcs,
>                                                  split_list(optarg, ","));
> +            block_rpcs = true;
> +            break;
> +        }
> +        case 'a': {
> +            if (is_help_option(optarg)) {
> +                qmp_for_each_command(&ga_commands, ga_print_cmd, NULL);
> +                exit(EXIT_SUCCESS);
> +            }
> +            config->allowedrpcs = g_list_concat(config->allowedrpcs,
> +                                                split_list(optarg, ","));
> +            allow_rpcs = true;
>              break;
>          }
>  #ifdef _WIN32
> @@ -1246,6 +1302,12 @@ static void config_parse(GAConfig *config, int
> argc, char **argv)
>              exit(EXIT_FAILURE);
>          }
>      }
> +
> +    if (block_rpcs && allow_rpcs) {
> +        g_critical("wrong commandline, using --block-rpcs and
> --allow-rpcs at the"
> +                   " same time is not allowed");
> +        exit(EXIT_FAILURE);
> +    }
>  }
>
>  static void config_free(GAConfig *config)
> @@ -1256,10 +1318,12 @@ static void config_free(GAConfig *config)
>      g_free(config->state_dir);
>      g_free(config->channel_path);
>      g_free(config->bliststr);
> +    g_free(config->aliststr);
>  #ifdef CONFIG_FSFREEZE
>      g_free(config->fsfreeze_hook);
>  #endif
>      g_list_free_full(config->blockedrpcs, g_free);
> +    g_list_free_full(config->allowedrpcs, g_free);
>      g_free(config);
>  }
>
> @@ -1374,6 +1438,15 @@ static GAState *initialize_agent(GAConfig *config,
> int socket_activation)
>          return NULL;
>      }
>
> +    if (config->allowedrpcs) {
> +        qmp_for_each_command(&ga_commands, ga_disable_not_allowed,
> config->allowedrpcs);
> +        s->allowedrpcs = config->allowedrpcs;
> +    }
> +
> +    /*
> +     * Some commands can be blocked due to system limitation.
> +     * Initialize blockedrpcs list even if allowedrpcs specified.
> +     */
>      config->blockedrpcs =
> ga_command_init_blockedrpcs(config->blockedrpcs);
>      if (config->blockedrpcs) {
>          GList *l = config->blockedrpcs;
> --
> 2.34.1
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 12302 bytes --]

[PATCH 3/3] qga: Add tests for --allow-rpcs option

[Konstantin Kostiuk]

Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com>
---
 tests/unit/test-qga.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/tests/unit/test-qga.c b/tests/unit/test-qga.c
index 360b4cab23..671e83cb86 100644
--- a/tests/unit/test-qga.c
+++ b/tests/unit/test-qga.c
@@ -665,6 +665,36 @@ static void test_qga_blockedrpcs(gconstpointer data)
     fixture_tear_down(&fix, NULL);
 }
 
+static void test_qga_allowedrpcs(gconstpointer data)
+{
+    TestFixture fix;
+    QDict *ret, *error;
+    const gchar *class, *desc;
+
+    fixture_setup(&fix, "-a guest-ping,guest-get-time", NULL);
+
+    /* check allowed RPCs */
+    ret = qmp_fd(fix.fd, "{'execute': 'guest-ping'}");
+    qmp_assert_no_error(ret);
+    qobject_unref(ret);
+
+    ret = qmp_fd(fix.fd, "{'execute': 'guest-get-time'}");
+    qmp_assert_no_error(ret);
+    qobject_unref(ret);
+
+    /* check something else */
+    ret = qmp_fd(fix.fd, "{'execute': 'guest-get-fsinfo'}");
+    g_assert_nonnull(ret);
+    error = qdict_get_qdict(ret, "error");
+    class = qdict_get_try_str(error, "class");
+    desc = qdict_get_try_str(error, "desc");
+    g_assert_cmpstr(class, ==, "CommandNotFound");
+    g_assert_nonnull(g_strstr_len(desc, -1, "has been disabled"));
+    qobject_unref(ret);
+
+    fixture_tear_down(&fix, NULL);
+}
+
 static void test_qga_config(gconstpointer data)
 {
     GError *error = NULL;
@@ -1090,6 +1120,7 @@ int main(int argc, char **argv)
                          test_qga_fsfreeze_status);
 
     g_test_add_data_func("/qga/blockedrpcs", NULL, test_qga_blockedrpcs);
+    g_test_add_data_func("/qga/allowedrpcs", NULL, test_qga_allowedrpcs);
     g_test_add_data_func("/qga/config", NULL, test_qga_config);
     g_test_add_data_func("/qga/guest-exec", &fix, test_qga_guest_exec);
     g_test_add_data_func("/qga/guest-exec-separated", &fix,
-- 
2.34.1

Re: [PATCH 3/3] qga: Add tests for --allow-rpcs option

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 2241 bytes --]

On Thu, Jul 6, 2023 at 12:32 PM Konstantin Kostiuk <kkostiuk@redhat.com>
wrote:

> Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com>
>


Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  tests/unit/test-qga.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>
> diff --git a/tests/unit/test-qga.c b/tests/unit/test-qga.c
> index 360b4cab23..671e83cb86 100644
> --- a/tests/unit/test-qga.c
> +++ b/tests/unit/test-qga.c
> @@ -665,6 +665,36 @@ static void test_qga_blockedrpcs(gconstpointer data)
>      fixture_tear_down(&fix, NULL);
>  }
>
> +static void test_qga_allowedrpcs(gconstpointer data)
> +{
> +    TestFixture fix;
> +    QDict *ret, *error;
> +    const gchar *class, *desc;
> +
> +    fixture_setup(&fix, "-a guest-ping,guest-get-time", NULL);
> +
> +    /* check allowed RPCs */
> +    ret = qmp_fd(fix.fd, "{'execute': 'guest-ping'}");
> +    qmp_assert_no_error(ret);
> +    qobject_unref(ret);
> +
> +    ret = qmp_fd(fix.fd, "{'execute': 'guest-get-time'}");
> +    qmp_assert_no_error(ret);
> +    qobject_unref(ret);
> +
> +    /* check something else */
> +    ret = qmp_fd(fix.fd, "{'execute': 'guest-get-fsinfo'}");
> +    g_assert_nonnull(ret);
> +    error = qdict_get_qdict(ret, "error");
> +    class = qdict_get_try_str(error, "class");
> +    desc = qdict_get_try_str(error, "desc");
> +    g_assert_cmpstr(class, ==, "CommandNotFound");
> +    g_assert_nonnull(g_strstr_len(desc, -1, "has been disabled"));
> +    qobject_unref(ret);
> +
> +    fixture_tear_down(&fix, NULL);
> +}
> +
>  static void test_qga_config(gconstpointer data)
>  {
>      GError *error = NULL;
> @@ -1090,6 +1120,7 @@ int main(int argc, char **argv)
>                           test_qga_fsfreeze_status);
>
>      g_test_add_data_func("/qga/blockedrpcs", NULL, test_qga_blockedrpcs);
> +    g_test_add_data_func("/qga/allowedrpcs", NULL, test_qga_allowedrpcs);
>      g_test_add_data_func("/qga/config", NULL, test_qga_config);
>      g_test_add_data_func("/qga/guest-exec", &fix, test_qga_guest_exec);
>      g_test_add_data_func("/qga/guest-exec-separated", &fix,
> --
> 2.34.1
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 3355 bytes --]