Stefan Bader wrote:
On 16.07.2014 23:05, Jim Fehlig wrote:
> While testing this, I noticed that libvirt will set vram to 9216 if not
> specified. E.g.
>
> # cat test.xml
> ...
> <video>
> <model type='vga'/>
> </video>
> ...
> # virsh define test.xml
> # virsh dumpxml test
> ...
> <video>
> <model type='vga' vram='9216' heads='1'/>
> </video>
> ...
>
> With type='vga', libxl will then fail to start the domain
>
> libxl: error: libxl_create.c:253:libxl__domain_build_info_setdefault:
> videoram must be at least 16 MB for STDVGA on QEMU_XEN
>
Heh, thats "funny". At least this was the same thing I observed when creating
new guests with virt-manager. Maybe I blamed the wrong part of the stack. Or
they both do it. This lead the the second part of my changes which was not so
clear about whether it should or should not go upstream.
So from my side a fixup of some kind would be good but we can work on this in a
follow-up.
The problem is, some domains may not start after applying this patch.
Consider a domain with the following video config
<video>
<model type='vga' vram='9216' heads='1'/>
</video>
This would work pre-patch since libxl_domain_build_info->video_memkb
would be initialized to LIBXL_MEMKB_DEFAULT when calling
libxl_domain_build_info_init(). But the domain will fail to start
post-patch since video_memkb is set to an invalid value.
I know the current behavior is not correct either, but would be nice to
fix everything up in one series. I started on a second patch that would
validate input or set sane defaults in libxlDomainDeviceDefPostParse(),
but while testing realized that sane defaults depend on which qemu is
used. E.g. the minimum video_memkb values have doubled with QEMU_XEN vs
QEMU_XEN_TRADITIONAL - see $xen_root/tools/libxl/libxl_create.c,
libxl__domain_build_info_setdefault(). [I'll mention again that it is
unfortunate that QEMU_XEN vs QEMU_XEN_TRADITIONAL leaked through the
public API.]
I'm thinking of writing a utility function to detect the old vs new
qemu, hoping there is something in the help output or similar to
determine which one <emulator> is. Such a function would be useful for
David Scott's old patch to support arbitrary user-provided <emulator>
https://www.redhat.com/archives/libvir-list/2013-April/msg02119.html
But alas, this will have to wait until I return from vacation.
Regards,
Jim