* [PATCH] libxl: only query VNC when enabled
@ 2020-10-01 23:53 Jason Andryuk
2020-10-07 10:50 ` Wei Liu
0 siblings, 1 reply; 5+ messages in thread
From: Jason Andryuk @ 2020-10-01 23:53 UTC (permalink / raw)
To: xen-devel; +Cc: Jason Andryuk, Ian Jackson, Wei Liu
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@gmail.com>
---
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;
+
qmp->callback = device_model_postconfig_vnc;
rc = libxl__ev_qmp_send(egc, qmp, "query-vnc", NULL);
if (rc) goto out;
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] libxl: only query VNC when enabled
2020-10-01 23:53 [PATCH] libxl: only query VNC when enabled Jason Andryuk
@ 2020-10-07 10:50 ` Wei Liu
2020-10-08 13:31 ` Jason Andryuk
0 siblings, 1 reply; 5+ messages in thread
From: Wei Liu @ 2020-10-07 10:50 UTC (permalink / raw)
To: Jason Andryuk; +Cc: xen-devel, Ian Jackson, Wei Liu
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@gmail.com>
> ---
> 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?
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);
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] libxl: only query VNC when enabled
2020-10-07 10:50 ` Wei Liu
@ 2020-10-08 13:31 ` Jason Andryuk
2020-10-08 15:25 ` Jason Andryuk
2020-10-08 15:27 ` Wei Liu
0 siblings, 2 replies; 5+ messages in thread
From: Jason Andryuk @ 2020-10-08 13:31 UTC (permalink / raw)
To: Wei Liu; +Cc: xen-devel, Ian Jackson
On Wed, Oct 7, 2020 at 6:50 AM Wei Liu <wl@xen.org> 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@gmail.com>
> > ---
> > 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);
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] libxl: only query VNC when enabled
2020-10-08 13:31 ` Jason Andryuk
@ 2020-10-08 15:25 ` Jason Andryuk
2020-10-08 15:27 ` Wei Liu
1 sibling, 0 replies; 5+ messages in thread
From: Jason Andryuk @ 2020-10-08 15:25 UTC (permalink / raw)
To: Wei Liu; +Cc: xen-devel, Ian Jackson
On Thu, Oct 8, 2020 at 9:31 AM Jason Andryuk <jandryuk@gmail.com> wrote:
>
> On Wed, Oct 7, 2020 at 6:50 AM Wei Liu <wl@xen.org> 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@gmail.com>
> > > ---
> > > 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.
Oh, I should add that QEMU needs a small patch to allow -vnc none in
ui/vnc-stub.c when vnc is disabled. This is because libxl always
passes -vnc none to ensure a default vnc is not created.
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);
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] libxl: only query VNC when enabled
2020-10-08 13:31 ` Jason Andryuk
2020-10-08 15:25 ` Jason Andryuk
@ 2020-10-08 15:27 ` Wei Liu
1 sibling, 0 replies; 5+ messages in thread
From: Wei Liu @ 2020-10-08 15:27 UTC (permalink / raw)
To: Jason Andryuk; +Cc: Wei Liu, xen-devel, Ian Jackson
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@xen.org> 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@gmail.com>
> > > ---
> > > 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@xen.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-10-08 15:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-01 23:53 [PATCH] libxl: only query VNC when enabled Jason Andryuk
2020-10-07 10:50 ` Wei Liu
2020-10-08 13:31 ` Jason Andryuk
2020-10-08 15:25 ` Jason Andryuk
2020-10-08 15:27 ` Wei Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).