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

[Fabiano Fidêncio]

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.

[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