[PATCH] libxl: Set VNC password through QMP

[PATCH] libxl: Set VNC password through QMP

[Anthony PERARD]

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@citrix.com>

---
 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);
+            }
         }
         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);
+        }
+        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;
-- 
tg: (6674c38..) qmp-vnc-password (depends on: master)

Re: [PATCH] libxl: Set VNC password through QMP

[Ian Campbell]

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@citrix.com>
> 
> ---
>  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;

Re: [PATCH] libxl: Set VNC password through QMP

[Anthony PERARD]

On Wed, 8 Feb 2012, Ian Campbell wrote:

> 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@citrix.com>
> >
> > ---
> >  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 ;-)

:), I forgot that.

> 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?

Yes, I'll do that.

> > +            }
> >          }
> >          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.

QEMU will search for an open port until it reach the vnc display port 99
(5999 TCP port).

For the 99, I probably read this number somewhere. But after checking in
QEMU code, I do not see any limitation for this number. So we could
probably change this number for the port number max (-5900, default vnc
port).

> > +        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;
>
>
>

-- 
Anthony PERARD

Re: [PATCH] libxl: Set VNC password through QMP

[Ian Campbell]

On Wed, 2012-02-08 at 17:24 +0000, Anthony PERARD wrote:
> On Wed, 8 Feb 2012, Ian Campbell wrote:
> > > @@ -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.
> 
> QEMU will search for an open port until it reach the vnc display port 99
> (5999 TCP port).
> 
> For the 99, I probably read this number somewhere. But after checking in
> QEMU code, I do not see any limitation for this number. So we could
> probably change this number for the port number max (-5900, default vnc
> port).

Can we not just omit it altogether and let qemu do its default thing?

Ian.

Re: [PATCH] libxl: Set VNC password through QMP

[Anthony PERARD]

On Thu, 9 Feb 2012, Ian Campbell wrote:

> On Wed, 2012-02-08 at 17:24 +0000, Anthony PERARD wrote:
> > On Wed, 8 Feb 2012, Ian Campbell wrote:
> > > > @@ -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.
> >
> > QEMU will search for an open port until it reach the vnc display port 99
> > (5999 TCP port).
> >
> > For the 99, I probably read this number somewhere. But after checking in
> > QEMU code, I do not see any limitation for this number. So we could
> > probably change this number for the port number max (-5900, default vnc
> > port).
>
> Can we not just omit it altogether and let qemu do its default thing?

I did not find any default configuration for a VNC display in QEMU. We
have to provide every option we want. The minimum is hostname:display,
and QEMU will only tries to bind this address, or fails and quit. So to
"simulated" -vncunused, we have to provide a ",to=i" options to try
every single port between 5900 and 5900+i, and this option needs to be
last :(.

I hope this respond to your question.

-- 
Anthony PERARD

Re: [PATCH] libxl: Set VNC password through QMP

[Ian Campbell]

On Thu, 2012-02-09 at 10:58 +0000, Anthony PERARD wrote:
> On Thu, 9 Feb 2012, Ian Campbell wrote:
> 
> > On Wed, 2012-02-08 at 17:24 +0000, Anthony PERARD wrote:
> > > On Wed, 8 Feb 2012, Ian Campbell wrote:
> > > > > @@ -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.
> > >
> > > QEMU will search for an open port until it reach the vnc display port 99
> > > (5999 TCP port).
> > >
> > > For the 99, I probably read this number somewhere. But after checking in
> > > QEMU code, I do not see any limitation for this number. So we could
> > > probably change this number for the port number max (-5900, default vnc
> > > port).
> >
> > Can we not just omit it altogether and let qemu do its default thing?
> 
> I did not find any default configuration for a VNC display in QEMU. We
> have to provide every option we want. The minimum is hostname:display,
> and QEMU will only tries to bind this address, or fails and quit. So to
> "simulated" -vncunused, we have to provide a ",to=i" options to try
> every single port between 5900 and 5900+i, and this option needs to be
> last :(.

I see, I didn't realise the upstream qemu was missing a "just find me a
port" option.

> I hope this respond to your question.

Yes, thanks.

Re: [PATCH] libxl: Set VNC password through QMP

[chris]

[-- Attachment #1.1: Type: text/plain, Size: 2966 bytes --]

Any chance we can have an option to set a static VNC port? :) Also, does
upstream qemu still have the issue where if your vnc connection goes stale
you lose vnc because it only takes a single client connection?

On Thu, Feb 9, 2012 at 6:17 AM, Ian Campbell <Ian.Campbell@citrix.com>wrote:

> On Thu, 2012-02-09 at 10:58 +0000, Anthony PERARD wrote:
> > On Thu, 9 Feb 2012, Ian Campbell wrote:
> >
> > > On Wed, 2012-02-08 at 17:24 +0000, Anthony PERARD wrote:
> > > > On Wed, 8 Feb 2012, Ian Campbell wrote:
> > > > > > @@ -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.
> > > >
> > > > QEMU will search for an open port until it reach the vnc display
> port 99
> > > > (5999 TCP port).
> > > >
> > > > For the 99, I probably read this number somewhere. But after
> checking in
> > > > QEMU code, I do not see any limitation for this number. So we could
> > > > probably change this number for the port number max (-5900, default
> vnc
> > > > port).
> > >
> > > Can we not just omit it altogether and let qemu do its default thing?
> >
> > I did not find any default configuration for a VNC display in QEMU. We
> > have to provide every option we want. The minimum is hostname:display,
> > and QEMU will only tries to bind this address, or fails and quit. So to
> > "simulated" -vncunused, we have to provide a ",to=i" options to try
> > every single port between 5900 and 5900+i, and this option needs to be
> > last :(.
>
> I see, I didn't realise the upstream qemu was missing a "just find me a
> port" option.
>
> > I hope this respond to your question.
>
> Yes, thanks.
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>

[-- Attachment #1.2: Type: text/html, Size: 4211 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

Re: [PATCH] libxl: Set VNC password through QMP

[Anthony PERARD]

On Thu, Feb 9, 2012 at 13:15, chris <tknchris@gmail.com> wrote:
> Any chance we can have an option to set a static VNC port? :)

It's already there ;-)
vncunused=0
vncdisplay=42

And QEMU will only try one port.

> Also, does
> upstream qemu still have the issue where if your vnc connection goes stale
> you lose vnc because it only takes a single client connection?

No, you can have several vnc client connected to QEMU.

Regards,

-- 
Anthony PERARD