Re: [libvirt] [PATCH v2 3/4] qemu: use newer -device video device in qemu commandline
On 12/12/2012 02:25 AM, Eric Blake wrote:
On 12/11/2012 07:14 AM, Guannan Ren wrote:
> '-device VGA' maps to '-vga std'
> '-device cirrus-vga' maps to '-vga cirrus'
> '-device qxl-vga' maps to '-vga qxl'
> (there is also '-device qxl' for secondary devices)
> '-device vmware-svga' maps to '-vga vmware'
>
> For qemu(>=1.2), we can use -device to replace -vga for video
> device. For the primary video device, the patch trys to use 0x2
> slot for matching old qemu. If the 0x2 slot is allocated already,
> the addr property could help for using any available slot.
> For qemu(< 1.2), we keep using -vga for primary device.
> ---
> src/qemu/qemu_command.c | 132 ++++++++++++++++++++++++++++++++++++-----------
> src/qemu/qemu_command.h | 4 +-
> tests/qemuxml2argvtest.c | 9 ++--
> 3 files changed, 110 insertions(+), 35 deletions(-)
>
> {
> size_t i, j;
> bool reservedIDE = false;
> bool reservedUSB = false;
> int function;
> + bool qemu1dot2plus = qemuCapsGetVersion(caps) >= 1002000;
Eww. Version string comparison is wrong, as it is feasible that someone
could backport the improved command line to older qemu. We should
instead be going off of whether specific '-device XXX' is supported;
probably by checking for QEMU_CAPS_DEVICE_QXL.
> @@ -6427,22 +6477,42 @@ qemuBuildCommandLine(virConnectPtr conn,
> goto error;
> }
> if (def->nvideos > 0) {
> - if (qemuCapsGet(caps, QEMU_CAPS_VGA)) {
> - if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_XEN) {
> + int primaryVideoType = def->videos[0]->type;
> + if (qemuCapsGetVersion(caps) >= 1002000 &&
Again, version comparison feels wrong. What are you really gating on,
whether qemu is new enough to support primary video on a non-default
address? If so, set up the right capability bit for that.
Although '-device XXX' is there but it probably doesn't work
for qemu(<1.2)
the following is the comments from Gerd Hoffmann in v1
"That will not work. You must check the qemu version. 1.2+ is safe,
IIRC 1.1 will work too.
Technical background: '-device VGA' (+variants) exists for a looooooong
time, pretty much day one of the -device switch. Actually using it did
not work though due to a nasty initialization order issue in qemu. The
qemu memory api created by avi fixed it."
> + ((primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_VGA &&
> + qemuCapsGet(caps, QEMU_CAPS_DEVICE_VGA)) ||
> + (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_CIRRUS &&
> + qemuCapsGet(caps, QEMU_CAPS_DEVICE_CIRRUS_VGA)) ||
> + (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_VMVGA &&
> + qemuCapsGet(caps, QEMU_CAPS_DEVICE_VMWARE_SVGA)) ||
> + (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_QXL &&
> + qemuCapsGet(caps, QEMU_CAPS_DEVICE_QXL_VGA)))
Or is the existence of these four device capability bits sufficient?
device capability bits checking here is in case that one of
four video device
is missing on some qemu(>=1.2) though.