[PATCH for-7.2 0/3] Replace 'blacklist' and 'whitelist' in the guest agent

[PATCH for-7.2 0/3] Replace 'blacklist' and 'whitelist' in the guest agent

[Thomas Huth]

Let's use more appropriate words here - the feature in the guest agent
is about blocking RPCs, so we should talk about that instead of using
the term "blacklist" here.

The patches are based on earlier work by Philippe Mathieu-Daudé,
with the idea for the new option name suggested by BALATON Zoltan.

Thomas Huth (3):
  qga: Replace 'blacklist' command line and config file options by
    'block-rpcs'
  qga: Replace 'blacklist' and 'whitelist' in the guest agent sources
  tests/unit/test-qga: Replace the word 'blacklist' in the guest agent
    unit test

 docs/about/deprecated.rst  | 19 ++++++++++
 docs/interop/qemu-ga.rst   |  8 ++--
 qga/qapi-schema.json       |  4 +-
 qga/guest-agent-core.h     |  2 +-
 qga/commands-posix.c       | 16 ++++----
 qga/commands-win32.c       | 10 ++---
 qga/main.c                 | 77 +++++++++++++++++++++-----------------
 tests/unit/test-qga.c      |  8 ++--
 tests/data/test-qga-config |  2 +-
 9 files changed, 87 insertions(+), 59 deletions(-)

-- 
2.31.1

[PATCH 1/3] qga: Replace 'blacklist' command line and config file options by 'block-rpcs'

[Thomas Huth]

Let's use a more appropriate wording for this command line and config
file option. The old ones are still accepted for compatibility reasons,
but marked as deprecated now so that it could be removed in a future
version of QEMU.

This change is based on earlier patches from Philippe Mathieu-Daudé,
with the idea for the new option name suggested by BALATON Zoltan.

And while we're at it, replace the "?" in the help text with "help"
since that does not have the problem of conflicting with the wildcard
character of the shells.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 docs/about/deprecated.rst  | 19 +++++++++++++++++++
 docs/interop/qemu-ga.rst   |  8 ++++----
 qga/main.c                 | 18 +++++++++++++-----
 tests/unit/test-qga.c      |  2 +-
 tests/data/test-qga-config |  2 +-
 5 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 7ee26626d5..61c505db97 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -379,3 +379,22 @@ be deleted from this tree.
 New deployments should use the Rust version, and existing systems
 should consider moving to it.  The command line and feature set
 is very close and moving should be simple.
