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

Re: [libvirt] [PATCH 05/17] qemu_hotplug: cleanup qemuDomainChangeGraphics



On Thu, May 05, 2016 at 18:20:24 +0200, Pavel Hrdina wrote:

In subject: This mostly improves error messages, so cleanup is not
really a spot-on description of this patch.

> Signed-off-by: Pavel Hrdina <phrdina redhat com>
> ---
>  src/qemu/qemu_hotplug.c | 33 +++++++++++++++++----------------
>  1 file changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 03e5309..97f4152 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c

> @@ -2618,8 +2619,9 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver,
>      }
>  
>      if (dev->nListens != olddev->nListens) {
> -        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> -                       _("cannot change the number of listen addresses"));
> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> +                       _("cannot change the number of listen addresses "
> +                         "on '%s' graphics"), type);

I'm not quite sure whether the 'on' preposition is correct in this
context, but I don't have a better suggestion.

>          goto cleanup;
>      }
>  
> @@ -2628,30 +2630,30 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver,
>          virDomainGraphicsListenDefPtr oldlisten = &olddev->listens[i];
>  
>          if (newlisten->type != oldlisten->type) {
> -            virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> -                           _("cannot change the type of listen address"));
> +            virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> +                           _("cannot change the type of listen address "
> +                             "on '%s' graphics"), type);
>              goto cleanup;
>          }
>  
>          switch ((virDomainGraphicsListenType) newlisten->type) {
>          case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS:
>              if (STRNEQ_NULLABLE(newlisten->address, oldlisten->address)) {
> -                virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> -                               dev->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC ?
> -                               _("cannot change listen address setting on vnc graphics") :
> -                               _("cannot change listen address setting on spice graphics"));
> +                virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> +                               _("cannot change listen address setting "
> +                                 "on '%s' graphics"), type);
>                  goto cleanup;
>              }
> -            break;
>  
> +            break;
>          case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK:

I prefer the empty line before the next 'case' statement.

>              if (STRNEQ_NULLABLE(newlisten->network, oldlisten->network)) {
> -                virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> -                               dev->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC ?
> -                           _("cannot change listen network setting on vnc graphics") :
> -                           _("cannot change listen network setting on spice graphics"));
... ah so it was pre-existing. I don't care about it then :)
> +                virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> +                               _("cannot change listen address setting "
> +                                 "on '%s' graphics"), type);
>                  goto cleanup;
>              }
> +
>              break;
>  
>          case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE:

ACK

Attachment: signature.asc
Description: Digital signature


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