From 7bc9027dac553014e79440126a8e7535b29899ff Mon Sep 17 00:00:00 2001 Message-Id: <7bc9027dac553014e79440126a8e7535b29899ff.1342518105.git.minovotn@redhat.com> In-Reply-To: <27a73856ecc481c66c7afac8171f753887f32e31.1342518105.git.minovotn@redhat.com> References: <27a73856ecc481c66c7afac8171f753887f32e31.1342518105.git.minovotn@redhat.com> From: Luiz Capitulino Date: Tue, 5 Jun 2012 14:58:31 +0200 Subject: [PATCH 22/41] qemu-ga: add a whitelist for fsfreeze-safe commands RH-Author: Luiz Capitulino Message-id: <1338908331-15633-17-git-send-email-lcapitulino@redhat.com> Patchwork-id: 39913 O-Subject: [PATCH RHEL6.4 qemu-kvm 16/36] qemu-ga: add a whitelist for fsfreeze-safe commands Bugzilla: 827612 RH-Acked-by: Paolo Bonzini RH-Acked-by: Juan Quintela RH-Acked-by: Jeffrey Cody From: Michael Roth Currently we rely on fsfreeze/thaw commands disabling/enabling logging then having other commands check whether logging is disabled to avoid executing if they aren't safe for running while a filesystem is frozen. Instead, have an explicit whitelist of fsfreeze-safe commands, and consolidate logging and command enablement/disablement into a pair of helper functions: ga_set_frozen()/ga_unset_frozen() Signed-off-by: Michael Roth (cherry picked from commit f22d85e9e67262db34504f4079745f9843da6a92) Signed-off-by: Luiz Capitulino --- qapi/qmp-core.h | 1 + qapi/qmp-registry.c | 14 +++++- qemu-ga.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++--- qga/commands-posix.c | 35 ++++----------- qga/guest-agent-core.h | 3 ++ 5 files changed, 140 insertions(+), 33 deletions(-) Signed-off-by: Michal Novotny --- qapi/qmp-core.h | 1 + qapi/qmp-registry.c | 14 +++++- qemu-ga.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++-- qga/commands-posix.c | 35 ++++---------- qga/guest-agent-core.h | 3 ++ 5 files changed, 140 insertions(+), 33 deletions(-) diff --git a/qapi/qmp-core.h b/qapi/qmp-core.h index 3bb3acb..431ddbb 100644 --- a/qapi/qmp-core.h +++ b/qapi/qmp-core.h @@ -38,6 +38,7 @@ void qmp_register_command(const char *name, QmpCommandFunc *fn); QmpCommand *qmp_find_command(const char *name); QObject *qmp_dispatch(QObject *request); void qmp_disable_command(const char *name); +void qmp_enable_command(const char *name); bool qmp_command_is_enabled(const char *name); char **qmp_get_command_list(void); diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c index d1639e6..9ffb542 100644 --- a/qapi/qmp-registry.c +++ b/qapi/qmp-registry.c @@ -40,18 +40,28 @@ QmpCommand *qmp_find_command(const char *name) return NULL; } -void qmp_disable_command(const char *name) +static void qmp_toggle_command(const char *name, bool enabled) { QmpCommand *cmd; QTAILQ_FOREACH(cmd, &qmp_commands, node) { if (strcmp(cmd->name, name) == 0) { - cmd->enabled = false; + cmd->enabled = enabled; return; } } } +void qmp_disable_command(const char *name) +{ + qmp_toggle_command(name, false); +} + +void qmp_enable_command(const char *name) +{ + qmp_toggle_command(name, true); +} + bool qmp_command_is_enabled(const char *name) { QmpCommand *cmd; diff --git a/qemu-ga.c b/qemu-ga.c index e881b72..939d696 100644 --- a/qemu-ga.c +++ b/qemu-ga.c @@ -56,10 +56,22 @@ struct GAState { GAService service; #endif bool delimit_response; + bool frozen; + GList *blacklist; }; struct GAState *ga_state; +/* commands that are safe to issue while filesystems are frozen */ +static const char *ga_freeze_whitelist[] = { + "guest-ping", + "guest-info", + "guest-sync", + "guest-fsfreeze-status", + "guest-fsfreeze-thaw", + NULL +}; + #ifdef _WIN32 DWORD WINAPI service_ctrl_handler(DWORD ctrl, DWORD type, LPVOID data, LPVOID ctx); @@ -68,6 +80,15 @@ VOID WINAPI service_main(DWORD argc, TCHAR *argv[]); static void quit_handler(int sig) { + /* if we're frozen, don't exit unless we're absolutely forced to, + * because it's basically impossible for graceful exit to complete + * unless all log/pid files are on unfreezable filesystems. there's + * also a very likely chance killing the agent before unfreezing + * the filesystems is a mistake (or will be viewed as one later). + */ + if (ga_is_frozen(ga_state)) { + return; + } g_debug("received signal num %d, quitting", sig); if (g_main_loop_is_running(ga_state->main_loop)) { @@ -206,6 +227,87 @@ void ga_set_response_delimited(GAState *s) s->delimit_response = true; } +static gint ga_strcmp(gconstpointer str1, gconstpointer str2) +{ + return strcmp(str1, str2); +} + +/* disable commands that aren't safe for fsfreeze */ +static void ga_disable_non_whitelisted(void) +{ + char **list_head, **list; + bool whitelisted; + int i; + + list_head = list = qmp_get_command_list(); + while (*list != NULL) { + whitelisted = false; + i = 0; + while (ga_freeze_whitelist[i] != NULL) { + if (strcmp(*list, ga_freeze_whitelist[i]) == 0) { + whitelisted = true; + } + i++; + } + if (!whitelisted) { + g_debug("disabling command: %s", *list); + qmp_disable_command(*list); + } + g_free(*list); + list++; + } + g_free(list_head); +} + +/* [re-]enable all commands, except those explictly blacklisted by user */ +static void ga_enable_non_blacklisted(GList *blacklist) +{ + char **list_head, **list; + + list_head = list = qmp_get_command_list(); + while (*list != NULL) { + if (g_list_find_custom(blacklist, *list, ga_strcmp) == NULL && + !qmp_command_is_enabled(*list)) { + g_debug("enabling command: %s", *list); + qmp_enable_command(*list); + } + g_free(*list); + list++; + } + g_free(list_head); +} + +bool ga_is_frozen(GAState *s) +{ + return s->frozen; +} + +void ga_set_frozen(GAState *s) +{ + if (ga_is_frozen(s)) { + return; + } + /* disable all non-whitelisted (for frozen state) commands */ + ga_disable_non_whitelisted(); + g_warning("disabling logging due to filesystem freeze"); + ga_disable_logging(s); + s->frozen = true; +} + +void ga_unset_frozen(GAState *s) +{ + if (!ga_is_frozen(s)) { + return; + } + + ga_enable_logging(s); + g_warning("logging re-enabled"); + + /* enable all disabled, non-blacklisted commands */ + ga_enable_non_blacklisted(s->blacklist); + s->frozen = false; +} + #ifndef _WIN32 static void become_daemon(const char *pidfile) { @@ -513,12 +615,13 @@ int main(int argc, char **argv) { "blacklist", 1, NULL, 'b' }, #ifdef _WIN32 { "service", 1, NULL, 's' }, -#endif +#endif { NULL, 0, NULL, 0 } }; int opt_ind = 0, ch, daemonize = 0, i, j, len; GLogLevelFlags log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL; FILE *log_file = stderr; + GList *blacklist = NULL; GAState *s; module_call_init(MODULE_INIT_QAPI); @@ -568,14 +671,12 @@ int main(int argc, char **argv) for (j = 0, i = 0, len = strlen(optarg); i < len; i++) { if (optarg[i] == ',') { optarg[i] = 0; - qmp_disable_command(&optarg[j]); - g_debug("disabling command: %s", &optarg[j]); + blacklist = g_list_append(blacklist, &optarg[j]); j = i + 1; } } if (j < i) { - qmp_disable_command(&optarg[j]); - g_debug("disabling command: %s", &optarg[j]); + blacklist = g_list_append(blacklist, &optarg[j]); } break; } @@ -615,6 +716,15 @@ int main(int argc, char **argv) g_log_set_default_handler(ga_log, s); g_log_set_fatal_mask(NULL, G_LOG_LEVEL_ERROR); s->logging_enabled = true; + s->frozen = false; + if (blacklist) { + s->blacklist = blacklist; + do { + g_debug("disabling command: %s", (char *)blacklist->data); + qmp_disable_command(blacklist->data); + blacklist = g_list_next(blacklist); + } while (blacklist); + } s->command_state = ga_command_state_new(); ga_command_state_init(s, s->command_state); ga_command_state_init_all(s->command_state); diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 6c185f4..c2144a4 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -351,16 +351,6 @@ static void guest_file_init(void) #if defined(CONFIG_FSFREEZE) -static void disable_logging(void) -{ - ga_disable_logging(ga_state); -} - -static void enable_logging(void) -{ - ga_enable_logging(ga_state); -} - typedef struct GuestFsfreezeMount { char *dirname; char *devtype; @@ -369,10 +359,6 @@ typedef struct GuestFsfreezeMount { typedef QTAILQ_HEAD(, GuestFsfreezeMount) GuestFsfreezeMountList; -struct { - GuestFsfreezeStatus status; -} guest_fsfreeze_state; - static void guest_fsfreeze_free_mount_list(GuestFsfreezeMountList *mounts) { GuestFsfreezeMount *mount, *temp; @@ -435,7 +421,11 @@ static int guest_fsfreeze_build_mount_list(GuestFsfreezeMountList *mounts) */ GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **err) { - return guest_fsfreeze_state.status; + if (ga_is_frozen(ga_state)) { + return GUEST_FSFREEZE_STATUS_FROZEN; + } + + return GUEST_FSFREEZE_STATUS_THAWED; } /* @@ -459,7 +449,7 @@ int64_t qmp_guest_fsfreeze_freeze(Error **err) } /* cannot risk guest agent blocking itself on a write in this state */ - disable_logging(); + ga_set_frozen(ga_state); QTAILQ_FOREACH(mount, &mounts, next) { fd = qemu_open(mount->dirname, O_RDONLY); @@ -494,7 +484,6 @@ int64_t qmp_guest_fsfreeze_freeze(Error **err) close(fd); } - guest_fsfreeze_state.status = GUEST_FSFREEZE_STATUS_FROZEN; guest_fsfreeze_free_mount_list(&mounts); return i; @@ -554,23 +543,17 @@ int64_t qmp_guest_fsfreeze_thaw(Error **err) close(fd); } - guest_fsfreeze_state.status = GUEST_FSFREEZE_STATUS_THAWED; - enable_logging(); + ga_unset_frozen(ga_state); guest_fsfreeze_free_mount_list(&mounts); return i; } -static void guest_fsfreeze_init(void) -{ - guest_fsfreeze_state.status = GUEST_FSFREEZE_STATUS_THAWED; -} - static void guest_fsfreeze_cleanup(void) { int64_t ret; Error *err = NULL; - if (guest_fsfreeze_state.status == GUEST_FSFREEZE_STATUS_FROZEN) { + if (ga_is_frozen(ga_state) == GUEST_FSFREEZE_STATUS_FROZEN) { ret = qmp_guest_fsfreeze_thaw(&err); if (ret < 0 || err) { slog("failed to clean up frozen filesystems"); @@ -999,7 +982,7 @@ int64_t qmp_guest_fsfreeze_thaw(Error **err) void ga_command_state_init(GAState *s, GACommandState *cs) { #if defined(CONFIG_FSFREEZE) - ga_command_state_add(cs, guest_fsfreeze_init, guest_fsfreeze_cleanup); + ga_command_state_add(cs, NULL, guest_fsfreeze_cleanup); #endif ga_command_state_add(cs, guest_file_init, NULL); } diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h index 304525d..bbb8b9b 100644 --- a/qga/guest-agent-core.h +++ b/qga/guest-agent-core.h @@ -32,3 +32,6 @@ void ga_disable_logging(GAState *s); void ga_enable_logging(GAState *s); void slog(const gchar *fmt, ...); void ga_set_response_delimited(GAState *s); +bool ga_is_frozen(GAState *s); +void ga_set_frozen(GAState *s); +void ga_unset_frozen(GAState *s); -- 1.7.10.4