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(a)redhat.com&gt; > --- > 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....