09.02.2022 21:33, Daniel P. Berrangé wrote:
On Tue, Jan 18, 2022 at 05:09:08PM +0100, Vladimir Sementsov-Ogievskiy wrote:Add possibility to change addresses where VNC server listens for new connections. Prior to 6.0 this functionality was available through 'change' qmp command which was deleted.Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]> --- docs/about/removed-features.rst | 3 ++- qapi/ui.json | 6 +++++- include/ui/console.h | 2 +- monitor/qmp-cmds.c | 4 +--- ui/vnc.c | 37 ++++++++++++++++++++++++++------- 5 files changed, 39 insertions(+), 13 deletions(-) diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst index 4c4da20d0f..b92626a74e 100644 --- a/docs/about/removed-features.rst +++ b/docs/about/removed-features.rst @@ -355,7 +355,8 @@ documentation of ``query-hotpluggable-cpus`` for additional details. ``change`` (removed in 6.0) '''''''''''''''''''''''''''-Use ``blockdev-change-medium`` or ``change-vnc-password`` instead.+Use ``blockdev-change-medium`` or ``change-vnc-password`` or +``display-reload`` instead.``query-events`` (removed in 6.0)''''''''''''''''''''''''''''''''' diff --git a/qapi/ui.json b/qapi/ui.json index 9354f4c467..4c4448f378 100644 --- a/qapi/ui.json +++ b/qapi/ui.json @@ -1293,12 +1293,16 @@ # Specify the VNC reload options. # # @tls-certs: reload tls certs or not. +# @addresses: If specified, change set of addresses +# to listen for connections. Addresses configured +# for websockets are not touched. (since 7.0) # # Since: 6.0 # ## { 'struct': 'DisplayReloadOptionsVNC', - 'data': { '*tls-certs': 'bool' } } + 'data': { '*tls-certs': 'bool', + '*addresses': ['SocketAddress'] } }So I'm having second thoughts on this, because I think we in fact need to distinguish between reloads of state related to existing configuration vs applying changes to the actual configuration. For example, when I think about the 'tls-certs' option here, it lets us reload the existing TLS credentials from disk. ie it lets the user re-write the TLS file content on disk and then tell QEMU to reload the files. An equally valuable option would be to tell QEMU to simply use a completely different set of TLS files on disk, rather than re-writing in place. We don't have this feature now, but I think we should anticipate it in the design. So I'm going to suggest that instead of 'display-reload', we should have a general purpose 'display-update' command for modifying existing config and , leave 'display-reload' purely for re-loading state, not changing config. Essentially 'display-reload' is about re-starting QEMU displays with the same config, while 'display-update' is about restarting with a new config. This shouldn't be too much work to achieve in your patch. Just clone everything related to the 'display-reload' QMP command boilerplate, replacing 'reload' with 'update' throughout and discarding the 'tls-certs' bits leaving only your 'addresses' bit.
Yes, that's simple to do, I'll resend soon.
We can use this 'display-update' command for making pasword and auth config changes possible too later.### @DisplayReloadOptions: diff --git a/include/ui/console.h b/include/ui/console.h index f590819880..b052027915 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -512,7 +512,7 @@ int vnc_display_password(const char *id, const char *password); int vnc_display_pw_expire(const char *id, time_t expires); void vnc_parse(const char *str); int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp); -bool vnc_display_reload_certs(const char *id, Error **errp); +bool vnc_display_reload(DisplayReloadOptionsVNC *arg, Error **errp);/* input.c */int index_from_key(const char *key, size_t key_length); diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c index 14e3beeaaf..ad45baa12b 100644 --- a/monitor/qmp-cmds.c +++ b/monitor/qmp-cmds.c @@ -356,9 +356,7 @@ void qmp_display_reload(DisplayReloadOptions *arg, Error **errp) switch (arg->type) { case DISPLAY_RELOAD_TYPE_VNC: #ifdef CONFIG_VNC - if (arg->u.vnc.has_tls_certs && arg->u.vnc.tls_certs) { - vnc_display_reload_certs(NULL, errp); - } + vnc_display_reload(&arg->u.vnc, errp); #else error_setg(errp, "vnc is invalid, missing 'CONFIG_VNC'"); #endif diff --git a/ui/vnc.c b/ui/vnc.c index fa0fb736d3..a86bd6335e 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -587,16 +587,10 @@ VncInfo2List *qmp_query_vnc_servers(Error **errp) return prev; }-bool vnc_display_reload_certs(const char *id, Error **errp)+static bool vnc_display_reload_certs(VncDisplay *vd, Error **errp) { - VncDisplay *vd = vnc_display_find(id); QCryptoTLSCredsClass *creds = NULL;- if (!vd) {- error_setg(errp, "Can not find vnc display"); - return false; - } - if (!vd->tlscreds) { error_setg(errp, "vnc tls is not enabled"); return false; @@ -3973,6 +3967,35 @@ static int vnc_display_listen(VncDisplay *vd, return 0; }+bool vnc_display_reload(DisplayReloadOptionsVNC *arg, Error **errp)+{ + VncDisplay *vd = vnc_display_find(NULL); + + if (!vd) { + error_setg(errp, "Can not find vnc display"); + return false; + } + + if (arg->has_tls_certs && arg->tls_certs) { + if (!vnc_display_reload_certs(vd, errp)) { + return false; + } + } + + if (arg->has_addresses) { + if (vd->listener) { + qio_net_listener_disconnect(vd->listener); + object_unref(OBJECT(vd->listener)); + vd->listener = NULL; + } + + if (vnc_display_listen(vd, arg->addresses, NULL, errp) < 0) { + return false; + } + } + + return true; +}void vnc_display_open(const char *id, Error **errp){ -- 2.31.1Regards, Daniel
-- Best regards, Vladimir
