| Message ID | 20210104071128.754-1-changzihao1@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vnc: add qmp to support reload vnc tls certificates | expand |
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
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. [...]
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' }
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
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(+)