From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:40718) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QCtjr-0000GE-Qm for qemu-devel@nongnu.org; Thu, 21 Apr 2011 09:16:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QCtjq-0001BS-GE for qemu-devel@nongnu.org; Thu, 21 Apr 2011 09:16:07 -0400 Received: from e6.ny.us.ibm.com ([32.97.182.146]:60649) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QCtjq-0001BG-DL for qemu-devel@nongnu.org; Thu, 21 Apr 2011 09:16:06 -0400 Received: from d01relay06.pok.ibm.com (d01relay06.pok.ibm.com [9.56.227.116]) by e6.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id p3LCpnXv025207 for ; Thu, 21 Apr 2011 08:51:49 -0400 Received: from d01av01.pok.ibm.com (d01av01.pok.ibm.com [9.56.224.215]) by d01relay06.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p3LDG1561445914 for ; Thu, 21 Apr 2011 09:16:01 -0400 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 p3LDG0st030741 for ; Thu, 21 Apr 2011 09:16:01 -0400 Message-ID: <4DB02E0E.5080809@linux.vnet.ibm.com> Date: Thu, 21 Apr 2011 08:15:58 -0500 From: Michael Roth MIME-Version: 1.0 References: <1303138953-1334-1-git-send-email-mdroth@linux.vnet.ibm.com> <1303138953-1334-13-git-send-email-mdroth@linux.vnet.ibm.com> <4DAFEE7F.5030607@redhat.com> In-Reply-To: <4DAFEE7F.5030607@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC][PATCH v2 12/17] guest agent: worker thread class List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jes Sorensen Cc: aliguori@linux.vnet.ibm.com, agl@linux.vnet.ibm.com, qemu-devel@nongnu.org On 04/21/2011 03:44 AM, Jes Sorensen wrote: > On 04/18/11 17:02, Michael Roth wrote: >> diff --git a/qga/guest-agent-worker.c b/qga/guest-agent-worker.c >> new file mode 100644 >> index 0000000..e3295da >> --- /dev/null >> +++ b/qga/guest-agent-worker.c >> @@ -0,0 +1,173 @@ >> +/* >> + * QEMU Guest Agent worker thread interfaces >> + * >> + * Copyright IBM Corp. 2011 >> + * >> + * Authors: >> + * Michael Roth >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >> + * See the COPYING file in the top-level directory. >> + */ >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include "guest-agent.h" >> +#include "../error.h" > > Oh dear! do not do that please! Fix the Makefile to include the > appropriate path. > >> +struct GAWorker { >> + pthread_t thread; >> + ga_worker_func execute; >> + pthread_mutex_t input_mutex; >> + pthread_cond_t input_avail_cond; >> + void *input; >> + bool input_avail; >> + pthread_mutex_t output_mutex; >> + pthread_cond_t output_avail_cond; > > You really should use QemuMutex and friends here. > >> + void *output; >> + Error *output_error; >> + bool output_avail; >> +}; >> + >> +static void *worker_run(void *worker_p) >> +{ >> + GAWorker *worker = worker_p; >> + Error *err; >> + void *input, *output; >> + >> + while (1) { >> + /* wait for input */ >> + pthread_mutex_lock(&worker->input_mutex); > > qemu_mutex_lock() > >> + while (!worker->input_avail) { >> + pthread_cond_wait(&worker->input_avail_cond,&worker->input_mutex); >> + } > > again > >> + input = worker->input; >> + worker->input_avail = false; >> + pthread_mutex_unlock(&worker->input_mutex); > > and again.... I'll stop. Basically there really should be no references > to pthread_* This is on the guest side of things where I'm trying to use GLib wherever possible to keep things somewhat portable: logging/list utilities/io events/etc. And I *really* wanted to use GThreads here, but the problem is that GThread does not have any sane means to kill off a thread when a timeout occurs: there's no analogue to pthread_cancel(), and to use signals you need to break the abstraction to get the underlying pid. The new QemuThread stuff is using GThread underneath the covers so same limitation there. pthreads provides these things and is fairly portable however, so I opted to make it an explicit dependency on the guest side. So glib+pthreads are the current dependencies. > > Jes