From patchwork Tue Feb 7 01:09:13 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Roth X-Patchwork-Id: 139869 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [140.186.70.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id F054EB7267 for ; Tue, 7 Feb 2012 12:09:43 +1100 (EST) Received: from localhost ([::1]:60269 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RuZYz-0000xN-IW for incoming@patchwork.ozlabs.org; Mon, 06 Feb 2012 20:09:41 -0500 Received: from eggs.gnu.org ([140.186.70.92]:32955) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RuZYm-0000Px-SH for qemu-devel@nongnu.org; Mon, 06 Feb 2012 20:09:30 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RuZYk-0001Up-Od for qemu-devel@nongnu.org; Mon, 06 Feb 2012 20:09:28 -0500 Received: from mail-pz0-f45.google.com ([209.85.210.45]:56588) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RuZYk-0001Uj-FO for qemu-devel@nongnu.org; Mon, 06 Feb 2012 20:09:26 -0500 Received: by dadp14 with SMTP id p14so7038829dad.4 for ; Mon, 06 Feb 2012 17:09:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=sender:from:to:cc:subject:date:message-id:x-mailer:in-reply-to :references; bh=KBmREMM4tYUOX3+WTsKOHL59RKxaM+IkvlqfYeRqdos=; b=ChYGjMH/4RPdugFUWxvaiReAgum4LNPbn3nVNzkdfyabDeg/a2NFm6JuXFc4QTV36b dfjIF0pspf5WOX45yXYMhGtZPmg29aJ9WGRjnc+RAU1wZTD+lpFg/Q7u/Q4BHULmtVzN QeuhBhm5YN4riKErl8wcccv+wHouVK0vD2q7k= Received: by 10.68.232.202 with SMTP id tq10mr52774529pbc.68.1328576965640; Mon, 06 Feb 2012 17:09:25 -0800 (PST) Received: from localhost.localdomain ([32.97.110.59]) by mx.google.com with ESMTPS id e10sm42650693pbv.0.2012.02.06.17.09.23 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 06 Feb 2012 17:09:24 -0800 (PST) From: Michael Roth To: qemu-devel@nongnu.org Date: Mon, 6 Feb 2012 19:09:13 -0600 Message-Id: <1328576953-19701-2-git-send-email-mdroth@linux.vnet.ibm.com> X-Mailer: git-send-email 1.7.4.1 In-Reply-To: <1328576953-19701-1-git-send-email-mdroth@linux.vnet.ibm.com> References: <1328576953-19701-1-git-send-email-mdroth@linux.vnet.ibm.com> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.85.210.45 Cc: libvir-list@redhat.com, mprivozn@redhat.com Subject: [Qemu-devel] [PATCH v2] qemu-ga: add guest-sync-delimited X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org 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 | 29 +++++++++++++++++++++++------ qga/commands-posix.c | 3 --- qga/commands.c | 6 ++++++ qga/guest-agent-core.h | 2 ++ 5 files changed, 78 insertions(+), 10 deletions(-) diff --git a/qapi-schema-guest.json b/qapi-schema-guest.json index 706925d..e2cb9ef 100644 --- a/qapi-schema-guest.json +++ b/qapi-schema-guest.json @@ -1,6 +1,41 @@ # *-*- 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: # # Echo back a unique integer value @@ -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 92f81ed..d567241 100644 --- a/qemu-ga.c +++ b/qemu-ga.c @@ -40,6 +40,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; @@ -53,9 +54,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, @@ -140,6 +142,11 @@ static const char *ga_log_level_str(GLogLevelFlags level) } } +void ga_set_response_delimited(GAState *s) +{ + s->delimit_response = true; +} + bool ga_logging_enabled(GAState *s) { return s->logging_enabled; @@ -236,8 +243,8 @@ fail: static int send_response(GAState *s, QObject *payload) { - const char *buf; - QString *payload_qstr; + const char *buf, *buf2; + QString *payload_qstr, *response_qstr; GIOStatus status; g_assert(payload && s->channel); @@ -247,10 +254,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 126127a..11963a1 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -28,8 +28,6 @@ #include "qerror.h" #include "qemu-queue.h" -static GAState *ga_state; - void qmp_guest_shutdown(bool has_mode, const char *mode, Error **err) { int ret; @@ -520,7 +518,6 @@ int64_t qmp_guest_fsfreeze_thaw(Error **err) /* 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 b27407d..5bcceaa 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 b5dfa5b..304525d 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);