[libvirt] [PATCH] libxl: fix framebuffer port setting for HVM domains
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@suse.com> Signed-off-by: David Kiarie <davidkiarie4@gmail.com> --- 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: -- 1.8.1.4
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@suse.com> Signed-off-by: David Kiarie <davidkiarie4@gmail.com> --- 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. 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. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
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@suse.com> Signed-off-by: David Kiarie <davidkiarie4@gmail.com> --- 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_the_... I've only had one semi-serious inquiry thus far, so looks like I'll be doing this myself during free cycles. Regards, Jim
On Tue, 2014-03-18 at 15:12 -0600, Jim Fehlig wrote:
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@suse.com> Signed-off-by: David Kiarie <davidkiarie4@gmail.com> --- 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 don't think we would want to expose such a function to actual users of the library, but I think the infrastructure Ian added in eb5cdc52756e "libxl: events: Makefile builds internal unit tests" could be used to expose such a function only to the test application which needs it.
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.
TBH I thought the library did this copy already, but apparently not. Adding this would need to be done carefully since it is at least in principal possible today to have the emulated VGA vnc/sdl output entirely separate from the first PV FB.
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_the_...
I've only had one semi-serious inquiry thus far, so looks like I'll be doing this myself during free cycles.
Regardless of which org it ends up under It might be worth also adding it to http://wiki.xen.org/wiki/GSoc_2014 where it might get more eyes from people thinking about doing Xen stuff. Ian.
participants (3)
-
Daniel P. Berrange -
Ian Campbell -
Jim Fehlig