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'] } }
##
# @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
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 :|
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 <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.
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.1
>>
>
> Regards,
> Daniel
>
--
Best regards,
Vladimir
© 2016 - 2026 Red Hat, Inc.