* [PATCH] vnc: add qmp to support reload vnc tls certificates
@ 2021-01-04 7:11 Zihao Chang
2021-01-04 12:27 ` Daniel P. Berrangé
2021-01-19 19:23 ` Eric Blake
0 siblings, 2 replies; 4+ messages in thread
From: Zihao Chang @ 2021-01-04 7:11 UTC (permalink / raw)
To: qemu-devel; +Cc: oscar.zhangbo, changzihao1, armbru, xiexiangyou, kraxel
QEMU loads vnc tls certificates only when vm is started. This patch
provides a new qmp command to reload vnc tls certificates without
restart vnc-server/VM.
Signed-off-by: Zihao Chang <changzihao1@huawei.com>
---
include/ui/console.h | 1 +
monitor/qmp-cmds.c | 7 +++++++
qapi/ui.json | 17 +++++++++++++++++
ui/vnc.c | 20 ++++++++++++++++++++
4 files changed, 45 insertions(+)
diff --git a/include/ui/console.h b/include/ui/console.h
index 5dd21976a3..f05140b662 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -441,6 +441,7 @@ int vnc_display_password(const char *id, const char *password);
int vnc_display_pw_expire(const char *id, time_t expires);
QemuOpts *vnc_parse(const char *str, Error **errp);
int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp);
+int vnc_display_reload_cert(const char *id, 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 34f7e75b7b..90bd08c8ed 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -287,6 +287,13 @@ static void qmp_change_vnc(const char *target, bool has_arg, const char *arg,
qmp_change_vnc_listen(target, errp);
}
}
+
+void qmp_reload_vnc_cert(Error **errp)
+{
+ if (vnc_display_reload_cert(NULL, errp) < 0) {
+ error_setg(errp, "Reload vnc tls cert failed");
+ }
+}
#endif /* !CONFIG_VNC */
void qmp_change(const char *device, const char *target,
diff --git a/qapi/ui.json b/qapi/ui.json
index d08d72b439..bc3ffdb20f 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1179,3 +1179,20 @@
##
{ 'command': 'query-display-options',
'returns': 'DisplayOptions' }
+
+##
+# @reload-vnc-cert:
+#
+# Reload certificates for vnc.
+#
+# Returns: nothing
+#
+# Since: 5.2
+#
+# Example:
+#
+# -> { "execute": "reload-vnc-cert" }
+# <- { "return": {} }
+#
+##
+{ 'command': 'reload-vnc-cert' }
diff --git a/ui/vnc.c b/ui/vnc.c
index 7452ac7df2..b0cfbcf47c 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -582,6 +582,26 @@ VncInfo2List *qmp_query_vnc_servers(Error **errp)
return prev;
}
+int vnc_display_reload_cert(const char *id, Error **errp)
+{
+ VncDisplay *vd = vnc_display_find(id);
+
+ if (!vd) {
+ return -EINVAL;
+ }
+
+ if (!vd->tlscreds) {
+ error_printf_unless_qmp("If you want use vnc tls please enable "
+ "vnc tls using '-vnc tls-creds=${tls-obj-id}'.\n");
+ return -EPERM;
+ }
+
+ object_property_set_bool(OBJECT(vd->tlscreds), "loaded", false, NULL);
+ object_property_set_bool(OBJECT(vd->tlscreds), "loaded", true, NULL);
+
+ return 0;
+}
+
/* TODO
1) Get the queue working for IO.
2) there is some weirdness when using the -S option (the screen is grey
--
2.23.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] vnc: add qmp to support reload vnc tls certificates
2021-01-04 7:11 [PATCH] vnc: add qmp to support reload vnc tls certificates Zihao Chang
@ 2021-01-04 12:27 ` Daniel P. Berrangé
2021-01-15 13:25 ` Markus Armbruster
2021-01-19 19:23 ` Eric Blake
1 sibling, 1 reply; 4+ messages in thread
From: Daniel P. Berrangé @ 2021-01-04 12:27 UTC (permalink / raw)
To: Zihao Chang; +Cc: oscar.zhangbo, kraxel, qemu-devel, xiexiangyou, armbru
On Mon, Jan 04, 2021 at 03:11:28PM +0800, Zihao Chang wrote:
> QEMU loads vnc tls certificates only when vm is started. This patch
> provides a new qmp command to reload vnc tls certificates without
> restart vnc-server/VM.
>
> Signed-off-by: Zihao Chang <changzihao1@huawei.com>
> ---
> include/ui/console.h | 1 +
> monitor/qmp-cmds.c | 7 +++++++
> qapi/ui.json | 17 +++++++++++++++++
> ui/vnc.c | 20 ++++++++++++++++++++
> 4 files changed, 45 insertions(+)
>
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 5dd21976a3..f05140b662 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -441,6 +441,7 @@ int vnc_display_password(const char *id, const char *password);
> int vnc_display_pw_expire(const char *id, time_t expires);
> QemuOpts *vnc_parse(const char *str, Error **errp);
> int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp);
> +int vnc_display_reload_cert(const char *id, 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 34f7e75b7b..90bd08c8ed 100644
> --- a/monitor/qmp-cmds.c
> +++ b/monitor/qmp-cmds.c
> @@ -287,6 +287,13 @@ static void qmp_change_vnc(const char *target, bool has_arg, const char *arg,
> qmp_change_vnc_listen(target, errp);
> }
> }
> +
> +void qmp_reload_vnc_cert(Error **errp)
> +{
> + if (vnc_display_reload_cert(NULL, errp) < 0) {
> + error_setg(errp, "Reload vnc tls cert failed");
This error message is entirely useless at helping diagnose what
failed. We need to propagate real error messages from the root
cause.
> + }
> +}
> #endif /* !CONFIG_VNC */
>
> void qmp_change(const char *device, const char *target,
> diff --git a/qapi/ui.json b/qapi/ui.json
> index d08d72b439..bc3ffdb20f 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -1179,3 +1179,20 @@
> ##
> { 'command': 'query-display-options',
> 'returns': 'DisplayOptions' }
> +
> +##
> +# @reload-vnc-cert:
> +#
> +# Reload certificates for vnc.
> +#
> +# Returns: nothing
> +#
> +# Since: 5.2
> +#
> +# Example:
> +#
> +# -> { "execute": "reload-vnc-cert" }
> +# <- { "return": {} }
> +#
> +##
> +{ 'command': 'reload-vnc-cert' }
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 7452ac7df2..b0cfbcf47c 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -582,6 +582,26 @@ VncInfo2List *qmp_query_vnc_servers(Error **errp)
> return prev;
> }
>
> +int vnc_display_reload_cert(const char *id, Error **errp)
> +{
> + VncDisplay *vd = vnc_display_find(id);
> +
> + if (!vd) {
> + return -EINVAL;
> + }
> +
> + if (!vd->tlscreds) {
> + error_printf_unless_qmp("If you want use vnc tls please enable "
> + "vnc tls using '-vnc tls-creds=${tls-obj-id}'.\n");
> + return -EPERM;
You're passing in a 'Error' object, so use that and don't retyrn errnos
> + }
> +
> + object_property_set_bool(OBJECT(vd->tlscreds), "loaded", false, NULL);
> + object_property_set_bool(OBJECT(vd->tlscreds), "loaded", true, NULL);
This is ignoring all errors which is not at all acceptable. It means on
failure to load the new certs, we're left with a broken creds object
which callers will not expect.
We need to be able to do this in a safe way such that we carry on using
the original TLS certs if loading the new ones fails. This can't be done
using the 'loaded' property- we need an explicit API to reload things.
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 :|
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] vnc: add qmp to support reload vnc tls certificates
2021-01-04 12:27 ` Daniel P. Berrangé
@ 2021-01-15 13:25 ` Markus Armbruster
0 siblings, 0 replies; 4+ messages in thread
From: Markus Armbruster @ 2021-01-15 13:25 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: oscar.zhangbo, Zihao Chang, kraxel, xiexiangyou, qemu-devel
Daniel P. Berrangé <berrange@redhat.com> writes:
> On Mon, Jan 04, 2021 at 03:11:28PM +0800, Zihao Chang wrote:
>> QEMU loads vnc tls certificates only when vm is started. This patch
>> provides a new qmp command to reload vnc tls certificates without
>> restart vnc-server/VM.
>>
>> Signed-off-by: Zihao Chang <changzihao1@huawei.com>
[...]
>> diff --git a/ui/vnc.c b/ui/vnc.c
>> index 7452ac7df2..b0cfbcf47c 100644
>> --- a/ui/vnc.c
>> +++ b/ui/vnc.c
>> @@ -582,6 +582,26 @@ VncInfo2List *qmp_query_vnc_servers(Error **errp)
>> return prev;
>> }
>>
>> +int vnc_display_reload_cert(const char *id, Error **errp)
>> +{
>> + VncDisplay *vd = vnc_display_find(id);
>> +
>> + if (!vd) {
>> + return -EINVAL;
>> + }
>> +
>> + if (!vd->tlscreds) {
>> + error_printf_unless_qmp("If you want use vnc tls please enable "
>> + "vnc tls using '-vnc tls-creds=${tls-obj-id}'.\n");
>> + return -EPERM;
>
> You're passing in a 'Error' object, so use that and don't retyrn errnos
Do return a value that indicates success / failure. See the big comment
in include/qapi/error.h for why.
I recommend bool unless callers need to distinguish between several
failures.
[...]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] vnc: add qmp to support reload vnc tls certificates
2021-01-04 7:11 [PATCH] vnc: add qmp to support reload vnc tls certificates Zihao Chang
2021-01-04 12:27 ` Daniel P. Berrangé
@ 2021-01-19 19:23 ` Eric Blake
1 sibling, 0 replies; 4+ messages in thread
From: Eric Blake @ 2021-01-19 19:23 UTC (permalink / raw)
To: Zihao Chang, qemu-devel; +Cc: oscar.zhangbo, kraxel, xiexiangyou, armbru
On 1/4/21 1:11 AM, Zihao Chang wrote:
> QEMU loads vnc tls certificates only when vm is started. This patch
> provides a new qmp command to reload vnc tls certificates without
> restart vnc-server/VM.
>
> Signed-off-by: Zihao Chang <changzihao1@huawei.com>
> ---
> +++ b/qapi/ui.json
> @@ -1179,3 +1179,20 @@
> ##
> { 'command': 'query-display-options',
> 'returns': 'DisplayOptions' }
> +
> +##
> +# @reload-vnc-cert:
> +#
> +# Reload certificates for vnc.
> +#
> +# Returns: nothing
> +#
> +# Since: 5.2
You've missed 5.2. The next release will be 6.0.
> +#
> +# Example:
> +#
> +# -> { "execute": "reload-vnc-cert" }
> +# <- { "return": {} }
> +#
> +##
> +{ 'command': 'reload-vnc-cert' }
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-01-19 20:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-04 7:11 [PATCH] vnc: add qmp to support reload vnc tls certificates Zihao Chang
2021-01-04 12:27 ` Daniel P. Berrangé
2021-01-15 13:25 ` Markus Armbruster
2021-01-19 19:23 ` Eric Blake
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).