From 3cf0bed8369267184e5dc5b58882811519d67437 Mon Sep 17 00:00:00 2001 From: Michael Roth Date: Tue, 7 Feb 2012 13:56:48 -0600 Subject: [PATCH] qemu-ga: add guest-sync-delimited guest-sync leaves it as an exercise to the user as to how to reliably obtain the response to guest-sync if the client had previously read in a partial response (due qemu-ga previously being restarted mid-"sentence" due to reboot, forced restart, etc). qemu-ga handles this situation on its end by having a client precede their guest-sync request with a 0xFF byte (invalid UTF-8), which qemu-ga/QEMU JSON parsers will treat as a flush event. Thus we can reliably flush the qemu-ga parser state in preparation for receiving the guest-sync request. guest-sync-delimited provides the same functionality for a client: when a guest-sync-delimited is issued, qemu-ga will precede it's response with a 0xFF byte that the client can use as an indicator to flush its buffer/parser state in preparation for reliably receiving the guest-sync-delimited response. It is also useful as an optimization for clients, since, after issuing a guest-sync-delimited, clients can safely discard all stale data read from the channel until the 0xFF is found. More information available on the wiki: http://wiki.qemu.org/Features/QAPI/GuestAgent#QEMU_Guest_Agent_Protocol Signed-off-by: Michael Roth --- qapi-schema-guest.json | 48 +++++++++++++++++++++++++++++++++++++++++- qemu-ga.c | 27 +++++++++++++++++++----- qga/commands-posix.c | 3 --- qga/commands.c | 6 ++++++ qga/guest-agent-core.h | 2 ++ 5 files changed, 77 insertions(+), 9 deletions(-) diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json index 12b5d4fdef..cf18876c57 100644 --- a/qapi-schema-guest.json +++ b/qapi-schema-guest.json @@ -1,5 +1,40 @@ # *-*- Mode: Python -*-* +## +# +# Echo back a unique integer value, and prepend to response a +# leading sentinel byte (0xFF) the client can check scan for. +# +# This is used by clients talking to the guest agent over the +# wire to ensure the stream is in sync and doesn't contain stale +# data from previous client. It must be issued upon initial +# connection, and after any client-side timeouts (including +# timeouts on receiving a response to this command). +# +# After issuing this request, all guest agent responses should be +# ignored until the response containing the unique integer value +# the client passed in is returned. Receival of the 0xFF sentinel +# byte must be handled as an indication that the client's +# lexer/tokenizer/parser state should be flushed/reset in +# preparation for reliably receiving the subsequent response. As +# an optimization, clients may opt to ignore all data until a +# sentinel value is receiving to avoid unecessary processing of +# stale data. +# +# Similarly, clients should also precede this *request* +# with a 0xFF byte to make sure the guest agent flushes any +# partially read JSON data from a previous client connection. +# +# @id: randomly generated 64-bit integer +# +# Returns: The unique integer id passed in by the client +# +# Since: 1.1 +# ## +{ 'command': 'guest-sync-delimited' + 'data': { 'id': 'int' }, + 'returns': 'int' } + ## # @guest-sync: # @@ -13,8 +48,19 @@ # partially-delivered JSON text in such a way that this response # can be obtained. # +# In cases where a partial stale response was previously +# received by the client, this cannot always be done reliably. +# One particular scenario being if qemu-ga responses are fed +# character-by-character into a JSON parser. In these situations, +# using guest-sync-delimited may be optimal. +# +# For clients that fetch responses line by line and convert them +# to JSON objects, guest-sync should be sufficient, but note that +# in cases where the channel is dirty some attempts at parsing the +# response may result in a parser error. +# # Such clients should also precede this command -# with a 0xFF byte to make such the guest agent flushes any +# with a 0xFF byte to make sure the guest agent flushes any # partially read JSON data from a previous session. # # @id: randomly generated 64-bit integer diff --git a/qemu-ga.c b/qemu-ga.c index 1c90e6ef0d..d6f786e50d 100644 --- a/qemu-ga.c +++ b/qemu-ga.c @@ -41,6 +41,7 @@ #define QGA_VIRTIO_PATH_DEFAULT "\\\\.\\Global\\org.qemu.guest_agent.0" #endif #define QGA_PIDFILE_DEFAULT "/var/run/qemu-ga.pid" +#define QGA_SENTINEL_BYTE 0xFF struct GAState { JSONMessageParser parser; @@ -54,9 +55,10 @@ struct GAState { #ifdef _WIN32 GAService service; #endif + bool delimit_response; }; -static struct GAState *ga_state; +struct GAState *ga_state; #ifdef _WIN32 DWORD WINAPI service_ctrl_handler(DWORD ctrl, DWORD type, LPVOID data, @@ -198,6 +200,11 @@ static void ga_log(const gchar *domain, GLogLevelFlags level, } } +void ga_set_response_delimited(GAState *s) +{ + s->delimit_response = true; +} + #ifndef _WIN32 static void become_daemon(const char *pidfile) { @@ -254,7 +261,7 @@ fail: static int send_response(GAState *s, QObject *payload) { const char *buf; - QString *payload_qstr; + QString *payload_qstr, *response_qstr; GIOStatus status; g_assert(payload && s->channel); @@ -264,10 +271,20 @@ static int send_response(GAState *s, QObject *payload) return -EINVAL; } - qstring_append_chr(payload_qstr, '\n'); - buf = qstring_get_str(payload_qstr); + if (s->delimit_response) { + s->delimit_response = false; + response_qstr = qstring_new(); + qstring_append_chr(response_qstr, QGA_SENTINEL_BYTE); + qstring_append(response_qstr, qstring_get_str(payload_qstr)); + QDECREF(payload_qstr); + } else { + response_qstr = payload_qstr; + } + + qstring_append_chr(response_qstr, '\n'); + buf = qstring_get_str(response_qstr); status = ga_channel_write_all(s->channel, buf, strlen(buf)); - QDECREF(payload_qstr); + QDECREF(response_qstr); if (status != G_IO_STATUS_NORMAL) { return -EIO; } diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 5b77fa9eee..7b2be2f936 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -35,8 +35,6 @@ #include "qemu-queue.h" #include "host-utils.h" -static GAState *ga_state; - static void reopen_fd_to_null(int fd) { int nullfd; @@ -909,7 +907,6 @@ error: /* register init/cleanup routines for stateful command groups */ void ga_command_state_init(GAState *s, GACommandState *cs) { - ga_state = s; #if defined(CONFIG_FSFREEZE) ga_command_state_add(cs, guest_fsfreeze_init, guest_fsfreeze_cleanup); #endif diff --git a/qga/commands.c b/qga/commands.c index b27407d5d7..5bcceaae34 100644 --- a/qga/commands.c +++ b/qga/commands.c @@ -29,6 +29,12 @@ void slog(const gchar *fmt, ...) va_end(ap); } +int64_t qmp_guest_sync_delimited(int64_t id, Error **errp) +{ + ga_set_response_delimited(ga_state); + return id; +} + int64_t qmp_guest_sync(int64_t id, Error **errp) { return id; diff --git a/qga/guest-agent-core.h b/qga/guest-agent-core.h index b5dfa5bbda..304525d3c2 100644 --- a/qga/guest-agent-core.h +++ b/qga/guest-agent-core.h @@ -18,6 +18,7 @@ typedef struct GAState GAState; typedef struct GACommandState GACommandState; +extern GAState *ga_state; void ga_command_state_init(GAState *s, GACommandState *cs); void ga_command_state_add(GACommandState *cs, @@ -30,3 +31,4 @@ bool ga_logging_enabled(GAState *s); void ga_disable_logging(GAState *s); void ga_enable_logging(GAState *s); void slog(const gchar *fmt, ...); +void ga_set_response_delimited(GAState *s);