Re: [libvirt] [PATCH] qxl: return 16M instead of 64M in virDomainVideoDefaultRAM
On Mon, Jul 27, 2015 at 03:26:52PM +0800, Lin Ma wrote:
在 2015年07月27日 14:37, Martin Kletzander 写道:
> On Mon, Jul 27, 2015 at 02:19:11PM +0800, Lin Ma wrote:
>>
>> 在 2015年07月27日 13:34, Martin Kletzander 写道:
>>> On Mon, Jul 27, 2015 at 12:48:50PM +0800, Lin Ma wrote:
>>>> Return 16M for qxl because QEMU uses 16MB as the default video ram
>>>> size
>>>> for qxl device since pc-1.2.
>>>>
>>>
>>> NACK, that function is not meant to return the default video ram size
>>> of QEMU, but rather video ram size that libvirt will set by default.
>> For qxl, The return value of that function is used to set ram and vram
>> attributes which
>> qemu doesn't care of. The proper and effective attribute is vgamem
>> which was set
>> to 16M already(commit#0e50246).
The QXL video device is complicated than just simply setting *vgamem* to some
value. There are some restriction and rules how it works and QEMU does care
about the *ram* and *vram* values. For example, ram >= 2*vgamem.
QXL device has two basic memory regions, *ram* and *vram*. The *ram* region
contains VGA framebuffer and it's size can be modified by *vgamem* attribute.
In addition the *ram* region contains some other information.
All the values are required to properly set parameters of QXL video device.
You can check this document [1], on page 4 there is detailed description of QXL
memory layout.
[1]
http://www.rachacuca.org/~fidencio/VirtioQXL%20-%20a%20virtio%20video%20d...
>>
>> It causes the strange attribute list in guest xml:
>> <video>
>> <model type='qxl' ram='65536' vram='65536'
vgamem='16384'
>> heads='1'/>
>> ......
>> </video>
>>
>> and virt-manager also shows the inproper video ram size for qxl, It
>> shows 64MB
>> qxl video ram size in guest detailed page, but guest reports only 16MB.
Virt-manager shows 64MB video ram because all other video devices uses only
*vram* to set the video ram. To let virt-manager show the correct framebuffer
size of each video device, we need to add an exception for QXL to get the value
from *vgamem* attribute if it's available.
>>
>> So should I keep 64MB in virDomainVideoDefaultRAM and set vgamem by
>> virDomainVideoDefaultRAM?
>> Or something else?
There is noting to do in libvirt. NACK to this patch.
>>
>
> Well, in this case, I think the aim was to set everything to 64MB, but
> I remember some migration problems needed to be taken care of. So
> maybe we need to keep it for some reason. To be sure, I'll let Pavel
> handle this as he dealt with all the stuff related. But we need to be
> consistent and be able to migrate from older versions that had these
> attributes set. Also if this change needs to be done for QEMU, it
> should be somewhere in src/qemu and not in src/conf where it changes
> behaviour for all drivers.
Thanks, Let's wait Pave's suggestion.
>
>>>
>>>> Signed-off-by: Lin Ma <lma(a)suse.com>
>>>> ---
>>>> src/conf/domain_conf.c | 6 ++++--
>>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>>>> index 6b557d1..8efc973 100644
>>>> --- a/src/conf/domain_conf.c
>>>> +++ b/src/conf/domain_conf.c
>>>> @@ -11651,8 +11651,10 @@ virDomainVideoDefaultRAM(const
>>>> virDomainDef *def,
>>>> return 4 * 1024;
>>>>
>>>> case VIR_DOMAIN_VIDEO_TYPE_QXL:
>>>> - /* QEMU use 64M as the minimal video memory for qxl device */
>>>> - return 64 * 1024;
>>>> + /* By default, QEMU uses 16MB as video memory size
>>>> + * for qxl device since pc-1.2
>>>> + */
>>>> + return 16 * 1024;
>>>>
>>>> default:
>>>> return 0;
>>>> --
>>>> 2.1.4
>>>>
>>>> --
>>>> libvir-list mailing list
>>>> libvir-list(a)redhat.com
>>>> https://www.redhat.com/mailman/listinfo/libvir-list
>>
>> --
>> libvir-list mailing list
>> libvir-list(a)redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list
>
>
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list