On 04/30/2013 06:53 PM, John Ferlan wrote:
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.
This is actually counting only every 5th (but starting with 0, so it's the "nvram" /* 140 */ here.
);
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?
I think it doesn't matter, but I changed it to WebSocket everywhere.
+ "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"?
Same here.
+ 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....
Actually no. This is parsing the command line of qemu, which always adds 5700, but not our minimum. I failed to see your mail because you didn't reply-all, noticed it now. I'll go through Eric's comments and push afterwards. Martin