| Message ID | 20210107143032.752-3-changzihao1@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | support tls certificates reload | expand |
Zihao Chang <changzihao1@huawei.com> writes: > QEMU loads vnc tls certificates only when vm is started. This patch > provides a new qmp to reload vnc tls certificates without restart > vnc-server/VM. > {"execute": "reload-vnc-cert"} > > Signed-off-by: Zihao Chang <changzihao1@huawei.com> > --- > include/ui/console.h | 1 + > monitor/qmp-cmds.c | 5 +++++ > qapi/ui.json | 18 ++++++++++++++++++ > ui/vnc.c | 24 ++++++++++++++++++++++++ > 4 files changed, 48 insertions(+) > > diff --git a/include/ui/console.h b/include/ui/console.h > index 5dd21976a3..60a24a8bb5 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); > +void vnc_display_reload_cert(const char *id, Error **errp); Make this return bool, please. error.h's big comment: * = Rules = * * - Functions that use Error to report errors have an Error **errp * parameter. It should be the last parameter, except for functions * taking variable arguments. * * - You may pass NULL to not receive the error, &error_abort to abort * on error, &error_fatal to exit(1) on error, or a pointer to a * variable containing NULL to receive the error. * * - Separation of concerns: the function is responsible for detecting * errors and failing cleanly; handling the error is its caller's * job. Since the value of @errp is about handling the error, the * function should not examine it. * * - The function may pass @errp to functions it calls to pass on * their errors to its caller. If it dereferences @errp to check * for errors, it must use ERRP_GUARD(). * * - On success, the function should not touch *errp. On failure, it * should set a new error, e.g. with error_setg(errp, ...), or * propagate an existing one, e.g. with error_propagate(errp, ...). * * - Whenever practical, also return a value that indicates success / * failure. This can make the error checking more concise, and can * avoid useless error object creation and destruction. Note that * we still have many functions returning void. We recommend * • bool-valued functions return true on success / false on failure, * • pointer-valued functions return non-null / null pointer, and * • integer-valued functions return non-negative / negative. > > /* 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..84f2b74ea8 100644 > --- a/monitor/qmp-cmds.c > +++ b/monitor/qmp-cmds.c > @@ -287,6 +287,11 @@ 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) > +{ > + vnc_display_reload_cert(NULL, errp); > +} > #endif /* !CONFIG_VNC */ > > void qmp_change(const char *device, const char *target, > diff --git a/qapi/ui.json b/qapi/ui.json > index d08d72b439..855b1ac007 100644 > --- a/qapi/ui.json > +++ b/qapi/ui.json > @@ -1179,3 +1179,21 @@ > ## > { 'command': 'query-display-options', > 'returns': 'DisplayOptions' } > + > +## > +# @reload-vnc-cert: > +# > +# Reload certificates for vnc. > +# > +# Returns: nothing > +# > +# Since: 5.2 6.0 now. > +# > +# Example: > +# > +# -> { "execute": "reload-vnc-cert" } > +# <- { "return": {} } > +# > +## > +{ 'command': 'reload-vnc-cert', > + 'if': 'defined(CONFIG_VNC)' } Daniel's objection to another patch that adds a rather specialized QMP command may apply: "I'm not a fan of adding adhoc new commands for specific properties." From: Daniel P. Berrangé <berrange@redhat.com> Subject: Re: [PATCH] vnc: add qmp to support change authz Message-ID: <20210107161830.GE1029501@redhat.com> [...]
On Fri, Jan 15, 2021 at 02:37:33PM +0100, Markus Armbruster wrote: > Zihao Chang <changzihao1@huawei.com> writes: > > > QEMU loads vnc tls certificates only when vm is started. This patch > > provides a new qmp to reload vnc tls certificates without restart > > vnc-server/VM. > > {"execute": "reload-vnc-cert"} > > > > Signed-off-by: Zihao Chang <changzihao1@huawei.com> > > --- > > include/ui/console.h | 1 + > > monitor/qmp-cmds.c | 5 +++++ > > qapi/ui.json | 18 ++++++++++++++++++ > > ui/vnc.c | 24 ++++++++++++++++++++++++ > > 4 files changed, 48 insertions(+) > > > > diff --git a/include/ui/console.h b/include/ui/console.h > > index 5dd21976a3..60a24a8bb5 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); > > +void vnc_display_reload_cert(const char *id, Error **errp); > > Make this return bool, please. > > error.h's big comment: > > * = Rules = > * > * - Functions that use Error to report errors have an Error **errp > * parameter. It should be the last parameter, except for functions > * taking variable arguments. > * > * - You may pass NULL to not receive the error, &error_abort to abort > * on error, &error_fatal to exit(1) on error, or a pointer to a > * variable containing NULL to receive the error. > * > * - Separation of concerns: the function is responsible for detecting > * errors and failing cleanly; handling the error is its caller's > * job. Since the value of @errp is about handling the error, the > * function should not examine it. > * > * - The function may pass @errp to functions it calls to pass on > * their errors to its caller. If it dereferences @errp to check > * for errors, it must use ERRP_GUARD(). > * > * - On success, the function should not touch *errp. On failure, it > * should set a new error, e.g. with error_setg(errp, ...), or > * propagate an existing one, e.g. with error_propagate(errp, ...). > * > * - Whenever practical, also return a value that indicates success / > * failure. This can make the error checking more concise, and can > * avoid useless error object creation and destruction. Note that > * we still have many functions returning void. We recommend > * • bool-valued functions return true on success / false on failure, > * • pointer-valued functions return non-null / null pointer, and > * • integer-valued functions return non-negative / negative. > > > > > /* 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..84f2b74ea8 100644 > > --- a/monitor/qmp-cmds.c > > +++ b/monitor/qmp-cmds.c > > @@ -287,6 +287,11 @@ 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) > > +{ > > + vnc_display_reload_cert(NULL, errp); > > +} > > #endif /* !CONFIG_VNC */ > > > > void qmp_change(const char *device, const char *target, > > diff --git a/qapi/ui.json b/qapi/ui.json > > index d08d72b439..855b1ac007 100644 > > --- a/qapi/ui.json > > +++ b/qapi/ui.json > > @@ -1179,3 +1179,21 @@ > > ## > > { 'command': 'query-display-options', > > 'returns': 'DisplayOptions' } > > + > > +## > > +# @reload-vnc-cert: > > +# > > +# Reload certificates for vnc. > > +# > > +# Returns: nothing > > +# > > +# Since: 5.2 > > 6.0 now. > > > +# > > +# Example: > > +# > > +# -> { "execute": "reload-vnc-cert" } > > +# <- { "return": {} } > > +# > > +## > > +{ 'command': 'reload-vnc-cert', > > + 'if': 'defined(CONFIG_VNC)' } > > Daniel's objection to another patch that adds a rather specialized QMP > command may apply: "I'm not a fan of adding adhoc new commands for > specific properties." > > From: Daniel P. Berrangé <berrange@redhat.com> > Subject: Re: [PATCH] vnc: add qmp to support change authz > Message-ID: <20210107161830.GE1029501@redhat.com> Yeah, though this scenario is a ittle more complicated. Tihs patch is not actually changing any object properties in the VNC server config. It is simply telling the VNC server to reload the existing object it has configured. My proposed "display-update" command would not directly map to what this "reload-vnc-cert" command does, unless we declared the certs are always reloaded after every display-update command. Or we could have a more generic "display-reload" command, which has fields indicating what aspect to reload. eg a 'tls-certs: bool' field to indicate reload of TLS certs in the display. This could be useful if we wanted the ability to reload authz access control lists, though we did actually wire them up to auto-reload using inotify. Regards, Daniel
On 2021/1/15 21:47, Daniel P. Berrangé wrote: > On Fri, Jan 15, 2021 at 02:37:33PM +0100, Markus Armbruster wrote: >> Zihao Chang <changzihao1@huawei.com> writes: >> >>> QEMU loads vnc tls certificates only when vm is started. This patch >>> provides a new qmp to reload vnc tls certificates without restart >>> vnc-server/VM. >>> {"execute": "reload-vnc-cert"} >>> >>> Signed-off-by: Zihao Chang <changzihao1@huawei.com> >>> --- >>> include/ui/console.h | 1 + >>> monitor/qmp-cmds.c | 5 +++++ >>> qapi/ui.json | 18 ++++++++++++++++++ >>> ui/vnc.c | 24 ++++++++++++++++++++++++ >>> 4 files changed, 48 insertions(+) >>> >>> diff --git a/include/ui/console.h b/include/ui/console.h >>> index 5dd21976a3..60a24a8bb5 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); >>> +void vnc_display_reload_cert(const char *id, Error **errp); >> >> Make this return bool, please. >> I will fix this in the next version. Thank your for your reviews. >> error.h's big comment: >> >> * = Rules = >> * >> * - Functions that use Error to report errors have an Error **errp >> * parameter. It should be the last parameter, except for functions >> * taking variable arguments. >> * >> * - You may pass NULL to not receive the error, &error_abort to abort >> * on error, &error_fatal to exit(1) on error, or a pointer to a >> * variable containing NULL to receive the error. >> * >> * - Separation of concerns: the function is responsible for detecting >> * errors and failing cleanly; handling the error is its caller's >> * job. Since the value of @errp is about handling the error, the >> * function should not examine it. >> * >> * - The function may pass @errp to functions it calls to pass on >> * their errors to its caller. If it dereferences @errp to check >> * for errors, it must use ERRP_GUARD(). >> * >> * - On success, the function should not touch *errp. On failure, it >> * should set a new error, e.g. with error_setg(errp, ...), or >> * propagate an existing one, e.g. with error_propagate(errp, ...). >> * >> * - Whenever practical, also return a value that indicates success / >> * failure. This can make the error checking more concise, and can >> * avoid useless error object creation and destruction. Note that >> * we still have many functions returning void. We recommend >> * • bool-valued functions return true on success / false on failure, >> * • pointer-valued functions return non-null / null pointer, and >> * • integer-valued functions return non-negative / negative. >> >>> >>> /* 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..84f2b74ea8 100644 >>> --- a/monitor/qmp-cmds.c >>> +++ b/monitor/qmp-cmds.c >>> @@ -287,6 +287,11 @@ 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) >>> +{ >>> + vnc_display_reload_cert(NULL, errp); >>> +} >>> #endif /* !CONFIG_VNC */ >>> >>> void qmp_change(const char *device, const char *target, >>> diff --git a/qapi/ui.json b/qapi/ui.json >>> index d08d72b439..855b1ac007 100644 >>> --- a/qapi/ui.json >>> +++ b/qapi/ui.json >>> @@ -1179,3 +1179,21 @@ >>> ## >>> { 'command': 'query-display-options', >>> 'returns': 'DisplayOptions' } >>> + >>> +## >>> +# @reload-vnc-cert: >>> +# >>> +# Reload certificates for vnc. >>> +# >>> +# Returns: nothing >>> +# >>> +# Since: 5.2 >> >> 6.0 now. >> >>> +# >>> +# Example: >>> +# >>> +# -> { "execute": "reload-vnc-cert" } >>> +# <- { "return": {} } >>> +# >>> +## >>> +{ 'command': 'reload-vnc-cert', >>> + 'if': 'defined(CONFIG_VNC)' } >> >> Daniel's objection to another patch that adds a rather specialized QMP >> command may apply: "I'm not a fan of adding adhoc new commands for >> specific properties." >> >> From: Daniel P. Berrangé <berrange@redhat.com> >> Subject: Re: [PATCH] vnc: add qmp to support change authz >> Message-ID: <20210107161830.GE1029501@redhat.com> > > Yeah, though this scenario is a ittle more complicated. Tihs patch is > not actually changing any object properties in the VNC server config. > It is simply telling the VNC server to reload the existing object it > has configured. > > My proposed "display-update" command would not directly map to what > this "reload-vnc-cert" command does, unless we declared the certs are > always reloaded after every display-update command. > > Or we could have a more generic "display-reload" command, which has > fields indicating what aspect to reload. eg a 'tls-certs: bool' field > to indicate reload of TLS certs in the display. This could be useful > if we wanted the ability to reload authz access control lists, though > we did actually wire them up to auto-reload using inotify. > > > Regards, > Daniel > If we add field for each reloadable attribute(tls-certs, authz,...), the number of parameters for qmp_display_reload() may increase sharply (bool has_tls_certs, bool tls_certs, ... twice the number of attributes). How about using a list[] to determine which attributes need to be reloaded["tls_certs", "authz*"...], and define an enum to ensure the validity of list elements. Regards, Zihao
Zihao Chang <changzihao1@huawei.com> writes: > On 2021/1/15 21:47, Daniel P. Berrangé wrote: >> On Fri, Jan 15, 2021 at 02:37:33PM +0100, Markus Armbruster wrote: >>> Zihao Chang <changzihao1@huawei.com> writes: [...] >>>> diff --git a/qapi/ui.json b/qapi/ui.json >>>> index d08d72b439..855b1ac007 100644 >>>> --- a/qapi/ui.json >>>> +++ b/qapi/ui.json >>>> @@ -1179,3 +1179,21 @@ >>>> ## >>>> { 'command': 'query-display-options', >>>> 'returns': 'DisplayOptions' } >>>> + >>>> +## >>>> +# @reload-vnc-cert: >>>> +# >>>> +# Reload certificates for vnc. >>>> +# >>>> +# Returns: nothing >>>> +# >>>> +# Since: 5.2 >>> >>> 6.0 now. >>> >>>> +# >>>> +# Example: >>>> +# >>>> +# -> { "execute": "reload-vnc-cert" } >>>> +# <- { "return": {} } >>>> +# >>>> +## >>>> +{ 'command': 'reload-vnc-cert', >>>> + 'if': 'defined(CONFIG_VNC)' } >>> >>> Daniel's objection to another patch that adds a rather specialized QMP >>> command may apply: "I'm not a fan of adding adhoc new commands for >>> specific properties." >>> >>> From: Daniel P. Berrangé <berrange@redhat.com> >>> Subject: Re: [PATCH] vnc: add qmp to support change authz >>> Message-ID: <20210107161830.GE1029501@redhat.com> >> >> Yeah, though this scenario is a ittle more complicated. Tihs patch is >> not actually changing any object properties in the VNC server config. >> It is simply telling the VNC server to reload the existing object it >> has configured. >> >> My proposed "display-update" command would not directly map to what >> this "reload-vnc-cert" command does, unless we declared the certs are >> always reloaded after every display-update command. >> >> Or we could have a more generic "display-reload" command, which has >> fields indicating what aspect to reload. eg a 'tls-certs: bool' field >> to indicate reload of TLS certs in the display. This could be useful >> if we wanted the ability to reload authz access control lists, though >> we did actually wire them up to auto-reload using inotify. >> >> >> Regards, >> Daniel >> > > If we add field for each reloadable attribute(tls-certs, authz,...), > the number of parameters for qmp_display_reload() may increase sharply > (bool has_tls_certs, bool tls_certs, ... twice the number of attributes). 'boxed': true can give you a reasonable C function even then. docs/devel/qapi-code-gen.txt: When member 'boxed' is absent, [the function generated for the command] takes the command arguments as arguments one by one, in QAPI schema order. Else it takes them wrapped in the C struct generated for the complex argument type. It takes an additional Error ** argument in either case. > How about using a list[] to determine which attributes need to be > reloaded["tls_certs", "authz*"...], and define an enum to ensure the > validity of list elements. Representing a set of named things as a "list of enum of thing names" is workable. It's a fairly rigid design, though. When you start with "struct of bool members", you can add members of other types, and the whole still looks regular. You can even evolve an existing bool into an alternate of bool and something else. What kind of evolution do you want to prepare for? Two foolish answers to avoid: "any and all, regardless of cost" (you should not brush off cost like that, ever), and "none, because I can predict the future confidently" (no, you can't).
On Mon, Jan 18, 2021 at 03:27:33PM +0800, Zihao Chang wrote: > > > On 2021/1/15 21:47, Daniel P. Berrangé wrote: > > On Fri, Jan 15, 2021 at 02:37:33PM +0100, Markus Armbruster wrote: > >> Zihao Chang <changzihao1@huawei.com> writes: > >> > >>> QEMU loads vnc tls certificates only when vm is started. This patch > >>> provides a new qmp to reload vnc tls certificates without restart > >>> vnc-server/VM. > >>> {"execute": "reload-vnc-cert"} > >>> > >>> Signed-off-by: Zihao Chang <changzihao1@huawei.com> > >>> --- > >>> include/ui/console.h | 1 + > >>> monitor/qmp-cmds.c | 5 +++++ > >>> qapi/ui.json | 18 ++++++++++++++++++ > >>> ui/vnc.c | 24 ++++++++++++++++++++++++ > >>> 4 files changed, 48 insertions(+) > >>> > >>> diff --git a/include/ui/console.h b/include/ui/console.h > >>> index 5dd21976a3..60a24a8bb5 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); > >>> +void vnc_display_reload_cert(const char *id, Error **errp); > >> > >> Make this return bool, please. > >> > I will fix this in the next version. > Thank your for your reviews. > > >> error.h's big comment: > >> > >> * = Rules = > >> * > >> * - Functions that use Error to report errors have an Error **errp > >> * parameter. It should be the last parameter, except for functions > >> * taking variable arguments. > >> * > >> * - You may pass NULL to not receive the error, &error_abort to abort > >> * on error, &error_fatal to exit(1) on error, or a pointer to a > >> * variable containing NULL to receive the error. > >> * > >> * - Separation of concerns: the function is responsible for detecting > >> * errors and failing cleanly; handling the error is its caller's > >> * job. Since the value of @errp is about handling the error, the > >> * function should not examine it. > >> * > >> * - The function may pass @errp to functions it calls to pass on > >> * their errors to its caller. If it dereferences @errp to check > >> * for errors, it must use ERRP_GUARD(). > >> * > >> * - On success, the function should not touch *errp. On failure, it > >> * should set a new error, e.g. with error_setg(errp, ...), or > >> * propagate an existing one, e.g. with error_propagate(errp, ...). > >> * > >> * - Whenever practical, also return a value that indicates success / > >> * failure. This can make the error checking more concise, and can > >> * avoid useless error object creation and destruction. Note that > >> * we still have many functions returning void. We recommend > >> * • bool-valued functions return true on success / false on failure, > >> * • pointer-valued functions return non-null / null pointer, and > >> * • integer-valued functions return non-negative / negative. > >> > >>> > >>> /* 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..84f2b74ea8 100644 > >>> --- a/monitor/qmp-cmds.c > >>> +++ b/monitor/qmp-cmds.c > >>> @@ -287,6 +287,11 @@ 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) > >>> +{ > >>> + vnc_display_reload_cert(NULL, errp); > >>> +} > >>> #endif /* !CONFIG_VNC */ > >>> > >>> void qmp_change(const char *device, const char *target, > >>> diff --git a/qapi/ui.json b/qapi/ui.json > >>> index d08d72b439..855b1ac007 100644 > >>> --- a/qapi/ui.json > >>> +++ b/qapi/ui.json > >>> @@ -1179,3 +1179,21 @@ > >>> ## > >>> { 'command': 'query-display-options', > >>> 'returns': 'DisplayOptions' } > >>> + > >>> +## > >>> +# @reload-vnc-cert: > >>> +# > >>> +# Reload certificates for vnc. > >>> +# > >>> +# Returns: nothing > >>> +# > >>> +# Since: 5.2 > >> > >> 6.0 now. > >> > >>> +# > >>> +# Example: > >>> +# > >>> +# -> { "execute": "reload-vnc-cert" } > >>> +# <- { "return": {} } > >>> +# > >>> +## > >>> +{ 'command': 'reload-vnc-cert', > >>> + 'if': 'defined(CONFIG_VNC)' } > >> > >> Daniel's objection to another patch that adds a rather specialized QMP > >> command may apply: "I'm not a fan of adding adhoc new commands for > >> specific properties." > >> > >> From: Daniel P. Berrangé <berrange@redhat.com> > >> Subject: Re: [PATCH] vnc: add qmp to support change authz > >> Message-ID: <20210107161830.GE1029501@redhat.com> > > > > Yeah, though this scenario is a ittle more complicated. Tihs patch is > > not actually changing any object properties in the VNC server config. > > It is simply telling the VNC server to reload the existing object it > > has configured. > > > > My proposed "display-update" command would not directly map to what > > this "reload-vnc-cert" command does, unless we declared the certs are > > always reloaded after every display-update command. > > > > Or we could have a more generic "display-reload" command, which has > > fields indicating what aspect to reload. eg a 'tls-certs: bool' field > > to indicate reload of TLS certs in the display. This could be useful > > if we wanted the ability to reload authz access control lists, though > > we did actually wire them up to auto-reload using inotify. > > > > > > Regards, > > Daniel > > > > If we add field for each reloadable attribute(tls-certs, authz,...), > the number of parameters for qmp_display_reload() may increase sharply > (bool has_tls_certs, bool tls_certs, ... twice the number of attributes). There's a fairly limited conceptual set of VNC features which are going to require a reload operation, so I don't think it'll grow too unreasonably large. > How about using a list[] to determine which attributes need to be > reloaded["tls_certs", "authz*"...], and define an enum to ensure the > validity of list elements. The enum is simple, but if we require data to be provided fr the reload operation, instead of a simple boolean, then it gets a bit more limiting. Regards, Daniel
Hi, > Or we could have a more generic "display-reload" command, which has > fields indicating what aspect to reload. eg a 'tls-certs: bool' field > to indicate reload of TLS certs in the display. This could be useful > if we wanted the ability to reload authz access control lists, though > we did actually wire them up to auto-reload using inotify. Maybe we should just use inotify-based reload for the certs too? take care, Gerd
On Mon, Jan 18, 2021 at 05:13:16PM +0100, Gerd Hoffmann wrote: > Hi, > > > Or we could have a more generic "display-reload" command, which has > > fields indicating what aspect to reload. eg a 'tls-certs: bool' field > > to indicate reload of TLS certs in the display. This could be useful > > if we wanted the ability to reload authz access control lists, though > > we did actually wire them up to auto-reload using inotify. > > Maybe we should just use inotify-based reload for the certs too? The authz access control is easy because it is just one file. When updating the certs though, we have 1-4 files that need loading, and they can only be reloaded once all of them are updated on disk. This gives a synchronization challenge for use of inotify, as when we see 1 updated, we don't know if we need to wait for the others to be updated or not. Regards, Daniel
diff --git a/include/ui/console.h b/include/ui/console.h index 5dd21976a3..60a24a8bb5 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); +void 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..84f2b74ea8 100644 --- a/monitor/qmp-cmds.c +++ b/monitor/qmp-cmds.c @@ -287,6 +287,11 @@ 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) +{ + vnc_display_reload_cert(NULL, errp); +} #endif /* !CONFIG_VNC */ void qmp_change(const char *device, const char *target, diff --git a/qapi/ui.json b/qapi/ui.json index d08d72b439..855b1ac007 100644 --- a/qapi/ui.json +++ b/qapi/ui.json @@ -1179,3 +1179,21 @@ ## { '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', + 'if': 'defined(CONFIG_VNC)' } diff --git a/ui/vnc.c b/ui/vnc.c index 7452ac7df2..2cc88c2421 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -582,6 +582,30 @@ VncInfo2List *qmp_query_vnc_servers(Error **errp) return prev; } +void vnc_display_reload_cert(const char *id, Error **errp) +{ + VncDisplay *vd = vnc_display_find(id); + QCryptoTLSCredsClass *creds = NULL; + + if (!vd) { + error_setg(errp, "Can not find Vnc Display"); + return; + } + + if (!vd->tlscreds) { + error_setg(errp, "Vnc tls is not enable"); + return; + } + + creds = QCRYPTO_TLS_CREDS_GET_CLASS(OBJECT(vd->tlscreds)); + if (creds->reload == NULL) { + error_setg(errp, "%s doesn't support to reload TLS credential", + object_get_typename(OBJECT(vd->tlscreds))); + return; + } + creds->reload(vd->tlscreds, errp); +} + /* 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 to reload vnc tls certificates without restart vnc-server/VM. {"execute": "reload-vnc-cert"} Signed-off-by: Zihao Chang <changzihao1@huawei.com> --- include/ui/console.h | 1 + monitor/qmp-cmds.c | 5 +++++ qapi/ui.json | 18 ++++++++++++++++++ ui/vnc.c | 24 ++++++++++++++++++++++++ 4 files changed, 48 insertions(+)