qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 22/22] qga: centralize logic for disabling/enabling comman


From: Philippe Mathieu-Daudé
Subject: Re: [PATCH v2 22/22] qga: centralize logic for disabling/enabling commands
Date: Wed, 3 Jul 2024 14:09:10 +0200
User-agent: Mozilla Thunderbird

On 3/7/24 12:01, Manos Pitsidianakis wrote:
Hello Daniel,

This cleanup seems like a good idea,

On Thu, 13 Jun 2024 18:44, "Daniel P. Berrangé" <berrange@redhat.com> wrote:
It is confusing having many different pieces of code enabling and
disabling commands, and it is not clear that they all have the same
semantics, especially wrt prioritization of the block/allow lists.
The code attempted to prevent the user from setting both the block
and allow lists concurrently, however, the logic was flawed as it
checked settings in the configuration file  separately from the
command line arguments. Thus it was possible to set a block list
in the config file and an allow list via a command line argument.
The --dump-conf option also creates a configuration file with both
keys present, even if unset, which means it is creating a config
that cannot actually be loaded again.

Centralizing the code in a single method "ga_apply_command_filters"
will provide a strong guarantee of consistency and clarify the
intended behaviour. With this there is no compelling technical
reason to prevent concurrent setting of both the allow and block
lists, so this flawed restriction is removed.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
docs/interop/qemu-ga.rst |  14 +++++
qga/commands-posix.c     |   6 --
qga/commands-win32.c     |   6 --
qga/main.c               | 128 +++++++++++++++++----------------------
4 files changed, 70 insertions(+), 84 deletions(-)


+static void ga_apply_command_filters(GAState *state)

Nit: inline?

No, consensus is today's compilers are smart enough to
notice inlining, developers shouldn't worry about this
anymore.

+{
+    qmp_for_each_command(&ga_commands, ga_apply_command_filters_iter, state);
+}




reply via email to

[Prev in Thread] Current Thread [Next in Thread]