[virt-tools-list] [virt-viewer 1/2] app: monitor-config - do it all or nothing

Pavel Grunt pgrunt at redhat.com
Mon Mar 7 10:53:30 UTC 2016


On Mon, 2016-03-07 at 10:37 +0100, Fabiano Fidêncio wrote:
> Don't keep trying to use a monitor config when it already failed for
> one
> monitor, otherwise virt-viewer can end up in a situation where none
> of
> the displays are enabled but the program is still running.
> So, in case of any failure, let's skip the whole monitor config,
> forcing
> virt-viewer to use the "fallback" one instead.
> 
> Resolves: rhbz#1315206
> 
> Signed-off-by: Fabiano Fidêncio <fidencio at redhat.com>
> ---
>  src/virt-viewer-app.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> index 660acef..bbb4928 100644
> --- a/src/virt-viewer-app.c
> +++ b/src/virt-viewer-app.c
> @@ -404,9 +404,10 @@ virt_viewer_app_parse_monitor_mappings(gchar
> **mappings, gsize nmappings)
>          }
>          g_strfreev(tokens);
>  
> -        if (monitor > nmonitors)
> -            g_warning("Initial monitor #%i for display #%i does not
> exist, skipping...", monitor, display);
> -        else {
> +        if (monitor > nmonitors) {
> +            g_warning("Initial monitor #%i for display #%i does not
> exist", monitor, display);
> +            goto configerror;
> +        } else {
>              /* config file format is 1-based, not 0-based */
>              display--;
>              monitor--;

Hi, ack the change.

It is considered as the error now, so I would use the same prefix as in
any other monitor config message:

"Invalid monitor-mapping configuration: the monitor #%i for display #%i
does not exist"

The patch is good.

Pavel




More information about the virt-tools-list mailing list