Re: [Xen-devel] [libvirt] [PATCH 4/5] libxl: Implement basic video device selection

[Jim Fehlig <jfehlig@xxxxxxxx>]

John Ferlan wrote:
> Coverity complains...
> On 09/19/2014 03:23 PM, Jim Fehlig wrote:
>   
>> From: Stefan Bader <stefan.bader@xxxxxxxxxxxxx>
>>
>> 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@xxxxxxxxxxxxx>
>> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
>> ---
>>  src/libxl/libxl_conf.c   | 74 
>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>  src/libxl/libxl_domain.c | 22 ++++++++++++++
>>  2 files changed, 96 insertions(+)
>>
>>     
>
> <...snip...>
>
>   
>> 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;
>> +                }
>>     
>
> There's no break here..
>
>   
>> +        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;
>> +            }
>>     
>
> And of course here - not that it matters yet
>   

Opps.  Thanks for catching those.  Patch sent

https://www.redhat.com/archives/libvir-list/2014-October/msg00486.html

Regards,
Jim


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