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

Osier Yang jyang at redhat.com
Mon Oct 22 09:33:57 UTC 2012


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




More information about the libvir-list mailing list