+
+
+QEMU guest agent
+----------------
+
+``--blacklist`` command line option (since 7.2)
+'''''''''''''''''''''''''''''''''''''''''''''''
+
+``--blacklist`` has been replaced by ``--block-rpcs`` (which is a better
+wording for what this option does). The short form ``-b`` still stays
+the same and thus is the preferred way for scripts that should run with
+both, older and future versions of QEMU.
+
+``blacklist`` config file option (since 7.2)
+''''''''''''''''''''''''''''''''''''''''''''
+
+The ``blacklist`` config file option has been renamed to ``block-rpcs``
+(to be in sync with the renaming of the corresponding command line
+option).
diff --git a/docs/interop/qemu-ga.rst b/docs/interop/qemu-ga.rst
index 3063357bb5..a9183802d1 100644
--- a/docs/interop/qemu-ga.rst
+++ b/docs/interop/qemu-ga.rst
@@ -79,10 +79,10 @@ Options
 
   Daemonize after startup (detach from terminal).
 
-.. option:: -b, --blacklist=LIST
+.. option:: -b, --block-rpcs=LIST
 
-  Comma-separated list of RPCs to disable (no spaces, ``?`` to list
-  available RPCs).
+  Comma-separated list of RPCs to disable (no spaces, use ``help`` to
+  list available RPCs).
 
 .. option:: -D, --dump-conf
 
@@ -125,7 +125,7 @@ pidfile        string
 fsfreeze-hook  string
 statedir       string
 verbose        boolean
-blacklist      string list
+block-rpcs     string list
 =============  ===========
 
 See also
diff --git a/qga/main.c b/qga/main.c
index 5f1efa2333..74e5c9b10c 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -260,8 +260,8 @@ QEMU_COPYRIGHT "\n"
 #ifdef _WIN32
 "  -s, --service     service commands: install, uninstall, vss-install, vss-uninstall\n"
 #endif
-"  -b, --blacklist   comma-separated list of RPCs to disable (no spaces, \"?\"\n"
-"                    to list available RPCs)\n"
+"  -b, --block-rpcs  comma-separated list of RPCs to disable (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"
@@ -963,6 +963,7 @@ static void config_load(GAConfig *config)
     GError *gerr = NULL;
     GKeyFile *keyfile;
     g_autofree char *conf = g_strdup(g_getenv("QGA_CONF")) ?: get_relocated_path(QGA_CONF_DEFAULT);
+    const gchar *blockrpcs_key = "block-rpcs";
 
     /* read system config */
     keyfile = g_key_file_new();
@@ -1009,9 +1010,15 @@ static void config_load(GAConfig *config)
         config->retry_path =
             g_key_file_get_boolean(keyfile, "general", "retry-path", &gerr);
     }
+
     if (g_key_file_has_key(keyfile, "general", "blacklist", NULL)) {
+        g_warning("config using deprecated 'blacklist' key, should be replaced"
+                  " with the 'block-rpcs' key.");
+        blockrpcs_key = "blacklist";
+    }
+    if (g_key_file_has_key(keyfile, "general", blockrpcs_key, NULL)) {
         config->bliststr =
-            g_key_file_get_string(keyfile, "general", "blacklist", &gerr);
+            g_key_file_get_string(keyfile, "general", blockrpcs_key, &gerr);
         config->blacklist = g_list_concat(config->blacklist,
                                           split_list(config->bliststr, ","));
     }
@@ -1073,7 +1080,7 @@ static void config_dump(GAConfig *config)
     g_key_file_set_boolean(keyfile, "general", "retry-path",
                            config->retry_path);
     tmp = list_join(config->blacklist, ',');
-    g_key_file_set_string(keyfile, "general", "blacklist", tmp);
+    g_key_file_set_string(keyfile, "general", "block-rpcs", tmp);
     g_free(tmp);
 
     tmp = g_key_file_to_data(keyfile, NULL, &error);
@@ -1105,7 +1112,8 @@ static void config_parse(GAConfig *config, int argc, char **argv)
         { "method", 1, NULL, 'm' },
         { "path", 1, NULL, 'p' },
         { "daemonize", 0, NULL, 'd' },
-        { "blacklist", 1, NULL, 'b' },
+        { "block-rpcs", 1, NULL, 'b' },
+        { "blacklist", 1, NULL, 'b' },  /* deprecated alias for 'block-rpcs' */
 #ifdef _WIN32
         { "service", 1, NULL, 's' },
 #endif
diff --git a/tests/unit/test-qga.c b/tests/unit/test-qga.c
index 530317044b..c398866f46 100644
--- a/tests/unit/test-qga.c
+++ b/tests/unit/test-qga.c
@@ -729,7 +729,7 @@ static void test_qga_config(gconstpointer data)
     g_assert_true(g_key_file_get_boolean(kf, "general", "verbose", &error));
     g_assert_no_error(error);
 
-    strv = g_key_file_get_string_list(kf, "general", "blacklist", &n, &error);
+    strv = g_key_file_get_string_list(kf, "general", "block-rpcs", &n, &error);
     g_assert_cmpint(n, ==, 2);
     g_assert_true(g_strv_contains((const char * const *)strv,
                                   "guest-ping"));
diff --git a/tests/data/test-qga-config b/tests/data/test-qga-config
index 4bb721a4a1..b6b7bc9dfd 100644
--- a/tests/data/test-qga-config
+++ b/tests/data/test-qga-config
@@ -5,4 +5,4 @@ path=/path/to/org.qemu.guest_agent.0
 pidfile=/var/foo/qemu-ga.pid
 statedir=/var/state
 verbose=true
-blacklist=guest-ping;guest-get-time
+block-rpcs=guest-ping;guest-get-time
-- 
2.31.1

[PATCH 2/3] qga: Replace 'blacklist' and 'whitelist' in the guest agent sources

[Thomas Huth]

Let's use better, more inclusive wording here.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 qga/qapi-schema.json   |  4 +--
 qga/guest-agent-core.h |  2 +-
 qga/commands-posix.c   | 16 ++++++------
 qga/commands-win32.c   | 10 +++----
 qga/main.c             | 59 +++++++++++++++++++++---------------------
 5 files changed, 46 insertions(+), 45 deletions(-)

diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 869399ea1a..026a56f76c 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -16,8 +16,8 @@
 
 { 'pragma': { 'doc-required': true } }
 
-# Whitelists to permit QAPI rule violations; think twice before you
-# add to them!
+# Lists with items allowed to permit QAPI rule violations; think twice
+# before you add to them!
 { 'pragma': {
     # Types whose member names may use '_'
     'member-name-exceptions': [
diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h
index 29cd50402f..b4e7c52c61 100644
--- a/qga/guest-agent-core.h
+++ b/qga/guest-agent-core.h
@@ -24,7 +24,7 @@ typedef struct GACommandState GACommandState;
 extern GAState *ga_state;
 extern QmpCommandList ga_commands;
 
-GList *ga_command_blacklist_init(GList *blacklist);
+GList *ga_command_init_blockedrpcs(GList *blockedrpcs);
 void ga_command_state_init(GAState *s, GACommandState *cs);
 void ga_command_state_add(GACommandState *cs,
                           void (*init)(void),
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 954efed01b..eea819cff0 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -3356,8 +3356,8 @@ qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp)
 }
 #endif
 
-/* add unsupported commands to the blacklist */
-GList *ga_command_blacklist_init(GList *blacklist)
+/* add unsupported commands to the list of blocked RPCs */
+GList *ga_command_init_blockedrpcs(GList *blockedrpcs)
 {
 #if !defined(__linux__)
     {
@@ -3370,13 +3370,13 @@ GList *ga_command_blacklist_init(GList *blacklist)
         char **p = (char **)list;
 
         while (*p) {
-            blacklist = g_list_append(blacklist, g_strdup(*p++));
+            blockedrpcs = g_list_append(blockedrpcs, g_strdup(*p++));
         }
     }
 #endif
 
 #if !defined(HAVE_GETIFADDRS)
-    blacklist = g_list_append(blacklist,
+    blockedrpcs = g_list_append(blockedrpcs,
                               g_strdup("guest-network-get-interfaces"));
 #endif
 
@@ -3390,18 +3390,18 @@ GList *ga_command_blacklist_init(GList *blacklist)
         char **p = (char **)list;
 
         while (*p) {
-            blacklist = g_list_append(blacklist, g_strdup(*p++));
+            blockedrpcs = g_list_append(blockedrpcs, g_strdup(*p++));
         }
     }
 #endif
 
 #if !defined(CONFIG_FSTRIM)
-    blacklist = g_list_append(blacklist, g_strdup("guest-fstrim"));
+    blockedrpcs = g_list_append(blockedrpcs, g_strdup("guest-fstrim"));
 #endif
 
-    blacklist = g_list_append(blacklist, g_strdup("guest-get-devices"));
+    blockedrpcs = g_list_append(blockedrpcs, g_strdup("guest-get-devices"));
 
-    return blacklist;
+    return blockedrpcs;
 }
 
 /* register init/cleanup routines for stateful command groups */
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 7ed7664715..ec9f55b453 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -2005,8 +2005,8 @@ GuestMemoryBlockInfo *qmp_guest_get_memory_block_info(Error **errp)
     return NULL;
 }
 
-/* add unsupported commands to the blacklist */
-GList *ga_command_blacklist_init(GList *blacklist)
+/* add unsupported commands to the list of blocked RPCs */
+GList *ga_command_init_blockedrpcs(GList *blockedrpcs)
 {
     const char *list_unsupported[] = {
         "guest-suspend-hybrid",
@@ -2017,7 +2017,7 @@ GList *ga_command_blacklist_init(GList *blacklist)
     char **p = (char **)list_unsupported;
 
     while (*p) {
-        blacklist = g_list_append(blacklist, g_strdup(*p++));
+        blockedrpcs = g_list_append(blockedrpcs, g_strdup(*p++));
     }
 
     if (!vss_init(true)) {
@@ -2028,11 +2028,11 @@ GList *ga_command_blacklist_init(GList *blacklist)
         p = (char **)list;
 
         while (*p) {
-            blacklist = g_list_append(blacklist, g_strdup(*p++));
+            blockedrpcs = g_list_append(blockedrpcs, g_strdup(*p++));
         }
     }
 
-    return blacklist;
+    return blockedrpcs;
 }
 
 /* register init/cleanup routines for stateful command groups */
diff --git a/qga/main.c b/qga/main.c
index 74e5c9b10c..5a9d8252e0 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -87,7 +87,7 @@ struct GAState {
 #endif
     bool delimit_response;
     bool frozen;
-    GList *blacklist;
+    GList *blockedrpcs;
     char *state_filepath_isfrozen;
     struct {
         const char *log_filepath;
@@ -107,7 +107,7 @@ struct GAState *ga_state;
 QmpCommandList ga_commands;
 
 /* commands that are safe to issue while filesystems are frozen */
-static const char *ga_freeze_whitelist[] = {
+static const char *ga_freeze_allowlist[] = {
     "guest-ping",
     "guest-info",
     "guest-sync",
@@ -363,31 +363,31 @@ static gint ga_strcmp(gconstpointer str1, gconstpointer str2)
 }
 
 /* disable commands that aren't safe for fsfreeze */
-static void ga_disable_non_whitelisted(const QmpCommand *cmd, void *opaque)
+static void ga_disable_not_allowed(const QmpCommand *cmd, void *opaque)
 {
-    bool whitelisted = false;
+    bool allowed = false;
     int i = 0;
     const char *name = qmp_command_name(cmd);
 
-    while (ga_freeze_whitelist[i] != NULL) {
-        if (strcmp(name, ga_freeze_whitelist[i]) == 0) {
-            whitelisted = true;
+    while (ga_freeze_allowlist[i] != NULL) {
+        if (strcmp(name, ga_freeze_allowlist[i]) == 0) {
+            allowed = true;
         }
         i++;
     }
-    if (!whitelisted) {
+    if (!allowed) {
         g_debug("disabling command: %s", name);
         qmp_disable_command(&ga_commands, name, "the agent is in frozen state");
     }
 }
 
-/* [re-]enable all commands, except those explicitly blacklisted by user */
-static void ga_enable_non_blacklisted(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 *blacklist = opaque;
+    GList *blockedrpcs = opaque;
     const char *name = qmp_command_name(cmd);
 
-    if (g_list_find_custom(blacklist, name, ga_strcmp) == NULL &&
+    if (g_list_find_custom(blockedrpcs, name, ga_strcmp) == NULL &&
         !qmp_command_is_enabled(cmd)) {
         g_debug("enabling command: %s", name);
         qmp_enable_command(&ga_commands, name);
@@ -426,8 +426,8 @@ void ga_set_frozen(GAState *s)
     if (ga_is_frozen(s)) {
         return;
     }
-    /* disable all non-whitelisted (for frozen state) commands */
-    qmp_for_each_command(&ga_commands, ga_disable_non_whitelisted, NULL);
+    /* disable all forbidden (for frozen state) commands */
+    qmp_for_each_command(&ga_commands, ga_disable_not_allowed, NULL);
     g_warning("disabling logging due to filesystem freeze");
     ga_disable_logging(s);
     s->frozen = true;
@@ -465,8 +465,8 @@ void ga_unset_frozen(GAState *s)
         s->deferred_options.pid_filepath = NULL;
     }
 
-    /* enable all disabled, non-blacklisted commands */
-    qmp_for_each_command(&ga_commands, ga_enable_non_blacklisted, s->blacklist);
+    /* enable all disabled, non-blocked commands */
+    qmp_for_each_command(&ga_commands, ga_enable_non_blocked, s->blockedrpcs);
     s->frozen = false;
     if (!ga_delete_file(s->state_filepath_isfrozen)) {
         g_warning("unable to delete %s, fsfreeze may not function properly",
@@ -896,7 +896,8 @@ int64_t ga_get_fd_handle(GAState *s, Error **errp)
     int64_t handle;
 
     g_assert(s->pstate_filepath);
-    /* we blacklist commands and avoid operations that potentially require
+    /*
+     * We block commands and avoid operations that potentially require
      * writing to disk when we're in a frozen state. this includes opening
      * new files, so we should never get here in that situation
      */
@@ -950,8 +951,8 @@ struct GAConfig {
 #ifdef _WIN32
     const char *service;
 #endif
-    gchar *bliststr; /* blacklist may point to this string */
-    GList *blacklist;
+    gchar *bliststr; /* blockedrpcs may point to this string */
+    GList *blockedrpcs;
     int daemonize;
     GLogLevelFlags log_level;
     int dumpconf;
@@ -1019,7 +1020,7 @@ static void config_load(GAConfig *config)
     if (g_key_file_has_key(keyfile, "general", blockrpcs_key, NULL)) {
         config->bliststr =
             g_key_file_get_string(keyfile, "general", blockrpcs_key, &gerr);
-        config->blacklist = g_list_concat(config->blacklist,
+        config->blockedrpcs = g_list_concat(config->blockedrpcs,
                                           split_list(config->bliststr, ","));
     }
 
@@ -1079,7 +1080,7 @@ static void config_dump(GAConfig *config)
                            config->log_level == G_LOG_LEVEL_MASK);
     g_key_file_set_boolean(keyfile, "general", "retry-path",
                            config->retry_path);
-    tmp = list_join(config->blacklist, ',');
+    tmp = list_join(config->blockedrpcs, ',');
     g_key_file_set_string(keyfile, "general", "block-rpcs", tmp);
     g_free(tmp);
 
@@ -1171,8 +1172,8 @@ static void config_parse(GAConfig *config, int argc, char **argv)
                 qmp_for_each_command(&ga_commands, ga_print_cmd, NULL);
                 exit(EXIT_SUCCESS);
             }
-            config->blacklist = g_list_concat(config->blacklist,
-                                             split_list(optarg, ","));
+            config->blockedrpcs = g_list_concat(config->blockedrpcs,
+                                                split_list(optarg, ","));
             break;
         }
 #ifdef _WIN32
@@ -1226,7 +1227,7 @@ static void config_free(GAConfig *config)
 #ifdef CONFIG_FSFREEZE
     g_free(config->fsfreeze_hook);
 #endif
-    g_list_free_full(config->blacklist, g_free);
+    g_list_free_full(config->blockedrpcs, g_free);
     g_free(config);
 }
 
@@ -1310,7 +1311,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_non_whitelisted, NULL);
+        qmp_for_each_command(&ga_commands, ga_disable_not_allowed, NULL);
     } else {
         if (config->daemonize) {
             become_daemon(config->pid_filepath);
@@ -1334,10 +1335,10 @@ static GAState *initialize_agent(GAConfig *config, int socket_activation)
         return NULL;
     }
 
-    config->blacklist = ga_command_blacklist_init(config->blacklist);
-    if (config->blacklist) {
-        GList *l = config->blacklist;
-        s->blacklist = config->blacklist;
+    config->blockedrpcs = ga_command_init_blockedrpcs(config->blockedrpcs);
+    if (config->blockedrpcs) {
+        GList *l = config->blockedrpcs;
+        s->blockedrpcs = config->blockedrpcs;
         do {
             g_debug("disabling command: %s", (char *)l->data);
             qmp_disable_command(&ga_commands, l->data, NULL);
-- 
2.31.1

[PATCH 3/3] tests/unit/test-qga: Replace the word 'blacklist' in the guest agent unit test

[Thomas Huth]

Let's use better, more inclusive wording here.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/unit/test-qga.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/unit/test-qga.c b/tests/unit/test-qga.c
index c398866f46..80bb6b0866 100644
--- a/tests/unit/test-qga.c
+++ b/tests/unit/test-qga.c
@@ -629,7 +629,7 @@ static void test_qga_get_time(gconstpointer fix)
     g_assert_cmpint(time, >, 0);
 }
 
-static void test_qga_blacklist(gconstpointer data)
+static void test_qga_blockedrpcs(gconstpointer data)
 {
     TestFixture fix;
     QDict *ret, *error;
@@ -637,7 +637,7 @@ static void test_qga_blacklist(gconstpointer data)
 
     fixture_setup(&fix, "-b guest-ping,guest-get-time", NULL);
 
-    /* check blacklist */
+    /* check blocked RPCs */
     ret = qmp_fd(fix.fd, "{'execute': 'guest-ping'}");
     g_assert_nonnull(ret);
     error = qdict_get_qdict(ret, "error");
@@ -968,7 +968,7 @@ int main(int argc, char **argv)
     g_test_add_data_func("/qga/fsfreeze-status", &fix,
                          test_qga_fsfreeze_status);
 
-    g_test_add_data_func("/qga/blacklist", NULL, test_qga_blacklist);
+    g_test_add_data_func("/qga/blockedrpcs", NULL, test_qga_blockedrpcs);
     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-invalid", &fix,
-- 
2.31.1

Re: [PATCH 1/3] qga: Replace 'blacklist' command line and config file options by 'block-rpcs'

[Daniel P. Berrangé]

On Wed, Jul 27, 2022 at 11:21:33AM +0200, Thomas Huth wrote:
> Let's use a more appropriate wording for this command line and config
> file option. The old ones are still accepted for compatibility reasons,
> but marked as deprecated now so that it could be removed in a future
> version of QEMU.
> 
> This change is based on earlier patches from Philippe Mathieu-Daudé,
> with the idea for the new option name suggested by BALATON Zoltan.
> 
> And while we're at it, replace the "?" in the help text with "help"
> since that does not have the problem of conflicting with the wildcard
> character of the shells.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  docs/about/deprecated.rst  | 19 +++++++++++++++++++
>  docs/interop/qemu-ga.rst   |  8 ++++----
>  qga/main.c                 | 18 +++++++++++++-----
>  tests/unit/test-qga.c      |  2 +-
>  tests/data/test-qga-config |  2 +-
>  5 files changed, 38 insertions(+), 11 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 1/3] qga: Replace 'blacklist' command line and config file options by 'block-rpcs'

[Konstantin Kostiuk]

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

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



On Wed, Jul 27, 2022 at 12:29 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Wed, Jul 27, 2022 at 11:21:33AM +0200, Thomas Huth wrote:
> > Let's use a more appropriate wording for this command line and config
> > file option. The old ones are still accepted for compatibility reasons,
> > but marked as deprecated now so that it could be removed in a future
> > version of QEMU.
> >
> > This change is based on earlier patches from Philippe Mathieu-Daudé,
> > with the idea for the new option name suggested by BALATON Zoltan.
> >
> > And while we're at it, replace the "?" in the help text with "help"
> > since that does not have the problem of conflicting with the wildcard
> > character of the shells.
> >
> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > ---
> >  docs/about/deprecated.rst  | 19 +++++++++++++++++++
> >  docs/interop/qemu-ga.rst   |  8 ++++----
> >  qga/main.c                 | 18 +++++++++++++-----
> >  tests/unit/test-qga.c      |  2 +-
> >  tests/data/test-qga-config |  2 +-
> >  5 files changed, 38 insertions(+), 11 deletions(-)
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>
>
> With regards,
> Daniel
> --
> |: https://berrange.com      -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-
> https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-
> https://www.instagram.com/dberrange :|
>
>

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

Re: [PATCH 2/3] qga: Replace 'blacklist' and 'whitelist' in the guest agent sources

[Daniel P. Berrangé]

On Wed, Jul 27, 2022 at 11:21:34AM +0200, Thomas Huth wrote:
> Let's use better, more inclusive wording here.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  qga/qapi-schema.json   |  4 +--
>  qga/guest-agent-core.h |  2 +-
>  qga/commands-posix.c   | 16 ++++++------
>  qga/commands-win32.c   | 10 +++----
>  qga/main.c             | 59 +++++++++++++++++++++---------------------
>  5 files changed, 46 insertions(+), 45 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 2/3] qga: Replace 'blacklist' and 'whitelist' in the guest agent sources

[Konstantin Kostiuk]

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

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

On Wed, Jul 27, 2022 at 12:21 PM Thomas Huth <thuth@redhat.com> wrote:

> Let's use better, more inclusive wording here.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  qga/qapi-schema.json   |  4 +--
>  qga/guest-agent-core.h |  2 +-
>  qga/commands-posix.c   | 16 ++++++------
>  qga/commands-win32.c   | 10 +++----
>  qga/main.c             | 59 +++++++++++++++++++++---------------------
>  5 files changed, 46 insertions(+), 45 deletions(-)
>
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index 869399ea1a..026a56f76c 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -16,8 +16,8 @@
>
>  { 'pragma': { 'doc-required': true } }
>
> -# Whitelists to permit QAPI rule violations; think twice before you
> -# add to them!
> +# Lists with items allowed to permit QAPI rule violations; think twice
> +# before you add to them!
>  { 'pragma': {
>      # Types whose member names may use '_'
>      'member-name-exceptions': [
> diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h
> index 29cd50402f..b4e7c52c61 100644
> --- a/qga/guest-agent-core.h
> +++ b/qga/guest-agent-core.h
> @@ -24,7 +24,7 @@ typedef struct GACommandState GACommandState;
>  extern GAState *ga_state;
>  extern QmpCommandList ga_commands;
>
> -GList *ga_command_blacklist_init(GList *blacklist);
> +GList *ga_command_init_blockedrpcs(GList *blockedrpcs);
>  void ga_command_state_init(GAState *s, GACommandState *cs);
>  void ga_command_state_add(GACommandState *cs,
>                            void (*init)(void),
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 954efed01b..eea819cff0 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -3356,8 +3356,8 @@ qmp_guest_fstrim(bool has_minimum, int64_t minimum,
> Error **errp)
>  }
>  #endif
>
> -/* add unsupported commands to the blacklist */
> -GList *ga_command_blacklist_init(GList *blacklist)
> +/* add unsupported commands to the list of blocked RPCs */
> +GList *ga_command_init_blockedrpcs(GList *blockedrpcs)
>  {
>  #if !defined(__linux__)
>      {
> @@ -3370,13 +3370,13 @@ GList *ga_command_blacklist_init(GList *blacklist)
>          char **p = (char **)list;
>
>          while (*p) {
> -            blacklist = g_list_append(blacklist, g_strdup(*p++));
> +            blockedrpcs = g_list_append(blockedrpcs, g_strdup(*p++));
>          }
>      }
>  #endif
>
>  #if !defined(HAVE_GETIFADDRS)
> -    blacklist = g_list_append(blacklist,
> +    blockedrpcs = g_list_append(blockedrpcs,
>                                g_strdup("guest-network-get-interfaces"));
>  #endif
>
> @@ -3390,18 +3390,18 @@ GList *ga_command_blacklist_init(GList *blacklist)
>          char **p = (char **)list;
>
>          while (*p) {
> -            blacklist = g_list_append(blacklist, g_strdup(*p++));
> +            blockedrpcs = g_list_append(blockedrpcs, g_strdup(*p++));
>          }
>      }
>  #endif
>
>  #if !defined(CONFIG_FSTRIM)
> -    blacklist = g_list_append(blacklist, g_strdup("guest-fstrim"));
> +    blockedrpcs = g_list_append(blockedrpcs, g_strdup("guest-fstrim"));
>  #endif
>
> -    blacklist = g_list_append(blacklist, g_strdup("guest-get-devices"));
> +    blockedrpcs = g_list_append(blockedrpcs,
> g_strdup("guest-get-devices"));
>
> -    return blacklist;
> +    return blockedrpcs;
>  }
>
>  /* register init/cleanup routines for stateful command groups */
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 7ed7664715..ec9f55b453 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -2005,8 +2005,8 @@ GuestMemoryBlockInfo
> *qmp_guest_get_memory_block_info(Error **errp)
>      return NULL;
>  }
>
> -/* add unsupported commands to the blacklist */
> -GList *ga_command_blacklist_init(GList *blacklist)
> +/* add unsupported commands to the list of blocked RPCs */
> +GList *ga_command_init_blockedrpcs(GList *blockedrpcs)
>  {
>      const char *list_unsupported[] = {
>          "guest-suspend-hybrid",
> @@ -2017,7 +2017,7 @@ GList *ga_command_blacklist_init(GList *blacklist)
>      char **p = (char **)list_unsupported;
>
>      while (*p) {
> -        blacklist = g_list_append(blacklist, g_strdup(*p++));
> +        blockedrpcs = g_list_append(blockedrpcs, g_strdup(*p++));
>      }
>
>      if (!vss_init(true)) {
> @@ -2028,11 +2028,11 @@ GList *ga_command_blacklist_init(GList *blacklist)
>          p = (char **)list;
>
>          while (*p) {
> -            blacklist = g_list_append(blacklist, g_strdup(*p++));
> +            blockedrpcs = g_list_append(blockedrpcs, g_strdup(*p++));
>          }
>      }
>
> -    return blacklist;
> +    return blockedrpcs;
>  }
>
>  /* register init/cleanup routines for stateful command groups */
> diff --git a/qga/main.c b/qga/main.c
> index 74e5c9b10c..5a9d8252e0 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -87,7 +87,7 @@ struct GAState {
>  #endif
>      bool delimit_response;
>      bool frozen;
> -    GList *blacklist;
> +    GList *blockedrpcs;
>      char *state_filepath_isfrozen;
>      struct {
>          const char *log_filepath;
> @@ -107,7 +107,7 @@ struct GAState *ga_state;
>  QmpCommandList ga_commands;
>
>  /* commands that are safe to issue while filesystems are frozen */
> -static const char *ga_freeze_whitelist[] = {
> +static const char *ga_freeze_allowlist[] = {
>      "guest-ping",
>      "guest-info",
>      "guest-sync",
> @@ -363,31 +363,31 @@ static gint ga_strcmp(gconstpointer str1,
> gconstpointer str2)
>  }
>
>  /* disable commands that aren't safe for fsfreeze */
> -static void ga_disable_non_whitelisted(const QmpCommand *cmd, void
> *opaque)
> +static void ga_disable_not_allowed(const QmpCommand *cmd, void *opaque)
>  {
> -    bool whitelisted = false;
> +    bool allowed = false;
>      int i = 0;
>      const char *name = qmp_command_name(cmd);
>
> -    while (ga_freeze_whitelist[i] != NULL) {
> -        if (strcmp(name, ga_freeze_whitelist[i]) == 0) {
> -            whitelisted = true;
> +    while (ga_freeze_allowlist[i] != NULL) {
> +        if (strcmp(name, ga_freeze_allowlist[i]) == 0) {
> +            allowed = true;
>          }
>          i++;
>      }
> -    if (!whitelisted) {
> +    if (!allowed) {
>          g_debug("disabling command: %s", name);
>          qmp_disable_command(&ga_commands, name, "the agent is in frozen
> state");
>      }
>  }
>
> -/* [re-]enable all commands, except those explicitly blacklisted by user
> */
> -static void ga_enable_non_blacklisted(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 *blacklist = opaque;
> +    GList *blockedrpcs = opaque;
>      const char *name = qmp_command_name(cmd);
>
> -    if (g_list_find_custom(blacklist, name, ga_strcmp) == NULL &&
> +    if (g_list_find_custom(blockedrpcs, name, ga_strcmp) == NULL &&
>          !qmp_command_is_enabled(cmd)) {
>          g_debug("enabling command: %s", name);
>          qmp_enable_command(&ga_commands, name);
> @@ -426,8 +426,8 @@ void ga_set_frozen(GAState *s)
>      if (ga_is_frozen(s)) {
>          return;
>      }
> -    /* disable all non-whitelisted (for frozen state) commands */
> -    qmp_for_each_command(&ga_commands, ga_disable_non_whitelisted, NULL);
> +    /* disable all forbidden (for frozen state) commands */
> +    qmp_for_each_command(&ga_commands, ga_disable_not_allowed, NULL);
>      g_warning("disabling logging due to filesystem freeze");
>      ga_disable_logging(s);
>      s->frozen = true;
> @@ -465,8 +465,8 @@ void ga_unset_frozen(GAState *s)
>          s->deferred_options.pid_filepath = NULL;
>      }
>
> -    /* enable all disabled, non-blacklisted commands */
> -    qmp_for_each_command(&ga_commands, ga_enable_non_blacklisted,
> s->blacklist);
> +    /* enable all disabled, non-blocked commands */
> +    qmp_for_each_command(&ga_commands, ga_enable_non_blocked,
> s->blockedrpcs);
>      s->frozen = false;
>      if (!ga_delete_file(s->state_filepath_isfrozen)) {
>          g_warning("unable to delete %s, fsfreeze may not function
> properly",
> @@ -896,7 +896,8 @@ int64_t ga_get_fd_handle(GAState *s, Error **errp)
>      int64_t handle;
>
>      g_assert(s->pstate_filepath);
> -    /* we blacklist commands and avoid operations that potentially require
> +    /*
> +     * We block commands and avoid operations that potentially require
>       * writing to disk when we're in a frozen state. this includes opening
>       * new files, so we should never get here in that situation
>       */
> @@ -950,8 +951,8 @@ struct GAConfig {
>  #ifdef _WIN32
>      const char *service;
>  #endif
> -    gchar *bliststr; /* blacklist may point to this string */
> -    GList *blacklist;
> +    gchar *bliststr; /* blockedrpcs may point to this string */
> +    GList *blockedrpcs;
>      int daemonize;
>      GLogLevelFlags log_level;
>      int dumpconf;
> @@ -1019,7 +1020,7 @@ static void config_load(GAConfig *config)
>      if (g_key_file_has_key(keyfile, "general", blockrpcs_key, NULL)) {
>          config->bliststr =
>              g_key_file_get_string(keyfile, "general", blockrpcs_key,
> &gerr);
> -        config->blacklist = g_list_concat(config->blacklist,
> +        config->blockedrpcs = g_list_concat(config->blockedrpcs,
>                                            split_list(config->bliststr,
> ","));
>      }
>
> @@ -1079,7 +1080,7 @@ static void config_dump(GAConfig *config)
>                             config->log_level == G_LOG_LEVEL_MASK);
>      g_key_file_set_boolean(keyfile, "general", "retry-path",
>                             config->retry_path);
> -    tmp = list_join(config->blacklist, ',');
> +    tmp = list_join(config->blockedrpcs, ',');
>      g_key_file_set_string(keyfile, "general", "block-rpcs", tmp);
>      g_free(tmp);
>
> @@ -1171,8 +1172,8 @@ static void config_parse(GAConfig *config, int argc,
> char **argv)
>                  qmp_for_each_command(&ga_commands, ga_print_cmd, NULL);
>                  exit(EXIT_SUCCESS);
>              }
> -            config->blacklist = g_list_concat(config->blacklist,
> -                                             split_list(optarg, ","));
> +            config->blockedrpcs = g_list_concat(config->blockedrpcs,
> +                                                split_list(optarg, ","));
>              break;
>          }
>  #ifdef _WIN32
> @@ -1226,7 +1227,7 @@ static void config_free(GAConfig *config)
>  #ifdef CONFIG_FSFREEZE
>      g_free(config->fsfreeze_hook);
>  #endif
> -    g_list_free_full(config->blacklist, g_free);
> +    g_list_free_full(config->blockedrpcs, g_free);
>      g_free(config);
>  }
>
> @@ -1310,7 +1311,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_non_whitelisted,
> NULL);
> +        qmp_for_each_command(&ga_commands, ga_disable_not_allowed, NULL);
>      } else {
>          if (config->daemonize) {
>              become_daemon(config->pid_filepath);
> @@ -1334,10 +1335,10 @@ static GAState *initialize_agent(GAConfig *config,
> int socket_activation)
>          return NULL;
>      }
>
> -    config->blacklist = ga_command_blacklist_init(config->blacklist);
> -    if (config->blacklist) {
> -        GList *l = config->blacklist;
> -        s->blacklist = config->blacklist;
> +    config->blockedrpcs =
> ga_command_init_blockedrpcs(config->blockedrpcs);
> +    if (config->blockedrpcs) {
> +        GList *l = config->blockedrpcs;
> +        s->blockedrpcs = config->blockedrpcs;
>          do {
>              g_debug("disabling command: %s", (char *)l->data);
>              qmp_disable_command(&ga_commands, l->data, NULL);
> --
> 2.31.1
>
>

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

Re: [PATCH 3/3] tests/unit/test-qga: Replace the word 'blacklist' in the guest agent unit test

[Konstantin Kostiuk]

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

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

On Wed, Jul 27, 2022 at 12:21 PM Thomas Huth <thuth@redhat.com> wrote:

> Let's use better, more inclusive wording here.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/unit/test-qga.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tests/unit/test-qga.c b/tests/unit/test-qga.c
> index c398866f46..80bb6b0866 100644
> --- a/tests/unit/test-qga.c
> +++ b/tests/unit/test-qga.c
> @@ -629,7 +629,7 @@ static void test_qga_get_time(gconstpointer fix)
>      g_assert_cmpint(time, >, 0);
>  }
>
> -static void test_qga_blacklist(gconstpointer data)
> +static void test_qga_blockedrpcs(gconstpointer data)
>  {
>      TestFixture fix;
>      QDict *ret, *error;
> @@ -637,7 +637,7 @@ static void test_qga_blacklist(gconstpointer data)
>
>      fixture_setup(&fix, "-b guest-ping,guest-get-time", NULL);
>
> -    /* check blacklist */
> +    /* check blocked RPCs */
>      ret = qmp_fd(fix.fd, "{'execute': 'guest-ping'}");
>      g_assert_nonnull(ret);
>      error = qdict_get_qdict(ret, "error");
> @@ -968,7 +968,7 @@ int main(int argc, char **argv)
>      g_test_add_data_func("/qga/fsfreeze-status", &fix,
>                           test_qga_fsfreeze_status);
>
> -    g_test_add_data_func("/qga/blacklist", NULL, test_qga_blacklist);
> +    g_test_add_data_func("/qga/blockedrpcs", NULL, test_qga_blockedrpcs);
>      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-invalid", &fix,
> --
> 2.31.1
>
>

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

Re: [PATCH 3/3] tests/unit/test-qga: Replace the word 'blacklist' in the guest agent unit test

[Daniel P. Berrangé]

On Wed, Jul 27, 2022 at 11:21:35AM +0200, Thomas Huth wrote:
> Let's use better, more inclusive wording here.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/unit/test-qga.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|