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

[Wei Liu <wl@xxxxxxx>]

On Thu, Oct 08, 2020 at 09:31:15AM -0400, Jason Andryuk wrote:
> 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.

Ah, my bad. I misread the code. By the time libxl enters
device_model_postconfig_vnc the query-vnc command will have already been
issued. So your patch is fine.

Acked-by: Wei Liu <wl@xxxxxxx>