[libvirt] [PATCH] properly set video ram size for qemu VGA video device
https://bugzilla.redhat.com/show_bug.cgi?id=1076098 We support vram option for video devices in domain xml, but so far only for QXL it had some effect. VGA video device in QEMU can also accept the size of video ram and we should pass it. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_command.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fb64cda..09e29c9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4800,6 +4800,9 @@ qemuBuildDeviceVideoStr(virDomainDefPtr def, /* QEMU accepts bytes for vram_size. */ virBufferAsprintf(&buf, ",vram_size=%u", video->vram * 1024); + } else if (video->type == VIR_DOMAIN_VIDEO_TYPE_VGA) { + /* QEMU accepts megabytes for vgamem_mb. */ + virBufferAsprintf(&buf, ",vgamem_mb=%u", video->vram / 1024); } if (qemuBuildDeviceAddressStr(&buf, def, &video->info, qemuCaps) < 0) -- 1.8.5.5
On 07/07/2014 01:44 PM, Pavel Hrdina wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1076098
We support vram option for video devices in domain xml, but so far only for QXL it had some effect. VGA video device in QEMU can also accept the size of video ram and we should pass it.
Even if vram is 9 MB as we made up in virDomainVideoDefaultRAM? IMO we should either treat 9 MB as "use the default" and not pass it to QEMU, or just hide the 'vram' attribute (just for the 'weird 9 MB default' cards and QEMU domains) from non-migratable XML and use a new attribute. BTW, there were also patches proposing a new attribute: https://www.redhat.com/archives/libvir-list/2014-June/msg00565.html
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_command.c | 3 +++ 1 file changed, 3 insertions(+)
Documentation and tests are missing.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fb64cda..09e29c9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4800,6 +4800,9 @@ qemuBuildDeviceVideoStr(virDomainDefPtr def,
/* QEMU accepts bytes for vram_size. */ virBufferAsprintf(&buf, ",vram_size=%u", video->vram * 1024); + } else if (video->type == VIR_DOMAIN_VIDEO_TYPE_VGA) {
VMVGA has vgamem_mb too. Don't we also need a new qemu capability to probe if this is supported by the QEMU binary?
+ /* QEMU accepts megabytes for vgamem_mb. */ + virBufferAsprintf(&buf, ",vgamem_mb=%u", video->vram / 1024); }
if (qemuBuildDeviceAddressStr(&buf, def, &video->info, qemuCaps) < 0)
Jan
On 2014/7/7 20:39, Ján Tomko wrote:
On 07/07/2014 01:44 PM, Pavel Hrdina wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1076098
We support vram option for video devices in domain xml, but so far only for QXL it had some effect. VGA video device in QEMU can also accept the size of video ram and we should pass it.
Even if vram is 9 MB as we made up in virDomainVideoDefaultRAM? IMO we should either treat 9 MB as "use the default" and not pass it to QEMU, or just hide the 'vram' attribute (just for the 'weird 9 MB default' cards and QEMU domains) from non-migratable XML and use a new attribute.
BTW, there were also patches proposing a new attribute: https://www.redhat.com/archives/libvir-list/2014-June/msg00565.html
I worked for these patches. Glad to discuss. I would introduce a new attribute "vgamem" to replace "vram" in some cases, and also support "secondary-vga" in qemu 2.1. Sorry for delay of the v2 patches. Now I'm making some test. I'll send v2 in this week.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_command.c | 3 +++ 1 file changed, 3 insertions(+)
Documentation and tests are missing.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fb64cda..09e29c9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4800,6 +4800,9 @@ qemuBuildDeviceVideoStr(virDomainDefPtr def,
/* QEMU accepts bytes for vram_size. */ virBufferAsprintf(&buf, ",vram_size=%u", video->vram * 1024); + } else if (video->type == VIR_DOMAIN_VIDEO_TYPE_VGA) {
VMVGA has vgamem_mb too. Don't we also need a new qemu capability to probe if this is supported by the QEMU binary?
+ /* QEMU accepts megabytes for vgamem_mb. */ + virBufferAsprintf(&buf, ",vgamem_mb=%u", video->vram / 1024); }
if (qemuBuildDeviceAddressStr(&buf, def, &video->info, qemuCaps) < 0)
Jan
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Jul 07, 2014 at 01:44:06PM +0200, Pavel Hrdina wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1076098
We support vram option for video devices in domain xml, but so far only for QXL it had some effect. VGA video device in QEMU can also accept the size of video ram and we should pass it.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_command.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fb64cda..09e29c9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4800,6 +4800,9 @@ qemuBuildDeviceVideoStr(virDomainDefPtr def,
/* QEMU accepts bytes for vram_size. */ virBufferAsprintf(&buf, ",vram_size=%u", video->vram * 1024); + } else if (video->type == VIR_DOMAIN_VIDEO_TYPE_VGA) {
The patch [1] that adds this option also adds it for VMWare VGA, can we use that too?
+ /* QEMU accepts megabytes for vgamem_mb. */ + virBufferAsprintf(&buf, ",vgamem_mb=%u", video->vram / 1024);
How will this behave with pre-1.1.0 qemu (without the commit mentioned [1])? Will it (a) just skip the parameter or will it (b) error out? In case of (b) this is a regression. Martin P.S.: Some tests in qemuxml2argv would go nicely with such change. [1] 4a1e244eb65c646bdd938d9d137ace42d76c95a7
participants (4)
-
Ján Tomko -
Martin Kletzander -
Pavel Hrdina -
Wang Rui