On 12/11/2012 08:32 PM, Guannan Ren wrote:
>>> + 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.
Still, I'd rather see us create a capability bit, even if that bit is
initially set according to a version check in qemu_capabilities.c, and
use that bit rather than polluting the rest of the qemu code with
additional version checks. That way, if someone manages to backport
fixed device handling even to RHEL qemu 0.12.x, as well as advertise
that backport, we can set the bit when the backport is detected. It may
even be as simple as a RHEL-specific patch that merely blindly sets the
capability bit at the one place where upstream sets the bit according to
a version check. But no matter how we approach it, having a dedicated
capability bit rather than multiple version checks will make backporting
much easier.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list