Re: [virt-tools-list] [PATCH virt-viewer] Avoid warning when loading initial monitor mapping
On Tue, 2017-04-04 at 10:40 -0500, Jonathon Jongsma wrote:
> On Mon, 2017-04-03 at 08:58 +0200, Pavel Grunt wrote:
> > Hi Jonathon,
> >
> > On Wed, 2017-03-29 at 15:24 -0500, Jonathon Jongsma wrote:
> > > When started in fullscreen mode with a monitor-mapping
> > > configuration
> > > option, we are getting the following warnings on the terminal:
> >
> > I haven't seen it. wayland ?
>
> Yes. Wayland on Fedora 25.
>
> >
> > >
> > > (process:27428): Gdk-CRITICAL **: gdk_screen_get_n_monitors:
> > > assertion 'GDK_IS_SCREEN (screen)' failed
> > >
> > > ** (process:27428): WARNING **: Invalid monitor-mapping
> > > configuration: monitor #1 for display #1 does not exist
> > >
> > > This is apparently because we were processing the fallback
> > > monitor
> > > mapping before the graphic server display was opened, so
> > > gdk_screen_get_default() returned NULL. This resulted in
> > > gdk_screen_get_n_monitors() reporting that we had 0 monitors.
> > >
> > > This patch moves the fallback monitor mapping parsing from
> > > virt_viewer_app_init() to
> > > virt_viewer_app_on_application_startup(),
> > > after chaining up to the base class startup() vfunc. The graphic
> > > server
> > > display is opened in the base class vfunc, so by the time that
> > > returns,
> > > we should be able to query the number of monitors.
> >
> > sure, it makes a sense. Ack
> >
> > >
> > > The patch also adds a check in
> > > virt_viewer_parse_monitor_mappings()
> > > to
> > > ensure that we pass a sane value for nmonitors.
> >
> > Sounds like testable
>
> Can you clarify what you mean here?
we have a test-monitor-mapping.c, we can trigger/check for that the
GCritical there.
>
>
> >
> > Thanks,
> > Pavel
> >
> >
> > > ---
> > > src/virt-viewer-app.c | 2 +-
> > > src/virt-viewer-util.c | 7 +--
> > > 2 files changed, 6 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> > > index 2e7e193..8c5c3f5 100644
> > > --- a/src/virt-viewer-app.c
> > > +++ b/src/virt-viewer-app.c
> > > @@ -1730,7 +1730,6 @@ virt_viewer_app_init(VirtViewerApp *self)
> > >
> > > g_clear_error();
> > >
> > > -self->priv->initial_display_map =
> > > virt_viewer_app_get_monitor_mapping_for_section(self,
> > > "fallback");
> > > g_signal_connect(self, "notify::guest-name",
> > > G_CALLBACK(title_maybe_changed), NULL);
> > > g_signal_connect(self, "notify::title",
> > > G_CALLBACK(title_maybe_changed), NULL);
> > > g_signal_connect(self, "notify::guri",
> > > G_CALLBACK(title_maybe_changed), NULL);
> > > @@ -1807,6 +1806,7 @@
> > > virt_viewer_app_on_application_startup(GApplication *app)
> > > self->priv->main_window = virt_viewer_app_window_new(self,
> > > virt_v
> > > iew
> > > e
> > > r_app_get_first_monitor(self));
> > > self->priv->main_notebook =
> > > GTK_WIDGET(virt_viewer_window_get_notebook(self->priv-
> > > > main_window));
> > >
> > > +self->priv->initial_display_map =
> > > virt_viewer_app_get_monitor_mapping_for_section(self,
> > > "fallback");
> > >
> > > virt_viewer_app_set_kiosk(self, opt_kiosk);
> > > virt_viewer_app_set_hotkeys(self, opt_hotkeys);
> > > diff --git a/src/virt-viewer-util.c b/src/virt-viewer-util.c
> > > index 0491f73..6b63133 100644
> > > --- a/src/virt-viewer-util.c
> > > +++ b/src/virt-viewer-util.c
> > > @@ -641,11 +641,14 @@
> > > virt_viewer_shift_monitors_to_origin(GHashTable *displays)
> > > GHashTable*
> > > virt_viewer_parse_monitor_mappings(gchar **mappings, const
> > > gsize
> > > nmappings, const gint nmonitors)
> > > {
> > > -GHashTable *displaymap = g_hash_table_new(g_direct_hash,
> > > g_direct_equal);
> > > -GHashTable *monitormap = g_hash_table_new(g_direct_hash,
> > > g_direct_equal);
> > > +GHashTable *displaymap;
> > > +GHashTable *monitormap;
> > > gint i, max_display_id = 0;
> > > gchar **tokens = NULL;
> > >
> > > +g_return_val_if_fail(nmonitors != 0, NULL);
> > > +displaymap = g_hash_table_new(g_direct_hash,
> > > g_direct_equal);
> > > +monitormap = g_hash_table_new(g_direct_hash,
> > > g_direct_equal);
> > > if (nmappings == 0) {
> > > g_warning("Empty monitor-mapping configuration");
> > > goto configerror;
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [PATCH virt-viewer] Avoid warning when loading initial monitor mapping
On Mon, 2017-04-03 at 08:58 +0200, Pavel Grunt wrote:
> Hi Jonathon,
>
> On Wed, 2017-03-29 at 15:24 -0500, Jonathon Jongsma wrote:
> > When started in fullscreen mode with a monitor-mapping
> > configuration
> > option, we are getting the following warnings on the terminal:
>
> I haven't seen it. wayland ?
Yes. Wayland on Fedora 25.
>
> >
> > (process:27428): Gdk-CRITICAL **: gdk_screen_get_n_monitors:
> > assertion 'GDK_IS_SCREEN (screen)' failed
> >
> > ** (process:27428): WARNING **: Invalid monitor-mapping
> > configuration: monitor #1 for display #1 does not exist
> >
> > This is apparently because we were processing the fallback monitor
> > mapping before the graphic server display was opened, so
> > gdk_screen_get_default() returned NULL. This resulted in
> > gdk_screen_get_n_monitors() reporting that we had 0 monitors.
> >
> > This patch moves the fallback monitor mapping parsing from
> > virt_viewer_app_init() to virt_viewer_app_on_application_startup(),
> > after chaining up to the base class startup() vfunc. The graphic
> > server
> > display is opened in the base class vfunc, so by the time that
> > returns,
> > we should be able to query the number of monitors.
>
> sure, it makes a sense. Ack
>
> >
> > The patch also adds a check in virt_viewer_parse_monitor_mappings()
> > to
> > ensure that we pass a sane value for nmonitors.
>
> Sounds like testable
Can you clarify what you mean here?
>
> Thanks,
> Pavel
>
>
> > ---
> > src/virt-viewer-app.c | 2 +-
> > src/virt-viewer-util.c | 7 +--
> > 2 files changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> > index 2e7e193..8c5c3f5 100644
> > --- a/src/virt-viewer-app.c
> > +++ b/src/virt-viewer-app.c
> > @@ -1730,7 +1730,6 @@ virt_viewer_app_init(VirtViewerApp *self)
> >
> > g_clear_error();
> >
> > -self->priv->initial_display_map =
> > virt_viewer_app_get_monitor_mapping_for_section(self, "fallback");
> > g_signal_connect(self, "notify::guest-name",
> > G_CALLBACK(title_maybe_changed), NULL);
> > g_signal_connect(self, "notify::title",
> > G_CALLBACK(title_maybe_changed), NULL);
> > g_signal_connect(self, "notify::guri",
> > G_CALLBACK(title_maybe_changed), NULL);
> > @@ -1807,6 +1806,7 @@
> > virt_viewer_app_on_application_startup(GApplication *app)
> > self->priv->main_window = virt_viewer_app_window_new(self,
> > virt_view
> > e
> > r_app_get_first_monitor(self));
> > self->priv->main_notebook =
> > GTK_WIDGET(virt_viewer_window_get_notebook(self->priv-
> > > main_window));
> >
> > +self->priv->initial_display_map =
> > virt_viewer_app_get_monitor_mapping_for_section(self, "fallback");
> >
> > virt_viewer_app_set_kiosk(self, opt_kiosk);
> > virt_viewer_app_set_hotkeys(self, opt_hotkeys);
> > diff --git a/src/virt-viewer-util.c b/src/virt-viewer-util.c
> > index 0491f73..6b63133 100644
> > --- a/src/virt-viewer-util.c
> > +++ b/src/virt-viewer-util.c
> > @@ -641,11 +641,14 @@
> > virt_viewer_shift_monitors_to_origin(GHashTable *displays)
> > GHashTable*
> > virt_viewer_parse_monitor_mappings(gchar **mappings, const gsize
> > nmappings, const gint nmonitors)
> > {
> > -GHashTable *displaymap = g_hash_table_new(g_direct_hash,
> > g_direct_equal);
> > -GHashTable *monitormap = g_hash_table_new(g_direct_hash,
> > g_direct_equal);
> > +GHashTable *displaymap;
> > +GHashTable *monitormap;
> > gint i, max_display_id = 0;
> > gchar **tokens = NULL;
> >
> > +g_return_val_if_fail(nmonitors != 0, NULL);
> > +displaymap = g_hash_table_new(g_direct_hash, g_direct_equal);
> > +monitormap = g_hash_table_new(g_direct_hash, g_direct_equal);
> > if (nmappings == 0) {
> > g_warning("Empty monitor-mapping configuration");
> > goto configerror;
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [PATCH virt-viewer] Avoid warning when loading initial monitor mapping
Hi Jonathon,
On Wed, 2017-03-29 at 15:24 -0500, Jonathon Jongsma wrote:
> When started in fullscreen mode with a monitor-mapping configuration
> option, we are getting the following warnings on the terminal:
I haven't seen it. wayland ?
>
> (process:27428): Gdk-CRITICAL **: gdk_screen_get_n_monitors:
> assertion 'GDK_IS_SCREEN (screen)' failed
>
> ** (process:27428): WARNING **: Invalid monitor-mapping
> configuration: monitor #1 for display #1 does not exist
>
> This is apparently because we were processing the fallback monitor
> mapping before the graphic server display was opened, so
> gdk_screen_get_default() returned NULL. This resulted in
> gdk_screen_get_n_monitors() reporting that we had 0 monitors.
>
> This patch moves the fallback monitor mapping parsing from
> virt_viewer_app_init() to virt_viewer_app_on_application_startup(),
> after chaining up to the base class startup() vfunc. The graphic
> server
> display is opened in the base class vfunc, so by the time that
> returns,
> we should be able to query the number of monitors.
sure, it makes a sense. Ack
>
> The patch also adds a check in virt_viewer_parse_monitor_mappings()
> to
> ensure that we pass a sane value for nmonitors.
Sounds like testable
Thanks,
Pavel
> ---
> src/virt-viewer-app.c | 2 +-
> src/virt-viewer-util.c | 7 +--
> 2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> index 2e7e193..8c5c3f5 100644
> --- a/src/virt-viewer-app.c
> +++ b/src/virt-viewer-app.c
> @@ -1730,7 +1730,6 @@ virt_viewer_app_init(VirtViewerApp *self)
>
> g_clear_error();
>
> -self->priv->initial_display_map =
> virt_viewer_app_get_monitor_mapping_for_section(self, "fallback");
> g_signal_connect(self, "notify::guest-name",
> G_CALLBACK(title_maybe_changed), NULL);
> g_signal_connect(self, "notify::title",
> G_CALLBACK(title_maybe_changed), NULL);
> g_signal_connect(self, "notify::guri",
> G_CALLBACK(title_maybe_changed), NULL);
> @@ -1807,6 +1806,7 @@
> virt_viewer_app_on_application_startup(GApplication *app)
> self->priv->main_window = virt_viewer_app_window_new(self,
> virt_viewe
> r_app_get_first_monitor(self));
> self->priv->main_notebook =
> GTK_WIDGET(virt_viewer_window_get_notebook(self->priv-
> >main_window));
> +self->priv->initial_display_map =
> virt_viewer_app_get_monitor_mapping_for_section(self, "fallback");
>
> virt_viewer_app_set_kiosk(self, opt_kiosk);
> virt_viewer_app_set_hotkeys(self, opt_hotkeys);
> diff --git a/src/virt-viewer-util.c b/src/virt-viewer-util.c
> index 0491f73..6b63133 100644
> --- a/src/virt-viewer-util.c
> +++ b/src/virt-viewer-util.c
> @@ -641,11 +641,14 @@
> virt_viewer_shift_monitors_to_origin(GHashTable *displays)
> GHashTable*
> virt_viewer_parse_monitor_mappings(gchar **mappings, const gsize
> nmappings, const gint nmonitors)
> {
> -GHashTable *displaymap = g_hash_table_new(g_direct_hash,
> g_direct_equal);
> -GHashTable *monitormap = g_hash_table_new(g_direct_hash,
> g_direct_equal);
> +GHashTable *displaymap;
> +GHashTable *monitormap;
> gint i, max_display_id = 0;
> gchar **tokens = NULL;
>
> +g_return_val_if_fail(nmonitors != 0, NULL);
> +displaymap = g_hash_table_new(g_direct_hash, g_direct_equal);
> +monitormap = g_hash_table_new(g_direct_hash, g_direct_equal);
> if (nmappings == 0) {
> g_warning("Empty monitor-mapping configuration");
> goto configerror;
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [PATCH virt-viewer] Avoid warning when loading initial monitor mapping
When started in fullscreen mode with a monitor-mapping configuration
option, we are getting the following warnings on the terminal:
(process:27428): Gdk-CRITICAL **: gdk_screen_get_n_monitors: assertion
'GDK_IS_SCREEN (screen)' failed
** (process:27428): WARNING **: Invalid monitor-mapping configuration:
monitor #1 for display #1 does not exist
This is apparently because we were processing the fallback monitor
mapping before the graphic server display was opened, so
gdk_screen_get_default() returned NULL. This resulted in
gdk_screen_get_n_monitors() reporting that we had 0 monitors.
This patch moves the fallback monitor mapping parsing from
virt_viewer_app_init() to virt_viewer_app_on_application_startup(),
after chaining up to the base class startup() vfunc. The graphic server
display is opened in the base class vfunc, so by the time that returns,
we should be able to query the number of monitors.
The patch also adds a check in virt_viewer_parse_monitor_mappings() to
ensure that we pass a sane value for nmonitors.
---
src/virt-viewer-app.c | 2 +-
src/virt-viewer-util.c | 7 +--
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
index 2e7e193..8c5c3f5 100644
--- a/src/virt-viewer-app.c
+++ b/src/virt-viewer-app.c
@@ -1730,7 +1730,6 @@ virt_viewer_app_init(VirtViewerApp *self)
g_clear_error();
-self->priv->initial_display_map =
virt_viewer_app_get_monitor_mapping_for_section(self, "fallback");
g_signal_connect(self, "notify::guest-name",
G_CALLBACK(title_maybe_changed), NULL);
g_signal_connect(self, "notify::title", G_CALLBACK(title_maybe_changed),
NULL);
g_signal_connect(self, "notify::guri", G_CALLBACK(title_maybe_changed),
NULL);
@@ -1807,6 +1806,7 @@ virt_viewer_app_on_application_startup(GApplication *app)
self->priv->main_window = virt_viewer_app_window_new(self,
virt_viewer_app_get_first_monitor(self));
self->priv->main_notebook =
GTK_WIDGET(virt_viewer_window_get_notebook(self->priv->main_window));
+self->priv->initial_display_map =
virt_viewer_app_get_monitor_mapping_for_section(self, "fallback");
virt_viewer_app_set_kiosk(self, opt_kiosk);
virt_viewer_app_set_hotkeys(self, opt_hotkeys);
diff --git a/src/virt-viewer-util.c b/src/virt-viewer-util.c
index 0491f73..6b63133 100644
--- a/src/virt-viewer-util.c
+++ b/src/virt-viewer-util.c
@@ -641,11 +641,14 @@ virt_viewer_shift_monitors_to_origin(GHashTable *displays)
GHashTable*
virt_viewer_parse_monitor_mappings(gchar **mappings, const gsize nmappings,
const gint nmonitors)
{
-GHashTable *displaymap = g_hash_table_new(g_direct_hash, g_direct_equal);
-GHashTable *monitormap = g_hash_table_new(g_direct_hash, g_direct_equal);
+GHashTable *displaymap;
+GHashTable *monitormap;
gint i, max_display_id = 0;
gchar **tokens = NULL;
+g_return_val_if_fail(nmonitors != 0, NULL);
+displaymap = g_hash_table_new(g_direct_hash, g_direct_equal);
+monitormap = g_hash_table_new(g_direct_hash, g_direct_equal);
if (nmappings == 0) {
g_warning("Empty monitor-mapping configuration");
goto configerror;
--
2.9.3
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list
