Re: [Qemu-devel] [RFC][PATCH v2 12/17] guest agent: worker thread class

[Jes Sorensen <Jes.Sorensen@redhat.com>]

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