[libvirt] [PATCH] RFC: support "vram" and "heads" for qxl vga

qemu command line to specify "vram": -global qxl.vram_size=uint qemu command line to specify "heads", (no need of '-device' for the first 'head'): -device qxl,id=qxl-N (N is natural number) This patch is just about the command line building, still left work on the command line parsing (hacking on "qemuBuildCommandLine"), as I'm even not sure if it's the right way to build the command line. Any advise/idea is appreciated. --- src/qemu/qemu_capabilities.c | 5 ++++- src/qemu/qemu_capabilities.h | 3 ++- src/qemu/qemu_command.c | 14 ++++++++++++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index cc5552c..78bcb4c 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -942,9 +942,12 @@ qemuCapsComputeCmdFlags(const char *help, * two features. The benefits of JSON mode now outweigh * the downside. */ - if (version >= 13000) + if (version >= 13000) flags |= QEMUD_CMD_FLAG_MONITOR_JSON; + if (strstr(help, "-global")) + flags |= QEMUD_CMD_FLAG_GLOBAL; + return flags; } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index a130a4f..b1feb87 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -91,7 +91,8 @@ enum qemuCapsFlags { QEMUD_CMD_FLAG_CCID_EMULATED = (1LL << 54), /* -device ccid-card-emulated */ QEMUD_CMD_FLAG_CCID_PASSTHRU = (1LL << 55), /* -device ccid-card-passthru */ QEMUD_CMD_FLAG_CHARDEV_SPICEVMC = (1LL << 56), /* newer -chardev spicevmc */ - QEMUD_CMD_FLAG_DEVICE_SPICEVMC = (1LL << 57), /* older -device spicevmc*/ + QEMUD_CMD_FLAG_DEVICE_SPICEVMC = (1LL << 57), /* older -device spicevmc */ + QEMUD_CMD_FLAG_GLOBAL = (1LL << 58), /* Is -global available */ }; virCapsPtr qemuCapsInit(virCapsPtr old_caps); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1687203..92af6cb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3964,6 +3964,20 @@ qemuBuildCommandLine(virConnectPtr conn, } virCommandAddArgList(cmd, "-vga", vgastr, NULL); + + if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_QXL) { + if (def->videos[0]->vram && + (qemuCmdFlags & QEMUD_CMD_FLAG_GLOBAL)) { + virCommandAddArgFormat(cmd, "-global qxl.vram_size=%u", + def->videos[0]->vram); + } + + if (def->videos[0]->heads > 0) { + for (i = 0; i < def->videos[0]->heads - 1; i++) { + virCommandAddArgFormat(cmd, "-device %s,id=qxl-%d", vgastr, i+1); + } + } + } } } else { -- 1.7.4

于 2011年02月15日 21:47, Osier Yang 写道:
qemu command line to specify "vram": -global qxl.vram_size=uint
qemu command line to specify "heads", (no need of '-device' for the first 'head'): -device qxl,id=qxl-N (N is natural number)
This patch is just about the command line building, still left work on the command line parsing (hacking on "qemuBuildCommandLine"), as I'm even not sure if it's the right way to build the command line.
Any advise/idea is appreciated. --- src/qemu/qemu_capabilities.c | 5 ++++- src/qemu/qemu_capabilities.h | 3 ++- src/qemu/qemu_command.c | 14 ++++++++++++++ 3 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index cc5552c..78bcb4c 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -942,9 +942,12 @@ qemuCapsComputeCmdFlags(const char *help, * two features. The benefits of JSON mode now outweigh * the downside. */ - if (version>= 13000) + if (version>= 13000) flags |= QEMUD_CMD_FLAG_MONITOR_JSON;
+ if (strstr(help, "-global")) + flags |= QEMUD_CMD_FLAG_GLOBAL; + return flags; }
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index a130a4f..b1feb87 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -91,7 +91,8 @@ enum qemuCapsFlags { QEMUD_CMD_FLAG_CCID_EMULATED = (1LL<< 54), /* -device ccid-card-emulated */ QEMUD_CMD_FLAG_CCID_PASSTHRU = (1LL<< 55), /* -device ccid-card-passthru */ QEMUD_CMD_FLAG_CHARDEV_SPICEVMC = (1LL<< 56), /* newer -chardev spicevmc */ - QEMUD_CMD_FLAG_DEVICE_SPICEVMC = (1LL<< 57), /* older -device spicevmc*/ + QEMUD_CMD_FLAG_DEVICE_SPICEVMC = (1LL<< 57), /* older -device spicevmc */ + QEMUD_CMD_FLAG_GLOBAL = (1LL<< 58), /* Is -global available */ };
virCapsPtr qemuCapsInit(virCapsPtr old_caps); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1687203..92af6cb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3964,6 +3964,20 @@ qemuBuildCommandLine(virConnectPtr conn, }
virCommandAddArgList(cmd, "-vga", vgastr, NULL); + + if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_QXL) { + if (def->videos[0]->vram&& + (qemuCmdFlags& QEMUD_CMD_FLAG_GLOBAL)) { + virCommandAddArgFormat(cmd, "-global qxl.vram_size=%u", + def->videos[0]->vram); + } + + if (def->videos[0]->heads> 0) {
urgh, s/> 0/> 1/ here..
+ for (i = 0; i< def->videos[0]->heads - 1; i++) { + virCommandAddArgFormat(cmd, "-device %s,id=qxl-%d", vgastr, i+1); + } + } + } } } else {
-- 1.7.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Feb 15, 2011 at 09:47:28PM +0800, Osier Yang wrote:
qemu command line to specify "vram": -global qxl.vram_size=uint
qemu command line to specify "heads", (no need of '-device' for the first 'head'): -device qxl,id=qxl-N (N is natural number)
This patch is just about the command line building, still left work on the command line parsing (hacking on "qemuBuildCommandLine"), as I'm even not sure if it's the right way to build the command line.
Any advise/idea is appreciated. --- src/qemu/qemu_capabilities.c | 5 ++++- src/qemu/qemu_capabilities.h | 3 ++- src/qemu/qemu_command.c | 14 ++++++++++++++ 3 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index cc5552c..78bcb4c 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -942,9 +942,12 @@ qemuCapsComputeCmdFlags(const char *help, * two features. The benefits of JSON mode now outweigh * the downside. */ - if (version >= 13000) + if (version >= 13000) flags |= QEMUD_CMD_FLAG_MONITOR_JSON;
+ if (strstr(help, "-global")) + flags |= QEMUD_CMD_FLAG_GLOBAL; +
There's no need for this. Just use QEMUD_CMD_FLAG_DEVICE since -global & -device arrived together.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1687203..92af6cb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3964,6 +3964,20 @@ qemuBuildCommandLine(virConnectPtr conn, }
virCommandAddArgList(cmd, "-vga", vgastr, NULL); + + if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_QXL) { + if (def->videos[0]->vram && + (qemuCmdFlags & QEMUD_CMD_FLAG_GLOBAL)) { + virCommandAddArgFormat(cmd, "-global qxl.vram_size=%u", + def->videos[0]->vram); + }
The '-global' parameter sets global defaults for all devices of a particular type. The video handling is a little wierd, because the first card we'll have todo via '-vga' and a '-set' and the 2nd, 3rd, etc cards will be normal using just '-device' (see qemuBuildVideoDevStr). So here you'll want to use -set instead of -global. And then also modify qemuBuildVideoDevStr
+ + if (def->videos[0]->heads > 0) { + for (i = 0; i < def->videos[0]->heads - 1; i++) { + virCommandAddArgFormat(cmd, "-device %s,id=qxl-%d", vgastr, i+1); + }
This isn't right. The 'heads' field indicates one graphics card, with multiple outputs. What you're doing here is creating multiple graphics cards each with one output. This is something you do by adding mulitple <video> elements in the XML instead. It doesn't look like QXL has multiple outputs on one card, so this setting is not relevant. Also be sure to create a test case for this in qemuxml2argvtest Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

于 2011年02月15日 22:13, Daniel P. Berrange 写道:
On Tue, Feb 15, 2011 at 09:47:28PM +0800, Osier Yang wrote:
qemu command line to specify "vram": -global qxl.vram_size=uint
qemu command line to specify "heads", (no need of '-device' for the first 'head'): -device qxl,id=qxl-N (N is natural number)
This patch is just about the command line building, still left work on the command line parsing (hacking on "qemuBuildCommandLine"), as I'm even not sure if it's the right way to build the command line.
Any advise/idea is appreciated. --- src/qemu/qemu_capabilities.c | 5 ++++- src/qemu/qemu_capabilities.h | 3 ++- src/qemu/qemu_command.c | 14 ++++++++++++++ 3 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index cc5552c..78bcb4c 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -942,9 +942,12 @@ qemuCapsComputeCmdFlags(const char *help, * two features. The benefits of JSON mode now outweigh * the downside. */ - if (version>= 13000) + if (version>= 13000) flags |= QEMUD_CMD_FLAG_MONITOR_JSON;
+ if (strstr(help, "-global")) + flags |= QEMUD_CMD_FLAG_GLOBAL; +
There's no need for this. Just use QEMUD_CMD_FLAG_DEVICE since -global& -device arrived together.
didn't known that, thanks.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1687203..92af6cb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3964,6 +3964,20 @@ qemuBuildCommandLine(virConnectPtr conn, }
virCommandAddArgList(cmd, "-vga", vgastr, NULL); + + if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_QXL) { + if (def->videos[0]->vram&& + (qemuCmdFlags& QEMUD_CMD_FLAG_GLOBAL)) { + virCommandAddArgFormat(cmd, "-global qxl.vram_size=%u", + def->videos[0]->vram); + }
The '-global' parameter sets global defaults for all devices of a particular type.
The video handling is a little wierd, because the first card we'll have todo via '-vga' and a '-set' and the 2nd, 3rd, etc cards will be normal using just '-device' (see qemuBuildVideoDevStr).
So here you'll want to use -set instead of -global. And then also modify qemuBuildVideoDevStr
+ + if (def->videos[0]->heads> 0) { + for (i = 0; i< def->videos[0]->heads - 1; i++) { + virCommandAddArgFormat(cmd, "-device %s,id=qxl-%d", vgastr, i+1); + }
This isn't right. The 'heads' field indicates one graphics card, with multiple outputs. What you're doing here is creating multiple graphics cards each with one output. This is something you do by adding mulitple <video> elements in the XML instead.
It doesn't look like QXL has multiple outputs on one card, so this setting is not relevant.
Also be sure to create a test case for this in qemuxml2argvtest
Thanks for the detailed explaination, as it's the RFC, didn't do it, will include in final patch.
Regards, Daniel

于 2011年02月15日 22:13, Daniel P. Berrange 写道:
On Tue, Feb 15, 2011 at 09:47:28PM +0800, Osier Yang wrote:
qemu command line to specify "vram": -global qxl.vram_size=uint
qemu command line to specify "heads", (no need of '-device' for the first 'head'): -device qxl,id=qxl-N (N is natural number)
This patch is just about the command line building, still left work on the command line parsing (hacking on "qemuBuildCommandLine"), as I'm even not sure if it's the right way to build the command line.
Any advise/idea is appreciated. --- src/qemu/qemu_capabilities.c | 5 ++++- src/qemu/qemu_capabilities.h | 3 ++- src/qemu/qemu_command.c | 14 ++++++++++++++ 3 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index cc5552c..78bcb4c 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -942,9 +942,12 @@ qemuCapsComputeCmdFlags(const char *help, * two features. The benefits of JSON mode now outweigh * the downside. */ - if (version>= 13000) + if (version>= 13000) flags |= QEMUD_CMD_FLAG_MONITOR_JSON;
+ if (strstr(help, "-global")) + flags |= QEMUD_CMD_FLAG_GLOBAL; +
There's no need for this. Just use QEMUD_CMD_FLAG_DEVICE since -global& -device arrived together.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1687203..92af6cb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3964,6 +3964,20 @@ qemuBuildCommandLine(virConnectPtr conn, }
virCommandAddArgList(cmd, "-vga", vgastr, NULL); + + if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_QXL) { + if (def->videos[0]->vram&& + (qemuCmdFlags& QEMUD_CMD_FLAG_GLOBAL)) { + virCommandAddArgFormat(cmd, "-global qxl.vram_size=%u", + def->videos[0]->vram); + }
The '-global' parameter sets global defaults for all devices of a particular type.
The video handling is a little wierd, because the first card we'll have todo via '-vga' and a '-set' and the 2nd, 3rd, etc cards will be normal using just '-device' (see qemuBuildVideoDevStr).
So here you'll want to use -set instead of -global. And then also modify qemuBuildVideoDevStr
+ + if (def->videos[0]->heads> 0) { + for (i = 0; i< def->videos[0]->heads - 1; i++) { + virCommandAddArgFormat(cmd, "-device %s,id=qxl-%d", vgastr, i+1); + }
This isn't right. The 'heads' field indicates one graphics card, with multiple outputs. What you're doing here is creating multiple graphics cards each with one output. This is something you do by adding mulitple <video> elements in the XML instead.
It doesn't look like QXL has multiple outputs on one card, so this setting is not relevant.
With more investigation, "-vga" doesn't support "id", that means "-set" cannot be used, and the only choice for us is "-global", and the multiheads for qxl means multilple "-device qxl" indeed (confirmed with Gerd Hoffmann). More detail: http://lists.gnu.org/archive/html/qemu-devel/2010-11/msg02275.html And for RHEL6.1/upstream, the primary qxl device is named "qxl-vga", However, for RHEL6.0, all qxl devices are names "qxl", so, it needs different solution for RHEL6.1/upstream and RHEL6.0/older. For RHEL6.1/upstream: -vga qxl -global qxl.vram_size=$SIZE -device qxl,id=qxl-1 -device qxl,id=qxl-2 -device qxl,id=qxl-N For RHEL6.0/older: -vga qxl -global qxl-vga.vram_size=$SIZE -device qxl,id=qxl-1,vram_size=$SIZE -device qxl,qxl-2,vram_size=$SIZE, -device ... OR: () -vga qxl -global qxl-vga.vram_size=$SIZE -global qxl.vram_size=$SIZE -device qxl,id=qxl-1 -device qxl,qxl-2, -device ... Regards Osier

On 02/15/2011 06:47 AM, Osier Yang wrote:
qemu command line to specify "vram": -global qxl.vram_size=uint
+ if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_QXL) { + if (def->videos[0]->vram && + (qemuCmdFlags & QEMUD_CMD_FLAG_GLOBAL)) { + virCommandAddArgFormat(cmd, "-global qxl.vram_size=%u", + def->videos[0]->vram); + }
Ouch - this needs to be two arguments: "-global", "qxl.vram_size=%u".
+ + if (def->videos[0]->heads > 0) { + for (i = 0; i < def->videos[0]->heads - 1; i++) { + virCommandAddArgFormat(cmd, "-device %s,id=qxl-%d", vgastr, i+1);
Likewise. I'm preparing a patch (having just hit the issue when trying to start a guest with a qxl graphics device). -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (3)
-
Daniel P. Berrange
-
Eric Blake
-
Osier Yang