On Thu, Jun 20, 2013 at 18:40:34 +0200, Michal Privoznik wrote:
> Currently, we have a bug when updating a graphics device. A graphics device can
> have a listen address set. This address is either defined by user (in which case
> it's type is VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS) or it can be inherited
> from a network (in which case it's type is
> VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK). However, in both cases we have a
> listen address to process (e.g. during migration, as I've tried to fix in
> 7f15ebc7).
> Later, when an user tries to update the graphics device (e.g. set a password),
> we check if listen addresses match the original as qemu doesn't know how to
> change listen address yet. Hence, users are required to not change the listen
> address. The implementation then just dumps listen addresses and compare them.
> Previously, while dumping the listen addresses, NULL was returned for NETWORK.
> After my patch, this is no longer true, and we get a listen address for olddev
> even if it is a type of NETWORK. So we have a real string on one side, the NULL
> from user's XML on the other side and hence we think user wants to change the
> listen address and we refuse it.
>
> Therefore, we must take the type of listen address into account as well.
> ---
> src/qemu/qemu_hotplug.c | 71 +++++++++++++++++++++++++++++++------------------
> 1 file changed, 45 insertions(+), 26 deletions(-)
>
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 6c07af5..8563457 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1971,10 +1970,50 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver,
...
> + switch (listen->type) {
> + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS:
> + if (STRNEQ_NULLABLE(listen->address, oldlisten->address)) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + dev->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC ?
> + _("cannot change listen address setting on vnc graphics") :
> + _("cannot change listen address setting on spice graphics"));
> + goto cleanup;
> + }
> + break;
> +
> + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK:
> + if (STRNEQ_NULLABLE(listen->network, oldlisten->network)) {
> + virReportError(VIR_ERR_INVALID_ARG, "%s",
> + dev->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC ?
> + _("cannot change listen network setting on vnc graphics") :
> + _("cannot change listen network setting on spice graphics"));
> + goto cleanup;
> + }
> + break;
> +
> + default:
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("listen of type %d not handled yet"),
> + listen->type);
> + goto cleanup;
The default case could only happen if a new listen type is introduced
and this switch was not properly updated, in which case detecting it in
run-time is too late. Just let compilers check that for us: use (enum
virDomainGraphicsListenType) listen->type in the switch and handle all
cases explicitly without using the default branch.
Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list