[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] qemuDomainChangeGraphics: Check listen address change by listen type



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]