Commit 4dfc34c3 missed copying the user-specified keymap to libxl_domain_build_info struct when creating a VFB device. Signed-off-by: Stefan Bader <stefan.bader(a)canonical.com&gt; Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; --- src/libxl/libxl_conf.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index acba69c..0a781f9 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1200,6 +1200,9 @@ libxlMakeVfbList(virPortAllocatorPtr graphicsports, if (VIR_STRDUP(b_info->u.hvm.sdl.xauthority, vfb.sdl.xauthority) < 0) goto error; } + + if (VIR_STRDUP(b_info->u.hvm.keymap, vfb.keymap) < 0) + goto error; } return 0; -- 1.8.4.5

On 19.09.2014 21:23, Jim Fehlig wrote: Indentation's off. Michal

On 19.09.2014 21:23, Jim Fehlig wrote: Well, we've tried hard to move away from parsing 'qemu -help' output. But on the other hand, this is trivial compared to virQEMUCaps code we've developed. So I'd give green flag to this. Michal

From: Stefan Bader <stefan.bader(a)canonical.com&gt; This started as an investigation into an issue where libvirt (using the libxl driver) and the Xen host, like an old couple, could not agree on who is responsible for selecting the VNC port to use. Things usually (and a bit surprisingly) did work because, just like that old couple, they had the same idea on what to do by default. However it was possible that this ended up in a big argument. The problem is that display information exists in two different places: in the vfbs list and in the build info. And for launching the device model, only the latter is used. But that never gets initialized from libvirt. So Xen allows the device model to select a default port while libvirt thinks it has told Xen that this is done by libvirt (though the vfbs config). While fixing that, I made a stab at actually evaluating the configuration of the video device. So that it is now possible to at least decide between a Cirrus or standard VGA emulation and to modify the VRAM within certain limits using libvirt. Signed-off-by: Stefan Bader <stefan.bader(a)canonical.com&gt; Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; --- src/libxl/libxl_conf.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++ src/libxl/libxl_domain.c | 22 ++++++++++++++ 2 files changed, 96 insertions(+) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index ff3f6b5..09211f8 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1433,6 +1433,72 @@ libxlMakePCIList(virDomainDefPtr def, libxl_domain_config *d_config) return -1; } +static int +libxlMakeVideo(virDomainDefPtr def, libxl_domain_config *d_config) + +{ + libxl_domain_build_info *b_info = &d_config->b_info; + int dm_type = libxlDomainGetEmulatorType(def); + + if (d_config->c_info.type != LIBXL_DOMAIN_TYPE_HVM) + return 0; + + /* + * Take the first defined video device (graphics card) to display + * on the first graphics device (display). + */ + if (def->nvideos) { + switch (def->videos[0]->type) { + case VIR_DOMAIN_VIDEO_TYPE_VGA: + case VIR_DOMAIN_VIDEO_TYPE_XEN: + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD; + if (dm_type == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) { + if (def->videos[0]->vram < 16 * 1024) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("videoram must be at least 16MB for VGA")); + return -1; + } + } else { + if (def->videos[0]->vram < 8 * 1024) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("videoram must be at least 8MB for VGA")); + return -1; + } + } + break; + + case VIR_DOMAIN_VIDEO_TYPE_CIRRUS: + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS; + if (dm_type == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) { + if (def->videos[0]->vram < 8 * 1024) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("videoram must be at least 8MB for CIRRUS")); + return -1; + } + } else { + if (def->videos[0]->vram < 4 * 1024) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("videoram must be at least 4MB for CIRRUS")); + return -1; + } + } + break; + + default: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("video type %s is not supported by libxl"), + virDomainVideoTypeToString(def->videos[0]->type)); + return -1; + } + /* vram validated for each video type, now set it */ + b_info->video_memkb = def->videos[0]->vram; + } else { + libxl_defbool_set(&b_info->u.hvm.nographic, 1); + } + + return 0; +} + int libxlDriverNodeGetInfo(libxlDriverPrivatePtr driver, virNodeInfoPtr info) { @@ -1523,6 +1589,14 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, if (libxlMakePCIList(def, d_config) < 0) return -1; + /* + * Now that any potential VFBs are defined, update the build info with + * the data of the primary display. Some day libxl might implicitely do + * so but as it does not right now, better be explicit. + */ + if (libxlMakeVideo(def, d_config) < 0) + return -1; + d_config->on_reboot = libxlActionFromVirLifecycle(def->onReboot); d_config->on_poweroff = libxlActionFromVirLifecycle(def->onPoweroff); d_config->on_crash = libxlActionFromVirLifecycleCrash(def->onCrash); diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 557fc20..f2cd07b 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -510,6 +510,28 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, pcisrc->backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN; } + if (dev->type == VIR_DOMAIN_DEVICE_VIDEO && STREQ(def->os.type, "hvm")) { + int dm_type = libxlDomainGetEmulatorType(def); + + switch (dev->data.video->type) { + case VIR_DOMAIN_VIDEO_TYPE_VGA: + case VIR_DOMAIN_VIDEO_TYPE_XEN: + if (dev->data.video->vram == 0) { + if (dm_type == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) + dev->data.video->vram = 16 * 1024; + else + dev->data.video->vram = 8 * 1024; + } + case VIR_DOMAIN_VIDEO_TYPE_CIRRUS: + if (dev->data.video->vram == 0) { + if (dm_type == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) + dev->data.video->vram = 8 * 1024; + else + dev->data.video->vram = 4 * 1024; + } + } + } + return 0; } -- 1.8.4.5

John Ferlan wrote: Opps. Thanks for catching those. Patch sent https://www.redhat.com/archives/libvir-list/2014-October/msg00486.html Regards, Jim

On 19.09.2014 21:23, Jim Fehlig wrote: Do we need virFileIsExecutable too? Michal