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

Re: [libvirt] [PATCH v2 2/2] qemu: Add VNC WebSocket support



On 04/30/2013 10:42 AM, Martin Kletzander wrote:
> Adding a VNC WebSocket support for QEMU driver.  This funcitonality is

s/funcitonality/functionality

> in upstream qemu from commit described as v1.3.0-982-g7536ee4, so the
> capability is being recognized based on QEMU version for now.
> 
> Signed-off-by: Martin Kletzander <mkletzan redhat com>
> ---
>  src/qemu/libvirtd_qemu.aug         |  2 ++
>  src/qemu/qemu.conf                 |  7 +++++
>  src/qemu/qemu_capabilities.c       | 11 +++++--
>  src/qemu/qemu_capabilities.h       |  1 +
>  src/qemu/qemu_command.c            | 60 ++++++++++++++++++++++++++++++++++++--
>  src/qemu/qemu_command.h            |  5 +++-
>  src/qemu/qemu_conf.c               | 32 ++++++++++++++++++++
>  src/qemu/qemu_conf.h               |  6 ++++
>  src/qemu/qemu_driver.c             |  5 ++++
>  src/qemu/qemu_process.c            | 31 ++++++++++++++++----
>  src/qemu/test_libvirtd_qemu.aug.in |  2 ++
>  tests/qemuargv2xmltest.c           |  1 +
>  tests/qemuxml2argvtest.c           |  1 +
>  tests/qemuxml2xmltest.c            |  1 +
>  14 files changed, 153 insertions(+), 12 deletions(-)
> 
> diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
> index a3dcb30..5344125 100644
> --- a/src/qemu/libvirtd_qemu.aug
> +++ b/src/qemu/libvirtd_qemu.aug
> @@ -41,6 +41,8 @@ module Libvirtd_qemu =
> 
>     let remote_display_entry = int_entry "remote_display_port_min"
>                   | int_entry "remote_display_port_max"
> +                 | int_entry "remote_websocket_port_min"
> +                 | int_entry "remote_websocket_port_max"
> 
>     let security_entry = str_entry "security_driver"
>                   | bool_entry "security_default_confined"
> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
> index 0f0a24c..9257d3d 100644
> --- a/src/qemu/qemu.conf
> +++ b/src/qemu/qemu.conf
> @@ -153,6 +153,13 @@
>  #remote_display_port_min = 5900
>  #remote_display_port_max = 65535
> 
> +# VNC WebSocket port policies, same rules apply as with remote display
> +# ports.  VNC WebSockets use similar display <-> port mappings, with
> +# the exception being that ports starts from 5700 instead of 5900.
> +# This is what may have be changed here.
> +#
> +#remote_websocket_port_min = 5700
> +#remote_websocket_port_max = 65535
> 
>  # The default security driver is SELinux. If SELinux is disabled
>  # on the host, then the security driver will automatically disable
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 2acf535..9e5eedf 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -222,9 +222,10 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
>                "tpm-tis",
> 
>                "nvram",  /* 140 */
> -              "pci-bridge", /* 141 */
> -              "vfio-pci", /* 142 */
> -              "vfio-pci.bootindex", /* 143 */
> +              "pci-bridge",
> +              "vfio-pci",
> +              "vfio-pci.bootindex",
> +              "vnc-websocket",

This list doesn't match below...  I also remember reading a comment
recently about counting every 5th and leaving proper spacing between
groups of 5.

