Re: [PATCH] qga: replace "blacklist" with "blocklist"

[Daniel P . Berrangé]

On Wed, Nov 24, 2021 at 01:09:37PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Let's use a more neutral language for that option.
> 
> "blacklist" is still silently accepted for compatibility reasons.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  docs/interop/qemu-ga.rst   |  4 ++--
>  qga/guest-agent-core.h     |  2 +-
>  qga/commands-posix.c       | 14 ++++++------
>  qga/commands-win32.c       | 10 ++++----
>  qga/main.c                 | 47 ++++++++++++++++++++++----------------
>  tests/unit/test-qga.c      |  8 +++----
>  tests/data/test-qga-config |  2 +-
>  7 files changed, 47 insertions(+), 40 deletions(-)
> 
> diff --git a/docs/interop/qemu-ga.rst b/docs/interop/qemu-ga.rst
> index 3063357bb5d6..706c7b189b0e 100644
> --- a/docs/interop/qemu-ga.rst
> +++ b/docs/interop/qemu-ga.rst
> @@ -79,7 +79,7 @@ Options
>  
>    Daemonize after startup (detach from terminal).
>  
> -.. option:: -b, --blacklist=LIST
> +.. option:: -b, --blocklist=LIST
>  
>    Comma-separated list of RPCs to disable (no spaces, ``?`` to list
>    available RPCs).

IMHO, neither is especially informative because they don't
express what is being disabled. What about.

   -f, --command-filter=LIST

   Comma-separate list of command names to disable.
   No spaces, use ``help`` to list available commands.


> diff --git a/qga/main.c b/qga/main.c
> index 15fd3a4149f4..016e3f160570 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -88,7 +88,7 @@ struct GAState {
>  #endif
>      bool delimit_response;
>      bool frozen;
> -    GList *blacklist;
> +    GList *blocklist;
>      char *state_filepath_isfrozen;
>      struct {
>          const char *log_filepath;
> @@ -257,7 +257,7 @@ 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"
> +"  -b, --blocklist   comma-separated list of RPCs to disable (no spaces, 
> \"?\"\n"
>  "                    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"
> @@ -379,13 +379,13 @@ static void ga_disable_non_whitelisted(const QmpCommand 
> *cmd, void *opaque)
>      }
>  }
>  
> -/* [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 blocklisted by user */
> +static void ga_enable_non_blocklisted(const QmpCommand *cmd, void *opaque)
>  {
> -    GList *blacklist = opaque;
> +    GList *blocklist = opaque;
>      const char *name = qmp_command_name(cmd);
>  
> -    if (g_list_find_custom(blacklist, name, ga_strcmp) == NULL &&
> +    if (g_list_find_custom(blocklist, name, ga_strcmp) == NULL &&
>          !qmp_command_is_enabled(cmd)) {
>          g_debug("enabling command: %s", name);
>          qmp_enable_command(&ga_commands, name);
> @@ -463,8 +463,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-blocklisted commands */
> +    qmp_for_each_command(&ga_commands, ga_enable_non_blocklisted, 
> s->blocklist);
>      s->frozen = false;
>      if (!ga_delete_file(s->state_filepath_isfrozen)) {
>          g_warning("unable to delete %s, fsfreeze may not function properly",
> @@ -894,7 +894,7 @@ 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 blocklist 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
>       */
> @@ -948,8 +948,8 @@ struct GAConfig {
>  #ifdef _WIN32
>      const char *service;
>  #endif
> -    gchar *bliststr; /* blacklist may point to this string */
> -    GList *blacklist;
> +    gchar *bliststr; /* blocklist may point to this string */
> +    GList *blocklist;
>      int daemonize;
>      GLogLevelFlags log_level;
>      int dumpconf;
> @@ -1007,10 +1007,16 @@ 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", "blocklist", NULL)) {
> +        config->bliststr =
> +            g_key_file_get_string(keyfile, "general", "blocklist", &gerr);
> +        config->blocklist = g_list_concat(config->blocklist,
> +                                          split_list(config->bliststr, ","));
> +    }
>      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,
> +        config->blocklist = g_list_concat(config->blocklist,
>                                            split_list(config->bliststr, ","));
>      }

This needs to print a deprecation warning.


We also need to update the QEMU deprecations documentation.


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 :|