Re: [PATCH] libxl: only query VNC when enabled

[Jason Andryuk <jandryuk@xxxxxxxxx>]

On Wed, Oct 7, 2020 at 6:50 AM Wei Liu <wl@xxxxxxx> wrote:
>
> On Thu, Oct 01, 2020 at 07:53:37PM -0400, Jason Andryuk wrote:
> > QEMU without VNC support (configure --disable-vnc) will return an error
> > when VNC is queried over QMP since it does not recognize the QMP
> > command.  This will cause libxl to fail starting the domain even if VNC
> > is not enabled.  Therefore only query QEMU for VNC support when using
> > VNC, so a VNC-less QEMU will function in this configuration.
> >
> > 'goto out' jumps to the call to device_model_postconfig_done(), the
> > final callback after the chain of vnc queries.  This bypasses all the
> > QMP VNC queries.
> >
> > Signed-off-by: Jason Andryuk <jandryuk@xxxxxxxxx>
> > ---
> >  tools/libs/light/libxl_dm.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
> > index a944181781..d1ff35dda3 100644
> > --- a/tools/libs/light/libxl_dm.c
> > +++ b/tools/libs/light/libxl_dm.c
> > @@ -3140,6 +3140,7 @@ static void 
> > device_model_postconfig_chardev(libxl__egc *egc,
> >  {
> >      EGC_GC;
> >      libxl__dm_spawn_state *dmss = CONTAINER_OF(qmp, *dmss, qmp);
> > +    const libxl_vnc_info *vnc = libxl__dm_vnc(dmss->guest_config);
> >      const libxl__json_object *item = NULL;
> >      const libxl__json_object *o = NULL;
> >      int i = 0;
> > @@ -3197,6 +3198,9 @@ static void 
> > device_model_postconfig_chardev(libxl__egc *egc,
> >          if (rc) goto out;
> >      }
> >
> > +    if (!vnc)
> > +        goto out;
> > +
>
> I would rather this check be done in device_model_postconfig_vnc.
>
> Does the following work for you?

I like your version, but it doesn't work:
libxl: debug: libxl_qmp.c:1883:libxl__ev_qmp_send: Domain 1: ev
0x55aa58417d88, cmd 'query-vnc'
libxl: error: libxl_qmp.c:1836:qmp_ev_parse_error_messages: Domain
1:The command query-vnc has not been found
libxl: error: libxl_dm.c:3321:device_model_postconfig_done: Domain
1:Post DM startup configs failed, rc=-29

When QEMU has vnc disabled, it doesn't recognize query-vnc.  I looked
at modifying qemu to support query-vnc even with --disable-vnc, but it
was messy to untangle the QMP definitions.  Since we are telling libxl
not to use VNC, it makes sense not to query about it.

Regards,
Jason

> diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
> index a944181781bb..c5db755a65d7 100644
> --- a/tools/libs/light/libxl_dm.c
> +++ b/tools/libs/light/libxl_dm.c
> @@ -3222,6 +3222,8 @@ static void device_model_postconfig_vnc(libxl__egc *egc,
>
>      if (rc) goto out;
>
> +    if (!vnc) goto out;
> +
>      /*
>       * query-vnc response:
>       * { 'enabled': 'bool', '*host': 'str', '*service': 'str' }
> @@ -3255,7 +3257,8 @@ static void device_model_postconfig_vnc(libxl__egc *egc,
>          if (rc) goto out;
>      }
>
> -    if (vnc && vnc->passwd && vnc->passwd[0]) {
> +    assert(vnc);
> +    if (vnc->passwd && vnc->passwd[0]) {
>          qmp->callback = device_model_postconfig_vnc_passwd;
>          libxl__qmp_param_add_string(gc, &args, "password", vnc->passwd);
>          rc = libxl__ev_qmp_send(egc, qmp, "change-vnc-password", args);
>