[libvirt] [PATCH] qxl: return 16M instead of 64M in virDomainVideoDefaultRAM

Return 16M for qxl because QEMU uses 16MB as the default video ram size for qxl device since pc-1.2. Signed-off-by: Lin Ma <lma@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

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.
Signed-off-by: Lin Ma <lma@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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

在 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).
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. So should I keep 64MB in virDomainVideoDefaultRAM and set vgamem by virDomainVideoDefaultRAM? Or something else?
Signed-off-by: Lin Ma <lma@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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

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).
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.
So should I keep 64MB in virDomainVideoDefaultRAM and set vgamem by virDomainVideoDefaultRAM? Or something else?
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.
Signed-off-by: Lin Ma <lma@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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

在 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).
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.
So should I keep 64MB in virDomainVideoDefaultRAM and set vgamem by virDomainVideoDefaultRAM? Or something else?
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@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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

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%20devi...
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@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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (3)
-
Lin Ma
-
Martin Kletzander
-
Pavel Hrdina