From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:52345) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QHF5r-00015u-EF for qemu-devel@nongnu.org; Tue, 03 May 2011 08:52:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QHF5m-0007ad-Im for qemu-devel@nongnu.org; Tue, 03 May 2011 08:52:47 -0400 Received: from mx1.redhat.com ([209.132.183.28]:2215) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QHF5m-0007aN-8x for qemu-devel@nongnu.org; Tue, 03 May 2011 08:52:42 -0400 Message-ID: <4DBFFA61.2020104@redhat.com> Date: Tue, 03 May 2011 14:51:45 +0200 From: Jes Sorensen MIME-Version: 1.0 References: <1303138953-1334-1-git-send-email-mdroth@linux.vnet.ibm.com> <4DAFFD05.20304@redhat.com> <4DB0376B.8010505@linux.vnet.ibm.com> In-Reply-To: <4DB0376B.8010505@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC][PATCH v2 00/11] QEMU Guest Agent: QMP-based host/guest communication (virtagent) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Roth Cc: aliguori@linux.vnet.ibm.com, agl@linux.vnet.ibm.com, qemu-devel@nongnu.org On 04/21/11 15:55, Michael Roth wrote: >> Did you do anything with the fsfreeze patches, or were they dropped in >> the migration to qapi? > > They were pending some changes required on the agent side that weren't > really addressed/doable until this patchset, namely: > > 1) server-side timeout mechanism to recover from RPCs that can hang > indefinitely or take a really long time (fsfreeze, fopen, etc), > currently it's 30 seconds, may need to bump it to 60 for fsfreeze, or > potentially add an RPC to change the server-side timeout > 2) a simple way to temporarily turn off logging so agent doesn't > deadlock itself > 3) a way to register a cleanup handler when a timeout occurs. > 4) disabling RPCs where proper accounting/logging is required > (guest-open-file, guest-shutdown, etc) > > #4 isn't implemented...I think this could be done fairly in-evasively > with something like: > > Response important_rpc(): > if (!ga_log("syslog", LEVEL_CRITICAL, "important stuff happening")) > return ERROR_LOGGING_CURRENTLY_DISABLED Either that, or maybe simply disable the full command while the freeze is in progress? I fear we're more likely to miss a case of checking for logging than we are to miss command disabling? It should still be very non evasive, maybe just a flag in the struct declaring the functions marking it as logging-required and if the no-logging flag is set, the command is made to wait, or return -EAGAIN > > bool ga_log(log_domain, level, msg): > if (log_domain == "syslog") > if (!logging_enabled && is_critical(log_level)) > return False; > syslog(msg, ...) > else > if (logging_enabled) > normallog(msg, ...) > return True > > With that I think we could actually drop the fsfreeze stuff in. Thoughts? IMHO it is better to disable the commands rather than just logging, but either way should allow it to drop in. Sorry for the late reply, been a bit swamped here. Cheers, Jes