qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] Re: [RFC][PATCH v5 03/21] virtagent: common code for managi


From: Michael Roth
Subject: [Qemu-devel] Re: [RFC][PATCH v5 03/21] virtagent: common code for managing client/server rpc jobs
Date: Mon, 06 Dec 2010 16:24:36 -0600
User-agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.2.12) Gecko/20101027 Thunderbird/3.1.6

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().



reply via email to

[Prev in Thread] Current Thread [Next in Thread]