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.
Centralizing the code in a single method "ga_apply_command_filters"
will provide a strong guarantee of consistency and clarify the
intended behaviour.
Signed-off-by: Daniel P. Berrangé <berra...@redhat.com>
---
qga/main.c | 110 ++++++++++++++++++++++++++---------------------------
1 file changed, 55 insertions(+), 55 deletions(-)
diff --git a/qga/main.c b/qga/main.c
index e8f52f0794..c7b7b0a9bc 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -419,60 +419,79 @@ static gint ga_strcmp(gconstpointer str1, gconstpointer
str2)
return strcmp(str1, str2);
}
-/* disable commands that aren't safe for fsfreeze */
-static void ga_disable_not_allowed_freeze(const QmpCommand *cmd, void *opaque)
+static bool ga_command_is_allowed(const QmpCommand *cmd, GAState *state)
{
- bool allowed = false;
int i = 0;
+ GAConfig *config = state->config;
const char *name = qmp_command_name(cmd);
+ /* Fallback policy is allow everything */
+ bool allowed = true;
- while (ga_freeze_allowlist[i] != NULL) {
- if (strcmp(name, ga_freeze_allowlist[i]) == 0) {
+ if (config->allowedrpcs) {
+ /*
+ * If an allow-list is given, this changes the fallback
+ * policy to deny everything
+ */
+ allowed = false;
+
+ if (g_list_find_custom(config->allowedrpcs, name, ga_strcmp) != NULL) {
allowed = true;
}
- i++;
- }
- 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 blocked by user */
-static void ga_enable_non_blocked(const QmpCommand *cmd, void *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) {
- if (qmp_command_is_enabled(cmd)) {
- return;
+ /*
+ * If both allowedrpcs and blockedrpcs are set, the blocked
+ * list will take priority
+ */
+ if (config->blockedrpcs) {
+ if (g_list_find_custom(config->blockedrpcs, name, ga_strcmp) != NULL) {
+ allowed = false;
}
+ }
- if (allowedrpcs &&
- g_list_find_custom(allowedrpcs, name, ga_strcmp) == NULL) {
- return;
- }
+ /*
+ * If frozen, this filtering must take priority over
+ * absolutely everything
+ */
+ if (state->frozen) {
+ allowed = false;
- g_debug("enabling command: %s", name);
- qmp_enable_command(&ga_commands, name);
+ while (ga_freeze_allowlist[i] != NULL) {
+ if (strcmp(name, ga_freeze_allowlist[i]) == 0) {
+ allowed = true;
+ }
+ i++;
+ }
}
+
+ return allowed;
}
-/* disable commands that aren't allowed */
-static void ga_disable_not_allowed(const QmpCommand *cmd, void *opaque)
+static void ga_apply_command_filters_iter(const QmpCommand *cmd, void *opaque)
{
- GList *allowedrpcs = opaque;
+ GAState *state = opaque;
+ bool want = ga_command_is_allowed(cmd, state);
+ bool have = qmp_command_is_enabled(cmd);
const char *name = qmp_command_name(cmd);
- if (g_list_find_custom(allowedrpcs, name, ga_strcmp) == NULL) {
+ if (want == have) {
+ return;
+ }
+
+ if (qmp_command_is_enabled(cmd)) {
g_debug("disabling command: %s", name);
qmp_disable_command(&ga_commands, name, "the command is not allowed");
+ } else {
+ g_debug("enabling command: %s", name);
+ qmp_enable_command(&ga_commands, name);
}
}
+static void ga_apply_command_filters(GAState *state)
+{
+ qmp_for_each_command(&ga_commands, ga_apply_command_filters_iter, state);
+}
+
static bool ga_create_file(const char *path)
{
int fd = open(path, O_CREAT | O_WRONLY, S_IWUSR | S_IRUSR);
@@ -505,15 +524,14 @@ void ga_set_frozen(GAState *s)
if (ga_is_frozen(s)) {
return;
}
- /* disable all forbidden (for frozen state) commands */
- 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;
if (!ga_create_file(s->state_filepath_isfrozen)) {
g_warning("unable to create %s, fsfreeze may not function properly",
s->state_filepath_isfrozen);
}
+ ga_apply_command_filters(s);
+ ga_disable_logging(s);
}
void ga_unset_frozen(GAState *s)
@@ -545,12 +563,12 @@ void ga_unset_frozen(GAState *s)
}
/* 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",
s->state_filepath_isfrozen);
}
+ ga_apply_command_filters(s);
}
#ifdef CONFIG_FSFREEZE
@@ -1414,7 +1432,6 @@ 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_freeze,
NULL);
} else {
if (config->daemonize) {
become_daemon(config->pid_filepath);
@@ -1438,25 +1455,8 @@ 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;
- }
+ ga_apply_command_filters(s);
- /*
- * 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;
- s->blockedrpcs = config->blockedrpcs;
- do {
- g_debug("disabling command: %s", (char *)l->data);
- qmp_disable_command(&ga_commands, l->data, NULL);
- l = g_list_next(l);
- } while (l);
- }
s->command_state = ga_command_state_new();
ga_command_state_init(s, s->command_state);
ga_command_state_init_all(s->command_state);
--
2.45.1
