From 333ffa0cb6a222f69cf85a87ee28b40bced93584 Mon Sep 17 00:00:00 2001 Message-Id: <333ffa0cb6a222f69cf85a87ee28b40bced93584.1367947969.git.minovotn@redhat.com> In-Reply-To: <707b9b97153063374d2530e72c49b1499fc21af9.1367947969.git.minovotn@redhat.com> References: <707b9b97153063374d2530e72c49b1499fc21af9.1367947969.git.minovotn@redhat.com> From: Michal Novotny Date: Tue, 7 May 2013 18:36:02 +0200 Subject: [PATCH 004/114] Revert "qemu-ga: use key-value store to avoid recycling fd handles after restart" This reverts commit 329b4240362d11aeb4856e1ce31f181600a9e60c. Reverting as asked by Laszlo in message <51892739.2030807@redhat.com> because of the ordering issue (most likely) related to supersed testing. Signed-off-by: Michal Novotny --- qemu-ga.c | 184 ------------------------------------------------- qga/commands-posix.c | 25 ++----- qga/guest-agent-core.h | 1 - 3 files changed, 6 insertions(+), 204 deletions(-) diff --git a/qemu-ga.c b/qemu-ga.c index 5d19991..0441ce0 100644 --- a/qemu-ga.c +++ b/qemu-ga.c @@ -15,7 +15,6 @@ #include #include #include -#include #ifndef _WIN32 #include #include @@ -31,7 +30,6 @@ #include "qerror.h" #include "qapi/qmp-core.h" #include "qga/channel.h" -#include "bswap.h" #ifdef _WIN32 #include "qga/service-win32.h" #include @@ -55,11 +53,6 @@ #endif #define QGA_SENTINEL_BYTE 0xFF -typedef struct GAPersistentState { -#define QGA_PSTATE_DEFAULT_FD_COUNTER 1000 - int64_t fd_counter; -} GAPersistentState; - struct GAState { JSONMessageParser parser; GMainLoop *main_loop; @@ -83,8 +76,6 @@ struct GAState { #ifdef CONFIG_FSFREEZE const char *fsfreeze_hook; #endif - const gchar *pstate_filepath; - GAPersistentState pstate; }; struct GAState *ga_state; @@ -734,171 +725,6 @@ VOID WINAPI service_main(DWORD argc, TCHAR *argv[]) } #endif -static void set_persistent_state_defaults(GAPersistentState *pstate) -{ - g_assert(pstate); - pstate->fd_counter = QGA_PSTATE_DEFAULT_FD_COUNTER; -} - -static void persistent_state_from_keyfile(GAPersistentState *pstate, - GKeyFile *keyfile) -{ - g_assert(pstate); - g_assert(keyfile); - /* if any fields are missing, either because the file was tampered with - * by agents of chaos, or because the field wasn't present at the time the - * file was created, the best we can ever do is start over with the default - * values. so load them now, and ignore any errors in accessing key-value - * pairs - */ - set_persistent_state_defaults(pstate); - - if (g_key_file_has_key(keyfile, "global", "fd_counter", NULL)) { - pstate->fd_counter = - g_key_file_get_integer(keyfile, "global", "fd_counter", NULL); - } -} - -static void persistent_state_to_keyfile(const GAPersistentState *pstate, - GKeyFile *keyfile) -{ - g_assert(pstate); - g_assert(keyfile); - - g_key_file_set_integer(keyfile, "global", "fd_counter", pstate->fd_counter); -} - -static gboolean write_persistent_state(const GAPersistentState *pstate, - const gchar *path) -{ - GKeyFile *keyfile = g_key_file_new(); - GError *gerr = NULL; - gboolean ret = true; - gchar *data = NULL; - gsize data_len; - - g_assert(pstate); - - persistent_state_to_keyfile(pstate, keyfile); - data = g_key_file_to_data(keyfile, &data_len, &gerr); - if (gerr) { - g_critical("failed to convert persistent state to string: %s", - gerr->message); - ret = false; - goto out; - } - - g_file_set_contents(path, data, data_len, &gerr); - if (gerr) { - g_critical("failed to write persistent state to %s: %s", - path, gerr->message); - ret = false; - goto out; - } - -out: - if (gerr) { - g_error_free(gerr); - } - if (keyfile) { - g_key_file_free(keyfile); - } - g_free(data); - return ret; -} - -static gboolean read_persistent_state(GAPersistentState *pstate, - const gchar *path, gboolean frozen) -{ - GKeyFile *keyfile = NULL; - GError *gerr = NULL; - struct stat st; - gboolean ret = true; - - g_assert(pstate); - - if (stat(path, &st) == -1) { - /* it's okay if state file doesn't exist, but any other error - * indicates a permissions issue or some other misconfiguration - * that we likely won't be able to recover from. - */ - if (errno != ENOENT) { - g_critical("unable to access state file at path %s: %s", - path, strerror(errno)); - ret = false; - goto out; - } - - /* file doesn't exist. initialize state to default values and - * attempt to save now. (we could wait till later when we have - * modified state we need to commit, but if there's a problem, - * such as a missing parent directory, we want to catch it now) - * - * there is a potential scenario where someone either managed to - * update the agent from a version that didn't use a key store - * while qemu-ga thought the filesystem was frozen, or - * deleted the key store prior to issuing a fsfreeze, prior - * to restarting the agent. in this case we go ahead and defer - * initial creation till we actually have modified state to - * write, otherwise fail to recover from freeze. - */ - set_persistent_state_defaults(pstate); - if (!frozen) { - ret = write_persistent_state(pstate, path); - if (!ret) { - g_critical("unable to create state file at path %s", path); - ret = false; - goto out; - } - } - ret = true; - goto out; - } - - keyfile = g_key_file_new(); - g_key_file_load_from_file(keyfile, path, 0, &gerr); - if (gerr) { - g_critical("error loading persistent state from path: %s, %s", - path, gerr->message); - ret = false; - goto out; - } - - persistent_state_from_keyfile(pstate, keyfile); - -out: - if (keyfile) { - g_key_file_free(keyfile); - } - if (gerr) { - g_error_free(gerr); - } - - return ret; -} - -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 - * writing to disk when we're in a frozen state. this includes opening - * new files, so we should never get here in that situation - */ - g_assert(!ga_is_frozen(s)); - - handle = s->pstate.fd_counter++; - if (s->pstate.fd_counter < 0) { - s->pstate.fd_counter = 0; - } - if (!write_persistent_state(&s->pstate, s->pstate_filepath)) { - error_setg(errp, "failed to commit persistent state to disk"); - } - - return handle; -} - int main(int argc, char **argv) { const char *sopt = "hVvdm:p:l:f:F::b:s:t:"; @@ -1028,9 +854,7 @@ int main(int argc, char **argv) ga_enable_logging(s); s->state_filepath_isfrozen = g_strdup_printf("%s/qga.state.isfrozen", state_dir); - s->pstate_filepath = g_strdup_printf("%s/qga.state", state_dir); s->frozen = false; - #ifndef _WIN32 /* check if a previous instance of qemu-ga exited with filesystems' state * marked as frozen. this could be a stale value (a non-qemu-ga process @@ -1087,14 +911,6 @@ int main(int argc, char **argv) } } - /* load persistent state from disk */ - if (!read_persistent_state(&s->pstate, - s->pstate_filepath, - ga_is_frozen(s))) { - g_critical("failed to load persistent state"); - goto out_bad; - } - if (blacklist) { s->blacklist = blacklist; do { diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 8a6179c..77e3a17 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -205,22 +205,14 @@ static struct { QTAILQ_HEAD(, GuestFileHandle) filehandles; } guest_file_state; -static int64_t guest_file_handle_add(FILE *fh, Error **errp) +static void guest_file_handle_add(FILE *fh) { GuestFileHandle *gfh; - int64_t handle; - - handle = ga_get_fd_handle(ga_state, errp); - if (error_is_set(errp)) { - return 0; - } gfh = g_malloc0(sizeof(GuestFileHandle)); - gfh->id = handle; + gfh->id = fileno(fh); gfh->fh = fh; QTAILQ_INSERT_TAIL(&guest_file_state.filehandles, gfh, next); - - return handle; } static GuestFileHandle *guest_file_handle_find(int64_t id, Error **err) @@ -242,7 +234,7 @@ int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode, E { FILE *fh; int fd; - int64_t ret = -1, handle; + int64_t ret = -1; if (!has_mode) { mode = "r"; @@ -268,14 +260,9 @@ int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode, E return -1; } - handle = guest_file_handle_add(fh, err); - if (error_is_set(err)) { - fclose(fh); - return -1; - } - - slog("guest-file-open, handle: %d", handle); - return handle; + guest_file_handle_add(fh); + slog("guest-file-open, handle: %d", fd); + return fd; } void qmp_guest_file_close(int64_t handle, Error **err) diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h index 7f73d48..c6e8de0 100644 --- a/qga/guest-agent-core.h +++ b/qga/guest-agent-core.h @@ -35,7 +35,6 @@ bool ga_is_frozen(GAState *s); void ga_set_frozen(GAState *s); void ga_unset_frozen(GAState *s); const char *ga_fsfreeze_hook(GAState *s); -int64_t ga_get_fd_handle(GAState *s, Error **errp); #ifndef _WIN32 void reopen_fd_to_null(int fd); -- 1.7.11.7