From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=54494 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1PJ54Z-00026o-Hs
	for qemu-devel@nongnu.org; Thu, 18 Nov 2010 09:02:48 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <Jes.Sorensen@redhat.com>) id 1PJ54Y-0001Bq-9t
	for qemu-devel@nongnu.org; Thu, 18 Nov 2010 09:02:47 -0500
Received: from mx1.redhat.com ([209.132.183.28]:19376)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <Jes.Sorensen@redhat.com>) id 1PJ54X-0001Bi-W7
	for qemu-devel@nongnu.org; Thu, 18 Nov 2010 09:02:46 -0500
Message-ID: <4CE531FC.5090608@redhat.com>
Date: Thu, 18 Nov 2010 15:02:36 +0100
From: Jes Sorensen <Jes.Sorensen@redhat.com>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [RFC][PATCH v4 03/18] virtagent: qemu-vp, integrate
	virtagent server
References: <1289923320-5638-1-git-send-email-mdroth@linux.vnet.ibm.com>
	<1289923320-5638-4-git-send-email-mdroth@linux.vnet.ibm.com>
In-Reply-To: <1289923320-5638-4-git-send-email-mdroth@linux.vnet.ibm.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/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: <http://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, ryanh@us.ibm.com, agl@linux.vnet.ibm.com, qemu-devel@nongnu.org, abeekhof@redhat.com

On 11/16/10 17:01, Michael Roth wrote:
> +    bool enable_virtagent;
>  
>      chr->opaque = drv;
>      chr->chr_write = vp_chr_write;
> @@ -2025,9 +2029,31 @@ static CharDriverState *qemu_chr_open_virtproxy(QemuOpts *opts)
>      /* parse socket forwarding options */
>      qemu_opt_foreach(opts, vp_init_forwards, drv, 1);
>  
> +    /* add forwarding options to enable virtagent server */
> +    enable_virtagent = qemu_opt_get_bool(opts, "virtagent", 0);

int qemu_opt_get_bool(QemuOpts *opts, const char *name, int defval)

Sorry qemu_opt_get_bool() actually returns an int.

> diff --git a/qemu-vp.c b/qemu-vp.c
> index cfd2a69..38959e5 100644
> --- a/qemu-vp.c
> +++ b/qemu-vp.c
> @@ -37,6 +37,8 @@
>  #include "qemu-option.h"
>  #include "qemu_socket.h"
>  #include "virtproxy.h"
> +#include "virtagent.h"
> +#include "virtagent-daemon.h"
>  
>  static bool verbose_enabled = 0;

You don't need to initialize global variables to zero, the compiler does
that.


> +static int init_agent(const VPData *agent_iforward) {
> +    QemuOpts *opts = agent_iforward->opts;
> +    VPDriver *drv;
> +    int ret, index;
> +
> +    INFO("initializing agent...");
> +    if (verbose_enabled) {
> +        qemu_opts_print(opts, NULL);
> +    }
> +
> +    index = qemu_opt_get_number(agent_iforward->opts, "index", 0);
> +    drv = get_channel_drv(index);
> +    if (drv == NULL) {
> +        warnx("unable to find channel with index: %d", index);
> +        goto err;
> +    }
> +
> +    /* outbound RPCs */
> +    ret = va_client_init(drv, false);
> +    if (ret) {
> +        warnx("error starting RPC server");
> +        goto err;
> +    }
> +
> +    /* start guest RPC server */
> +    ret = va_server_init(drv, false);
> +    if (ret != 0) {
> +        warnx("error starting RPC server");
> +        goto err;
> +    }
> +
> +    return 0;
> +
> +err:
> +    return -1;
> +}

Please set appropriate error codes and return something meaningful
instead of just -1.

Cheers,
Jes