From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:41422)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <Jes.Sorensen@redhat.com>) id 1QCpVv-0001qn-Fp
	for qemu-devel@nongnu.org; Thu, 21 Apr 2011 04:45:28 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <Jes.Sorensen@redhat.com>) id 1QCpVu-0006ye-EP
	for qemu-devel@nongnu.org; Thu, 21 Apr 2011 04:45:27 -0400
Received: from mx1.redhat.com ([209.132.183.28]:28360)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <Jes.Sorensen@redhat.com>) id 1QCpVu-0006xy-2W
	for qemu-devel@nongnu.org; Thu, 21 Apr 2011 04:45:26 -0400
Message-ID: <4DAFEE7F.5030607@redhat.com>
Date: Thu, 21 Apr 2011 10:44:47 +0200
From: Jes Sorensen <Jes.Sorensen@redhat.com>
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>
In-Reply-To: <1303138953-1334-13-git-send-email-mdroth@linux.vnet.ibm.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [RFC][PATCH v2 12/17] guest agent: worker thread
	class
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Michael Roth <mdroth@linux.vnet.ibm.com>
Cc: aliguori@linux.vnet.ibm.com, agl@linux.vnet.ibm.com, qemu-devel@nongnu.org

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      <mdroth@linux.vnet.ibm.com>
> + *
> + * 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 <glib.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <stdbool.h>
> +#include <pthread.h>
> +#include <errno.h>
> +#include <string.h>
> +#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_*

Jes