Re: [Xen-devel] [PATCH] libxl: Set VNC password through QMP

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2012-02-08 at 11:49 +0000, Anthony PERARD wrote:
> This patch provide the code to set the VNC password to QEMU upstream through
> VNC. The password is still stored in xenstore but will not be used by QEMU
> upstream.
> 
> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> 
> ---
>  tools/libxl/libxl_create.c   |    3 +++
>  tools/libxl/libxl_dm.c       |   21 ++++++++++++---------
>  tools/libxl/libxl_internal.h |    1 +
>  tools/libxl/libxl_qmp.c      |   37 +++++++++++++++++++++++++++++++++++++
>  4 files changed, 53 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index ebf2ed7..ae0d668 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -619,6 +619,9 @@ static int do_domain_create(libxl__gc *gc, 
> libxl_domain_config *d_config,
>          if (dm_info->device_model_version
>              == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
>              libxl__qmp_initializations(ctx, domid);
> +            if (dm_info->vncpasswd) {
> +                libxl__qmp_vnc_password(gc, domid, dm_info->vncpasswd);

Is your tree up to date? I thought I'd removed dm_info ;-)

This seems like the sort of thing libxl__qmp_initializations ought to
handle. Perhaps we should pass the domain_config down and do it there?

> +            }
>          }
>          ret = libxl__confirm_device_model_startup(gc, dm_info, dm_starting);
>          if (ret < 0) {
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 97d91b4..6f7d0d5 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -271,10 +271,8 @@ static char ** 
> libxl__build_device_model_args_new(libxl__gc *gc,
>      if (info->vnc) {
>          int display = 0;
>          const char *listen = "127.0.0.1";
> +        char *vncarg = NULL;
>  
> -        if (info->vncpasswd && info->vncpasswd[0]) {
> -            assert(!"missing code for supplying vnc password to qemu");
> -        }
>          flexarray_append(dm_args, "-vnc");
>  
>          if (info->vncdisplay) {
> @@ -287,13 +285,16 @@ static char ** 
> libxl__build_device_model_args_new(libxl__gc *gc,
>          }
>  
>          if (strchr(listen, ':') != NULL)
> -            flexarray_append(dm_args,
> -                    libxl__sprintf(gc, "%s%s", listen,
> -                        info->vncunused ? ",to=99" : ""));
> +            vncarg = libxl__sprintf(gc, "%s", listen);
>          else
> -            flexarray_append(dm_args,
> -                    libxl__sprintf(gc, "%s:%d%s", listen, display,
> -                        info->vncunused ? ",to=99" : ""));
> +            vncarg = libxl__sprintf(gc, "%s:%d", listen, display);
> +        if (info->vncpasswd && info->vncpasswd[0]) {
> +            vncarg = libxl__sprintf(gc, "%s,password", vncarg);
> +        }
> +        if (info->vncunused) {
> +            vncarg = libxl__sprintf(gc, "%s,to=99", vncarg);

Not new, but I've been meaning to ask: what is to=99 for here? It seems
a bit arbitrary and the default behaviour without it appears to be to
keep looking for an available port which sounds like what we want.

> +        }
> +        flexarray_append(dm_args, vncarg);
>      }
>      if (info->sdl) {
>          flexarray_append(dm_args, "-sdl");
> @@ -862,6 +863,8 @@ int libxl__create_device_model(libxl__gc *gc,
>      }
>  
>      if (info->vncpasswd) {
> +        /* This xenstore key will only be used by qemu-xen-traditionnal.
> +         * The code to supply vncpasswd to qemu-xen is later. */
>  retry_transaction:
>          /* Find uuid and the write the vnc password to xenstore for qemu. */
>          t = xs_transaction_start(ctx->xsh);
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index fa7fb16..b33be99 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -592,6 +592,7 @@ _hidden int libxl__qmp_pci_del(libxl__gc *gc, int domid,
>                                 libxl_device_pci *pcidev);
>  /* Save current QEMU state into fd. */
>  _hidden int libxl__qmp_migrate(libxl__gc *gc, int domid, int fd);
> +_hidden int libxl__qmp_vnc_password(libxl__gc *gc, int domid, char 
> *password);
>  /* close and free the QMP handler */
>  _hidden void libxl__qmp_close(libxl__qmp_handler *qmp);
>  /* remove the socket file, if the file has already been removed,
> diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
> index 1777e44..274db19 100644
> --- a/tools/libxl/libxl_qmp.c
> +++ b/tools/libxl/libxl_qmp.c
> @@ -879,6 +879,43 @@ out:
>      return rc;
>  }
>  
> +static int qmp_change(libxl__gc *gc, int domid,
> +                      char *device, char *target, char *arg)
> +{
> +    libxl__qmp_handler *qmp = NULL;
> +    flexarray_t *parameters = NULL;
> +    libxl_key_value_list args = NULL;
> +    int rc = 0;
> +
> +    qmp = libxl__qmp_initialize(libxl__gc_owner(gc), domid);
> +    if (!qmp)
> +        return ERROR_FAIL;
> +
> +    parameters = flexarray_make(6, 1);
> +    flexarray_append_pair(parameters, "device", device);
> +    flexarray_append_pair(parameters, "target", target);
> +    if (arg)
> +        flexarray_append_pair(parameters, "arg", arg);
> +    args = libxl__xs_kvs_of_flexarray(gc, parameters, parameters->count);
> +    if (!args)
> +        return ERROR_NOMEM;
> +
> +    rc = qmp_synchronous_send(qmp, "change", &args,
> +                              NULL, NULL, qmp->timeout);
> +
> +    flexarray_free(parameters);
> +    libxl__qmp_close(qmp);
> +    return rc;
> +}
> +
> +int libxl__qmp_vnc_password(libxl__gc *gc, int domid, char *password)
> +{
> +    if (!password)
> +        return ERROR_FAIL;
> +
> +    return qmp_change(gc, domid, "vnc", "password", password);
> +}
> +
>  int libxl__qmp_initializations(libxl_ctx *ctx, uint32_t domid)
>  {
>      libxl__qmp_handler *qmp = NULL;



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel