Re: [PATCH] libxl: Implement basic video device selection

[Stefan Bader <stefan.bader@canonical.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3522 bytes --]

On 19.09.2014 05:01, Jim Fehlig wrote:
> Stefan Bader wrote:
>> Re-pushing this as the old thread got rather stale.
> 
> Thanks.
> 
>>  Some of the
>> VFB setup went in a bug fix. Not sure I missed a detail in rebasing
>> bug the keyboard setting may be the only thing missing...
>>   
> 
> Yes, agreed.
> 
>> -Stefan
>>
>> [v2: Check return code of VIR_STRDUP and fix indentation]
>> [v3: Split out VRAM fixup and return error for unsupported video type]
>> [v4: Re-arrange code and move VFB setup into libxlMakeVfbList]
>> [v5: Rebased against head which already had some VFB setup code]
>>
>> >From b3ff8f4c658d29f15e673af88b9ae2fdfa3c1317 Mon Sep 17 00:00:00 2001
>> From: Stefan Bader <stefan.bader@canonical.com>
>> Date: Thu, 27 Mar 2014 16:01:18 +0100
>> Subject: [PATCH] libxl: Implement basic video device selection
>>
>> 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 | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 50 insertions(+)
>>   
> 
> This patch suffers the same issues as the last version.  And when
> commenting on that version, I promised to work on a followup to address
> my concerns
> 
> https://www.redhat.com/archives/libvir-list/2014-July/msg00931.html

Oh my, I have to admit I completely forgot about that.

> 
> Your repost poked me into reworking my first attempt, the result of
> which is below.  I should probably look at a sensible split-up of these
> patches that would be easier to review, but in the meantime comments on
> my followup would be appreciated.
> 
> With both patches, my tests are passing and my concerns are subdued :-).

There seem to be some parts of code suggesting support for anything beside Xen
(assumed to be alias to VGA) and Cirrus. As much as I know Xen handles only the
two (not qxl or anything else).

I believe the xen specific qemu variant was the only one called qemu-dm (and the
upstream variant always being qemu-system-i386). But checking the help message
probably is safer and not called that often so I should not worry about
performance).

I tried the variant you proposed and it looks to be ok. Still have to carry a
piece which is not going upstream if I wan't to paper over the virt-manager
issue. But at least now the failure is clearly showing what is wrong. So I am
happy enough. :)

-Stefan
> 
> Regards,
> Jim
>  
> 



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel