All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Bader <stefan.bader@canonical.com>
To: libvir-list@redhat.com, xen-devel@lists.xen.org
Cc: Jim Fehlig <jfehlig@suse.com>, Ian Campbell <ian.campbell@citrix.com>
Subject: [PATCH 3/3] libxl: Implement basic video device selection
Date: Thu, 27 Mar 2014 17:55:04 +0100	[thread overview]
Message-ID: <1395939304-9017-4-git-send-email-stefan.bader@canonical.com> (raw)
In-Reply-To: <1395939304-9017-1-git-send-email-stefan.bader@canonical.com>

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@canonical.com>
---
 src/libxl/libxl_conf.c |   85 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index b8de72a..fdd2726 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1298,6 +1298,82 @@ libxlMakeCapabilities(libxl_ctx *ctx)
     return NULL;
 }
 
+static void
+libxlSetBuildGraphics(virDomainDefPtr def, libxl_domain_config *d_config)
+{
+    libxl_domain_build_info *b_info = &d_config->b_info;
+
+    /*
+     * Take the first defined video device (graphics card) to display
+     * on the first graphics device (display).
+     * Right now only type and vram info is used and anything beside
+     * type xen and vga is mapped to cirrus.
+     */
+    if (def->nvideos) {
+        unsigned int min_vram = 8 * 1024;
+
+        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;
+                /*
+                 * Libxl enforces a minimal VRAM size of 8M when using
+                 * LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL or
+                 * 16M for LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN.
+                 * Avoid build failures and go with the minimum if less
+                 * is specified.
+                 */
+                switch (b_info->device_model_version) {
+                    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+                        min_vram = 8 * 1024;
+                        break;
+                    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+                    default:
+                        min_vram = 16 * 1024;
+                }
+                break;
+            default:
+                /*
+                 * Ignore any other device type and use Cirrus. Again fix
+                 * up the minimal VRAM to what libxl expects.
+                 */
+                b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+                switch (b_info->device_model_version) {
+                    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+                        min_vram = 4 * 1024; /* Actually the max, too */
+                        break;
+                    case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+                    default:
+                        min_vram = 8 * 1024;
+                }
+        }
+        b_info->video_memkb = (def->videos[0]->vram > min_vram) ?
+                               def->videos[0]->vram :
+                               LIBXL_MEMKB_DEFAULT;
+    } else {
+        libxl_defbool_set(&b_info->u.hvm.nographic, 1);
+    }
+
+    /*
+     * When making the list of displays, only VNC and SDL types were
+     * taken into account. So it seems sensible to connect the default
+     * video device to the first in the vfb list.
+     *
+     * FIXME: Copy the structures and fixing the strings feels a bit dirty.
+     */
+    if (d_config->num_vfbs) {
+        libxl_device_vfb *vfb0 = &d_config->vfbs[0];
+
+	b_info->u.hvm.vnc = vfb0->vnc;
+        VIR_STRDUP(b_info->u.hvm.vnc.listen, vfb0->vnc.listen);
+        VIR_STRDUP(b_info->u.hvm.vnc.passwd, vfb0->vnc.passwd);
+        b_info->u.hvm.sdl = vfb0->sdl;
+        VIR_STRDUP(b_info->u.hvm.sdl.display, vfb0->sdl.display);
+        VIR_STRDUP(b_info->u.hvm.sdl.xauthority, vfb0->sdl.xauthority);
+        VIR_STRDUP(b_info->u.hvm.keymap, vfb0->keymap);
+    }
+}
+
 int
 libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
                        virDomainObjPtr vm, libxl_domain_config *d_config)
@@ -1325,6 +1401,15 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
     if (libxlMakePciList(def, d_config) < 0)
         return -1;
 
+    /*
+     * Now that any potential VFBs are defined, it is time to 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 (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM)
+        libxlSetBuildGraphics(def, d_config);
+
     d_config->on_reboot = def->onReboot;
     d_config->on_poweroff = def->onPoweroff;
     d_config->on_crash = def->onCrash;
-- 
1.7.9.5

  parent reply	other threads:[~2014-03-27 16:55 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1395939304-9017-1-git-send-email-stefan.bader@canonical.com>
2014-03-27 16:55 ` [PATCH 1/3] libxl: Use id from virDomainObj inside the driver Stefan Bader
2014-04-03 14:16   ` [libvirt] " Michal Privoznik
2014-03-27 16:55 ` [PATCH 2/3] libxl: Set disk format for empty cdrom device Stefan Bader
2014-04-03 14:16   ` [libvirt] " Michal Privoznik
2014-03-27 16:55 ` Stefan Bader [this message]
2014-04-03 14:16   ` [libvirt] [PATCH 3/3] libxl: Implement basic video device selection Michal Privoznik
2014-03-31 17:14 ` libxl fixes/improvements for libvirt Jim Fehlig
2014-04-03 15:45 ` [libvirt] " Michal Privoznik
     [not found] ` <533D8207.8040402@redhat.com>
2014-04-03 15:49   ` Stefan Bader
2014-04-04  9:36   ` [PATCH v2] libxl: Implement basic video device selection Stefan Bader
     [not found]   ` <1396604199-8198-1-git-send-email-stefan.bader@canonical.com>
2014-04-04  9:48     ` Ian Campbell
     [not found]     ` <1396604917.4211.184.camel@kazak.uk.xensource.com>
2014-04-04 10:31       ` Stefan Bader
     [not found]       ` <533E8A1A.4030904@canonical.com>
2014-04-04 10:34         ` Ian Campbell
2014-04-04 10:34         ` Stefan Bader
     [not found]         ` <1396607657.4211.190.camel@kazak.uk.xensource.com>
2014-04-04 10:52           ` Stefan Bader
2014-04-04 12:51           ` [libvirt] " Daniel P. Berrange
     [not found]           ` <20140404125113.GA13990@redhat.com>
2014-04-04 12:56             ` Ian Campbell
     [not found]             ` <1396616169.4211.222.camel@kazak.uk.xensource.com>
2014-04-04 13:08               ` Daniel P. Berrange
2014-04-04 13:10               ` Stefan Bader
2014-04-04 13:17     ` Daniel P. Berrange
     [not found]     ` <20140404131711.GD13990@redhat.com>
2014-04-04 13:36       ` Stefan Bader
     [not found]       ` <533EB566.7000903@canonical.com>
2014-04-04 13:51         ` Daniel P. Berrange

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1395939304-9017-4-git-send-email-stefan.bader@canonical.com \
    --to=stefan.bader@canonical.com \
    --cc=ian.campbell@citrix.com \
    --cc=jfehlig@suse.com \
    --cc=libvir-list@redhat.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.