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

Pavel Grunt pgrunt at redhat.com
Mon Mar 7 13:06:42 UTC 2016


On Mon, 2016-03-07 at 13:16 +0100, Fabiano Fidêncio wrote:
> On Mon, Mar 7, 2016 at 11:53 AM, Pavel Grunt <pgrunt at redhat.com>
> wrote:
> > 
> > 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"
> Thanks for the suggestion.
> I'm going to squash this patch[0] to the one you've acked.

ack,
Pavel

> 
> [0]:
> diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> index 1cab1d9..f599a13 100644
> --- a/src/virt-viewer-app.c
> +++ b/src/virt-viewer-app.c
> @@ -405,7 +405,7 @@ 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", monitor, display);
> +            g_warning("Invalid monitor-mapping configuration:
> monitor
> #%i for display #%i does not exist", monitor, display);
>              goto configerror;
>          }
> 
> 
> Best Regards,
> --
> Fabiano Fidêncio




More information about the virt-tools-list mailing list