From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=37107 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PPz3Z-0005zm-Jz for qemu-devel@nongnu.org; Tue, 07 Dec 2010 10:02:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PPz3Y-0002IQ-6X for qemu-devel@nongnu.org; Tue, 07 Dec 2010 10:02:17 -0500 Received: from e5.ny.us.ibm.com ([32.97.182.145]:44869) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PPz3Y-0002II-2o for qemu-devel@nongnu.org; Tue, 07 Dec 2010 10:02:16 -0500 Received: from d01dlp02.pok.ibm.com (d01dlp02.pok.ibm.com [9.56.224.85]) by e5.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id oB7Ee2Sl012000 for ; Tue, 7 Dec 2010 09:40:04 -0500 Received: from d01relay05.pok.ibm.com (d01relay05.pok.ibm.com [9.56.227.237]) by d01dlp02.pok.ibm.com (Postfix) with ESMTP id DA8D94DE8070 for ; Tue, 7 Dec 2010 10:00:21 -0500 (EST) Received: from d01av01.pok.ibm.com (d01av01.pok.ibm.com [9.56.224.215]) by d01relay05.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id oB7F2B1H135154 for ; Tue, 7 Dec 2010 10:02:12 -0500 Received: from d01av01.pok.ibm.com (loopback [127.0.0.1]) by d01av01.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id oB7F2AMN024662 for ; Tue, 7 Dec 2010 10:02:11 -0500 Message-ID: <4CFE4C6D.9000201@linux.vnet.ibm.com> Date: Tue, 07 Dec 2010 09:02:05 -0600 From: Michael Roth MIME-Version: 1.0 Subject: Re: [Qemu-devel] Re: [RFC][PATCH v5 03/21] virtagent: common code for managing client/server rpc jobs References: <1291399402-20366-1-git-send-email-mdroth@linux.vnet.ibm.com> <1291399402-20366-4-git-send-email-mdroth@linux.vnet.ibm.com> <4CFE38CB.10003@redhat.com> In-Reply-To: <4CFE38CB.10003@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jes Sorensen Cc: agl@linux.vnet.ibm.com, stefanha@linux.vnet.ibm.com, abeekhof@redhat.com, qemu-devel@nongnu.org, aliguori@linux.vnet.ibm.com, ryanh@us.ibm.com On 12/07/2010 07:38 AM, Jes Sorensen wrote: > On 12/03/10 19:03, Michael Roth wrote: >> This implements a simple state machine to manage client/server rpc >> jobs being multiplexed over a single channel. >> >> A client job consists of sending an rpc request, reading an >> rpc response, then making the appropriate callbacks. We allow one >> client job to be processed at a time, which will make the following >> state transitions: >> >> VA_CLIENT_IDLE -> VA_CLIENT_SEND (job queued, send channel open) >> VA_CLIENT_SEND -> VA_CLIENT_WAIT (request sent, awaiting response) >> VA_CLIENT_WAIT -> VA_CLIENT_IDLE (response recieved, callbacks made) >> >> A server job consists of recieving an rpc request, generating a >> response, then sending the response. We expect to receive one server >> request at a time due to the 1 at a time restriction for client jobs. >> Server jobs make the following transitions: >> >> VA_SERVER_IDLE -> VA_SERVER_WAIT (recieved/executed request, send >> channel busy, response deferred) >> VA_SERVER_IDLE -> VA_SERVER_SEND (recieved/executed request, send >> channel open, sending response) >> VA_SERVER_WAIT -> VA_SERVER_SEND (send channel now open, sending >> response) >> VA_SERVER_SEND -> VA_SERVER_IDLE (response sent) >> >> Signed-off-by: Michael Roth > > As mentioned before, I really don't understand why this is part of QEMU, > the guest agent really should be able to run totally outside of QEMU. > >> + >> +#define DEBUG_VA >> + >> +#ifdef DEBUG_VA >> +#define TRACE(msg, ...) do { \ >> + fprintf(stderr, "%s:%s():L%d: " msg "\n", \ >> + __FILE__, __FUNCTION__, __LINE__, ## __VA_ARGS__); \ >> +} while(0) >> +#else >> +#define TRACE(msg, ...) \ >> + do { } while (0) >> +#endif >> + >> +#define LOG(msg, ...) do { \ >> + fprintf(stderr, "%s:%s(): " msg "\n", \ >> + __FILE__, __FUNCTION__, ## __VA_ARGS__); \ >> +} while(0) > > This must be like the 217th copy of these functions, could you please > use some of the code that is already in the tree, and make it generic if > needed. > >> + >> +#define VERSION "1.0" >> +#define EOL "\r\n" >> + >> +#define VA_HDR_LEN_MAX 4096 /* http header limit */ >> +#define VA_CONTENT_LEN_MAX 2*1024*1024 /* rpc/http send limit */ >> +#define VA_CLIENT_JOBS_MAX 5 /* max client rpcs we can queue */ >> +#define VA_SERVER_JOBS_MAX 1 /* max server rpcs we can queue */ > > As mentioned last time, please make this stuff configurable and not hard > coded. > Yup, definitely on the TODO. Should be in the next round. > Cheers, > Jes >