[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 2/3] qapi/ui: display-reload: add possibility to change li
From: |
Daniel P . Berrangé |
Subject: |
Re: [PATCH v3 2/3] qapi/ui: display-reload: add possibility to change listen address |
Date: |
Wed, 9 Feb 2022 18:33:21 +0000 |
User-agent: |
Mutt/2.1.5 (2021-12-30) |
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 <vsementsov@virtuozzo.com>
> ---
> 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.
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.1
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
- Re: [PATCH v3 2/3] qapi/ui: display-reload: add possibility to change listen address,
Daniel P . Berrangé <=