[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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.

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]