Re: [Qemu-devel] [RFC][PATCH v4 03/18] virtagent: qemu-vp, integrate virtagent server

[Jes Sorensen]

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