From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=55176 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PPjUA-00079O-7k for qemu-devel@nongnu.org; Mon, 06 Dec 2010 17:24:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PPjU9-000082-1z for qemu-devel@nongnu.org; Mon, 06 Dec 2010 17:24:42 -0500 Received: from e7.ny.us.ibm.com ([32.97.182.137]:40170) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PPjU8-00007w-T4 for qemu-devel@nongnu.org; Mon, 06 Dec 2010 17:24:41 -0500 Received: from d01dlp01.pok.ibm.com (d01dlp01.pok.ibm.com [9.56.224.56]) by e7.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id oB6M6qCf007762 for ; Mon, 6 Dec 2010 17:07:08 -0500 Received: from d01relay07.pok.ibm.com (d01relay07.pok.ibm.com [9.56.227.147]) by d01dlp01.pok.ibm.com (Postfix) with ESMTP id DBFDC728051 for ; Mon, 6 Dec 2010 17:24:38 -0500 (EST) Received: from d01av01.pok.ibm.com (d01av01.pok.ibm.com [9.56.224.215]) by d01relay07.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id oB6MOcwf1773810 for ; Mon, 6 Dec 2010 17:24:38 -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 oB6MOb5s028399 for ; Mon, 6 Dec 2010 17:24:37 -0500 Message-ID: <4CFD62A4.2040002@linux.vnet.ibm.com> Date: Mon, 06 Dec 2010 16:24:36 -0600 From: Michael Roth MIME-Version: 1.0 References: <1291399402-20366-1-git-send-email-mdroth@linux.vnet.ibm.com> <1291399402-20366-4-git-send-email-mdroth@linux.vnet.ibm.com> <1291672629.2213.4.camel@aglitke> In-Reply-To: <1291672629.2213.4.camel@aglitke> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [RFC][PATCH v5 03/21] virtagent: common code for managing client/server rpc jobs List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Adam Litke Cc: agl@linux.vnet.ibm.com, stefanha@linux.vnet.ibm.com, Jes.Sorensen@redhat.com, qemu-devel@nongnu.org, aliguori@linux.vnet.ibm.com, ryanh@us.ibm.com, abeekhof@redhat.com On 12/06/2010 03:57 PM, Adam Litke wrote: > On Fri, 2010-12-03 at 12:03 -0600, Michael Roth wrote: >> +/* create new client job and then put it on the queue. this can be >> + * called externally from virtagent. Since there can only be one virtagent >> + * instance we access state via an object-scoped global rather than pass >> + * it around. >> + * >> + * if this is successful virtagent will handle cleanup of req_xml after >> + * making the appropriate callbacks, otherwise callee should handle it >> + */ > > Explain please. Do you mean caller should handle it? Are you trying to > say that this function, when successful, "steals" the reference to > req_xml? > >> +int va_client_job_add(xmlrpc_mem_block *req_xml, VAClientCallback *cb, >> + MonitorCompletion *mon_cb, void *mon_data) >> +{ >> + int ret; >> + VAClientJob *client_job; >> + TRACE("called"); >> + >> + client_job = va_client_job_new(req_xml, cb, mon_cb, mon_data); >> + if (client_job == NULL) { >> + return -EINVAL; >> + } >> + >> + ret = va_push_client_job(client_job); >> + if (ret != 0) { >> + LOG("error adding client to queue: %s", strerror(ret)); >> + qemu_free(client_job); >> + return ret; >> + } >> + >> + return 0; >> +} >> + >> +/* create new server job and then put it on the queue in wait state >> + * this should only be called from within our read handler callback >> + */ > > Since this function is only 4 lines and has only one valid call site. > perhaps its better to fold it directly into the read handler callback. > >> +static int va_server_job_add(xmlrpc_mem_block *resp_xml) >> +{ >> + VAServerJob *server_job; >> + TRACE("called"); >> + >> + server_job = va_server_job_new(resp_xml); >> + assert(server_job != NULL); >> + va_push_server_job(server_job); >> + return 0; >> +} > What I was mainly shooting for was to have the entry-points for adding client and server jobs be clear and somewhat similar. I've actually moved the read handler callback body into virtagent-server.c:va_do_server_rpc() since then. So client jobs get added by hmp/qmp->virtagent:va_do_rpc()->va_push_client_job() and server jobs by read handler->virtagent-server.c:va_do_server_rpc()->va_push_server_job().