>      );
> 
>  struct _virQEMUCaps {
> @@ -2520,6 +2521,10 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps,
>      if (qemuCaps->version >= 1003000)
>          virQEMUCapsSet(qemuCaps, QEMU_CAPS_MACHINE_USB_OPT);
> 
> +    /* WebSockets were introduced between 1.3.0 and 1.3.1 */
> +    if (qemuCaps->version >= 1003001)
> +        virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_WEBSOCKET);
> +
>      if (!(archstr = qemuMonitorGetTargetArch(mon)))
>          goto cleanup;
> 
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 213f63c..c647274 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -182,6 +182,7 @@ enum virQEMUCapsFlags {
>      QEMU_CAPS_DEVICE_PCI_BRIDGE  = 141, /* -device pci-bridge */
>      QEMU_CAPS_DEVICE_VFIO_PCI    = 142, /* -device vfio-pci */
>      QEMU_CAPS_VFIO_PCI_BOOTINDEX = 143, /* bootindex param for vfio-pci device */
> +    QEMU_CAPS_VNC_WEBSOCKET      = 144, /* bootindex param for vfio-pci device */

This doesn't match above
> 
>      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 3184e5b..f718434 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -5903,6 +5903,17 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg,
>          }
>      }
> 
> +    if (graphics->data.vnc.websocket) {
> +        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_WEBSOCKET)) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("VNC websockets are not supported "

Should it be "WebSockets" or does it matter?

> +                             "with this QEMU binary"));
> +            goto error;
> +        }
> +
> +        virBufferAsprintf(&opt, ",websocket=%d", graphics->data.vnc.websocket);
> +    }
> +
>      virCommandAddArg(cmd, "-vnc");
>      virCommandAddArgBuffer(cmd, &opt);
>      if (graphics->data.vnc.keymap)
> @@ -9726,6 +9737,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps,
>                   * -vnc some.host.name:4
>                   */
>                  char *opts;
> +                char *port;
>                  const char *sep = ":";
>                  if (val[0] == '[')
>                      sep = "]:";
> @@ -9736,11 +9748,12 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps,
>                                     _("missing VNC port number in '%s'"), val);
>                      goto error;
>                  }
> -                if (virStrToLong_i(tmp+strlen(sep), &opts, 10,
> +                port = tmp + strlen(sep);
> +                if (virStrToLong_i(port, &opts, 10,
>                                     &vnc->data.vnc.port) < 0) {
>                      virDomainGraphicsDefFree(vnc);
>                      virReportError(VIR_ERR_INTERNAL_ERROR,
> -                                   _("cannot parse VNC port '%s'"), tmp+1);
> +                                   _("cannot parse VNC port '%s'"), port);
>                      goto error;
>                  }
>                  if (val[0] == '[')
> @@ -9753,6 +9766,49 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps,
>                      virDomainGraphicsDefFree(vnc);
>                      goto no_memory;
>                  }
> +
> +                if (*opts == ',') {
> +                    char *orig_opts = strdup(opts + 1);
> +                    if (!orig_opts) {
> +                        virDomainGraphicsDefFree(vnc);
> +                        goto no_memory;
> +                    }
> +                    opts = orig_opts;
> +
> +                    while (opts && *opts) {
> +                        char *nextopt = strchr(opts, ',');
> +                        if (nextopt)
> +                            *(nextopt++) = '\0';
> +
> +                        if (STRPREFIX(opts, "websocket")) {
> +                            char *websocket = opts + strlen("websocket");
> +                            if (*(websocket++) == '=' &&
> +                                *websocket) {
> +                                /* If the websocket continues with
> +                                 * '=<something>', we'll parse it */
> +                                if (virStrToLong_i(websocket,
> +                                                   NULL, 0,
> +                                                   &vnc->data.vnc.websocket) < 0) {
> +                                    virReportError(VIR_ERR_INTERNAL_ERROR,
> +                                                   _("cannot parse VNC "
> +                                                     "websocket port '%s'"),

Do we care to make it "WebSocket"?

> +                                                   websocket);
> +                                    virDomainGraphicsDefFree(vnc);
> +                                    VIR_FREE(orig_opts);
> +                                }
> +                            } else {
> +                                /* Otherwise, we'll compute the port the same
> +                                 * way QEMU does, by adding a 5700 to the
> +                                 * display value. */
> +                                vnc->data.vnc.websocket =
> +                                    vnc->data.vnc.port + 5700;

s/5700/QEMU_WEBSOCKET_PORT_MIN

I see there is a QEMU_REMOTE_PORT_MIN in qemu_command.h, but it's not
used in qemu_command.c either....

> +                            }
> +                        }
> +
> +                        opts = nextopt;
> +                    }
> +                    VIR_FREE(orig_opts);
> +                }
>                  vnc->data.vnc.port += 5900;
>                  vnc->data.vnc.autoport = false;
>              }
> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
> index a706942..724e88e 100644
> --- a/src/qemu/qemu_command.h
> +++ b/src/qemu/qemu_command.h
> @@ -1,7 +1,7 @@
>  /*
>   * qemu_command.h: QEMU command generation
>   *
> - * Copyright (C) 2006-2012 Red Hat, Inc.
> + * Copyright (C) 2006-2013 Red Hat, Inc.
>   * Copyright (C) 2006 Daniel P. Berrange
>   *
>   * This library is free software; you can redistribute it and/or
> @@ -48,6 +48,9 @@
>  # define QEMU_REMOTE_PORT_MIN  5900
>  # define QEMU_REMOTE_PORT_MAX  65535
> 
> +# define QEMU_WEBSOCKET_PORT_MIN  5700
> +# define QEMU_WEBSOCKET_PORT_MAX  65535
> +
> 
>  virCommandPtr qemuBuildCommandLine(virConnectPtr conn,
>                                     virQEMUDriverPtr driver,
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 7c3f317..1e56c5b 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -228,6 +228,9 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
>      cfg->remotePortMin = QEMU_REMOTE_PORT_MIN;
>      cfg->remotePortMax = QEMU_REMOTE_PORT_MAX;
> 
> +    cfg->webSocketPortMin = QEMU_WEBSOCKET_PORT_MIN;
> +    cfg->webSocketPortMax = QEMU_WEBSOCKET_PORT_MAX;
> +
>  #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R
>      /* For privileged driver, try and find hugepage mount automatically.
>       * Non-privileged driver requires admin to create a dir for the
> @@ -404,6 +407,35 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
>      GET_VALUE_STR("spice_password", cfg->spicePassword);
> 
> 
> +    GET_VALUE_LONG("remote_websocket_port_min", cfg->webSocketPortMin);
> +    if (cfg->webSocketPortMin < QEMU_WEBSOCKET_PORT_MIN) {
> +        /* if the port is too low, we can't get the display name
> +         * to tell to vnc (usually subtract 5700, e.g. localhost:1
> +         * for port 5701) */
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("%s: remote_websocket_port_min: port must be greater "
> +                         "than or equal to %d"),
> +                        filename, QEMU_WEBSOCKET_PORT_MIN);
> +        goto cleanup;
> +    }
> +
> +    GET_VALUE_LONG("remote_websocket_port_max", cfg->webSocketPortMax);
> +    if (cfg->webSocketPortMax > QEMU_WEBSOCKET_PORT_MAX ||
> +        cfg->webSocketPortMax < cfg->webSocketPortMin) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                        _("%s: remote_websocket_port_max: port must be between "
> +                          "the minimal port and %d"),
> +                       filename, QEMU_WEBSOCKET_PORT_MAX);
> +        goto cleanup;
> +    }
> +
> +    if (cfg->webSocketPortMin > cfg->webSocketPortMax) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                        _("%s: remote_websocket_port_min: min port must not be "
> +                          "greater than max port"), filename);
> +        goto cleanup;
> +    }
> +
>      GET_VALUE_LONG("remote_display_port_min", cfg->remotePortMin);
>      if (cfg->remotePortMin < QEMU_REMOTE_PORT_MIN) {
>          /* if the port is too low, we can't get the display name
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index 77d3d2f..8392729 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -114,6 +114,9 @@ struct _virQEMUDriverConfig {
>      int remotePortMin;
>      int remotePortMax;
> 
> +    int webSocketPortMin;
> +    int webSocketPortMax;
> +
>      char *hugetlbfsMount;
>      char *hugepagePath;
>      char *bridgeHelperName;
> @@ -210,6 +213,9 @@ struct _virQEMUDriver {
>      /* Immutable pointer, self-locking APIs */
>      virPortAllocatorPtr remotePorts;
> 
> +    /* Immutable pointer, self-locking APIs */
> +    virPortAllocatorPtr webSocketPorts;
> +
>      /* Immutable pointer, lockless APIs*/
>      virSysinfoDefPtr hostsysinfo;
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 296efe3..cf0bc55 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -663,6 +663,11 @@ qemuStateInitialize(bool privileged,
>                               cfg->remotePortMax)) == NULL)
>          goto error;
> 
> +    if ((qemu_driver->webSocketPorts =
> +         virPortAllocatorNew(cfg->webSocketPortMin,
> +                             cfg->webSocketPortMax)) == NULL)
> +        goto error;
> +
>      if (qemuSecurityInit(qemu_driver) < 0)
>          goto error;
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index e75c8c9..296e9b3 100644You'd have to use %d and def->data.vnc.port, assuming it's valid...

> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -3233,6 +3233,29 @@ qemuSetUnprivSGIO(virDomainDiskDefPtr disk)
>      return ret;
>  }
> 
> +static int
> +qemuProcessVNCAllocatePorts(virQEMUDriverPtr driver,
> +                            virDomainGraphicsDefPtr graphics)
> +{
> +    unsigned short port;
> +
> +    if (graphics->data.vnc.socket)
> +        return 0;
> +
> +    if (graphics->data.vnc.autoport) {
> +        if (virPortAllocatorAcquire(driver->remotePorts, &port) < 0)
> +            return -1;
> +        graphics->data.vnc.port = port;
> +    }
> +
> +    if (graphics->data.vnc.websocket == -1) {
> +        if (virPortAllocatorAcquire(driver->webSocketPorts, &port) < 0)
> +            return -1;
> +        graphics->data.vnc.websocket = port;
> +    }
> +
> +    return 0;
> +}
> 
>  static int
>  qemuProcessSPICEAllocatePorts(virQEMUDriverPtr driver,
> @@ -3448,13 +3471,9 @@ int qemuProcessStart(virConnectPtr conn,
> 
>      for (i = 0 ; i < vm->def->ngraphics; ++i) {
>          virDomainGraphicsDefPtr graphics = vm->def->graphics[i];
> -        if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
> -            !graphics->data.vnc.socket &&
> -            graphics->data.vnc.autoport) {
> -            unsigned short port;
> -            if (virPortAllocatorAcquire(driver->remotePorts, &port) < 0)
> +        if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) {
> +            if (qemuProcessVNCAllocatePorts(driver, graphics) < 0)
>                  goto cleanup;
> -            graphics->data.vnc.port = port;
>          } else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
>              if (qemuProcessSPICEAllocatePorts(driver, cfg, graphics) < 0)
>                  goto cleanup;
> diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in
> index 26ca068..d4e4fae 100644
> --- a/src/qemu/test_libvirtd_qemu.aug.in
> +++ b/src/qemu/test_libvirtd_qemu.aug.in
> @@ -17,6 +17,8 @@ module Test_libvirtd_qemu =
>  { "spice_password" = "XYZ12345" }
>  { "remote_display_port_min" = "5900" }
>  { "remote_display_port_max" = "65535" }
> +{ "remote_websocket_port_min" = "5700" }
> +{ "remote_websocket_port_max" = "65535" }
>  { "security_driver" = "selinux" }
>  { "security_default_confined" = "1" }
>  { "security_require_confined" = "1" }
> diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c
> index 9f1bb24..c8e7825 100644
> --- a/tests/qemuargv2xmltest.c
> +++ b/tests/qemuargv2xmltest.c
> @@ -200,6 +200,7 @@ mymain(void)
>      DO_TEST("disk-usb");
>      DO_TEST("graphics-vnc");
>      DO_TEST("graphics-vnc-socket");
> +    DO_TEST("graphics-vnc-websocket");
> 
>      DO_TEST("graphics-vnc-sasl");
>      DO_TEST("graphics-vnc-tls");
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 9699f77..0db3181 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -597,6 +597,7 @@ mymain(void)
> 
>      DO_TEST("graphics-vnc", QEMU_CAPS_VNC);
>      DO_TEST("graphics-vnc-socket", QEMU_CAPS_VNC);
> +    DO_TEST("graphics-vnc-websocket", QEMU_CAPS_VNC, QEMU_CAPS_VNC_WEBSOCKET);
> 
>      driver.config->vncSASL = 1;
>      VIR_FREE(driver.config->vncSASLdir);
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index a81cfcf..7400779 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -186,6 +186,7 @@ mymain(void)
>      DO_TEST_FULL("disk-mirror", true, WHEN_INACTIVE);
>      DO_TEST("graphics-listen-network");
>      DO_TEST("graphics-vnc");
> +    DO_TEST("graphics-vnc-websocket");
>      DO_TEST("graphics-vnc-sasl");
>      DO_TEST("graphics-vnc-tls");
>      DO_TEST("graphics-sdl");
> 


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