On 12/12/2012 09:22 PM, Eric Blake wrote:
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.
Agree, I will do that in v3.
and setting a specific caps flags also make writing new testcase easier
than checking qemu version directly.
Guannan
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list