libxl uses the libxl_vnc_info and libxl_sdl_info fields from the hvm union in libxl_domain_build_info struct when generating QEMU args for VNC or SDL. These fields were left unset by the libxl driver, causing libxl to ignore any user settings. E.g. with <graphics type='vnc' port='5950'/> port would be ignored and QEMU would instead be invoked with -vnc 127.0.0.1:0,to=99 Unlike the libxl_domain_config struct, the libxl_domain_build_info contains only a single libxl_vnc_info and libxl_sdl_info, so populate these fields from the first vfb in libxl_domain_config->vfbs. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; Signed-off-by: David Kiarie <davidkiarie4(a)gmail.com&gt; --- src/libxl/libxl_conf.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index aa5d958..cfac847 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1022,6 +1022,20 @@ libxlMakeVfbList(libxlDriverPrivatePtr driver, d_config->vkbs = x_vkbs; d_config->num_vfbs = d_config->num_vkbs = nvfbs; + /* + * VNC or SDL info must also be set in libxl_domain_build_info + * for HVM domains. Use the first vfb device. + */ + if (STREQ(def->os.type, "hvm")) { + libxl_domain_build_info *b_info = &d_config->b_info; + libxl_device_vfb vfb = d_config->vfbs[0]; + + if (libxl_defbool_val(vfb.vnc.enable)) + memcpy(&b_info->u.hvm.vnc, &vfb.vnc, sizeof(libxl_vnc_info)); + else if (libxl_defbool_val(vfb.sdl.enable)) + memcpy(&b_info->u.hvm.sdl, &vfb.sdl, sizeof(libxl_sdl_info)); + } + return 0; error:

Daniel P. Berrange wrote: > On Tue, Mar 18, 2014 at 12:19:47PM -0600, Jim Fehlig wrote: > >> libxl uses the libxl_vnc_info and libxl_sdl_info fields from the >> hvm union in libxl_domain_build_info struct when generating QEMU >> args for VNC or SDL. These fields were left unset by the libxl >> driver, causing libxl to ignore any user settings. E.g. with >> >> <graphics type='vnc' port='5950'/> >> >> port would be ignored and QEMU would instead be invoked with >> >> -vnc 127.0.0.1:0,to=99 >> >> Unlike the libxl_domain_config struct, the libxl_domain_build_info >> contains only a single libxl_vnc_info and libxl_sdl_info, so >> populate these fields from the first vfb in >> libxl_domain_config->vfbs. >> >> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; >> Signed-off-by: David Kiarie <davidkiarie4(a)gmail.com&gt; >> --- >> src/libxl/libxl_conf.c | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c >> index aa5d958..cfac847 100644 >> --- a/src/libxl/libxl_conf.c >> +++ b/src/libxl/libxl_conf.c >> @@ -1022,6 +1022,20 @@ libxlMakeVfbList(libxlDriverPrivatePtr driver, >> d_config->vkbs = x_vkbs; >> d_config->num_vfbs = d_config->num_vkbs = nvfbs; >> >> + /* >> + * VNC or SDL info must also be set in libxl_domain_build_info >> + * for HVM domains. Use the first vfb device. >> + */ >> + if (STREQ(def->os.type, "hvm")) { >> + libxl_domain_build_info *b_info = &d_config->b_info; >> + libxl_device_vfb vfb = d_config->vfbs[0]; >> + >> + if (libxl_defbool_val(vfb.vnc.enable)) >> + memcpy(&b_info->u.hvm.vnc, &vfb.vnc, sizeof(libxl_vnc_info)); >> + else if (libxl_defbool_val(vfb.sdl.enable)) >> + memcpy(&b_info->u.hvm.sdl, &vfb.sdl, sizeof(libxl_sdl_info)); >> + } >> + >> return 0; >> >> error: >> > > ACK. > Thanks, I'll push this shortly. > It sure would be nice if there were some kind of API in libxl which > took a 'libxl_domain_config*' and returned a char **argv string for > the QEMU command line args that it would later invoke. That would let > us create a test suite for the XML -> libxl_domain_config conversion > to spot these kind of problems. > Yes, agreed.

I meant to CC xen-devel on this patch (done now) since it is rather cumbersome to supply the same info in two places for HVM guests.

The improvement you suggest would also be very welcome. The libxl driver in general needs some in-tree testing love. Although I was a bit worried it would be too short for a GSoC project, I proposed this work as potential project through the openSUSE org https://en.opensuse.org/openSUSE:GSOC_ideas#Implement_a_test_driver_for_t... I've only had one semi-serious inquiry thus far, so looks like I'll be doing this myself during free cycles.