Re: [PATCH 2/2] qemu: Don't blindly assume VNC is supported

Osier Yang <jyang@xxxxxxxxxx> · Mon, 22 Oct 2012 17:33:57 +0800

On 2012年10月20日 03:40, Doug Goldstein wrote:
Currently its assumed that qemu always supports VNC, however it is

s/its/it's/

definitely possible to compile qemu without VNC support so we should at
the very least check for it and handle that correctly.
---
  src/qemu/qemu_capabilities.c |    5 ++++
  src/qemu/qemu_capabilities.h |    1 +
  src/qemu/qemu_command.c      |    6 +++++
  tests/qemuhelptest.c         |   48 ++++++++++++++++++++++++++++--------------
  tests/qemuxml2argvtest.c     |   10 ++++----
  5 files changed, 49 insertions(+), 21 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 7c391b3..9c7cbca 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -187,6 +187,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
                "reboot-timeout", /* 110 */
                "dump-guest-core",
                "seamless-migration",
+              "vnc",
      );

  struct _qemuCaps {
@@ -937,6 +938,8 @@ qemuCapsComputeCmdFlags(const char *help,
      }
      if (strstr(help, "-spice"))
          qemuCapsSet(caps, QEMU_CAPS_SPICE);
+    if (strstr(help, "-vnc"))
+        qemuCapsSet(caps, QEMU_CAPS_VNC);
      if (strstr(help, "seamless-migration="))
          qemuCapsSet(caps, QEMU_CAPS_SEAMLESS_MIGRATION);
      if (strstr(help, "boot=on"))
@@ -1881,6 +1884,8 @@ qemuCapsProbeQMPCommands(qemuCapsPtr caps,
              qemuCapsSet(caps, QEMU_CAPS_SPICE);
          else if (STREQ(name, "query-kvm"))
              qemuCapsSet(caps, QEMU_CAPS_KVM);
+        else if (STREQ(name, "query-vnc"))
+            qemuCapsSet(caps, QEMU_CAPS_VNC);

It's better to name it as "QEMU_CAPS_GRAPHIC_VNC". With
assuming that we will have flags like QEMU_CAPS_GRAPHIC_SPICE
in future.

          VIR_FREE(name);
      }
      VIR_FREE(commands);
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 5d343c1..1dd6f0c 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -150,6 +150,7 @@ enum qemuCapsFlags {
      QEMU_CAPS_REBOOT_TIMEOUT     = 110, /* -boot reboot-timeout */
      QEMU_CAPS_DUMP_GUEST_CORE    = 111, /* dump-guest-core-parameter */
      QEMU_CAPS_SEAMLESS_MIGRATION = 112, /* seamless-migration for SPICE */
+    QEMU_CAPS_VNC                = 113, /* Is -vnc avail */

To keep consistent, better to use "available?".


      QEMU_CAPS_LAST,                   /* this must always be the last item */
  };
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9096b3c..40c6417 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5847,6 +5847,12 @@ qemuBuildCommandLine(virConnectPtr conn,
          def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
          virBuffer opt = VIR_BUFFER_INITIALIZER;

+        if (!qemuCapsGet(caps, QEMU_CAPS_VNC)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("vnc graphics are not supported with this QEMU"));

I'd reword it like:

"vnc graphic is not supported by this QEMU". Plural is no need here.

The rest looks just fine, ACK with the changes.

Regards,
Osier

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list






