[virt-tools-list] [PATCH v5 2/3] remote-viewer: Remove unused properties
The reason for using properties to access those members was to ensure that they would only be set during the creation of the object. Now that we removed that restriction, we set private members directly. Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/remote-viewer.c | 101 +++- 1 file changed, 4 insertions(+), 97 deletions(-) diff --git a/src/remote-viewer.c b/src/remote-viewer.c index 2703a24..aa99baa 100644 --- a/src/remote-viewer.c +++ b/src/remote-viewer.c @@ -67,15 +67,6 @@ G_DEFINE_TYPE (RemoteViewer, remote_viewer, VIRT_VIEWER_TYPE_APP) #define GET_PRIVATE(o)\ (G_TYPE_INSTANCE_GET_PRIVATE ((o), REMOTE_VIEWER_TYPE, RemoteViewerPrivate)) -enum { -PROP_0, -#ifdef HAVE_SPICE_GTK -PROP_CONTROLLER, -PROP_CTRL_FOREIGN_MENU, -#endif -PROP_OPEN_RECENT_DIALOG -}; - #ifdef HAVE_OVIRT static OvirtVm * choose_vm(GtkWindow *main_window, char **vm_name, @@ -122,56 +113,6 @@ remote_viewer_dispose (GObject *object) } static void -remote_viewer_get_property (GObject *object, guint property_id, -GValue *value, GParamSpec *pspec) -{ -RemoteViewer *self = REMOTE_VIEWER(object); -RemoteViewerPrivate *priv = self->priv; - -switch (property_id) { -#ifdef HAVE_SPICE_GTK -case PROP_CONTROLLER: -g_value_set_object(value, priv->controller); -break; -case PROP_CTRL_FOREIGN_MENU: -g_value_set_object(value, priv->ctrl_foreign_menu); -break; -#endif -case PROP_OPEN_RECENT_DIALOG: -g_value_set_boolean(value, priv->open_recent_dialog); -break; -default: -G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec); -} -} - -static void -remote_viewer_set_property (GObject *object, guint property_id, -const GValue *value, GParamSpec *pspec) -{ -RemoteViewer *self = REMOTE_VIEWER(object); -RemoteViewerPrivate *priv = self->priv; - -switch (property_id) { -#ifdef HAVE_SPICE_GTK -case PROP_CONTROLLER: -g_return_if_fail(priv->controller == NULL); -priv->controller = g_value_dup_object(value); -break; -case PROP_CTRL_FOREIGN_MENU: -g_return_if_fail(priv->ctrl_foreign_menu == NULL); -priv->ctrl_foreign_menu = g_value_dup_object(value); -break; -#endif -case PROP_OPEN_RECENT_DIALOG: -priv->open_recent_dialog = g_value_get_boolean(value); -break; -default: -G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec); -} -} - -static void remote_viewer_deactivated(VirtViewerApp *app, gboolean connect_error) { RemoteViewer *self = REMOTE_VIEWER(app); @@ -271,20 +212,14 @@ remote_viewer_local_command_line (GApplication *gapp, GOTO_END; } -SpiceCtrlController *ctrl = spice_ctrl_controller_new(); -SpiceCtrlForeignMenu *menu = spice_ctrl_foreign_menu_new(); +self->priv->controller = spice_ctrl_controller_new(); +self->priv->ctrl_foreign_menu = spice_ctrl_foreign_menu_new(); -g_object_set(self, "guest-name", "defined by Spice controller", - "controller", ctrl, - "foreign-menu", menu, - NULL); +g_object_set(self, "guest-name", "defined by Spice controller", NULL); -g_signal_connect(menu, "notify::title", +g_signal_connect(self->priv->ctrl_foreign_menu, "notify::title", G_CALLBACK(foreign_menu_title_changed), self); - -g_object_unref(ctrl); -g_object_unref(menu); } #endif @@ -311,8 +246,6 @@ remote_viewer_class_init (RemoteViewerClass *klass) g_type_class_add_private (klass, sizeof (RemoteViewerPrivate)); -object_class->get_property = remote_viewer_get_property; -object_class->set_property = remote_viewer_set_property; object_class->dispose = remote_viewer_dispose; g_app_class->local_command_line = remote_viewer_local_command_line; @@ -322,36 +255,10 @@ remote_viewer_class_init (RemoteViewerClass *klass) app_class->add_option_entries = remote_viewer_add_option_entries; #ifdef HAVE_SPICE_GTK app_class->activate = remote_viewer_activate; - gtk_app_class->window_added = remote_viewer_window_added; - -g_object_class_install_property(object_class, -PROP_CONTROLLER, -g_param_spec_object("controller", -"Controller", -"
[virt-tools-list] [PATCH v5 virt-viewer 0/3] Port to GtkApplication API's
In this version: Rebased to latest master Eduardo Lima (Etrunko) (3): Port to GtkApplication API's remote-viewer: Remove unused properties Drop old compatibility code configure.ac| 6 +- src/Makefile.am | 2 - src/ovirt-foreign-menu.c| 1 - src/remote-viewer-main.c| 167 +--- src/remote-viewer.c | 274 +++- src/remote-viewer.h | 4 +- src/virt-glib-compat.c | 34 - src/virt-glib-compat.h | 83 src/virt-viewer-app.c | 145 + src/virt-viewer-app.h | 11 +- src/virt-viewer-events.c| 1 - src/virt-viewer-file.h | 1 - src/virt-viewer-main.c | 108 +--- src/virt-viewer-session-spice.c | 1 - src/virt-viewer-util.h | 1 + src/virt-viewer.c | 137 src/virt-viewer.h | 9 +- src/virt-viewer.xml | 2 +- 18 files changed, 385 insertions(+), 602 deletions(-) delete mode 100644 src/virt-glib-compat.c delete mode 100644 src/virt-glib-compat.h -- 2.5.0 ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [PATCH v5 1/3] Port to GtkApplication API's
Most of this patch consists in code being shuffled around to fit the expected flow while using the new APIs. I tried my best to make this patch the less intrusive as possible. Main changes are: - Updated build requirements * glib version 2.38 * gtk+ version 3.10 * gio - VirtViewerApp is now a subclass of GtkApplication. Some mainloop calls were replaced: * gtk_main() -> g_application_run() * gtk_quit() -> g_application_quit() - Unified command line option handling. The logic has moved from the main functions and split in common options, and specific ones for each application. With this, the main functions were highly simplified, and now basically responsible for instantiating the App object and running the main loop. - All Window objects must be associated with the Application. With this, there is no need to emit our own 'window-added'/'window- removed' signals, as those will be emited by GtkApplication whenever gtk_application_add_window() and gtk_application_remove_window() are called. Also, 'window-removed' was not being used anywhere. Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- configure.ac | 6 +- src/remote-viewer-main.c | 167 ++ src/remote-viewer.c | 207 +++ src/remote-viewer.h | 4 +- src/virt-viewer-app.c| 145 - src/virt-viewer-app.h| 11 +-- src/virt-viewer-main.c | 108 ++--- src/virt-viewer-util.h | 1 + src/virt-viewer.c| 137 +-- src/virt-viewer.h| 9 +-- src/virt-viewer.xml | 2 +- 11 files changed, 398 insertions(+), 399 deletions(-) diff --git a/configure.ac b/configure.ac index 2b979f4..5503d46 100644 --- a/configure.ac +++ b/configure.ac @@ -12,10 +12,10 @@ AC_CANONICAL_HOST m4_ifndef([AM_SILENT_RULES], [m4_define([AM_SILENT_RULES],[])]) AM_SILENT_RULES([yes]) -GLIB2_REQUIRED=2.22.0 +GLIB2_REQUIRED="2.38.0" LIBXML2_REQUIRED="2.6.0" LIBVIRT_REQUIRED="0.10.0" -GTK_REQUIRED="3.0" +GTK_REQUIRED="3.10" GTK_VNC_REQUIRED="0.4.0" SPICE_GTK_REQUIRED="0.30" SPICE_PROTOCOL_REQUIRED="0.12.7" @@ -93,7 +93,7 @@ PKG_PROG_PKG_CONFIG GLIB_MKENUMS=`$PKG_CONFIG --variable=glib_mkenums glib-2.0` AC_SUBST(GLIB_MKENUMS) -PKG_CHECK_MODULES(GLIB2, glib-2.0 >= $GLIB2_REQUIRED gthread-2.0 gmodule-export-2.0) +PKG_CHECK_MODULES(GLIB2, glib-2.0 >= $GLIB2_REQUIRED gio-2.0 gthread-2.0 gmodule-export-2.0) PKG_CHECK_MODULES(LIBXML2, libxml-2.0 >= $LIBXML2_REQUIRED) AC_ARG_WITH([libvirt], diff --git a/src/remote-viewer-main.c b/src/remote-viewer-main.c index 81cf736..b05f27b 100644 --- a/src/remote-viewer-main.c +++ b/src/remote-viewer-main.c @@ -22,184 +22,29 @@ #include #include +#include #include #include #include -#ifdef G_OS_WIN32 -#include -#include -#endif - -#ifdef HAVE_GTK_VNC -#include -#endif -#ifdef HAVE_SPICE_GTK -#include -#endif -#ifdef HAVE_OVIRT -#include -#endif #include "remote-viewer.h" -#include "virt-viewer-app.h" -#include "virt-viewer-session.h" - -static void -remote_viewer_version(void) -{ -g_print(_("remote-viewer version %s"), VERSION BUILDID); -#ifdef REMOTE_VIEWER_OS_ID -g_print(" (OS ID: %s)", REMOTE_VIEWER_OS_ID); -#endif -g_print("\n"); -exit(EXIT_SUCCESS); -} - -static void -recent_add(gchar *uri, const gchar *mime_type) -{ -GtkRecentManager *recent; -GtkRecentData meta = { -.app_name = (char*)"remote-viewer", -.app_exec = (char*)"remote-viewer %u", -.mime_type= (char*)mime_type, -}; - -if (uri == NULL) -return; - -recent = gtk_recent_manager_get_default(); -meta.display_name = uri; -if (!gtk_recent_manager_add_full(recent, uri, )) -g_warning("Recent item couldn't be added"); -} - -static void connected(VirtViewerSession *session, - VirtViewerApp *self G_GNUC_UNUSED) -{ -gchar *uri = virt_viewer_session_get_uri(session); -const gchar *mime = virt_viewer_session_mime_type(session); - -recent_add(uri, mime); -g_free(uri); -} int main(int argc, char **argv) { -GOptionContext *context; -GError *error = NULL; int ret = 1; -gchar **args = NULL; -gchar *uri = NULL; -char *title = NULL; RemoteViewer *viewer = NULL; -#ifdef HAVE_SPICE_GTK -gboolean controller = FALSE; -#endif -VirtViewerApp *app; -const GOptionEntry options [] = { -{ "version", 'V', G_OPTION_FLAG_NO_ARG, G_OPTION_ARG_CALLBACK, - remote_viewer_version, N_("Display version information"), NULL }, -{ "title", 't', 0, G_OPTION_ARG_STRING, , - N_("Set window
Re: [virt-tools-list] [PATCH v5 1/3] Port to GtkApplication API's
On 02/16/2016 10:41 AM, Pavel Grunt wrote: > Hi, > > On Mon, 2016-02-15 at 19:22 -0200, Eduardo Lima (Etrunko) wrote: >> Most of this patch consists in code being shuffled around to fit the >> expected flow while using the new APIs. I tried my best to make this >> patch the less intrusive as possible. Main changes are: >> >> - Updated build requirements >>* glib version 2.38 >>* gtk+ version 3.10 >>* gio > > For what we need gio ? GApplication code lives in gio. Although Gtk+ should already be pulling it anyway, I thought it should be better to have it explicit. >> diff --git a/src/remote-viewer-main.c b/src/remote-viewer-main.c >> index 81cf736..b05f27b 100644 >> --- a/src/remote-viewer-main.c >> +++ b/src/remote-viewer-main.c >> @@ -22,184 +22,29 @@ >> >> #include >> #include >> +#include >> #include >> #include >> #include >> -#ifdef G_OS_WIN32 >> -#include >> -#include >> -#endif >> - >> -#ifdef HAVE_GTK_VNC >> -#include >> -#endif >> -#ifdef HAVE_SPICE_GTK >> -#include >> -#endif >> -#ifdef HAVE_OVIRT >> -#include >> -#endif >> >> #include "remote-viewer.h" >> -#include "virt-viewer-app.h" >> -#include "virt-viewer-session.h" >> - >> -static void >> -remote_viewer_version(void) >> -{ >> -g_print(_("remote-viewer version %s"), VERSION BUILDID); >> -#ifdef REMOTE_VIEWER_OS_ID >> -g_print(" (OS ID: %s)", REMOTE_VIEWER_OS_ID); >> -#endif >> -g_print("\n"); >> -exit(EXIT_SUCCESS); >> -} >> - >> -static void >> -recent_add(gchar *uri, const gchar *mime_type) >> -{ >> -GtkRecentManager *recent; >> -GtkRecentData meta = { >> -.app_name = (char*)"remote-viewer", >> -.app_exec = (char*)"remote-viewer %u", >> -.mime_type= (char*)mime_type, >> -}; >> - >> -if (uri == NULL) >> -return; >> - >> -recent = gtk_recent_manager_get_default(); >> -meta.display_name = uri; >> -if (!gtk_recent_manager_add_full(recent, uri, )) >> -g_warning("Recent item couldn't be added"); >> -} >> - >> -static void connected(VirtViewerSession *session, >> - VirtViewerApp *self G_GNUC_UNUSED) >> -{ >> -gchar *uri = virt_viewer_session_get_uri(session); >> -const gchar *mime = virt_viewer_session_mime_type(session); >> - >> -recent_add(uri, mime); >> -g_free(uri); >> -} >> >> int >> main(int argc, char **argv) >> { > >> -GOptionContext *context; >> -GError *error = NULL; >> int ret = 1; >> -gchar **args = NULL; >> -gchar *uri = NULL; >> -char *title = NULL; >> RemoteViewer *viewer = NULL; >> -#ifdef HAVE_SPICE_GTK >> -gboolean controller = FALSE; >> -#endif >> -VirtViewerApp *app; >> -const GOptionEntry options [] = { >> -{ "version", 'V', G_OPTION_FLAG_NO_ARG, >> G_OPTION_ARG_CALLBACK, >> - remote_viewer_version, N_("Display version information"), >> NULL }, >> -{ "title", 't', 0, G_OPTION_ARG_STRING, , >> - N_("Set window title"), NULL }, >> -#ifdef HAVE_SPICE_GTK >> -{ "spice-controller", '\0', 0, G_OPTION_ARG_NONE, >> , >> - N_("Open connection using Spice controller >> communication"), NULL }, >> -#endif >> -{ G_OPTION_REMAINING, '\0', 0, G_OPTION_ARG_STRING_ARRAY, >> , >> - NULL, "URI|VV-FILE" }, >> -{ NULL, 0, 0, G_OPTION_ARG_NONE, NULL, NULL, NULL } >> -}; >> -GOptionGroup *app_options = NULL; >> >> virt_viewer_util_init(_("Remote Viewer")); >> >> -/* Setup command line options */ >> -context = g_option_context_new (NULL); >> -g_option_context_set_summary(context, _("Remote viewer >> client")); >> -app_options = virt_viewer_app_get_option_group(); >> -g_option_group_add_entries (app_options, options); >> -g_option_context_set_main_group (context, app_options); >> -g_option_context_add_group (context, gtk_get_option_group >> (TRUE)); >> -#ifdef HAVE_GTK_VNC >> -g_option_context_add_group (context, >> vnc_display_get_option_group (
[virt-tools-list] [PATCH v6 1/3] Port to GtkApplication API's
Most of this patch consists in code being shuffled around to fit the expected flow while using the new APIs. I tried my best to make this patch the less intrusive as possible. Main changes are: - Updated build requirements * glib version 2.38 * gtk+ version 3.10 * gio - VirtViewerApp is now a subclass of GtkApplication. Some mainloop calls were replaced: * gtk_main() -> g_application_run() * gtk_quit() -> g_application_quit() - Unified command line option handling. The logic has moved from the main functions and split in common options, and specific ones for each application. With this, the main functions were highly simplified, and now basically responsible for instantiating the App object and running the main loop. - All Window objects must be associated with the Application. With this, there is no need to emit our own 'window-added'/'window- removed' signals, as those will be emited by GtkApplication whenever gtk_application_add_window() and gtk_application_remove_window() are called. Also, 'window-removed' was not being used anywhere. Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- configure.ac | 6 +- src/remote-viewer-main.c | 173 ++- src/remote-viewer.c | 187 +-- src/remote-viewer.h | 3 - src/virt-viewer-app.c| 158 +-- src/virt-viewer-app.h| 11 +-- src/virt-viewer-main.c | 113 +++- src/virt-viewer-util.h | 2 +- src/virt-viewer.c| 120 ++ src/virt-viewer.h| 8 -- src/virt-viewer.xml | 2 +- 11 files changed, 365 insertions(+), 418 deletions(-) diff --git a/configure.ac b/configure.ac index 2b979f4..5503d46 100644 --- a/configure.ac +++ b/configure.ac @@ -12,10 +12,10 @@ AC_CANONICAL_HOST m4_ifndef([AM_SILENT_RULES], [m4_define([AM_SILENT_RULES],[])]) AM_SILENT_RULES([yes]) -GLIB2_REQUIRED=2.22.0 +GLIB2_REQUIRED="2.38.0" LIBXML2_REQUIRED="2.6.0" LIBVIRT_REQUIRED="0.10.0" -GTK_REQUIRED="3.0" +GTK_REQUIRED="3.10" GTK_VNC_REQUIRED="0.4.0" SPICE_GTK_REQUIRED="0.30" SPICE_PROTOCOL_REQUIRED="0.12.7" @@ -93,7 +93,7 @@ PKG_PROG_PKG_CONFIG GLIB_MKENUMS=`$PKG_CONFIG --variable=glib_mkenums glib-2.0` AC_SUBST(GLIB_MKENUMS) -PKG_CHECK_MODULES(GLIB2, glib-2.0 >= $GLIB2_REQUIRED gthread-2.0 gmodule-export-2.0) +PKG_CHECK_MODULES(GLIB2, glib-2.0 >= $GLIB2_REQUIRED gio-2.0 gthread-2.0 gmodule-export-2.0) PKG_CHECK_MODULES(LIBXML2, libxml-2.0 >= $LIBXML2_REQUIRED) AC_ARG_WITH([libvirt], diff --git a/src/remote-viewer-main.c b/src/remote-viewer-main.c index 81cf736..6871965 100644 --- a/src/remote-viewer-main.c +++ b/src/remote-viewer-main.c @@ -22,184 +22,27 @@ #include #include +#include #include #include #include -#ifdef G_OS_WIN32 -#include -#include -#endif - -#ifdef HAVE_GTK_VNC -#include -#endif -#ifdef HAVE_SPICE_GTK -#include -#endif -#ifdef HAVE_OVIRT -#include -#endif #include "remote-viewer.h" -#include "virt-viewer-app.h" -#include "virt-viewer-session.h" - -static void -remote_viewer_version(void) -{ -g_print(_("remote-viewer version %s"), VERSION BUILDID); -#ifdef REMOTE_VIEWER_OS_ID -g_print(" (OS ID: %s)", REMOTE_VIEWER_OS_ID); -#endif -g_print("\n"); -exit(EXIT_SUCCESS); -} - -static void -recent_add(gchar *uri, const gchar *mime_type) -{ -GtkRecentManager *recent; -GtkRecentData meta = { -.app_name = (char*)"remote-viewer", -.app_exec = (char*)"remote-viewer %u", -.mime_type= (char*)mime_type, -}; - -if (uri == NULL) -return; - -recent = gtk_recent_manager_get_default(); -meta.display_name = uri; -if (!gtk_recent_manager_add_full(recent, uri, )) -g_warning("Recent item couldn't be added"); -} - -static void connected(VirtViewerSession *session, - VirtViewerApp *self G_GNUC_UNUSED) -{ -gchar *uri = virt_viewer_session_get_uri(session); -const gchar *mime = virt_viewer_session_mime_type(session); - -recent_add(uri, mime); -g_free(uri); -} int main(int argc, char **argv) { -GOptionContext *context; -GError *error = NULL; int ret = 1; -gchar **args = NULL; -gchar *uri = NULL; -char *title = NULL; -RemoteViewer *viewer = NULL; -#ifdef HAVE_SPICE_GTK -gboolean controller = FALSE; -#endif -VirtViewerApp *app; -const GOptionEntry options [] = { -{ "version", 'V', G_OPTION_FLAG_NO_ARG, G_OPTION_ARG_CALLBACK, - remote_viewer_version, N_("Display version information"), NULL }, -{ "title", 't', 0, G_OPTION_ARG_STRING, , - N_("Set window
[virt-tools-list] [PATCH v6 virt-viewer 0/3] Port to GtkApplication API's
In this version I addressed the comments made from review of v5. - Rename virt_viewer_app_startup() to virt_viewer_app_on_application_startup() - Remove VIRT_VIEWER_VERSION error code and move --version option handler to VirtViewerApp. - Removed {remote,virt}_viewer_new() functions and call g_object_new() directly. Eduardo Lima (Etrunko) (3): Port to GtkApplication API's remote-viewer: Remove unused properties Drop old compatibility code configure.ac| 6 +- src/Makefile.am | 2 - src/ovirt-foreign-menu.c| 1 - src/remote-viewer-main.c| 173 ++- src/remote-viewer.c | 256 +--- src/remote-viewer.h | 3 - src/virt-glib-compat.c | 34 -- src/virt-glib-compat.h | 83 - src/virt-viewer-app.c | 158 - src/virt-viewer-app.h | 11 +- src/virt-viewer-events.c| 1 - src/virt-viewer-file.h | 1 - src/virt-viewer-main.c | 113 ++ src/virt-viewer-session-spice.c | 1 - src/virt-viewer-util.h | 2 +- src/virt-viewer.c | 120 ++- src/virt-viewer.h | 8 -- src/virt-viewer.xml | 2 +- 18 files changed, 353 insertions(+), 622 deletions(-) delete mode 100644 src/virt-glib-compat.c delete mode 100644 src/virt-glib-compat.h -- 2.5.0 ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [PATCH v6 3/3] Drop old compatibility code
With glib requirements now being 2.38, these functions do not make sense anymore. Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/Makefile.am | 2 - src/ovirt-foreign-menu.c| 1 - src/virt-glib-compat.c | 34 - src/virt-glib-compat.h | 83 - src/virt-viewer-events.c| 1 - src/virt-viewer-file.h | 1 - src/virt-viewer-session-spice.c | 1 - 7 files changed, 123 deletions(-) delete mode 100644 src/virt-glib-compat.c delete mode 100644 src/virt-glib-compat.h diff --git a/src/Makefile.am b/src/Makefile.am index 42d30fd..a4a420b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -40,8 +40,6 @@ $(BUILT_SOURCES): %: %.etemplate $(ENUMS_FILES) libvirt_viewer_la_SOURCES =\ $(BUILT_SOURCES)\ - virt-glib-compat.h \ - virt-glib-compat.c \ virt-viewer-util.h \ virt-viewer-util.c \ virt-viewer-auth.h \ diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c index 9b6d3b8..9859439 100644 --- a/src/ovirt-foreign-menu.c +++ b/src/ovirt-foreign-menu.c @@ -28,7 +28,6 @@ #include #include "ovirt-foreign-menu.h" -#include "virt-glib-compat.h" #include "virt-viewer-util.h" typedef enum { diff --git a/src/virt-glib-compat.c b/src/virt-glib-compat.c deleted file mode 100644 index c15ff28..000 --- a/src/virt-glib-compat.c +++ /dev/null @@ -1,34 +0,0 @@ -/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */ -/* - This library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - This library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with this library; if not, see <http://www.gnu.org/licenses/>. -*/ -#include - -#include "virt-glib-compat.h" - -#if !GLIB_CHECK_VERSION(2,32,0) -GByteArray *g_byte_array_new_take (guint8 *data, gsize len) -{ - GByteArray *array; - - array = g_byte_array_new (); - g_assert (array->data == NULL); - g_assert (array->len == 0); - - array->data = data; - array->len = len; - - return array; -} -#endif diff --git a/src/virt-glib-compat.h b/src/virt-glib-compat.h deleted file mode 100644 index 1242289..000 --- a/src/virt-glib-compat.h +++ /dev/null @@ -1,83 +0,0 @@ -/* - * Virt Viewer: A virtual machine console viewer - * - * Copyright (C) 2007-2009 Red Hat, Inc. - * Copyright (C) 2009-2012 Daniel P. Berrange - * Copyright (C) 2010 Marc-André Lureau - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA - * - * Author: Daniel P. Berrange <berra...@redhat.com> - */ - -#include - -#ifndef _VIRT_GLIB_COMPAT_H -# define _VIRT_GLIB_COMPAT_H 1 - -#include - -G_BEGIN_DECLS - -#ifndef g_clear_pointer -#define g_clear_pointer(pp, destroy) \ - G_STMT_START { \ -G_STATIC_ASSERT (sizeof *(pp) == sizeof (gpointer)); \ -/* Only one access, please */ \ -gpointer *_pp = (gpointer *) (pp); \ -gpointer _p; \ -/* This assignment is needed to avoid a gcc warning */ \ -GDestroyNotify _destroy = (GDestroyNotify) (destroy); \ - \ -(void) (0 ? (gpointer) *(pp) : 0); \ -do
[virt-tools-list] [PATCH v6 2/3] remote-viewer: Remove unused properties
The reason for using properties to access those members was to ensure that they would only be set during the creation of the object. Now that we removed that restriction, we set private members directly. Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/remote-viewer.c | 101 +++- 1 file changed, 4 insertions(+), 97 deletions(-) diff --git a/src/remote-viewer.c b/src/remote-viewer.c index 1cef19e..6b12056 100644 --- a/src/remote-viewer.c +++ b/src/remote-viewer.c @@ -66,15 +66,6 @@ G_DEFINE_TYPE (RemoteViewer, remote_viewer, VIRT_VIEWER_TYPE_APP) #define GET_PRIVATE(o)\ (G_TYPE_INSTANCE_GET_PRIVATE ((o), REMOTE_VIEWER_TYPE, RemoteViewerPrivate)) -enum { -PROP_0, -#ifdef HAVE_SPICE_GTK -PROP_CONTROLLER, -PROP_CTRL_FOREIGN_MENU, -#endif -PROP_OPEN_RECENT_DIALOG -}; - #ifdef HAVE_OVIRT static OvirtVm * choose_vm(GtkWindow *main_window, char **vm_name, @@ -121,56 +112,6 @@ remote_viewer_dispose (GObject *object) } static void -remote_viewer_get_property (GObject *object, guint property_id, -GValue *value, GParamSpec *pspec) -{ -RemoteViewer *self = REMOTE_VIEWER(object); -RemoteViewerPrivate *priv = self->priv; - -switch (property_id) { -#ifdef HAVE_SPICE_GTK -case PROP_CONTROLLER: -g_value_set_object(value, priv->controller); -break; -case PROP_CTRL_FOREIGN_MENU: -g_value_set_object(value, priv->ctrl_foreign_menu); -break; -#endif -case PROP_OPEN_RECENT_DIALOG: -g_value_set_boolean(value, priv->open_recent_dialog); -break; -default: -G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec); -} -} - -static void -remote_viewer_set_property (GObject *object, guint property_id, -const GValue *value, GParamSpec *pspec) -{ -RemoteViewer *self = REMOTE_VIEWER(object); -RemoteViewerPrivate *priv = self->priv; - -switch (property_id) { -#ifdef HAVE_SPICE_GTK -case PROP_CONTROLLER: -g_return_if_fail(priv->controller == NULL); -priv->controller = g_value_dup_object(value); -break; -case PROP_CTRL_FOREIGN_MENU: -g_return_if_fail(priv->ctrl_foreign_menu == NULL); -priv->ctrl_foreign_menu = g_value_dup_object(value); -break; -#endif -case PROP_OPEN_RECENT_DIALOG: -priv->open_recent_dialog = g_value_get_boolean(value); -break; -default: -G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec); -} -} - -static void remote_viewer_deactivated(VirtViewerApp *app, gboolean connect_error) { RemoteViewer *self = REMOTE_VIEWER(app); @@ -248,20 +189,14 @@ remote_viewer_local_command_line (GApplication *gapp, if (opt_args) ERROR_GOTO_END(_("\nError: extra arguments given while using Spice controller\n\n")); -SpiceCtrlController *ctrl = spice_ctrl_controller_new(); -SpiceCtrlForeignMenu *menu = spice_ctrl_foreign_menu_new(); +self->priv->controller = spice_ctrl_controller_new(); +self->priv->ctrl_foreign_menu = spice_ctrl_foreign_menu_new(); -g_object_set(self, "guest-name", "defined by Spice controller", - "controller", ctrl, - "foreign-menu", menu, - NULL); +g_object_set(self, "guest-name", "defined by Spice controller", NULL); -g_signal_connect(menu, "notify::title", +g_signal_connect(self->priv->ctrl_foreign_menu, "notify::title", G_CALLBACK(foreign_menu_title_changed), self); - -g_object_unref(ctrl); -g_object_unref(menu); } #endif @@ -288,8 +223,6 @@ remote_viewer_class_init (RemoteViewerClass *klass) g_type_class_add_private (klass, sizeof (RemoteViewerPrivate)); -object_class->get_property = remote_viewer_get_property; -object_class->set_property = remote_viewer_set_property; object_class->dispose = remote_viewer_dispose; g_app_class->local_command_line = remote_viewer_local_command_line; @@ -299,36 +232,10 @@ remote_viewer_class_init (RemoteViewerClass *klass) app_class->add_option_entries = remote_viewer_add_option_entries; #ifdef HAVE_SPICE_GTK app_class->activate = remote_viewer_activate; - gtk_app_class->window_added = remote_viewer_window_added; - -g_object_class_install_property(object_class, -PROP_CONTROLLER, -g_param_spec_object("controller", -
Re: [virt-tools-list] [PATCH v5 1/3] Port to GtkApplication API's
On 02/17/2016 01:22 PM, Jonathon Jongsma wrote: >>> I disagree, but again, I think it is basically a matter of >>> preference. >> >> It is. From the macro name it is not clear for me how is different >> "GOTO_END" from "goto end" > > For what it's worth, I agree with Pavel here. I think it actually obscures > things instead of making them clearer. > > Well I think that's already a part of our day-to-day coding, we are using GObjects all over the place after all. Macros do have their value if used correctly, and I think this one is a perfect example. So, what do you think about this "improved" one I proposed? If you say you don't like it, I can change it without problems. >> >>> Looking at it again, I could improve this macro a bit more, by moving >>> the g_printerr() call inside and receive the message as a parameter: >> >> any improvements welcome >> >> Pavel >> >>> >>> +#define ERROR_GOTO_END(x, ...) \ >>> +do { \ >>> +g_printerr(x, ## __VA_ARGS__); \ >>> +ret = TRUE; \ >>> +*status = 1; \ >>> +goto end; \ >>> + } while (FALSE) >>> + >>> >>> Regards, Eduardo. >>> >> >> ___ >> virt-tools-list mailing list >> virt-tools-list@redhat.com >> https://www.redhat.com/mailman/listinfo/virt-tools-list -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ 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] display: Set value of desktop width and height property directly
On 02/02/2016 10:59 AM, Pavel Grunt wrote: > Hi, > > On Tue, 2016-02-02 at 09:47 -0200, Eduardo Lima (Etrunko) wrote: >> On 01/27/2016 03:33 PM, Pavel Grunt wrote: >>> Avoid calling gtk_widget_queue_resize() and emitting >>> the "display-desktop-resize" signal. >> >> Just curious about why this change is necessary? I mean, isn't this >> signal necessary somewhere else? I see it is handled in >> VirtViewerWindow. >> > these properties are only used > by virt_viewer_display_spice_set_desktop() which will emit the signal > and call the function. I want to prevent multiple calls > to virt_viewer_display_queue_resize() when changing both properties. > >> If so, please provide some more details in the commit message. > > what about: > > display: Set value of desktop width and height property directly > > Avoid calling gtk_widget_queue_resize() and emiting > the "display-desktop-resize" signal. > The only user of the properties is > virt_viewer_display_spice_set_desktop() > which will call the function and emit the signal after setting both > "desktop-width" and "desktop-height" properties. > Yes, much better, thanks. :) -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [PATCH 2/2] configure: cleanup {GLIB2, GTK}_CFLAGS
On 03/22/2016 08:02 AM, Pavel Grunt wrote: > Hi, > > imo with the change the line is too long > > About the AC_SUBST calls - iirc they are defined automatically when > pkg-config is >= 0.24 (not the case of rhel6). > Or is it about something else (why just _LIBS and not _CFLAGS) ? > About line being to long it is easily fixed by breaking the line. This patch is only here because when I reviewed the original patch, it wal already merged. About calling AC_SUBST for _LIBS, only _CFLAGS varible is touched, so it is only required for the latter. I will provide more details in the commit message. -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [PATCH 2/2] configure: cleanup {GLIB2, GTK}_CFLAGS
On 03/22/2016 10:42 AM, Eduardo Lima (Etrunko) wrote: > On 03/22/2016 08:02 AM, Pavel Grunt wrote: >> Hi, >> >> imo with the change the line is too long >> >> About the AC_SUBST calls - iirc they are defined automatically when >> pkg-config is >= 0.24 (not the case of rhel6). >> Or is it about something else (why just _LIBS and not _CFLAGS) ? >> > > About line being to long it is easily fixed by breaking the line. This > patch is only here because when I reviewed the original patch, it wal > already merged. > > About calling AC_SUBST for _LIBS, only _CFLAGS varible is touched, so it > is only required for the latter. I will provide more details in the > commit message. > Forgot to add that about RHEL6, it does not matter anyway, as virt-viewer dropped Gtk+ 2.0 support recently. -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-viewer v2] Use GResource for loading ui files
On 03/02/2016 02:46 PM, Eduardo Lima (Etrunko) wrote: > On 03/02/2016 12:23 PM, Fabiano Fidêncio wrote: >> Let's take advantage of GResource for loading ui files in a better and >> cleaner way than virt_viewer_util_load_ui() was doing. >> It also brings the benefit, at least for developers, of being able to >> test ui changes without having to "make install" virt-viewer. >> >> Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> >> --- >> Changes since v1: >> - Adressed all comments from Eduardo and Jonathon. >> --- >> mingw-virt-viewer.spec.in | 18 ++-- >> src/Makefile.am | 21 +-- >> src/virt-viewer-about.xml | 1 - >> src/virt-viewer-app.c | 5 + >> src/virt-viewer-util.c| 48 >> +-- >> src/virt-viewer-util.h| 1 + >> src/virt-viewer-window.c | 20 ++ >> src/virt-viewer.gresource.xml | 19 + >> virt-viewer.spec.in | 9 >> 9 files changed, 63 insertions(+), 79 deletions(-) >> create mode 100644 src/virt-viewer.gresource.xml >> >> diff --git a/mingw-virt-viewer.spec.in b/mingw-virt-viewer.spec.in >> index b200db7..ddea296 100644 >> --- a/mingw-virt-viewer.spec.in >> +++ b/mingw-virt-viewer.spec.in >> @@ -114,10 +114,12 @@ MinGW Windows virt-viewer MSI >> %mingw_make_install DESTDIR=$RPM_BUILD_ROOT >> >> %if 0%{?mingw_build_win32} == 1 >> +mkdir $RPM_BUILD_ROOT/%{mingw32_datadir}/virt-viewer >> cp build_win32$MINGW_BUILDDIR_SUFFIX/data/virt-viewer-x86-@VERSION@.msi >> $RPM_BUILD_ROOT/%{mingw32_datadir}/virt-viewer >> %endif >> >> %if 0%{?mingw_build_win64} == 1 >> +mkdir $RPM_BUILD_ROOT/%{mingw64_datadir}/virt-viewer >> cp build_win64$MINGW_BUILDDIR_SUFFIX/data/virt-viewer-x64-@VERSION@.msi >> $RPM_BUILD_ROOT/%{mingw64_datadir}/virt-viewer >> %endif >> >> @@ -138,14 +140,6 @@ rm -rf $RPM_BUILD_ROOT >> %{mingw32_bindir}/debug-helper.exe >> >> %dir %{mingw32_datadir}/virt-viewer/ >> -%dir %{mingw32_datadir}/virt-viewer/ui/ >> -%{mingw32_datadir}/virt-viewer/ui/virt-viewer.xml >> -%{mingw32_datadir}/virt-viewer/ui/virt-viewer-about.xml >> -%{mingw32_datadir}/virt-viewer/ui/virt-viewer-auth.xml >> -%{mingw32_datadir}/virt-viewer/ui/virt-viewer-guest-details.xml >> -%{mingw32_datadir}/virt-viewer/ui/virt-viewer-vm-connection.xml >> -%{mingw32_datadir}/virt-viewer/ui/virt-viewer-preferences.xml >> -%{mingw32_datadir}/virt-viewer/ui/remote-viewer-connect.xml >> %{mingw32_datadir}/icons/hicolor/*/apps/* >> %{mingw32_datadir}/icons/hicolor/*/devices/* >> >> @@ -163,14 +157,6 @@ rm -rf $RPM_BUILD_ROOT >> %{mingw64_bindir}/debug-helper.exe >> >> %dir %{mingw64_datadir}/virt-viewer/ >> -%dir %{mingw64_datadir}/virt-viewer/ui/ >> -%{mingw64_datadir}/virt-viewer/ui/virt-viewer.xml >> -%{mingw64_datadir}/virt-viewer/ui/virt-viewer-about.xml >> -%{mingw64_datadir}/virt-viewer/ui/virt-viewer-auth.xml >> -%{mingw64_datadir}/virt-viewer/ui/virt-viewer-guest-details.xml >> -%{mingw64_datadir}/virt-viewer/ui/virt-viewer-vm-connection.xml >> -%{mingw64_datadir}/virt-viewer/ui/virt-viewer-preferences.xml >> -%{mingw64_datadir}/virt-viewer/ui/remote-viewer-connect.xml >> %{mingw64_datadir}/icons/hicolor/*/apps/* >> %{mingw64_datadir}/icons/hicolor/*/devices/* >> >> diff --git a/src/Makefile.am b/src/Makefile.am >> index f42a7bf..a42c01e 100644 >> --- a/src/Makefile.am >> +++ b/src/Makefile.am >> @@ -5,8 +5,7 @@ bin_PROGRAMS = >> >> noinst_LTLIBRARIES = libvirt-viewer.la >> >> -builderxmldir = $(pkgdatadir)/ui >> -builderxml_DATA = \ >> +noinst_DATA = \ >> virt-viewer.xml \ >> virt-viewer-about.xml \ >> virt-viewer-auth.xml\ >> @@ -17,9 +16,12 @@ builderxml_DATA = \ >> $(NULL) >> >> EXTRA_DIST =\ >> -$(builderxml_DATA) \ >> +$(noinst_DATA) \ >> +virt-viewer-resources.c \ >> +virt-viewer-resources.h \ >> virt-viewer-enums.c.etemplate \ >> virt-viewer-enums.h.etemplate \ >> +virt-viewer.gresource.xml \ >> $(NULL) >> Ooops, I overlooked this part. It seems you don't need resources.[ch] in EXTRA_DIST. They will be automatically generated. -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-viewer v2] Use GResource for loading ui files
VIRT_VIEWER_RESOURCE_PREFIX, > + name); > > -if (error) { > -g_error("Cannot load UI description %s: %s", name, > -error->message); > -g_clear_error(); > -goto failed; > -} > +builder = gtk_builder_new_from_resource(resource); > > +g_free(resource); > return builder; > - failed: > -g_error("failed to find UI description file"); > -g_object_unref(builder); > -return NULL; > } > > int > diff --git a/src/virt-viewer-util.h b/src/virt-viewer-util.h > index 0a7dd97..c453804 100644 > --- a/src/virt-viewer-util.h > +++ b/src/virt-viewer-util.h > @@ -34,6 +34,7 @@ enum { > }; > > #define VIRT_VIEWER_ERROR virt_viewer_error_quark () > +#define VIRT_VIEWER_RESOURCE_PREFIX "/org/virt-manager/virt-viewer" > > GQuark virt_viewer_error_quark(void); > > diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c > index 8ce34ca..ef62d9a 100644 > --- a/src/virt-viewer-window.c > +++ b/src/virt-viewer-window.c > @@ -1030,11 +1030,24 @@ G_MODULE_EXPORT void > virt_viewer_window_menu_help_about(GtkWidget *menu G_GNUC_UNUSED, > VirtViewerWindow *self) > { > -GtkBuilder *about = virt_viewer_util_load_ui("virt-viewer-about.xml"); > +GtkBuilder *about; > +GtkWidget *dialog; > +GdkPixbuf *icon; > + > +about = virt_viewer_util_load_ui("virt-viewer-about.xml"); > + > +dialog = GTK_WIDGET(gtk_builder_get_object(about, "about")); > > -GtkWidget *dialog = GTK_WIDGET(gtk_builder_get_object(about, "about")); > gtk_about_dialog_set_version(GTK_ABOUT_DIALOG(dialog), VERSION BUILDID); > > +icon = > gdk_pixbuf_new_from_resource(VIRT_VIEWER_RESOURCE_PREFIX"/icons/48x48/virt-viewer.png", > NULL); > +if (icon != NULL) { > +gtk_about_dialog_set_logo(GTK_ABOUT_DIALOG(dialog), icon); > +g_object_unref(icon); > +} else { > +gtk_about_dialog_set_logo_icon_name(GTK_ABOUT_DIALOG(dialog), > "virt-viewer"); > +} > + > gtk_window_set_transient_for(GTK_WINDOW(dialog), > GTK_WINDOW(self->priv->window)); > > @@ -1066,8 +1079,7 @@ virt_viewer_window_toolbar_setup(VirtViewerWindow *self) > g_signal_connect(button, "clicked", > G_CALLBACK(virt_viewer_window_menu_file_quit), self); > > /* USB Device selection */ > -button = gtk_image_new_from_icon_name("virt-viewer-usb", > - GTK_ICON_SIZE_INVALID); > +button = > gtk_image_new_from_resource(VIRT_VIEWER_RESOURCE_PREFIX"/icons/24x24/virt-viewer-usb.png"); > button = GTK_WIDGET(gtk_tool_button_new(button, NULL)); > gtk_tool_button_set_label(GTK_TOOL_BUTTON(button), _("USB device > selection")); > gtk_tool_item_set_tooltip_text(GTK_TOOL_ITEM(button), _("USB device > selection")); > diff --git a/src/virt-viewer.gresource.xml b/src/virt-viewer.gresource.xml > new file mode 100644 > index 000..596889a > --- /dev/null > +++ b/src/virt-viewer.gresource.xml > @@ -0,0 +1,19 @@ > + > + > + > +remote-viewer-connect.xml > +virt-viewer-about.xml > +virt-viewer-auth.xml > +virt-viewer-guest-details.xml > +virt-viewer-preferences.xml > +virt-viewer-vm-connection.xml > +virt-viewer.xml > + alias="icons/16x16/virt-viewer.png">../icons/16x16/virt-viewer.png > + alias="icons/22x22/virt-viewer.png">../icons/22x22/virt-viewer.png > + alias="icons/24x24/virt-viewer.png">../icons/24x24/virt-viewer.png > + alias="icons/24x24/virt-viewer-usb.png">../icons/24x24/virt-viewer-usb.png > + alias="icons/32x32/virt-viewer.png">../icons/32x32/virt-viewer.png > + alias="icons/48x48/virt-viewer.png">../icons/48x48/virt-viewer.png > + alias="icons/256x256/virt-viewer.png">../icons/256x256/virt-viewer.png > + > + > diff --git a/virt-viewer.spec.in b/virt-viewer.spec.in > index 274e7e7..3c483ea 100644 > --- a/virt-viewer.spec.in > +++ b/virt-viewer.spec.in > @@ -122,15 +122,6 @@ fi > %doc README COPYING AUTHORS ChangeLog NEWS > %{_bindir}/%{name} > %{_bindir}/remote-viewer > -%dir %{_datadir}/%{name} > -%dir %{_datadir}/%{name}/ui/ > -%{_datadir}/%{name}/ui/virt-viewer.xml > -%{_datadir}/%{name}/ui/virt-viewer-auth.xml > -%{_datadir}/%{name}/ui/virt-viewer-about.xml > -%{_datadir}/%{name}/ui/virt-viewer-guest-details.xml > -%{_datadir}/%{name}/ui/virt-viewer-vm-connection.xml > -%{_datadir}/%{name}/ui/virt-viewer-preferences.xml > -%{_datadir}/%{name}/ui/remote-viewer-connect.xml > %{_datadir}/icons/hicolor/*/apps/* > %{_datadir}/icons/hicolor/*/devices/* > %{_datadir}/applications/remote-viewer.desktop > Looking good from my side. :) Acked-by: Eduardo Lima (Etrunko) <etru...@redhat.com> -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-viewer v2] Use GResource for loading ui files
On 03/02/2016 05:24 PM, Fabiano Fidêncio wrote: > On Wed, Mar 2, 2016 at 8:05 PM, Eduardo Lima (Etrunko) > <etru...@redhat.com> wrote: >> On 03/02/2016 02:46 PM, Eduardo Lima (Etrunko) wrote: >>> On 03/02/2016 12:23 PM, Fabiano Fidêncio wrote: >>>> Let's take advantage of GResource for loading ui files in a better and >>>> cleaner way than virt_viewer_util_load_ui() was doing. >>>> It also brings the benefit, at least for developers, of being able to >>>> test ui changes without having to "make install" virt-viewer. >>>> >>>> Signed-off-by: Fabiano Fidêncio <fiden...@redhat.com> >>>> --- >>>> Changes since v1: >>>> - Adressed all comments from Eduardo and Jonathon. >>>> --- >>>> mingw-virt-viewer.spec.in | 18 ++-- >>>> src/Makefile.am | 21 +-- >>>> src/virt-viewer-about.xml | 1 - >>>> src/virt-viewer-app.c | 5 + >>>> src/virt-viewer-util.c| 48 >>>> +-- >>>> src/virt-viewer-util.h| 1 + >>>> src/virt-viewer-window.c | 20 ++ >>>> src/virt-viewer.gresource.xml | 19 + >>>> virt-viewer.spec.in | 9 >>>> 9 files changed, 63 insertions(+), 79 deletions(-) >>>> create mode 100644 src/virt-viewer.gresource.xml >>>> >>>> diff --git a/mingw-virt-viewer.spec.in b/mingw-virt-viewer.spec.in >>>> index b200db7..ddea296 100644 >>>> --- a/mingw-virt-viewer.spec.in >>>> +++ b/mingw-virt-viewer.spec.in >>>> @@ -114,10 +114,12 @@ MinGW Windows virt-viewer MSI >>>> %mingw_make_install DESTDIR=$RPM_BUILD_ROOT >>>> >>>> %if 0%{?mingw_build_win32} == 1 >>>> +mkdir $RPM_BUILD_ROOT/%{mingw32_datadir}/virt-viewer >>>> cp build_win32$MINGW_BUILDDIR_SUFFIX/data/virt-viewer-x86-@VERSION@.msi >>>> $RPM_BUILD_ROOT/%{mingw32_datadir}/virt-viewer >>>> %endif >>>> >>>> %if 0%{?mingw_build_win64} == 1 >>>> +mkdir $RPM_BUILD_ROOT/%{mingw64_datadir}/virt-viewer >>>> cp build_win64$MINGW_BUILDDIR_SUFFIX/data/virt-viewer-x64-@VERSION@.msi >>>> $RPM_BUILD_ROOT/%{mingw64_datadir}/virt-viewer >>>> %endif >>>> >>>> @@ -138,14 +140,6 @@ rm -rf $RPM_BUILD_ROOT >>>> %{mingw32_bindir}/debug-helper.exe >>>> >>>> %dir %{mingw32_datadir}/virt-viewer/ >>>> -%dir %{mingw32_datadir}/virt-viewer/ui/ >>>> -%{mingw32_datadir}/virt-viewer/ui/virt-viewer.xml >>>> -%{mingw32_datadir}/virt-viewer/ui/virt-viewer-about.xml >>>> -%{mingw32_datadir}/virt-viewer/ui/virt-viewer-auth.xml >>>> -%{mingw32_datadir}/virt-viewer/ui/virt-viewer-guest-details.xml >>>> -%{mingw32_datadir}/virt-viewer/ui/virt-viewer-vm-connection.xml >>>> -%{mingw32_datadir}/virt-viewer/ui/virt-viewer-preferences.xml >>>> -%{mingw32_datadir}/virt-viewer/ui/remote-viewer-connect.xml >>>> %{mingw32_datadir}/icons/hicolor/*/apps/* >>>> %{mingw32_datadir}/icons/hicolor/*/devices/* >>>> >>>> @@ -163,14 +157,6 @@ rm -rf $RPM_BUILD_ROOT >>>> %{mingw64_bindir}/debug-helper.exe >>>> >>>> %dir %{mingw64_datadir}/virt-viewer/ >>>> -%dir %{mingw64_datadir}/virt-viewer/ui/ >>>> -%{mingw64_datadir}/virt-viewer/ui/virt-viewer.xml >>>> -%{mingw64_datadir}/virt-viewer/ui/virt-viewer-about.xml >>>> -%{mingw64_datadir}/virt-viewer/ui/virt-viewer-auth.xml >>>> -%{mingw64_datadir}/virt-viewer/ui/virt-viewer-guest-details.xml >>>> -%{mingw64_datadir}/virt-viewer/ui/virt-viewer-vm-connection.xml >>>> -%{mingw64_datadir}/virt-viewer/ui/virt-viewer-preferences.xml >>>> -%{mingw64_datadir}/virt-viewer/ui/remote-viewer-connect.xml >>>> %{mingw64_datadir}/icons/hicolor/*/apps/* >>>> %{mingw64_datadir}/icons/hicolor/*/devices/* >>>> >>>> diff --git a/src/Makefile.am b/src/Makefile.am >>>> index f42a7bf..a42c01e 100644 >>>> --- a/src/Makefile.am >>>> +++ b/src/Makefile.am >>>> @@ -5,8 +5,7 @@ bin_PROGRAMS = >>>> >>>> noinst_LTLIBRARIES = libvirt-viewer.la >>>> >>>> -builderxmldir = $(pkgdatadir)/ui >>>> -builderxml_DATA = \ >>>> +noinst_DATA =
Re: [virt-tools-list] [virt-viewer] app: Remove useless libxml includes
On 03/02/2016 12:16 PM, Fabiano Fidêncio wrote: > --- > src/virt-viewer-app.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c > index 3fa80a5..b51bf4f 100644 > --- a/src/virt-viewer-app.c > +++ b/src/virt-viewer-app.c > @@ -36,9 +36,6 @@ > #include > #include > > -#include > -#include > - > #ifdef HAVE_SYS_SOCKET_H > #include > #endif > Acked-by: Eduardo Lima (Etrunko) <etru...@redhat.com> -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-viewer] ovirt: Only use active ISO domains for foreign menu
On 03/01/2016 10:36 AM, Christophe Fergeau wrote: > oVirt storage domains can be in various states (inactive, in > maintainance, ...). We only want to show the ISOs it contains in the > foreign menu when the storage domain is actually active, not in the > other states. > --- > src/ovirt-foreign-menu.c | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c > index 9859439..82f0f2a 100644 > --- a/src/ovirt-foreign-menu.c > +++ b/src/ovirt-foreign-menu.c > @@ -650,12 +650,17 @@ static void storage_domains_fetched_cb(GObject > *source_object, > while (g_hash_table_iter_next(, NULL, (gpointer *))) { > OvirtCollection *file_collection; > int type; > +int state; > > -g_object_get(domain, "type", , NULL); > +g_object_get(domain, "type", , "state", , NULL); > if (type != OVIRT_STORAGE_DOMAIN_TYPE_ISO) { > continue; > } > > +if (state != OVIRT_STORAGE_DOMAIN_STATE_ACTIVE) { > +continue; > +} > + Maybe put this test together with the previous one with || ? -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-viewer v2] ovirt: Only use active ISO domains for foreign menu
On 03/02/2016 09:04 AM, Fabiano Fidêncio wrote: > On Wed, Mar 2, 2016 at 12:02 PM, Christophe Fergeau <cferg...@redhat.com> > wrote: >> On Wed, Mar 02, 2016 at 08:52:28AM +0100, Christophe Fergeau wrote: >>> oVirt storage domains can be in various states (inactive, in >>> maintainance, ...). We only want to show the ISOs it contains in the >>> foreign menu when the storage domain is actually active, not in the >>> other states. >>> >>> https://bugzilla.redhat.com/show_bug.cgi?id=1310450 >>> --- >>> Changes since v1: >>> - added new condition to an existing if() block >> >> Fwiw, I prefer the v1 way which I find much easier to read. > > Same here. V1 it is then :) Acked-by: Eduardo Lima (Etrunko) <etru...@redhat.com> -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [PATCH v2 virt-viewer 1/2] Fix spice includes
Spice release version 0.31 requires that only spice-client.h or spice-client-gtk.h should be included directly. As a result, compilation is now throwing warnings like: warning: #warning "Only can be included directly" [-Wcpp] warning: #warning "Only can be included directly" [-Wcpp] This patch also bumps spice version requirement to 0.31, to ensure those files are available. Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- configure.ac| 2 +- src/virt-viewer-display-spice.c | 2 +- src/virt-viewer-display-spice.h | 3 +-- src/virt-viewer-session-spice.c | 5 + src/virt-viewer-session-spice.h | 3 +-- 5 files changed, 5 insertions(+), 10 deletions(-) diff --git a/configure.ac b/configure.ac index 6d8475b..115ad81 100644 --- a/configure.ac +++ b/configure.ac @@ -24,7 +24,7 @@ LIBXML2_REQUIRED="2.6.0" LIBVIRT_REQUIRED="0.10.0" LIBVIRT_GLIB_REQUIRED="0.1.8" GTK_VNC_REQUIRED="0.4.0" -SPICE_GTK_REQUIRED="0.30" +SPICE_GTK_REQUIRED="0.31" SPICE_PROTOCOL_REQUIRED="0.12.7" GOVIRT_REQUIRED="0.3.2" diff --git a/src/virt-viewer-display-spice.c b/src/virt-viewer-display-spice.c index 5114833..4abab2c 100644 --- a/src/virt-viewer-display-spice.c +++ b/src/virt-viewer-display-spice.c @@ -25,7 +25,7 @@ #include #include -#include +#include #include diff --git a/src/virt-viewer-display-spice.h b/src/virt-viewer-display-spice.h index 3c30270..598a1b7 100644 --- a/src/virt-viewer-display-spice.h +++ b/src/virt-viewer-display-spice.h @@ -25,8 +25,7 @@ #define _VIRT_VIEWER_DISPLAY_SPICE_H #include -#include -#include +#include #include "virt-viewer-display.h" #include "virt-viewer-session-spice.h" diff --git a/src/virt-viewer-session-spice.c b/src/virt-viewer-session-spice.c index dc0c1ff..f7f2dc9 100644 --- a/src/virt-viewer-session-spice.c +++ b/src/virt-viewer-session-spice.c @@ -24,12 +24,9 @@ #include -#include #include -#include -#include -#include +#include #include #include "virt-viewer-file.h" diff --git a/src/virt-viewer-session-spice.h b/src/virt-viewer-session-spice.h index 95bdcdf..aa1f7e8 100644 --- a/src/virt-viewer-session-spice.h +++ b/src/virt-viewer-session-spice.h @@ -25,8 +25,7 @@ #define _VIRT_VIEWER_SESSION_SPICE_H #include -#include -#include +#include #include "virt-viewer-session.h" -- 2.5.5 ___ 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 v2 09/12] Run iso-dialog when 'Change CD' menu is activated
Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/resources/ui/virt-viewer.ui | 1 + src/virt-viewer-window.c| 24 2 files changed, 25 insertions(+) diff --git a/src/resources/ui/virt-viewer.ui b/src/resources/ui/virt-viewer.ui index af3ae46..e9609ec 100644 --- a/src/resources/ui/virt-viewer.ui +++ b/src/resources/ui/virt-viewer.ui @@ -78,6 +78,7 @@ False _Change CD True + diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c index 7e6b93f..b82035e 100644 --- a/src/virt-viewer-window.c +++ b/src/virt-viewer-window.c @@ -43,6 +43,8 @@ #include "virt-viewer-util.h" #include "virt-viewer-timed-revealer.h" +#include "remote-viewer-iso-list-dialog.h" + #define ZOOM_STEP 10 /* Signal handlers for main window (move in a VirtViewerMainWindow?) */ @@ -62,6 +64,7 @@ void virt_viewer_window_menu_file_smartcard_insert(GtkWidget *menu, VirtViewerWi void virt_viewer_window_menu_file_smartcard_remove(GtkWidget *menu, VirtViewerWindow *self); void virt_viewer_window_menu_view_release_cursor(GtkWidget *menu, VirtViewerWindow *self); void virt_viewer_window_menu_preferences_cb(GtkWidget *menu, VirtViewerWindow *self); +void virt_viewer_window_menu_change_cd_activate(GtkWidget *menu, VirtViewerWindow *self); /* Internal methods */ @@ -1052,6 +1055,27 @@ virt_viewer_window_menu_help_about(GtkWidget *menu G_GNUC_UNUSED, g_object_unref(G_OBJECT(about)); } +static void +iso_dialog_response(GtkDialog *dialog, +gint response_id, +gpointer user_data G_GNUC_UNUSED) +{ +if (response_id == GTK_RESPONSE_NONE) +return; + +gtk_widget_destroy(GTK_WIDGET(dialog)); +} + +void +virt_viewer_window_menu_change_cd_activate(GtkWidget *menu G_GNUC_UNUSED, + VirtViewerWindow *self) +{ +VirtViewerWindowPrivate *priv = self->priv; +GtkWidget *dialog = remote_viewer_iso_list_dialog_new(GTK_WINDOW(priv->window)); +g_signal_connect(dialog, "response", G_CALLBACK(iso_dialog_response), ); +gtk_widget_show_all(dialog); +gtk_dialog_run(GTK_DIALOG(dialog)); +} static void virt_viewer_window_toolbar_setup(VirtViewerWindow *self) -- 2.7.4 ___ 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 v2 10/12] remote-viewer: Make ovirt-foreign-menu a property
The OvirtForeignMenu pointer is needed by the new ISO list dialog, and we make it acessible via property to avoid interdependency between objects. Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/remote-viewer-iso-list-dialog.c | 31 +++ src/remote-viewer-iso-list-dialog.h | 2 +- src/remote-viewer.c | 37 + src/virt-viewer-window.c| 17 +++-- 4 files changed, 76 insertions(+), 11 deletions(-) diff --git a/src/remote-viewer-iso-list-dialog.c b/src/remote-viewer-iso-list-dialog.c index a319adb..7a84d0c 100644 --- a/src/remote-viewer-iso-list-dialog.c +++ b/src/remote-viewer-iso-list-dialog.c @@ -24,6 +24,7 @@ #include "remote-viewer-iso-list-dialog.h" #include "virt-viewer-util.h" +#include "ovirt-foreign-menu.h" G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, GTK_TYPE_DIALOG) @@ -33,6 +34,7 @@ G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, GTK_TYPE struct _RemoteViewerISOListDialogPrivate { GtkWidget *stack; +OvirtForeignMenu *foreign_menu; }; enum RemoteViewerIsoListDialogPages @@ -44,6 +46,10 @@ enum RemoteViewerIsoListDialogPages static void remote_viewer_iso_list_dialog_dispose(GObject *object) { +RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object); +RemoteViewerISOListDialogPrivate *priv = self->priv; + +g_clear_object(>foreign_menu); G_OBJECT_CLASS(remote_viewer_iso_list_dialog_parent_class)->dispose(object); } @@ -100,13 +106,22 @@ remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self) } GtkWidget * -remote_viewer_iso_list_dialog_new(GtkWindow *parent) +remote_viewer_iso_list_dialog_new(GtkWindow *parent, GObject *foreign_menu) { -return g_object_new(REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG, -"title", _("Change CD"), -"transient-for", parent, -"border-width", 18, -"default-width", 400, -"default-height", 300, -NULL); +GtkWidget *dialog; +RemoteViewerISOListDialog *self; + +g_return_val_if_fail(foreign_menu != NULL, NULL); + +dialog = g_object_new(REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG, + "title", _("Change CD"), + "transient-for", parent, + "border-width", 18, + "default-width", 400, + "default-height", 300, + NULL); + +self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog); +self->priv->foreign_menu = OVIRT_FOREIGN_MENU(g_object_ref(foreign_menu)); +return dialog; } diff --git a/src/remote-viewer-iso-list-dialog.h b/src/remote-viewer-iso-list-dialog.h index def841b..8b936f5 100644 --- a/src/remote-viewer-iso-list-dialog.h +++ b/src/remote-viewer-iso-list-dialog.h @@ -51,7 +51,7 @@ struct _RemoteViewerISOListDialogClass GType remote_viewer_iso_list_dialog_get_type(void) G_GNUC_CONST; -GtkWidget *remote_viewer_iso_list_dialog_new(GtkWindow *parent); +GtkWidget *remote_viewer_iso_list_dialog_new(GtkWindow *parent, GObject *foreign_menu); G_END_DECLS diff --git a/src/remote-viewer.c b/src/remote-viewer.c index 95130dc..e0cd139 100644 --- a/src/remote-viewer.c +++ b/src/remote-viewer.c @@ -67,6 +67,13 @@ G_DEFINE_TYPE (RemoteViewer, remote_viewer, VIRT_VIEWER_TYPE_APP) #define GET_PRIVATE(o)\ (G_TYPE_INSTANCE_GET_PRIVATE ((o), REMOTE_VIEWER_TYPE, RemoteViewerPrivate)) +enum RemoteViewerProperties { +PROP_0, +#ifdef HAVE_OVIRT +PROP_OVIRT_FOREIGN_MENU, +#endif +}; + #ifdef HAVE_OVIRT static OvirtVm * choose_vm(GtkWindow *main_window, char **vm_name, @@ -213,6 +220,25 @@ end: } static void +remote_viewer_get_property(GObject *object, guint property_id, + GValue *value, GParamSpec *pspec) +{ +RemoteViewer *self = REMOTE_VIEWER(object); +RemoteViewerPrivate *priv = self->priv; + +switch (property_id) { +#ifdef HAVE_OVIRT +case PROP_OVIRT_FOREIGN_MENU: +g_value_set_object(value, priv->ovirt_foreign_menu); +break; +#endif + +default: +G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec); +} +} + +static void remote_viewer_class_init (RemoteViewerClass *klass) { GObjectClass *object_class = G_OBJECT_CLASS (klass); @@ -222,6 +248,7 @@ remote_viewer_class_init (RemoteViewerClass *klass) g_type_class_add_private (klass, sizeof (RemoteViewerPrivate)); +object_class->get_pro
[virt-tools-list] [PATCH virt-viewer v3 05/10] UI: Make 'Change CD' menu item a submenu under 'File' toplevel menu
Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/resources/ui/virt-viewer.ui | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/resources/ui/virt-viewer.ui b/src/resources/ui/virt-viewer.ui index 6e3c5ad..af3ae46 100644 --- a/src/resources/ui/virt-viewer.ui +++ b/src/resources/ui/virt-viewer.ui @@ -1,6 +1,7 @@ + - + False @@ -73,6 +74,13 @@ + +False +_Change CD +True + + + True False @@ -247,14 +255,6 @@ - - -False -False -_Change CD -True - - False -- 2.7.4 ___ 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 v3 04/10] ovirt-foreign-menu: Notify of new files even if nothing changed
When user presses the Refresh button in ISO dialog, the list is cleared, and currently, the only way it is informed of the new list is by the notify signal. The same applies when an error occurs while trying to change the current ISO. Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/ovirt-foreign-menu.c | 43 +++ 1 file changed, 15 insertions(+), 28 deletions(-) diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c index 493ca47..8320552 100644 --- a/src/ovirt-foreign-menu.c +++ b/src/ovirt-foreign-menu.c @@ -375,22 +375,24 @@ static void updated_cdrom_cb(GObject *source_object, g_free(foreign_menu->priv->current_iso_name); foreign_menu->priv->current_iso_name = foreign_menu->priv->next_iso_name; foreign_menu->priv->next_iso_name = NULL; -g_object_notify(G_OBJECT(foreign_menu), "file"); -} else { -/* Reset old state back as we were not successful in switching to - * the new ISO */ -const char *current_file = foreign_menu->priv->current_iso_name; - -if (error != NULL) { -g_warning("failed to update cdrom resource: %s", error->message); -g_clear_error(); -} -g_debug("setting OvirtCdrom:file back to '%s'", -current_file?current_file:NULL); -g_object_set(foreign_menu->priv->cdrom, "file", current_file, NULL); +goto end; } +/* Reset old state back as we were not successful in switching to + * the new ISO */ +if (error != NULL) { +g_warning("failed to update cdrom resource: %s", error->message); +g_clear_error(); +} +g_debug("setting OvirtCdrom:file back to '%s'", +foreign_menu->priv->current_iso_name); +g_object_set(foreign_menu->priv->cdrom, + "file", foreign_menu->priv->current_iso_name, + NULL); g_clear_pointer(_menu->priv->next_iso_name, g_free); + +end: +g_object_notify(G_OBJECT(foreign_menu), "file"); } @@ -399,7 +401,6 @@ static void ovirt_foreign_menu_set_files(OvirtForeignMenu *menu, { GList *sorted_files = NULL; const GList *it; -GList *it2; for (it = files; it != NULL; it = it->next) { char *name; @@ -416,20 +417,6 @@ static void ovirt_foreign_menu_set_files(OvirtForeignMenu *menu, (GCompareFunc)g_strcmp0); } -for (it = sorted_files, it2 = menu->priv->iso_names; - (it != NULL) && (it2 != NULL); - it = it->next, it2 = it2->next) { -if (g_strcmp0(it->data, it2->data) != 0) { -break; -} -} - -if ((it == NULL) && (it2 == NULL)) { -/* sorted_files and menu->priv->files content was the same */ -g_list_free_full(sorted_files, (GDestroyNotify)g_free); -return; -} - g_list_free_full(menu->priv->iso_names, (GDestroyNotify)g_free); menu->priv->iso_names = sorted_files; g_object_notify(G_OBJECT(menu), "files"); -- 2.7.4 ___ 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 v3 00/10] Replace oVirt foreign menu with dedicated dialog
I have pushed the first two patches of the series because they were already acknowledged and were pretty much self-contained. I tried to replace GtkTreeView in favor of new GtkListBox, but after some painful work, I decided to drop it because it is not possible to reproduce the exact same behavior of the former with the latter. For instance, once we have a GtkRadioButton activated, it is not possible to deactivate it. We also need to create a new GObject that would be used as the model, while with a tree view, things can be automatically done with Glade. Finally, it would be necessary to raise the requirements for both gtk+ (3.16) and glib (2.44). If some one wants to take a look on the work in progress, check out the 'dialog' branch in my github clone (http://github.com/etrunko/virt-viewer). In this version: * Removed leftover enum when changed from GtkNotebook to GtkStack * Some cosmetic fixes (indentation, renaming, etc) * UI Tweaks: - Added some padding between items of the list. - Set tree view selection to current ISO when the list is first loaded or refreshed. - Handle tree view "activate" signal the same way as radio button toggle - Removed "Select ISO" label and alignment in header bar patch. Eduardo Lima (Etrunko) (10): ovirt-foreign-menu: Remove timer used to refresh iso list ovirt-foreign-menu: Add accessors for current iso and iso list ovirt-foreign-menu: Remove GtkMenu related functions ovirt-foreign-menu: Notify of new files even if nothing changed UI: Make 'Change CD' menu item a submenu under 'File' toplevel menu Introduce ISO List dialog Run iso-dialog when 'Change CD' menu is activated remote-viewer: Make ovirt-foreign-menu a property iso-dialog: Implement functionality provided by oVirt foreign menu iso-dialog: Use header bar for buttons configure.ac | 4 +- po/POTFILES.in | 2 + src/Makefile.am| 3 + src/ovirt-foreign-menu.c | 182 + src/ovirt-foreign-menu.h | 5 +- src/remote-viewer-iso-list-dialog.c| 316 + src/remote-viewer-iso-list-dialog.h| 58 ++ src/remote-viewer.c| 89 src/resources/ui/remote-viewer-iso-list.ui | 135 src/resources/ui/virt-viewer.ui| 19 +- src/resources/virt-viewer.gresource.xml| 1 + src/virt-viewer-window.c | 37 12 files changed, 660 insertions(+), 191 deletions(-) create mode 100644 src/remote-viewer-iso-list-dialog.c create mode 100644 src/remote-viewer-iso-list-dialog.h create mode 100644 src/resources/ui/remote-viewer-iso-list.ui -- 2.7.4 ___ 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 v3 09/10] iso-dialog: Implement functionality provided by oVirt foreign menu
Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/remote-viewer-iso-list-dialog.c| 180 - src/resources/ui/remote-viewer-iso-list.ui | 5 +- 2 files changed, 181 insertions(+), 4 deletions(-) diff --git a/src/remote-viewer-iso-list-dialog.c b/src/remote-viewer-iso-list-dialog.c index 858719c..4a1cf44 100644 --- a/src/remote-viewer-iso-list-dialog.c +++ b/src/remote-viewer-iso-list-dialog.c @@ -20,6 +20,7 @@ #include +#include #include #include "remote-viewer-iso-list-dialog.h" @@ -33,17 +34,32 @@ G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, GTK_TYPE struct _RemoteViewerISOListDialogPrivate { +GtkListStore *list_store; GtkWidget *stack; +GtkWidget *tree_view; OvirtForeignMenu *foreign_menu; }; +enum RemoteViewerISOListDialogModel +{ +ISO_IS_ACTIVE = 0, +ISO_NAME, +FONT_WEIGHT, +}; + +void remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle *cell_renderer, gchar *path, gpointer user_data); +void remote_viewer_iso_list_dialog_row_activated(GtkTreeView *view, GtkTreePath *path, GtkTreeViewColumn *col, gpointer user_data); + static void remote_viewer_iso_list_dialog_dispose(GObject *object) { RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object); RemoteViewerISOListDialogPrivate *priv = self->priv; -g_clear_object(>foreign_menu); +if (priv->foreign_menu) { +g_signal_handlers_disconnect_by_data(priv->foreign_menu, object); +g_clear_object(>foreign_menu); +} G_OBJECT_CLASS(remote_viewer_iso_list_dialog_parent_class)->dispose(object); } @@ -58,6 +74,58 @@ remote_viewer_iso_list_dialog_class_init(RemoteViewerISOListDialogClass *klass) } static void +remote_viewer_iso_list_dialog_show_files(RemoteViewerISOListDialog *self) +{ +RemoteViewerISOListDialogPrivate *priv = self->priv = DIALOG_PRIVATE(self); +gtk_stack_set_visible_child_full(GTK_STACK(priv->stack), "iso-list", + GTK_STACK_TRANSITION_TYPE_SLIDE_LEFT); +gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, TRUE); +} + +static void +remote_viewer_iso_list_dialog_foreach(char *name, RemoteViewerISOListDialog *self) +{ +RemoteViewerISOListDialogPrivate *priv = self->priv; +char * current_iso = ovirt_foreign_menu_get_current_iso_name(self->priv->foreign_menu); +gboolean active = g_strcmp0(current_iso, name) == 0; +gint weight = active ? PANGO_WEIGHT_BOLD : PANGO_WEIGHT_NORMAL; +GtkTreeIter iter; + +gtk_list_store_append(priv->list_store, ); +gtk_list_store_set(priv->list_store, , + ISO_IS_ACTIVE, active, + ISO_NAME, name, + FONT_WEIGHT, weight, -1); + +if (active) { +GtkTreePath *path = gtk_tree_model_get_path(GTK_TREE_MODEL(priv->list_store), ); +gtk_tree_view_set_cursor(GTK_TREE_VIEW(priv->tree_view), path, NULL, FALSE); +gtk_tree_view_scroll_to_cell(GTK_TREE_VIEW(priv->tree_view), path, NULL, TRUE, 0.5, 0.5); +gtk_tree_path_free(path); +} + +free(current_iso); +} + +static void +remote_viewer_iso_list_dialog_refresh_iso_list(RemoteViewerISOListDialog *self, + gboolean refreshing) +{ +RemoteViewerISOListDialogPrivate *priv = self->priv; +GList *iso_list; + +if (refreshing) { +gtk_list_store_clear(priv->list_store); +ovirt_foreign_menu_start(priv->foreign_menu); +return; +} + +iso_list = ovirt_foreign_menu_get_iso_names(priv->foreign_menu); +g_list_foreach(iso_list, (GFunc) remote_viewer_iso_list_dialog_foreach, self); +remote_viewer_iso_list_dialog_show_files(self); +} + +static void remote_viewer_iso_list_dialog_response(GtkDialog *dialog, gint response_id, gpointer user_data G_GNUC_UNUSED) @@ -71,7 +139,45 @@ remote_viewer_iso_list_dialog_response(GtkDialog *dialog, gtk_stack_set_visible_child_full(GTK_STACK(priv->stack), "status", GTK_STACK_TRANSITION_TYPE_SLIDE_LEFT); gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, FALSE); -gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_CLOSE, FALSE); +remote_viewer_iso_list_dialog_refresh_iso_list(self, TRUE); +} + +void +remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle *cell_renderer G_GNUC_UNUSED, + gchar *path, + gpointer user_data) +{ +RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(user_data); +RemoteViewerISOListDialogPrivate *priv = self->priv; +GtkTreeModel *model = GTK_TREE_MODEL(priv->list_store)
[virt-tools-list] [PATCH virt-viewer v3 03/10] ovirt-foreign-menu: Remove GtkMenu related functions
With this commit, we finish cleaning up ovirt foreign menu code, which only deals with data, leaving the UI bits to be handled properly in the new ISO list dialog. This patch also updates remote-viewer to reflect the change. Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/ovirt-foreign-menu.c | 79 src/remote-viewer.c | 52 +-- 2 files changed, 8 insertions(+), 123 deletions(-) diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c index eec6bc6..493ca47 100644 --- a/src/ovirt-foreign-menu.c +++ b/src/ovirt-foreign-menu.c @@ -359,22 +359,6 @@ ovirt_foreign_menu_start(OvirtForeignMenu *menu) } -static void -ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, gpointer user_data); - - -static void -menu_item_set_active_no_signal(GtkMenuItem *menuitem, - gboolean active, - GCallback callback, - gpointer user_data) -{ -g_signal_handlers_block_by_func(menuitem, callback, user_data); -gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(menuitem), active); -g_signal_handlers_unblock_by_func(menuitem, callback, user_data); -} - - static void updated_cdrom_cb(GObject *source_object, GAsyncResult *result, gpointer user_data) @@ -410,69 +394,6 @@ static void updated_cdrom_cb(GObject *source_object, } -static void -ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, gpointer user_data) -{ -OvirtForeignMenu *foreign_menu; -const char *iso_name = NULL; -gboolean checked; - -checked = gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(menuitem)); -foreign_menu = OVIRT_FOREIGN_MENU(user_data); -g_return_if_fail(foreign_menu->priv->cdrom != NULL); -g_return_if_fail(foreign_menu->priv->next_iso_name == NULL); - -g_debug("'%s' clicked", gtk_menu_item_get_label(menuitem)); - -/* We only want to move the check mark for the currently selected ISO - * when ovirt_cdrom_update_async() is successful, so for now we move - * the check mark back to where it was before - */ -menu_item_set_active_no_signal(menuitem, !checked, - (GCallback)ovirt_foreign_menu_activate_item_cb, - foreign_menu); - -if (checked) { -iso_name = gtk_menu_item_get_label(menuitem); -} - -ovirt_foreign_menu_set_current_iso_name(foreign_menu, iso_name); -} - - -GtkWidget *ovirt_foreign_menu_get_gtk_menu(OvirtForeignMenu *foreign_menu) -{ -GtkWidget *gtk_menu; -GList *it; -char *current_iso; - -if (foreign_menu->priv->iso_names == NULL) { -g_debug("ISO list is empty, no menu to show"); -return NULL; -} -g_debug("Creating GtkMenu for foreign menu"); -current_iso = ovirt_foreign_menu_get_current_iso_name(foreign_menu); -gtk_menu = gtk_menu_new(); -for (it = foreign_menu->priv->iso_names; it != NULL; it = it->next) { -GtkWidget *menuitem; - -menuitem = gtk_check_menu_item_new_with_label((char *)it->data); -if (g_strcmp0((char *)it->data, current_iso) == 0) { -g_warn_if_fail(g_strcmp0(current_iso, foreign_menu->priv->current_iso_name) == 0); -gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(menuitem), - TRUE); -} -g_signal_connect(menuitem, "activate", - G_CALLBACK(ovirt_foreign_menu_activate_item_cb), - foreign_menu); -gtk_menu_shell_append(GTK_MENU_SHELL(gtk_menu), menuitem); -} -g_free(current_iso); - -return gtk_menu; -} - - static void ovirt_foreign_menu_set_files(OvirtForeignMenu *menu, const GList *files) { diff --git a/src/remote-viewer.c b/src/remote-viewer.c index 6d29bf2..95130dc 100644 --- a/src/remote-viewer.c +++ b/src/remote-viewer.c @@ -734,33 +734,11 @@ authenticate_cb(RestProxy *proxy, G_GNUC_UNUSED RestProxyAuth *auth, static void ovirt_foreign_menu_update(GtkApplication *gtkapp, GtkWindow *gtkwin, G_GNUC_UNUSED gpointer data) { -RemoteViewer *app = REMOTE_VIEWER(gtkapp); +RemoteViewer *self = REMOTE_VIEWER(gtkapp); VirtViewerWindow *win = g_object_get_data(G_OBJECT(gtkwin), "virt-viewer-window"); -GtkWidget *menu = g_object_get_data(G_OBJECT(win), "foreign-menu"); -GtkWidget *submenu; - -if (app->priv->ovirt_foreign_menu == NULL) { -/* nothing to do */ -return; -} - -submenu = ovirt_foreign_menu_get_gtk_menu(app->priv->ovirt_foreign_menu); -if (submenu == NULL) { -/* No items to show, no point in showing the menu */ -if (menu != NULL) -
[virt-tools-list] [PATCH virt-viewer v3 08/10] remote-viewer: Make ovirt-foreign-menu a property
The OvirtForeignMenu pointer is needed by the new ISO list dialog, and we make it acessible via property to avoid interdependency between objects. Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/remote-viewer-iso-list-dialog.c | 31 +++ src/remote-viewer-iso-list-dialog.h | 2 +- src/remote-viewer.c | 37 + src/virt-viewer-window.c| 15 ++- 4 files changed, 75 insertions(+), 10 deletions(-) diff --git a/src/remote-viewer-iso-list-dialog.c b/src/remote-viewer-iso-list-dialog.c index 0fbea28..858719c 100644 --- a/src/remote-viewer-iso-list-dialog.c +++ b/src/remote-viewer-iso-list-dialog.c @@ -24,6 +24,7 @@ #include "remote-viewer-iso-list-dialog.h" #include "virt-viewer-util.h" +#include "ovirt-foreign-menu.h" G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, GTK_TYPE_DIALOG) @@ -33,11 +34,16 @@ G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, GTK_TYPE struct _RemoteViewerISOListDialogPrivate { GtkWidget *stack; +OvirtForeignMenu *foreign_menu; }; static void remote_viewer_iso_list_dialog_dispose(GObject *object) { +RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object); +RemoteViewerISOListDialogPrivate *priv = self->priv; + +g_clear_object(>foreign_menu); G_OBJECT_CLASS(remote_viewer_iso_list_dialog_parent_class)->dispose(object); } @@ -94,13 +100,22 @@ remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self) } GtkWidget * -remote_viewer_iso_list_dialog_new(GtkWindow *parent) +remote_viewer_iso_list_dialog_new(GtkWindow *parent, GObject *foreign_menu) { -return g_object_new(REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG, -"title", _("Change CD"), -"transient-for", parent, -"border-width", 18, -"default-width", 400, -"default-height", 300, -NULL); +GtkWidget *dialog; +RemoteViewerISOListDialog *self; + +g_return_val_if_fail(foreign_menu != NULL, NULL); + +dialog = g_object_new(REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG, + "title", _("Change CD"), + "transient-for", parent, + "border-width", 18, + "default-width", 400, + "default-height", 300, + NULL); + +self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog); +self->priv->foreign_menu = OVIRT_FOREIGN_MENU(g_object_ref(foreign_menu)); +return dialog; } diff --git a/src/remote-viewer-iso-list-dialog.h b/src/remote-viewer-iso-list-dialog.h index def841b..8b936f5 100644 --- a/src/remote-viewer-iso-list-dialog.h +++ b/src/remote-viewer-iso-list-dialog.h @@ -51,7 +51,7 @@ struct _RemoteViewerISOListDialogClass GType remote_viewer_iso_list_dialog_get_type(void) G_GNUC_CONST; -GtkWidget *remote_viewer_iso_list_dialog_new(GtkWindow *parent); +GtkWidget *remote_viewer_iso_list_dialog_new(GtkWindow *parent, GObject *foreign_menu); G_END_DECLS diff --git a/src/remote-viewer.c b/src/remote-viewer.c index 95130dc..e0cd139 100644 --- a/src/remote-viewer.c +++ b/src/remote-viewer.c @@ -67,6 +67,13 @@ G_DEFINE_TYPE (RemoteViewer, remote_viewer, VIRT_VIEWER_TYPE_APP) #define GET_PRIVATE(o)\ (G_TYPE_INSTANCE_GET_PRIVATE ((o), REMOTE_VIEWER_TYPE, RemoteViewerPrivate)) +enum RemoteViewerProperties { +PROP_0, +#ifdef HAVE_OVIRT +PROP_OVIRT_FOREIGN_MENU, +#endif +}; + #ifdef HAVE_OVIRT static OvirtVm * choose_vm(GtkWindow *main_window, char **vm_name, @@ -213,6 +220,25 @@ end: } static void +remote_viewer_get_property(GObject *object, guint property_id, + GValue *value, GParamSpec *pspec) +{ +RemoteViewer *self = REMOTE_VIEWER(object); +RemoteViewerPrivate *priv = self->priv; + +switch (property_id) { +#ifdef HAVE_OVIRT +case PROP_OVIRT_FOREIGN_MENU: +g_value_set_object(value, priv->ovirt_foreign_menu); +break; +#endif + +default: +G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec); +} +} + +static void remote_viewer_class_init (RemoteViewerClass *klass) { GObjectClass *object_class = G_OBJECT_CLASS (klass); @@ -222,6 +248,7 @@ remote_viewer_class_init (RemoteViewerClass *klass) g_type_class_add_private (klass, sizeof (RemoteViewerPrivate)); +object_class->get_property = remote_viewer_get_property; object_class->dispose = remote_viewer_dispose; g_app_class->local_command_line = remote_viewer_local_command_line;
[virt-tools-list] [PATCH virt-viewer v3 01/10] ovirt-foreign-menu: Remove timer used to refresh iso list
With the new ISO dialog, the user triggers the refresh manually. Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/ovirt-foreign-menu.c | 23 +++ 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c index a51f2c9..565ef22 100644 --- a/src/ovirt-foreign-menu.c +++ b/src/ovirt-foreign-menu.c @@ -46,7 +46,7 @@ static void ovirt_foreign_menu_fetch_vm_async(OvirtForeignMenu *menu); static void ovirt_foreign_menu_fetch_storage_domain_async(OvirtForeignMenu *menu); static void ovirt_foreign_menu_fetch_vm_cdrom_async(OvirtForeignMenu *menu); static void ovirt_foreign_menu_refresh_cdrom_file_async(OvirtForeignMenu *menu); -static gboolean ovirt_foreign_menu_refresh_iso_list(gpointer user_data); +static void ovirt_foreign_menu_fetch_iso_list_async(OvirtForeignMenu *menu); G_DEFINE_TYPE (OvirtForeignMenu, ovirt_foreign_menu, G_TYPE_OBJECT) @@ -313,7 +313,7 @@ ovirt_foreign_menu_next_async_step(OvirtForeignMenu *menu, g_warn_if_fail(menu->priv->files != NULL); g_warn_if_fail(menu->priv->cdrom != NULL); -ovirt_foreign_menu_refresh_iso_list(menu); +ovirt_foreign_menu_fetch_iso_list_async(menu); break; default: g_warn_if_reached(); @@ -754,8 +754,6 @@ static void iso_list_fetched_cb(GObject *source_object, files = g_hash_table_get_values(ovirt_collection_get_resources(collection)); ovirt_foreign_menu_set_files(OVIRT_FOREIGN_MENU(user_data), files); g_list_free(files); - -g_timeout_add_seconds(300, ovirt_foreign_menu_refresh_iso_list, user_data); } @@ -765,27 +763,12 @@ static void ovirt_foreign_menu_fetch_iso_list_async(OvirtForeignMenu *menu) return; } +g_debug("Refreshing foreign menu iso list"); ovirt_collection_fetch_async(menu->priv->files, menu->priv->proxy, NULL, iso_list_fetched_cb, menu); } -static gboolean ovirt_foreign_menu_refresh_iso_list(gpointer user_data) -{ -OvirtForeignMenu *menu; - -g_debug("Refreshing foreign menu iso list"); -menu = OVIRT_FOREIGN_MENU(user_data); -ovirt_foreign_menu_fetch_iso_list_async(menu); - -/* ovirt_foreign_menu_fetch_iso_list_async() will schedule a new call to - * that function through iso_list_fetched_cb() when it has finished - * fetching the iso list - */ -return G_SOURCE_REMOVE; -} - - OvirtForeignMenu *ovirt_foreign_menu_new_from_file(VirtViewerFile *file) { OvirtProxy *proxy = NULL; -- 2.7.4 ___ 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 v3 10/10] iso-dialog: Use header bar for buttons
It seems to give more modern look to the dialog, but it requires Gtk+ 3.12, thus I will am keeping this commit separated. On the glade UI file, we get rid of the GtkAlignment object that was used to put some space between the tree view and dialog buttons. The "Select ISO" label is gone too. Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- configure.ac | 4 +- src/remote-viewer-iso-list-dialog.c| 31 +-- src/resources/ui/remote-viewer-iso-list.ui | 87 +++--- 3 files changed, 60 insertions(+), 62 deletions(-) diff --git a/configure.ac b/configure.ac index 7c437a3..7682f42 100644 --- a/configure.ac +++ b/configure.ac @@ -17,8 +17,8 @@ GLIB2_REQUIRED="2.38" GLIB2_ENCODED_VERSION="GLIB_VERSION_2_38" # Keep these two definitions in agreement. -GTK_REQUIRED="3.10" -GTK_ENCODED_VERSION="GDK_VERSION_3_10" +GTK_REQUIRED="3.12" +GTK_ENCODED_VERSION="GDK_VERSION_3_12" LIBXML2_REQUIRED="2.6.0" LIBVIRT_REQUIRED="0.10.0" diff --git a/src/remote-viewer-iso-list-dialog.c b/src/remote-viewer-iso-list-dialog.c index 4a1cf44..dd652d7 100644 --- a/src/remote-viewer-iso-list-dialog.c +++ b/src/remote-viewer-iso-list-dialog.c @@ -34,6 +34,7 @@ G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, GTK_TYPE struct _RemoteViewerISOListDialogPrivate { +GtkHeaderBar *header_bar; GtkListStore *list_store; GtkWidget *stack; GtkWidget *tree_view; @@ -83,6 +84,19 @@ remote_viewer_iso_list_dialog_show_files(RemoteViewerISOListDialog *self) } static void +remote_viewer_iso_list_dialog_set_subtitle(RemoteViewerISOListDialog *self, const char *iso_name) +{ +RemoteViewerISOListDialogPrivate *priv = self->priv; +gchar *subtitle = NULL; + +if (iso_name && strlen(iso_name) != 0) +subtitle = g_strdup_printf(_("Current: %s"), iso_name); + +gtk_header_bar_set_subtitle(priv->header_bar, subtitle); +g_free(subtitle); +} + +static void remote_viewer_iso_list_dialog_foreach(char *name, RemoteViewerISOListDialog *self) { RemoteViewerISOListDialogPrivate *priv = self->priv; @@ -102,6 +116,7 @@ remote_viewer_iso_list_dialog_foreach(char *name, RemoteViewerISOListDialog *sel gtk_tree_view_set_cursor(GTK_TREE_VIEW(priv->tree_view), path, NULL, FALSE); gtk_tree_view_scroll_to_cell(GTK_TREE_VIEW(priv->tree_view), path, NULL, TRUE, 0.5, 0.5); gtk_tree_path_free(path); +remote_viewer_iso_list_dialog_set_subtitle(self, current_iso); } free(current_iso); @@ -136,6 +151,7 @@ remote_viewer_iso_list_dialog_response(GtkDialog *dialog, if (response_id != GTK_RESPONSE_NONE) return; +remote_viewer_iso_list_dialog_set_subtitle(self, NULL); gtk_stack_set_visible_child_full(GTK_STACK(priv->stack), "status", GTK_STACK_TRANSITION_TYPE_SLIDE_LEFT); gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, FALSE); @@ -187,9 +203,13 @@ remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self) RemoteViewerISOListDialogPrivate *priv = self->priv = DIALOG_PRIVATE(self); GtkBuilder *builder = virt_viewer_util_load_ui("remote-viewer-iso-list.ui"); GtkCellRendererToggle *cell_renderer; +GtkWidget *button, *image; gtk_builder_connect_signals(builder, self); +priv->header_bar = GTK_HEADER_BAR(gtk_dialog_get_header_bar(GTK_DIALOG(self))); +gtk_header_bar_set_has_subtitle(priv->header_bar, TRUE); + priv->stack = GTK_WIDGET(gtk_builder_get_object(builder, "stack")); gtk_box_pack_start(GTK_BOX(content), priv->stack, TRUE, TRUE, 0); @@ -201,12 +221,11 @@ remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self) g_object_unref(builder); -gtk_dialog_add_buttons(GTK_DIALOG(self), - _("Refresh"), GTK_RESPONSE_NONE, - _("Close"), GTK_RESPONSE_CLOSE, - NULL); +button = gtk_dialog_add_button(GTK_DIALOG(self), "", GTK_RESPONSE_NONE); +image = gtk_image_new_from_icon_name("view-refresh-symbolic", GTK_ICON_SIZE_BUTTON); +gtk_button_set_image(GTK_BUTTON(button), image); +gtk_button_set_always_show_image(GTK_BUTTON(button), TRUE); -gtk_dialog_set_default_response(GTK_DIALOG(self), GTK_RESPONSE_CLOSE); gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, FALSE); g_signal_connect(self, "response", G_CALLBACK(remote_viewer_iso_list_dialog_response), NULL); } @@ -246,6 +265,7 @@ ovirt_foreign_menu_notify_file(OvirtForeignMenu *foreign_menu, g_free(name); } while (gtk_tree_model_iter_next(model, )); +remote_viewer_iso_list_dialog_set_subti
[virt-tools-list] [PATCH virt-viewer v3 06/10] Introduce ISO List dialog
Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- po/POTFILES.in | 2 + src/Makefile.am| 3 + src/remote-viewer-iso-list-dialog.c| 106 src/remote-viewer-iso-list-dialog.h| 58 +++ src/resources/ui/remote-viewer-iso-list.ui | 155 + src/resources/virt-viewer.gresource.xml| 1 + 6 files changed, 325 insertions(+) create mode 100644 src/remote-viewer-iso-list-dialog.c create mode 100644 src/remote-viewer-iso-list-dialog.h create mode 100644 src/resources/ui/remote-viewer-iso-list.ui diff --git a/po/POTFILES.in b/po/POTFILES.in index 6775f53..54de445 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -1,9 +1,11 @@ data/remote-viewer.appdata.xml.in data/remote-viewer.desktop.in data/virt-viewer-mime.xml.in +src/remote-viewer-iso-list-dialog.c src/remote-viewer-main.c src/remote-viewer.c [type: gettext/glade] src/resources/ui/remote-viewer-connect.ui +[type: gettext/glade] src/resources/ui/remote-viewer-iso-list.ui [type: gettext/glade] src/resources/ui/virt-viewer-about.ui src/virt-viewer-app.c src/virt-viewer-auth.c diff --git a/src/Makefile.am b/src/Makefile.am index 0c48e40..66a73f8 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -13,6 +13,7 @@ noinst_DATA = \ resources/ui/virt-viewer-vm-connection.ui \ resources/ui/virt-viewer-preferences.ui \ resources/ui/remote-viewer-connect.ui \ + resources/ui/remote-viewer-iso-list.ui \ $(NULL) EXTRA_DIST = \ @@ -96,6 +97,8 @@ if HAVE_OVIRT libvirt_viewer_la_SOURCES += \ ovirt-foreign-menu.h \ ovirt-foreign-menu.c \ + remote-viewer-iso-list-dialog.c \ + remote-viewer-iso-list-dialog.h \ $(NULL) endif diff --git a/src/remote-viewer-iso-list-dialog.c b/src/remote-viewer-iso-list-dialog.c new file mode 100644 index 000..0fbea28 --- /dev/null +++ b/src/remote-viewer-iso-list-dialog.c @@ -0,0 +1,106 @@ +/* + * Virt Viewer: A virtual machine console viewer + * + * Copyright (C) 2016 Red Hat, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#include + +#include + +#include "remote-viewer-iso-list-dialog.h" +#include "virt-viewer-util.h" + +G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, GTK_TYPE_DIALOG) + +#define DIALOG_PRIVATE(o) \ +(G_TYPE_INSTANCE_GET_PRIVATE((o), REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG, RemoteViewerISOListDialogPrivate)) + +struct _RemoteViewerISOListDialogPrivate +{ +GtkWidget *stack; +}; + +static void +remote_viewer_iso_list_dialog_dispose(GObject *object) +{ + G_OBJECT_CLASS(remote_viewer_iso_list_dialog_parent_class)->dispose(object); +} + +static void +remote_viewer_iso_list_dialog_class_init(RemoteViewerISOListDialogClass *klass) +{ +GObjectClass *object_class = G_OBJECT_CLASS(klass); + +g_type_class_add_private(klass, sizeof(RemoteViewerISOListDialogPrivate)); + +object_class->dispose = remote_viewer_iso_list_dialog_dispose; +} + +static void +remote_viewer_iso_list_dialog_response(GtkDialog *dialog, + gint response_id, + gpointer user_data G_GNUC_UNUSED) +{ +RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog); +RemoteViewerISOListDialogPrivate *priv = self->priv; + +if (response_id != GTK_RESPONSE_NONE) +return; + +gtk_stack_set_visible_child_full(GTK_STACK(priv->stack), "status", + GTK_STACK_TRANSITION_TYPE_SLIDE_LEFT); +gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, FALSE); +gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_CLOSE, FALSE); +} + +static void +remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self) +{ +GtkWidget *content = gtk_dialog_get_content_area(GTK_DIALOG(self)); +RemoteViewerISOListDialogPrivate *priv = self->priv = DIALOG_PRIVATE(self); +GtkBuilder *builder = virt_viewer_util_load_ui("remote-viewer-iso-list.ui"); + +gtk_builder_connect_signals(builder, self); + +priv->stack = GTK_WIDGET(gtk_builder_get_ob
[virt-tools-list] [PATCH virt-viewer v3 02/10] ovirt-foreign-menu: Add accessors for current iso and iso list
Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/ovirt-foreign-menu.c | 49 ++-- src/ovirt-foreign-menu.h | 5 - 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c index 565ef22..eec6bc6 100644 --- a/src/ovirt-foreign-menu.c +++ b/src/ovirt-foreign-menu.c @@ -47,6 +47,7 @@ static void ovirt_foreign_menu_fetch_storage_domain_async(OvirtForeignMenu *menu static void ovirt_foreign_menu_fetch_vm_cdrom_async(OvirtForeignMenu *menu); static void ovirt_foreign_menu_refresh_cdrom_file_async(OvirtForeignMenu *menu); static void ovirt_foreign_menu_fetch_iso_list_async(OvirtForeignMenu *menu); +static void updated_cdrom_cb(GObject *source_object, GAsyncResult *result, gpointer user_data); G_DEFINE_TYPE (OvirtForeignMenu, ovirt_foreign_menu, G_TYPE_OBJECT) @@ -85,7 +86,7 @@ enum { }; -static char * +char * ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *foreign_menu) { char *name; @@ -100,6 +101,36 @@ ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *foreign_menu) } +void +ovirt_foreign_menu_set_current_iso_name(OvirtForeignMenu *foreign_menu, char *name) +{ +g_return_if_fail(foreign_menu->priv->cdrom != NULL); +g_return_if_fail(foreign_menu->priv->next_iso_name == NULL); + +if (name) { +g_debug("Updating VM cdrom image to '%s'", name); +foreign_menu->priv->next_iso_name = g_strdup(name); +} else { +g_debug("Removing current cdrom image"); +foreign_menu->priv->next_iso_name = NULL; +} + +g_object_set(foreign_menu->priv->cdrom, + "file", name, + NULL); +ovirt_cdrom_update_async(foreign_menu->priv->cdrom, TRUE, + foreign_menu->priv->proxy, NULL, + updated_cdrom_cb, foreign_menu); +} + + +GList* +ovirt_foreign_menu_get_iso_names(OvirtForeignMenu *foreign_menu) +{ +return foreign_menu->priv->iso_names; +} + + static void ovirt_foreign_menu_get_property(GObject *object, guint property_id, GValue *value, GParamSpec *pspec) @@ -383,7 +414,7 @@ static void ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, gpointer user_data) { OvirtForeignMenu *foreign_menu; -const char *iso_name; +const char *iso_name = NULL; gboolean checked; checked = gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(menuitem)); @@ -403,19 +434,9 @@ ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, gpointer user_data) if (checked) { iso_name = gtk_menu_item_get_label(menuitem); -g_debug("Updating VM cdrom image to '%s'", iso_name); -foreign_menu->priv->next_iso_name = g_strdup(iso_name); -} else { -g_debug("Removing current cdrom image"); -iso_name = NULL; -foreign_menu->priv->next_iso_name = NULL; } -g_object_set(foreign_menu->priv->cdrom, - "file", iso_name, - NULL); -ovirt_cdrom_update_async(foreign_menu->priv->cdrom, TRUE, - foreign_menu->priv->proxy, NULL, - updated_cdrom_cb, foreign_menu); + +ovirt_foreign_menu_set_current_iso_name(foreign_menu, iso_name); } diff --git a/src/ovirt-foreign-menu.h b/src/ovirt-foreign-menu.h index cf18b52..f1a1ddb 100644 --- a/src/ovirt-foreign-menu.h +++ b/src/ovirt-foreign-menu.h @@ -70,7 +70,10 @@ OvirtForeignMenu* ovirt_foreign_menu_new(OvirtProxy *proxy); OvirtForeignMenu *ovirt_foreign_menu_new_from_file(VirtViewerFile *self); void ovirt_foreign_menu_start(OvirtForeignMenu *menu); -GtkWidget *ovirt_foreign_menu_get_gtk_menu(OvirtForeignMenu *foreign_menu); +char *ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *menu); +void ovirt_foreign_menu_set_current_iso_name(OvirtForeignMenu *menu, char *name); + +GList *ovirt_foreign_menu_get_iso_names(OvirtForeignMenu *menu); G_END_DECLS -- 2.7.4 ___ 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 v3 07/10] Run iso-dialog when 'Change CD' menu is activated
Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/resources/ui/virt-viewer.ui | 1 + src/virt-viewer-window.c| 24 2 files changed, 25 insertions(+) diff --git a/src/resources/ui/virt-viewer.ui b/src/resources/ui/virt-viewer.ui index af3ae46..e9609ec 100644 --- a/src/resources/ui/virt-viewer.ui +++ b/src/resources/ui/virt-viewer.ui @@ -78,6 +78,7 @@ False _Change CD True + diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c index 99fd102..d172af6 100644 --- a/src/virt-viewer-window.c +++ b/src/virt-viewer-window.c @@ -43,6 +43,8 @@ #include "virt-viewer-util.h" #include "virt-viewer-timed-revealer.h" +#include "remote-viewer-iso-list-dialog.h" + #define ZOOM_STEP 10 /* Signal handlers for main window (move in a VirtViewerMainWindow?) */ @@ -62,6 +64,7 @@ void virt_viewer_window_menu_file_smartcard_insert(GtkWidget *menu, VirtViewerWi void virt_viewer_window_menu_file_smartcard_remove(GtkWidget *menu, VirtViewerWindow *self); void virt_viewer_window_menu_view_release_cursor(GtkWidget *menu, VirtViewerWindow *self); void virt_viewer_window_menu_preferences_cb(GtkWidget *menu, VirtViewerWindow *self); +void virt_viewer_window_menu_change_cd_activate(GtkWidget *menu, VirtViewerWindow *self); /* Internal methods */ @@ -1056,6 +1059,27 @@ virt_viewer_window_menu_help_about(GtkWidget *menu G_GNUC_UNUSED, g_object_unref(G_OBJECT(about)); } +static void +iso_dialog_response(GtkDialog *dialog, +gint response_id, +gpointer user_data G_GNUC_UNUSED) +{ +if (response_id == GTK_RESPONSE_NONE) +return; + +gtk_widget_destroy(GTK_WIDGET(dialog)); +} + +void +virt_viewer_window_menu_change_cd_activate(GtkWidget *menu G_GNUC_UNUSED, + VirtViewerWindow *self) +{ +VirtViewerWindowPrivate *priv = self->priv; +GtkWidget *dialog = remote_viewer_iso_list_dialog_new(GTK_WINDOW(priv->window)); +g_signal_connect(dialog, "response", G_CALLBACK(iso_dialog_response), NULL); +gtk_widget_show_all(dialog); +gtk_dialog_run(GTK_DIALOG(dialog)); +} static void virt_viewer_window_toolbar_setup(VirtViewerWindow *self) -- 2.7.4 ___ 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] display: Do not override size-allocate handler
On 08/10/2016 07:41 AM, Pavel Grunt wrote: > On Tue, 2016-08-09 at 17:44 +0200, Christophe Fergeau wrote: >> On Tue, Aug 09, 2016 at 05:17:31PM +0200, Pavel Grunt wrote: >>> >>> Just connect to the signal >> >> I'm tempted to ask "why?". I think it's recommended (more efficient) to >> just override the vfunc directly when you can (ie when you derive a new >> widget) rather than connecting to a signal. > > no strong reason, just a personal preference coming from the fact that > virt-viewer-display is not "adjusting" its allocation, rather it is only using > it for setting the allocation for its child. > Well, in this case I think it would make sense. Did you get any collateral effects with this patch? -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ 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] display: Do not override size-allocate handler
On 08/09/2016 12:44 PM, Christophe Fergeau wrote: > On Tue, Aug 09, 2016 at 05:17:31PM +0200, Pavel Grunt wrote: >> Just connect to the signal > > I'm tempted to ask "why?". I think it's recommended (more efficient) to > just override the vfunc directly when you can (ie when you derive a new > widget) rather than connecting to a signal. > This is a gray area to me, I don't remember exactly which cases, but sometimes overriding the virtual function will produce weird behaviors, even though the parent class function is called. Meanwhile connecting to the signal will work fine. -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ 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 v2 01/11] ovirt-foreign-menu: Rework states logic
Use switch/case instead of lots of conditional blocks Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/ovirt-foreign-menu.c | 41 +++-- 1 file changed, 15 insertions(+), 26 deletions(-) diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c index 33ff4f1..80e40a1 100644 --- a/src/ovirt-foreign-menu.c +++ b/src/ovirt-foreign-menu.c @@ -312,51 +312,40 @@ ovirt_foreign_menu_next_async_step(OvirtForeignMenu *menu, g_return_if_fail(current_state >= STATE_0); g_return_if_fail(current_state < STATE_ISOS); -current_state++; - -if (current_state == STATE_API) { +switch (++current_state) { +case STATE_API: if (menu->priv->api == NULL) { ovirt_foreign_menu_fetch_api_async(menu); -} else { -current_state++; +break; } -} - -if (current_state == STATE_VM) { +case STATE_VM: if (menu->priv->vm == NULL) { ovirt_foreign_menu_fetch_vm_async(menu); -} else { -current_state++; +break; } -} - -if (current_state == STATE_STORAGE_DOMAIN) { +case STATE_STORAGE_DOMAIN: if (menu->priv->files == NULL) { ovirt_foreign_menu_fetch_storage_domain_async(menu); -} else { -current_state++; +break; } -} - -if (current_state == STATE_VM_CDROM) { +case STATE_VM_CDROM: if (menu->priv->cdrom == NULL) { ovirt_foreign_menu_fetch_vm_cdrom_async(menu); -} else { -current_state++; +break; } -} - -if (current_state == STATE_CDROM_FILE) { +case STATE_CDROM_FILE: ovirt_foreign_menu_refresh_cdrom_file_async(menu); -} - -if (current_state == STATE_ISOS) { +break; +case STATE_ISOS: g_warn_if_fail(menu->priv->api != NULL); g_warn_if_fail(menu->priv->vm != NULL); g_warn_if_fail(menu->priv->files != NULL); g_warn_if_fail(menu->priv->cdrom != NULL); ovirt_foreign_menu_refresh_iso_list(menu); +break; +default: +g_warn_if_reached(); } } -- 2.7.4 ___ 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 03/11] ovirt-foreign-menu: Remove timer used to refresh iso list
On 07/18/2016 09:14 AM, Pavel Grunt wrote: > On Sun, 2016-07-17 at 23:13 -0300, Eduardo Lima (Etrunko) wrote: >> With the new ISO dialog, the user triggers the refresh manually. > > This should come after the dialog is introduced> Well, I have no plans in merging any of these patches independently, they are only split so that it makes more sense to understand the whole series. -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ 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 01/11] ovirt-foreign-menu: Rework states logic
On 07/18/2016 06:15 AM, Pavel Grunt wrote: > Hi Eduardo, > > On Sun, 2016-07-17 at 23:13 -0300, Eduardo Lima (Etrunko) wrote: >> Use switch/case instead of lots of conditional blocks > Yes, it is more readable >> >> Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> >> --- >> src/ovirt-foreign-menu.c | 76 +++-- >> --- >> 1 file changed, 36 insertions(+), 40 deletions(-) >> >> diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c >> index 33ff4f1..b0b8fec 100644 >> --- a/src/ovirt-foreign-menu.c >> +++ b/src/ovirt-foreign-menu.c >> @@ -312,51 +312,47 @@ ovirt_foreign_menu_next_async_step(OvirtForeignMenu >> *menu, >> g_return_if_fail(current_state >= STATE_0); >> g_return_if_fail(current_state < STATE_ISOS); >> >> -current_state++; > my preference is to keep the increment outside the switch statement > >> - >> -if (current_state == STATE_API) { >> -if (menu->priv->api == NULL) { >> -ovirt_foreign_menu_fetch_api_async(menu); >> -} else { >> -current_state++; >> +switch (++current_state) { > Actually the increment is not needed at all thanks to your changes, imo > switch(current_state + 1) would be more readable Alright, I don't mind at all. >> +case STATE_API: { > 'case' should have the same indentation as its 'switch' > > Remove extra {}, no need to have the null check in the extra block (applies to > all cases) > I don't think so, the if checks are necessary for the initialization process, when we have everything initalized, it will fall straight to the STATE_ISOS case. Or maybe you are talking about something else and I misunderstood? -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ 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 01/11] ovirt-foreign-menu: Rework states logic
On 07/18/2016 11:58 AM, Pavel Grunt wrote: > On Mon, 2016-07-18 at 10:22 -0300, Eduardo Lima (Etrunko) wrote: >> On 07/18/2016 06:15 AM, Pavel Grunt wrote: >>> >>> Hi Eduardo, >>> >>> On Sun, 2016-07-17 at 23:13 -0300, Eduardo Lima (Etrunko) wrote: >>>> >>>> Use switch/case instead of lots of conditional blocks >>> Yes, it is more readable >>>> >>>> >>>> Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> >>>> --- >>>> src/ovirt-foreign-menu.c | 76 +++-- >>>> >>>> --- >>>> 1 file changed, 36 insertions(+), 40 deletions(-) >>>> >>>> diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c >>>> index 33ff4f1..b0b8fec 100644 >>>> --- a/src/ovirt-foreign-menu.c >>>> +++ b/src/ovirt-foreign-menu.c >>>> @@ -312,51 +312,47 @@ ovirt_foreign_menu_next_async_step(OvirtForeignMenu >>>> *menu, >>>> g_return_if_fail(current_state >= STATE_0); >>>> g_return_if_fail(current_state < STATE_ISOS); >>>> >>>> -current_state++; >>> my preference is to keep the increment outside the switch statement >>> >>>> >>>> - >>>> -if (current_state == STATE_API) { >>>> -if (menu->priv->api == NULL) { >>>> -ovirt_foreign_menu_fetch_api_async(menu); >>>> -} else { >>>> -current_state++; >>>> +switch (++current_state) { >>> Actually the increment is not needed at all thanks to your changes, imo >>> switch(current_state + 1) would be more readable >> >> Alright, I don't mind at all. >> >>> >>>> >>>> +case STATE_API: { >>> 'case' should have the same indentation as its 'switch' >>> >>> Remove extra {}, no need to have the null check in the extra block (applies >>> to >>> all cases) >>> >> >> I don't think so, the if checks are necessary for the initialization >> process, when we have everything initalized, it will fall straight to >> the STATE_ISOS case. Or maybe you are talking about something else and I >> misunderstood? >> > > the {} are not needed, but it is a minor: The break statements are put inside of the conditional block on purpose. If the NULL check passes, the initialization is done asynchronously and that function will be responsible for calling back with the next step. If it fails, it means that it was already initialized, so we move to the next initialization, one at a time, until everything is set up and it will fall straight to the STATE_ISOS. > ... > case STATE_API: > if (menu->priv->api == NULL) { > ovirt_foreign_menu_fetch_api_async(menu); > break; > } > case STATE_VM: > if (menu->priv->vm == NULL) { > ovirt_foreign_menu_fetch_vm_async(menu); > break; > } > ... > >> -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ 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 00/11] Replace oVirt foreign menu with dedicated dialog
After analyzing the causes of the bug reported in rhbz #1310624, we came to the conclusion that the whole idea of having a menu to display a list remote files seemed wrong. A temporary solution has been provided until the code was reworked, and this is the result. Most of the changes is in ovirt-foreign menu, which does not handle any UI widgets anymore, and some accessors were added so that other objects can interface with it. The dialog itself is pretty simple, and displays the list of files using a GtkTreeView widget. It was designed using glade, which made the development easier than programming the whole UI manually. Eduardo Lima (Etrunko) (11): ovirt-foreign-menu: Rework states logic ovirt-foreign-menu: Use g_clear_pointer/g_clear_object ovirt-foreign-menu: Remove timer used to refresh iso list ovirt-foreign-menu: Add accessors for current iso and iso list ovirt-foreign-menu: Remove GtkMenu related functions ovirt-foreign-menu: Notify of new files even if list did not change UI: Make 'Change CD' menu item a submenu under 'File' toplevel menu Introduce ISO List dialog Run iso-dialog when 'Change CD' menu is activated remote-viewer: Make ovirt-foreign-menu a property iso-dialog: Implement functionality provided by oVirt foreign menu src/Makefile.am| 3 + src/ovirt-foreign-menu.c | 290 + src/ovirt-foreign-menu.h | 5 +- src/remote-viewer-iso-list-dialog.c| 274 +++ src/remote-viewer-iso-list-dialog.h| 59 ++ src/remote-viewer.c| 88 - src/resources/ui/remote-viewer-iso-list.ui | 176 + src/resources/ui/virt-viewer.ui| 58 +++--- src/resources/virt-viewer.gresource.xml| 1 + src/virt-viewer-window.c | 43 + 10 files changed, 715 insertions(+), 282 deletions(-) create mode 100644 src/remote-viewer-iso-list-dialog.c create mode 100644 src/remote-viewer-iso-list-dialog.h create mode 100644 src/resources/ui/remote-viewer-iso-list.ui -- 2.7.4 ___ 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 08/11] Introduce ISO List dialog
Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/Makefile.am| 3 + src/remote-viewer-iso-list-dialog.c| 115 +++ src/remote-viewer-iso-list-dialog.h| 58 ++ src/resources/ui/remote-viewer-iso-list.ui | 174 + src/resources/virt-viewer.gresource.xml| 1 + 5 files changed, 351 insertions(+) create mode 100644 src/remote-viewer-iso-list-dialog.c create mode 100644 src/remote-viewer-iso-list-dialog.h create mode 100644 src/resources/ui/remote-viewer-iso-list.ui diff --git a/src/Makefile.am b/src/Makefile.am index 0c48e40..66a73f8 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -13,6 +13,7 @@ noinst_DATA = \ resources/ui/virt-viewer-vm-connection.ui \ resources/ui/virt-viewer-preferences.ui \ resources/ui/remote-viewer-connect.ui \ + resources/ui/remote-viewer-iso-list.ui \ $(NULL) EXTRA_DIST = \ @@ -96,6 +97,8 @@ if HAVE_OVIRT libvirt_viewer_la_SOURCES += \ ovirt-foreign-menu.h \ ovirt-foreign-menu.c \ + remote-viewer-iso-list-dialog.c \ + remote-viewer-iso-list-dialog.h \ $(NULL) endif diff --git a/src/remote-viewer-iso-list-dialog.c b/src/remote-viewer-iso-list-dialog.c new file mode 100644 index 000..b3972ac --- /dev/null +++ b/src/remote-viewer-iso-list-dialog.c @@ -0,0 +1,115 @@ +/* + * Virt Viewer: A virtual machine console viewer + * + * Copyright (C) 2016 Red Hat, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#include + +#include + +#include "remote-viewer-iso-list-dialog.h" +#include "virt-viewer-util.h" + +G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, GTK_TYPE_DIALOG) + +#define DIALOG_PRIVATE(o) \ +(G_TYPE_INSTANCE_GET_PRIVATE((o), REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG, RemoteViewerISOListDialogPrivate)) + +struct _RemoteViewerISOListDialogPrivate +{ +GtkBuilder *builder; +GtkWidget *notebook; +}; + +enum RemoteViewerIsoListDialogPages +{ +STATUS_PAGE = 0, +ISO_LIST_PAGE, +}; + +static void +remote_viewer_iso_list_dialog_dispose(GObject *object) +{ +RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object); +RemoteViewerISOListDialogPrivate *priv = self->priv; + +g_clear_object(>builder); + + G_OBJECT_CLASS(remote_viewer_iso_list_dialog_parent_class)->dispose(object); +} + +static void +remote_viewer_iso_list_dialog_class_init(RemoteViewerISOListDialogClass *klass) +{ +GObjectClass *object_class = G_OBJECT_CLASS(klass); + +g_type_class_add_private(klass, sizeof(RemoteViewerISOListDialogPrivate)); + +object_class->dispose = remote_viewer_iso_list_dialog_dispose; +} + +static void +remote_viewer_iso_list_dialog_response(GtkDialog *dialog, + gint response_id, + gpointer user_data G_GNUC_UNUSED) +{ +RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog); +RemoteViewerISOListDialogPrivate *priv = self->priv; + +if (response_id != GTK_RESPONSE_NONE) +return; + +gtk_notebook_set_current_page(GTK_NOTEBOOK(priv->notebook), STATUS_PAGE); +gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, FALSE); +gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_CLOSE, FALSE); +} + +static void +remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self) +{ +GtkWidget *content = gtk_dialog_get_content_area(GTK_DIALOG(self)); +RemoteViewerISOListDialogPrivate *priv = self->priv = DIALOG_PRIVATE(self); + +gtk_widget_set_size_request(GTK_WIDGET(self), 400, 300); +gtk_box_set_spacing(GTK_BOX(content), 6); + +priv->builder = virt_viewer_util_load_ui("remote-viewer-iso-list.ui"); +gtk_builder_connect_signals(priv->builder, self); + +priv->notebook = GTK_WIDGET(gtk_builder_get_object(priv->builder, "notebook")); +gtk_box_pack_start(GTK_BOX(content), priv->notebook, TRUE, TRUE, 0); + +gtk_dialog_add_buttons(GTK_DIALOG(self), + _("Refresh"), GTK_RESPONSE_NONE,
[virt-tools-list] [PATCH virt-viewer 01/11] ovirt-foreign-menu: Rework states logic
Use switch/case instead of lots of conditional blocks Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/ovirt-foreign-menu.c | 76 +++- 1 file changed, 36 insertions(+), 40 deletions(-) diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c index 33ff4f1..b0b8fec 100644 --- a/src/ovirt-foreign-menu.c +++ b/src/ovirt-foreign-menu.c @@ -312,51 +312,47 @@ ovirt_foreign_menu_next_async_step(OvirtForeignMenu *menu, g_return_if_fail(current_state >= STATE_0); g_return_if_fail(current_state < STATE_ISOS); -current_state++; - -if (current_state == STATE_API) { -if (menu->priv->api == NULL) { -ovirt_foreign_menu_fetch_api_async(menu); -} else { -current_state++; +switch (++current_state) { +case STATE_API: { +if (menu->priv->api == NULL) { +ovirt_foreign_menu_fetch_api_async(menu); +break; +} } -} - -if (current_state == STATE_VM) { -if (menu->priv->vm == NULL) { -ovirt_foreign_menu_fetch_vm_async(menu); -} else { -current_state++; +case STATE_VM: { +if (menu->priv->vm == NULL) { +ovirt_foreign_menu_fetch_vm_async(menu); +break; +} } -} - -if (current_state == STATE_STORAGE_DOMAIN) { -if (menu->priv->files == NULL) { -ovirt_foreign_menu_fetch_storage_domain_async(menu); -} else { -current_state++; +case STATE_STORAGE_DOMAIN: { +if (menu->priv->files == NULL) { +ovirt_foreign_menu_fetch_storage_domain_async(menu); +break; +} } -} - -if (current_state == STATE_VM_CDROM) { -if (menu->priv->cdrom == NULL) { -ovirt_foreign_menu_fetch_vm_cdrom_async(menu); -} else { -current_state++; +case STATE_VM_CDROM: { +if (menu->priv->cdrom == NULL) { +ovirt_foreign_menu_fetch_vm_cdrom_async(menu); +break; +} } -} - -if (current_state == STATE_CDROM_FILE) { -ovirt_foreign_menu_refresh_cdrom_file_async(menu); -} - -if (current_state == STATE_ISOS) { -g_warn_if_fail(menu->priv->api != NULL); -g_warn_if_fail(menu->priv->vm != NULL); -g_warn_if_fail(menu->priv->files != NULL); -g_warn_if_fail(menu->priv->cdrom != NULL); +case STATE_CDROM_FILE: { +ovirt_foreign_menu_refresh_cdrom_file_async(menu); +break; +} +case STATE_ISOS: { +g_warn_if_fail(menu->priv->api != NULL); +g_warn_if_fail(menu->priv->vm != NULL); +g_warn_if_fail(menu->priv->files != NULL); +g_warn_if_fail(menu->priv->cdrom != NULL); -ovirt_foreign_menu_refresh_iso_list(menu); +ovirt_foreign_menu_refresh_iso_list(menu); +break; +} +default: { +g_warn_if_reached(); +} } } -- 2.7.4 ___ 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 06/11] ovirt-foreign-menu: Notify of new files even if list did not change
Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/ovirt-foreign-menu.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c index 24b5af2..75154bb 100644 --- a/src/ovirt-foreign-menu.c +++ b/src/ovirt-foreign-menu.c @@ -428,12 +428,15 @@ static void ovirt_foreign_menu_set_files(OvirtForeignMenu *menu, if ((it == NULL) && (it2 == NULL)) { /* sorted_files and menu->priv->files content was the same */ +g_debug("Iso list unchanged"); g_list_free_full(sorted_files, (GDestroyNotify)g_free); -return; +goto end; } g_list_free_full(menu->priv->iso_names, (GDestroyNotify)g_free); menu->priv->iso_names = sorted_files; + +end: g_object_notify(G_OBJECT(menu), "files"); } -- 2.7.4 ___ 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 04/11] ovirt-foreign-menu: Add accessors for current iso and iso list
Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/ovirt-foreign-menu.c | 49 ++-- src/ovirt-foreign-menu.h | 5 - 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c index b071e27..2446239 100644 --- a/src/ovirt-foreign-menu.c +++ b/src/ovirt-foreign-menu.c @@ -47,6 +47,7 @@ static void ovirt_foreign_menu_fetch_storage_domain_async(OvirtForeignMenu *menu static void ovirt_foreign_menu_fetch_vm_cdrom_async(OvirtForeignMenu *menu); static void ovirt_foreign_menu_refresh_cdrom_file_async(OvirtForeignMenu *menu); static void ovirt_foreign_menu_fetch_iso_list_async(OvirtForeignMenu *menu); +static void updated_cdrom_cb(GObject *source_object, GAsyncResult *result, gpointer user_data); G_DEFINE_TYPE (OvirtForeignMenu, ovirt_foreign_menu, G_TYPE_OBJECT) @@ -85,7 +86,7 @@ enum { }; -static char * +char * ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *foreign_menu) { char *name; @@ -100,6 +101,36 @@ ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *foreign_menu) } +void +ovirt_foreign_menu_set_current_iso_name(OvirtForeignMenu *foreign_menu, char *name) +{ +g_return_if_fail(foreign_menu->priv->cdrom != NULL); +g_return_if_fail(foreign_menu->priv->next_iso_name == NULL); + +if (name) { +g_debug("Updating VM cdrom image to '%s'", name); +foreign_menu->priv->next_iso_name = g_strdup(name); +} else { +g_debug("Removing current cdrom image"); +foreign_menu->priv->next_iso_name = NULL; +} + +g_object_set(foreign_menu->priv->cdrom, + "file", name, + NULL); +ovirt_cdrom_update_async(foreign_menu->priv->cdrom, TRUE, + foreign_menu->priv->proxy, NULL, + updated_cdrom_cb, foreign_menu); +} + + +GList* +ovirt_foreign_menu_get_iso_names(OvirtForeignMenu *foreign_menu) +{ +return foreign_menu->priv->iso_names; +} + + static void ovirt_foreign_menu_get_property(GObject *object, guint property_id, GValue *value, GParamSpec *pspec) @@ -385,7 +416,7 @@ static void ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, gpointer user_data) { OvirtForeignMenu *foreign_menu; -const char *iso_name; +const char *iso_name = NULL; gboolean checked; checked = gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(menuitem)); @@ -405,19 +436,9 @@ ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, gpointer user_data) if (checked) { iso_name = gtk_menu_item_get_label(menuitem); -g_debug("Updating VM cdrom image to '%s'", iso_name); -foreign_menu->priv->next_iso_name = g_strdup(iso_name); -} else { -g_debug("Removing current cdrom image"); -iso_name = NULL; -foreign_menu->priv->next_iso_name = NULL; } -g_object_set(foreign_menu->priv->cdrom, - "file", iso_name, - NULL); -ovirt_cdrom_update_async(foreign_menu->priv->cdrom, TRUE, - foreign_menu->priv->proxy, NULL, - updated_cdrom_cb, foreign_menu); + +ovirt_foreign_menu_set_current_iso_name(foreign_menu, iso_name); } diff --git a/src/ovirt-foreign-menu.h b/src/ovirt-foreign-menu.h index cf18b52..f1a1ddb 100644 --- a/src/ovirt-foreign-menu.h +++ b/src/ovirt-foreign-menu.h @@ -70,7 +70,10 @@ OvirtForeignMenu* ovirt_foreign_menu_new(OvirtProxy *proxy); OvirtForeignMenu *ovirt_foreign_menu_new_from_file(VirtViewerFile *self); void ovirt_foreign_menu_start(OvirtForeignMenu *menu); -GtkWidget *ovirt_foreign_menu_get_gtk_menu(OvirtForeignMenu *foreign_menu); +char *ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *menu); +void ovirt_foreign_menu_set_current_iso_name(OvirtForeignMenu *menu, char *name); + +GList *ovirt_foreign_menu_get_iso_names(OvirtForeignMenu *menu); G_END_DECLS -- 2.7.4 ___ 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 02/11] ovirt-foreign-menu: Use g_clear_pointer/g_clear_object
Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/ovirt-foreign-menu.c | 68 1 file changed, 16 insertions(+), 52 deletions(-) diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c index b0b8fec..889e7bc 100644 --- a/src/ovirt-foreign-menu.c +++ b/src/ovirt-foreign-menu.c @@ -143,33 +143,23 @@ ovirt_foreign_menu_set_property(GObject *object, guint property_id, switch (property_id) { case PROP_PROXY: -if (priv->proxy != NULL) { -g_object_unref(priv->proxy); -} +g_clear_object(>proxy); priv->proxy = g_value_dup_object(value); break; case PROP_API: -if (priv->api != NULL) { -g_object_unref(priv->api); -} +g_clear_object(>api); priv->api = g_value_dup_object(value); break; case PROP_VM: -if (priv->vm != NULL) { -g_object_unref(priv->vm); -} +g_clear_object(>vm); priv->vm = g_value_dup_object(value); -g_free(priv->vm_guid); -priv->vm_guid = NULL; +g_clear_pointer(>vm_guid, g_free); if (priv->vm != NULL) { g_object_get(G_OBJECT(priv->vm), "guid", >vm_guid, NULL); } break; case PROP_VM_GUID: -if (priv->vm != NULL) { -g_object_unref(priv->vm); -priv->vm = NULL; -} +g_clear_object(>vm); g_free(priv->vm_guid); priv->vm_guid = g_value_dup_string(value); break; @@ -184,44 +174,20 @@ ovirt_foreign_menu_dispose(GObject *obj) { OvirtForeignMenu *self = OVIRT_FOREIGN_MENU(obj); -if (self->priv->proxy) { -g_object_unref(self->priv->proxy); -self->priv->proxy = NULL; -} - -if (self->priv->api != NULL) { -g_object_unref(self->priv->api); -self->priv->api = NULL; -} - -if (self->priv->vm) { -g_object_unref(self->priv->vm); -self->priv->vm = NULL; -} - -g_free(self->priv->vm_guid); -self->priv->vm_guid = NULL; - -if (self->priv->files) { -g_object_unref(self->priv->files); -self->priv->files = NULL; -} - -if (self->priv->cdrom) { -g_object_unref(self->priv->cdrom); -self->priv->cdrom = NULL; -} +g_clear_object(>priv->proxy); +g_clear_object(>priv->api); +g_clear_object(>priv->vm); +g_clear_pointer(>priv->vm_guid, g_free); +g_clear_object(>priv->files); +g_clear_object(>priv->cdrom); if (self->priv->iso_names) { g_list_free_full(self->priv->iso_names, (GDestroyNotify)g_free); self->priv->iso_names = NULL; } -g_free(self->priv->current_iso_name); -self->priv->current_iso_name = NULL; - -g_free(self->priv->next_iso_name); -self->priv->next_iso_name = NULL; +g_clear_pointer(>priv->current_iso_name, g_free); +g_clear_pointer(>priv->next_iso_name, g_free); G_OBJECT_CLASS(ovirt_foreign_menu_parent_class)->dispose(obj); } @@ -410,8 +376,8 @@ static void updated_cdrom_cb(GObject *source_object, current_file?current_file:NULL); g_object_set(foreign_menu->priv->cdrom, "file", current_file, NULL); } -g_free(foreign_menu->priv->next_iso_name); -foreign_menu->priv->next_iso_name = NULL; + +g_clear_pointer(_menu->priv->next_iso_name, g_free); } @@ -546,13 +512,11 @@ static void cdrom_file_refreshed_cb(GObject *source_object, } /* Content of OvirtCdrom is now current */ -g_free(menu->priv->current_iso_name); +g_clear_pointer(>priv->current_iso_name, g_free); if (menu->priv->cdrom != NULL) { g_object_get(G_OBJECT(menu->priv->cdrom), "file", >priv->current_iso_name, NULL); -} else { -menu->priv->current_iso_name = NULL; } g_object_notify(G_OBJECT(menu), "file"); if (menu->priv->cdrom != NULL) { -- 2.7.4 ___ 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 07/11] UI: Make 'Change CD' menu item a submenu under 'File' toplevel menu
Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/resources/ui/virt-viewer.ui | 57 +++-- 1 file changed, 21 insertions(+), 36 deletions(-) diff --git a/src/resources/ui/virt-viewer.ui b/src/resources/ui/virt-viewer.ui index 6e3c5ad..ee23ba5 100644 --- a/src/resources/ui/virt-viewer.ui +++ b/src/resources/ui/virt-viewer.ui @@ -1,6 +1,7 @@ + - + False @@ -23,7 +24,6 @@ True False -False _File True @@ -35,7 +35,6 @@ True False -False _Screenshot True @@ -46,7 +45,6 @@ True False False -False _USB device selection True @@ -55,7 +53,6 @@ False -False virt-viewer/file/smartcard-insert Smartcard insertion True @@ -65,7 +62,6 @@ False -False virt-viewer/file/smartcard-remove Smartcard removal True @@ -73,6 +69,13 @@ + +False +_Change CD +True + + + True False @@ -89,13 +92,12 @@ -_Quit True False -False +_Quit True - + @@ -106,7 +108,6 @@ True False -False _View True @@ -118,7 +119,6 @@ True False -False virt-viewer/view/toggle-fullscreen _Full screen True @@ -129,7 +129,6 @@ True False -False _Zoom True @@ -139,22 +138,22 @@ accelgroup -virt-viewer/view/zoom-in -Zoom _In +False True False -False +virt-viewer/view/zoom-in +Zoom _In True -virt-viewer/view/zoom-out -Zoom _Out +False True False -False +virt-viewer/view/zoom-out +Zoom _Out True @@ -167,11 +166,11 @@ -virt-viewer/view/zoom-reset -_Normal Size +False True
[virt-tools-list] [PATCH virt-viewer 10/11] remote-viewer: Make ovirt-foreign-menu a property
The OvirtForeignMenu pointer is needed by the new ISO list dialog, and we make it acessible via property to avoid interdependency between objects. Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/remote-viewer-iso-list-dialog.c | 20 +++- src/remote-viewer-iso-list-dialog.h | 3 ++- src/remote-viewer.c | 36 src/virt-viewer-window.c| 16 +++- 4 files changed, 68 insertions(+), 7 deletions(-) diff --git a/src/remote-viewer-iso-list-dialog.c b/src/remote-viewer-iso-list-dialog.c index b3972ac..996c1f2 100644 --- a/src/remote-viewer-iso-list-dialog.c +++ b/src/remote-viewer-iso-list-dialog.c @@ -34,6 +34,7 @@ struct _RemoteViewerISOListDialogPrivate { GtkBuilder *builder; GtkWidget *notebook; +OvirtForeignMenu *foreign_menu; }; enum RemoteViewerIsoListDialogPages @@ -106,10 +107,19 @@ remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self) } GtkWidget * -remote_viewer_iso_list_dialog_new(GtkWindow *parent) +remote_viewer_iso_list_dialog_new(GtkWindow *parent, OvirtForeignMenu *foreign_menu) { -return g_object_new(REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG, -"title", _("Change CD"), -"transient-for", parent, -NULL); +GtkWidget *dialog; +RemoteViewerISOListDialog *self; + +g_return_val_if_fail(foreign_menu != NULL, NULL); + +dialog = g_object_new(REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG, + "title", _("Change CD"), + "transient-for", parent, + NULL); + +self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog); +self->priv->foreign_menu = foreign_menu; +return dialog; } diff --git a/src/remote-viewer-iso-list-dialog.h b/src/remote-viewer-iso-list-dialog.h index def841b..ee7c70f 100644 --- a/src/remote-viewer-iso-list-dialog.h +++ b/src/remote-viewer-iso-list-dialog.h @@ -22,6 +22,7 @@ #define __REMOTE_VIEWER_ISO_LIST_DIALOG_H__ #include +#include "ovirt-foreign-menu.h" G_BEGIN_DECLS @@ -51,7 +52,7 @@ struct _RemoteViewerISOListDialogClass GType remote_viewer_iso_list_dialog_get_type(void) G_GNUC_CONST; -GtkWidget *remote_viewer_iso_list_dialog_new(GtkWindow *parent); +GtkWidget *remote_viewer_iso_list_dialog_new(GtkWindow *parent, OvirtForeignMenu *foreign_menu); G_END_DECLS diff --git a/src/remote-viewer.c b/src/remote-viewer.c index 95130dc..e4e41c4 100644 --- a/src/remote-viewer.c +++ b/src/remote-viewer.c @@ -67,6 +67,13 @@ G_DEFINE_TYPE (RemoteViewer, remote_viewer, VIRT_VIEWER_TYPE_APP) #define GET_PRIVATE(o)\ (G_TYPE_INSTANCE_GET_PRIVATE ((o), REMOTE_VIEWER_TYPE, RemoteViewerPrivate)) +enum RemoteViewerProperties { +PROP_0, +#ifdef HAVE_OVIRT +PROP_OVIRT_FOREIGN_MENU, +#endif +}; + #ifdef HAVE_OVIRT static OvirtVm * choose_vm(GtkWindow *main_window, char **vm_name, @@ -213,6 +220,25 @@ end: } static void +remote_viewer_get_property(GObject *object, guint property_id, + GValue *value, GParamSpec *pspec) +{ +RemoteViewer *self = REMOTE_VIEWER(object); +RemoteViewerPrivate *priv = self->priv; + +switch (property_id) { +#ifdef HAVE_OVIRT +case PROP_OVIRT_FOREIGN_MENU: +g_value_set_pointer(value, priv->ovirt_foreign_menu); +break; +#endif + +default: +G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec); +} +} + +static void remote_viewer_class_init (RemoteViewerClass *klass) { GObjectClass *object_class = G_OBJECT_CLASS (klass); @@ -222,6 +248,7 @@ remote_viewer_class_init (RemoteViewerClass *klass) g_type_class_add_private (klass, sizeof (RemoteViewerPrivate)); +object_class->get_property = remote_viewer_get_property; object_class->dispose = remote_viewer_dispose; g_app_class->local_command_line = remote_viewer_local_command_line; @@ -235,6 +262,15 @@ remote_viewer_class_init (RemoteViewerClass *klass) #else (void) gtk_app_class; #endif + +#ifdef HAVE_OVIRT +g_object_class_install_property(object_class, +PROP_OVIRT_FOREIGN_MENU, +g_param_spec_pointer("ovirt-foreign-menu", + "oVirt Foreign Menu", + "Object which is used as interface to oVirt", + G_PARAM_READABLE | G_PARAM_STATIC_STRINGS)); +#endif } static void diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c index 867fb86..76fe80f 100644 --- a/src/virt-viewer-window.c +
[virt-tools-list] [PATCH virt-viewer 05/11] ovirt-foreign-menu: Remove GtkMenu related functions
With this commit, we finish cleaning up ovirt foreign menu code, which only deals with data, leaving the UI bits to be handled properly in the new ISO list dialog. This patch also updates remote-viewer to reflect the change. Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/ovirt-foreign-menu.c | 79 src/remote-viewer.c | 52 +-- 2 files changed, 8 insertions(+), 123 deletions(-) diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c index 2446239..24b5af2 100644 --- a/src/ovirt-foreign-menu.c +++ b/src/ovirt-foreign-menu.c @@ -361,22 +361,6 @@ ovirt_foreign_menu_start(OvirtForeignMenu *menu) } -static void -ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, gpointer user_data); - - -static void -menu_item_set_active_no_signal(GtkMenuItem *menuitem, - gboolean active, - GCallback callback, - gpointer user_data) -{ -g_signal_handlers_block_by_func(menuitem, callback, user_data); -gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(menuitem), active); -g_signal_handlers_unblock_by_func(menuitem, callback, user_data); -} - - static void updated_cdrom_cb(GObject *source_object, GAsyncResult *result, gpointer user_data) @@ -412,69 +396,6 @@ static void updated_cdrom_cb(GObject *source_object, } -static void -ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, gpointer user_data) -{ -OvirtForeignMenu *foreign_menu; -const char *iso_name = NULL; -gboolean checked; - -checked = gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(menuitem)); -foreign_menu = OVIRT_FOREIGN_MENU(user_data); -g_return_if_fail(foreign_menu->priv->cdrom != NULL); -g_return_if_fail(foreign_menu->priv->next_iso_name == NULL); - -g_debug("'%s' clicked", gtk_menu_item_get_label(menuitem)); - -/* We only want to move the check mark for the currently selected ISO - * when ovirt_cdrom_update_async() is successful, so for now we move - * the check mark back to where it was before - */ -menu_item_set_active_no_signal(menuitem, !checked, - (GCallback)ovirt_foreign_menu_activate_item_cb, - foreign_menu); - -if (checked) { -iso_name = gtk_menu_item_get_label(menuitem); -} - -ovirt_foreign_menu_set_current_iso_name(foreign_menu, iso_name); -} - - -GtkWidget *ovirt_foreign_menu_get_gtk_menu(OvirtForeignMenu *foreign_menu) -{ -GtkWidget *gtk_menu; -GList *it; -char *current_iso; - -if (foreign_menu->priv->iso_names == NULL) { -g_debug("ISO list is empty, no menu to show"); -return NULL; -} -g_debug("Creating GtkMenu for foreign menu"); -current_iso = ovirt_foreign_menu_get_current_iso_name(foreign_menu); -gtk_menu = gtk_menu_new(); -for (it = foreign_menu->priv->iso_names; it != NULL; it = it->next) { -GtkWidget *menuitem; - -menuitem = gtk_check_menu_item_new_with_label((char *)it->data); -if (g_strcmp0((char *)it->data, current_iso) == 0) { -g_warn_if_fail(g_strcmp0(current_iso, foreign_menu->priv->current_iso_name) == 0); -gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(menuitem), - TRUE); -} -g_signal_connect(menuitem, "activate", - G_CALLBACK(ovirt_foreign_menu_activate_item_cb), - foreign_menu); -gtk_menu_shell_append(GTK_MENU_SHELL(gtk_menu), menuitem); -} -g_free(current_iso); - -return gtk_menu; -} - - static void ovirt_foreign_menu_set_files(OvirtForeignMenu *menu, const GList *files) { diff --git a/src/remote-viewer.c b/src/remote-viewer.c index 6d29bf2..95130dc 100644 --- a/src/remote-viewer.c +++ b/src/remote-viewer.c @@ -734,33 +734,11 @@ authenticate_cb(RestProxy *proxy, G_GNUC_UNUSED RestProxyAuth *auth, static void ovirt_foreign_menu_update(GtkApplication *gtkapp, GtkWindow *gtkwin, G_GNUC_UNUSED gpointer data) { -RemoteViewer *app = REMOTE_VIEWER(gtkapp); +RemoteViewer *self = REMOTE_VIEWER(gtkapp); VirtViewerWindow *win = g_object_get_data(G_OBJECT(gtkwin), "virt-viewer-window"); -GtkWidget *menu = g_object_get_data(G_OBJECT(win), "foreign-menu"); -GtkWidget *submenu; - -if (app->priv->ovirt_foreign_menu == NULL) { -/* nothing to do */ -return; -} - -submenu = ovirt_foreign_menu_get_gtk_menu(app->priv->ovirt_foreign_menu); -if (submenu == NULL) { -/* No items to show, no point in showing the menu */ -if (menu != NULL) -
[virt-tools-list] [PATCH virt-viewer 11/11] iso-dialog: Implement functionality provided by oVirt foreign menu
Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/remote-viewer-iso-list-dialog.c| 149 + src/resources/ui/remote-viewer-iso-list.ui | 4 +- 2 files changed, 152 insertions(+), 1 deletion(-) diff --git a/src/remote-viewer-iso-list-dialog.c b/src/remote-viewer-iso-list-dialog.c index 996c1f2..3b2e187 100644 --- a/src/remote-viewer-iso-list-dialog.c +++ b/src/remote-viewer-iso-list-dialog.c @@ -20,6 +20,7 @@ #include +#include #include #include "remote-viewer-iso-list-dialog.h" @@ -34,7 +35,9 @@ struct _RemoteViewerISOListDialogPrivate { GtkBuilder *builder; GtkWidget *notebook; +GtkWidget *treeview; OvirtForeignMenu *foreign_menu; +GtkListStore *list_store; }; enum RemoteViewerIsoListDialogPages @@ -43,6 +46,15 @@ enum RemoteViewerIsoListDialogPages ISO_LIST_PAGE, }; +enum RemoteViewerIsoListDialogModel +{ +ISO_IS_ACTIVE = 0, +ISO_NAME, +FONT_WEIGHT, +}; + +void remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle *cell_renderer, gchar *path, gpointer user_data); + static void remote_viewer_iso_list_dialog_dispose(GObject *object) { @@ -65,6 +77,52 @@ remote_viewer_iso_list_dialog_class_init(RemoteViewerISOListDialogClass *klass) } static void +remote_viewer_iso_list_dialog_show_files(RemoteViewerISOListDialog *self) +{ +RemoteViewerISOListDialogPrivate *priv = self->priv = DIALOG_PRIVATE(self); +gtk_notebook_set_current_page(GTK_NOTEBOOK(priv->notebook), ISO_LIST_PAGE); +gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, TRUE); +gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_CLOSE, TRUE); +} + +static void +remote_viewer_iso_list_dialog_foreach(char *name, RemoteViewerISOListDialog *self) +{ +RemoteViewerISOListDialogPrivate *priv = self->priv; +char * current_iso = ovirt_foreign_menu_get_current_iso_name(self->priv->foreign_menu); +gboolean active = g_strcmp0(current_iso, name) == 0; +gint weight = active ? PANGO_WEIGHT_BOLD : PANGO_WEIGHT_NORMAL; +GtkTreeIter iter; + +gtk_list_store_append(priv->list_store, ); +gtk_list_store_set(priv->list_store, , + ISO_IS_ACTIVE, active, + ISO_NAME, name, + FONT_WEIGHT, weight, -1); + +free(current_iso); +} + +static void +remote_viewer_iso_list_dialog_refresh_iso_list(RemoteViewerISOListDialog *self, + gboolean refreshing) +{ +RemoteViewerISOListDialogPrivate *priv = self->priv; +GList *iso_list; + +if (refreshing) { +ovirt_foreign_menu_start(priv->foreign_menu); +return; +} + +iso_list = ovirt_foreign_menu_get_iso_names(priv->foreign_menu); + +gtk_list_store_clear(priv->list_store); +g_list_foreach(iso_list, (GFunc) remote_viewer_iso_list_dialog_foreach, self); +remote_viewer_iso_list_dialog_show_files(self); +} + +static void remote_viewer_iso_list_dialog_response(GtkDialog *dialog, gint response_id, gpointer user_data G_GNUC_UNUSED) @@ -78,6 +136,32 @@ remote_viewer_iso_list_dialog_response(GtkDialog *dialog, gtk_notebook_set_current_page(GTK_NOTEBOOK(priv->notebook), STATUS_PAGE); gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, FALSE); gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_CLOSE, FALSE); + +remote_viewer_iso_list_dialog_refresh_iso_list(self, TRUE); +} + +void +remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle *cell_renderer G_GNUC_UNUSED, + gchar *path G_GNUC_UNUSED, + gpointer user_data) +{ +RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(user_data); +RemoteViewerISOListDialogPrivate *priv = self->priv; +GtkTreeModel *model = GTK_TREE_MODEL(priv->list_store); +GtkTreeIter iter; +gboolean active; +gchar *name; + +gtk_tree_model_get(model, , + ISO_IS_ACTIVE, , + ISO_NAME, , -1); + +gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, FALSE); +gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_CLOSE, FALSE); +gtk_widget_set_sensitive(priv->treeview, FALSE); + +ovirt_foreign_menu_set_current_iso_name(priv->foreign_menu, active ? NULL : name); +g_free(name); } static void @@ -85,6 +169,7 @@ remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self) { GtkWidget *content = gtk_dialog_get_content_area(GTK_DIALOG(self)); RemoteViewerISOListDialogPrivate *priv = self->priv = DIALOG_PRIVATE(self); +GtkCellRendererToggle *cell_renderer; gtk_widget_set_size_request(GTK_WIDGET(self), 400, 30
[virt-tools-list] [PATCH virt-viewer 03/11] ovirt-foreign-menu: Remove timer used to refresh iso list
With the new ISO dialog, the user triggers the refresh manually. Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/ovirt-foreign-menu.c | 23 +++ 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c index 889e7bc..b071e27 100644 --- a/src/ovirt-foreign-menu.c +++ b/src/ovirt-foreign-menu.c @@ -46,7 +46,7 @@ static void ovirt_foreign_menu_fetch_vm_async(OvirtForeignMenu *menu); static void ovirt_foreign_menu_fetch_storage_domain_async(OvirtForeignMenu *menu); static void ovirt_foreign_menu_fetch_vm_cdrom_async(OvirtForeignMenu *menu); static void ovirt_foreign_menu_refresh_cdrom_file_async(OvirtForeignMenu *menu); -static gboolean ovirt_foreign_menu_refresh_iso_list(gpointer user_data); +static void ovirt_foreign_menu_fetch_iso_list_async(OvirtForeignMenu *menu); G_DEFINE_TYPE (OvirtForeignMenu, ovirt_foreign_menu, G_TYPE_OBJECT) @@ -313,7 +313,7 @@ ovirt_foreign_menu_next_async_step(OvirtForeignMenu *menu, g_warn_if_fail(menu->priv->files != NULL); g_warn_if_fail(menu->priv->cdrom != NULL); -ovirt_foreign_menu_refresh_iso_list(menu); +ovirt_foreign_menu_fetch_iso_list_async(menu); break; } default: { @@ -756,8 +756,6 @@ static void iso_list_fetched_cb(GObject *source_object, files = g_hash_table_get_values(ovirt_collection_get_resources(collection)); ovirt_foreign_menu_set_files(OVIRT_FOREIGN_MENU(user_data), files); g_list_free(files); - -g_timeout_add_seconds(300, ovirt_foreign_menu_refresh_iso_list, user_data); } @@ -767,27 +765,12 @@ static void ovirt_foreign_menu_fetch_iso_list_async(OvirtForeignMenu *menu) return; } +g_debug("Refreshing foreign menu iso list"); ovirt_collection_fetch_async(menu->priv->files, menu->priv->proxy, NULL, iso_list_fetched_cb, menu); } -static gboolean ovirt_foreign_menu_refresh_iso_list(gpointer user_data) -{ -OvirtForeignMenu *menu; - -g_debug("Refreshing foreign menu iso list"); -menu = OVIRT_FOREIGN_MENU(user_data); -ovirt_foreign_menu_fetch_iso_list_async(menu); - -/* ovirt_foreign_menu_fetch_iso_list_async() will schedule a new call to - * that function through iso_list_fetched_cb() when it has finished - * fetching the iso list - */ -return G_SOURCE_REMOVE; -} - - OvirtForeignMenu *ovirt_foreign_menu_new_from_file(VirtViewerFile *file) { OvirtProxy *proxy = NULL; -- 2.7.4 ___ 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 09/11] Run iso-dialog when 'Change CD' menu is activated
Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/resources/ui/virt-viewer.ui | 1 + src/virt-viewer-window.c| 29 + 2 files changed, 30 insertions(+) diff --git a/src/resources/ui/virt-viewer.ui b/src/resources/ui/virt-viewer.ui index ee23ba5..e27ee77 100644 --- a/src/resources/ui/virt-viewer.ui +++ b/src/resources/ui/virt-viewer.ui @@ -73,6 +73,7 @@ False _Change CD True + diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c index 7e6b93f..867fb86 100644 --- a/src/virt-viewer-window.c +++ b/src/virt-viewer-window.c @@ -43,6 +43,8 @@ #include "virt-viewer-util.h" #include "virt-viewer-timed-revealer.h" +#include "remote-viewer-iso-list-dialog.h" + #define ZOOM_STEP 10 /* Signal handlers for main window (move in a VirtViewerMainWindow?) */ @@ -62,6 +64,7 @@ void virt_viewer_window_menu_file_smartcard_insert(GtkWidget *menu, VirtViewerWi void virt_viewer_window_menu_file_smartcard_remove(GtkWidget *menu, VirtViewerWindow *self); void virt_viewer_window_menu_view_release_cursor(GtkWidget *menu, VirtViewerWindow *self); void virt_viewer_window_menu_preferences_cb(GtkWidget *menu, VirtViewerWindow *self); +void virt_viewer_window_menu_change_cd_activate(GtkWidget *menu, VirtViewerWindow *self); /* Internal methods */ @@ -1052,6 +1055,32 @@ virt_viewer_window_menu_help_about(GtkWidget *menu G_GNUC_UNUSED, g_object_unref(G_OBJECT(about)); } +static void +iso_dialog_response(GtkDialog *dialog, +gint response_id, +GtkWidget **dialog_ptr) { +if (response_id == GTK_RESPONSE_NONE) +return; + +gtk_widget_destroy(GTK_WIDGET(dialog)); +*dialog_ptr = NULL; +} + +void +virt_viewer_window_menu_change_cd_activate(GtkWidget *menu G_GNUC_UNUSED, + VirtViewerWindow *self) +{ +VirtViewerWindowPrivate *priv = self->priv; +static GtkWidget *dialog = NULL; + +if (dialog) +return; + +dialog = remote_viewer_iso_list_dialog_new(GTK_WINDOW(priv->window)); +g_signal_connect(dialog, "response", G_CALLBACK(iso_dialog_response), ); +gtk_widget_show_all(dialog); +gtk_dialog_run(GTK_DIALOG(dialog)); +} static void virt_viewer_window_toolbar_setup(VirtViewerWindow *self) -- 2.7.4 ___ 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 05/11] ovirt-foreign-menu: Remove GtkMenu related functions
On 07/19/2016 12:43 PM, Christophe Fergeau wrote: > On Sun, Jul 17, 2016 at 11:13:05PM -0300, Eduardo Lima (Etrunko) wrote: >> With this commit, we finish cleaning up ovirt foreign menu code, which >> only deals with data, leaving the UI bits to be handled properly in the >> new ISO list dialog. > > I'm not clear exactly what this is doing from the commit log. This > removes the old toplevel "change cd" menu, replaces it with a > File/Change CD menu entry, which is going to be not doing anything for > now? > Yes, that's it. It is strange, but I avoided removing old code and replacing with lots of new code only to keep the functionality working. Same happens with the dialog code, which is introduced with no functionality at all, and only plugged into oVirt foreign menu later on. >> -ovirt_foreign_menu_start(foreign_menu); >> -} >> >> +ovirt_foreign_menu_updated(self); > > Why do we use _updated() instead of _start()? > _updated() will only make the menu option visible, while I moved the call to _start() when dialog is shown. This whole naming seems wrong btw, because those functions are static to remote-viewer.c and not present in ovirt-foreign-menu.c. -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ 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 08/11] Introduce ISO List dialog
On 07/19/2016 01:13 PM, Christophe Fergeau wrote: > On Sun, Jul 17, 2016 at 11:13:08PM -0300, Eduardo Lima (Etrunko) wrote: >> Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> >> --- >> src/Makefile.am| 3 + >> src/remote-viewer-iso-list-dialog.c| 115 +++ >> src/remote-viewer-iso-list-dialog.h| 58 ++ >> src/resources/ui/remote-viewer-iso-list.ui | 174 >> + >> src/resources/virt-viewer.gresource.xml| 1 + >> 5 files changed, 351 insertions(+) >> create mode 100644 src/remote-viewer-iso-list-dialog.c >> create mode 100644 src/remote-viewer-iso-list-dialog.h >> create mode 100644 src/resources/ui/remote-viewer-iso-list.ui >> >> diff --git a/src/Makefile.am b/src/Makefile.am >> index 0c48e40..66a73f8 100644 >> --- a/src/Makefile.am >> +++ b/src/Makefile.am >> @@ -13,6 +13,7 @@ noinst_DATA = \ >> resources/ui/virt-viewer-vm-connection.ui \ >> resources/ui/virt-viewer-preferences.ui \ >> resources/ui/remote-viewer-connect.ui \ >> +resources/ui/remote-viewer-iso-list.ui \ >> $(NULL) >> >> EXTRA_DIST =\ >> @@ -96,6 +97,8 @@ if HAVE_OVIRT >> libvirt_viewer_la_SOURCES += \ >> ovirt-foreign-menu.h \ >> ovirt-foreign-menu.c \ >> +remote-viewer-iso-list-dialog.c \ >> +remote-viewer-iso-list-dialog.h \ >> $(NULL) >> endif >> >> diff --git a/src/remote-viewer-iso-list-dialog.c >> b/src/remote-viewer-iso-list-dialog.c >> new file mode 100644 >> index 000..b3972ac >> --- /dev/null >> +++ b/src/remote-viewer-iso-list-dialog.c >> @@ -0,0 +1,115 @@ >> +/* >> + * Virt Viewer: A virtual machine console viewer >> + * >> + * Copyright (C) 2016 Red Hat, Inc. >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program; if not, write to the Free Software >> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA >> + */ >> + >> +#include >> + >> +#include >> + >> +#include "remote-viewer-iso-list-dialog.h" >> +#include "virt-viewer-util.h" >> + >> +G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, >> GTK_TYPE_DIALOG) >> + >> +#define DIALOG_PRIVATE(o) \ >> +(G_TYPE_INSTANCE_GET_PRIVATE((o), >> REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG, RemoteViewerISOListDialogPrivate)) >> + >> +struct _RemoteViewerISOListDialogPrivate >> +{ >> +GtkBuilder *builder; >> +GtkWidget *notebook; >> +}; >> + >> +enum RemoteViewerIsoListDialogPages >> +{ >> +STATUS_PAGE = 0, >> +ISO_LIST_PAGE, >> +}; >> + >> +static void >> +remote_viewer_iso_list_dialog_dispose(GObject *object) >> +{ >> +RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object); >> +RemoteViewerISOListDialogPrivate *priv = self->priv; >> + >> +g_clear_object(>builder); >> + >> + >> G_OBJECT_CLASS(remote_viewer_iso_list_dialog_parent_class)->dispose(object); >> +} >> + >> +static void >> +remote_viewer_iso_list_dialog_class_init(RemoteViewerISOListDialogClass >> *klass) >> +{ >> +GObjectClass *object_class = G_OBJECT_CLASS(klass); >> + >> +g_type_class_add_private(klass, >> sizeof(RemoteViewerISOListDialogPrivate)); >> + >> +object_class->dispose = remote_viewer_iso_list_dialog_dispose; >> +} >> + >> +static void >> +remote_viewer_iso_list_dialog_response(GtkDialog *dialog, >> + gint response_id, >> + gpointer user_data G_GNUC_UNUSED) >> +{ >> +RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog); >> +RemoteViewerISOListDialogPrivate *p
Re: [virt-tools-list] [PATCH virt-viewer 07/11] UI: Make 'Change CD' menu item a submenu under 'File' toplevel menu
On 07/19/2016 12:46 PM, Christophe Fergeau wrote: > Your glade seems to have added unrelated changes to that commit. If > newer versions of glade are always going to make these changes, we can > commit them separately. > Dunno why all those changes got in, most likely because of different versions of glade. I will remove the unrelated changes and keep only what the commit is meant to do. -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ 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 03/11] ovirt-foreign-menu: Remove timer used to refresh iso list
On 07/19/2016 12:11 PM, Christophe Fergeau wrote: > On Mon, Jul 18, 2016 at 10:18:14AM -0300, Eduardo Lima (Etrunko) wrote: >> On 07/18/2016 09:14 AM, Pavel Grunt wrote: >>> On Sun, 2016-07-17 at 23:13 -0300, Eduardo Lima (Etrunko) wrote: >>>> With the new ISO dialog, the user triggers the refresh manually. >>> >>> This should come after the dialog is introduced> >> >> Well, I have no plans in merging any of these patches independently, >> they are only split so that it makes more sense to understand the whole >> series. > > I agree with Pavel, the series makes more sense if it's removed _after_ > the dialog is there. But if it's a conflict mess to move it later, let's > keep it here. > I will try reordering the commits this way. My initial idea was to merge the series as a whole with --no-ff. -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com signature.asc Description: OpenPGP digital signature ___ 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 10/11] remote-viewer: Make ovirt-foreign-menu a property
On 07/19/2016 12:57 PM, Christophe Fergeau wrote: >> + >> +self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog); >> +self->priv->foreign_menu = foreign_menu; > > I'd g_object_ref it if you need to have it around (together with > g_clear_object > in dispose/finalize). Okay, fixed. >> + >> +#ifdef HAVE_OVIRT >> +g_object_class_install_property(object_class, >> +PROP_OVIRT_FOREIGN_MENU, >> + >> g_param_spec_pointer("ovirt-foreign-menu", > > This can be a g_param_spec_object, OvirtForeignMenu is a GObject. Also fixed. > >> + "oVirt Foreign >> Menu", >> + "Object which is >> used as interface to oVirt", >> + G_PARAM_READABLE | >> G_PARAM_STATIC_STRINGS)); >> +#endif >> } >> >> static void >> diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c >> index 867fb86..76fe80f 100644 >> --- a/src/virt-viewer-window.c >> +++ b/src/virt-viewer-window.c >> @@ -1072,11 +1072,25 @@ virt_viewer_window_menu_change_cd_activate(GtkWidget >> *menu G_GNUC_UNUSED, >> { >> VirtViewerWindowPrivate *priv = self->priv; >> static GtkWidget *dialog = NULL; >> +GValue foreign_menu = G_VALUE_INIT; >> >> if (dialog) >> return; >> >> -dialog = remote_viewer_iso_list_dialog_new(GTK_WINDOW(priv->window)); >> +g_value_init(_menu, G_TYPE_POINTER); >> +g_object_get_property(G_OBJECT(priv->app), "ovirt-foreign-menu", >> _menu); > > You can use g_object_get(G_OBJECT(priv->app), "ovirt-foreign_menu", > _menu, NULL); > rather than a GValue. > Thanks, fixed too. -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ 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 10/11] remote-viewer: Make ovirt-foreign-menu a property
On 07/19/2016 02:54 PM, Eduardo Lima (Etrunko) wrote: > On 07/19/2016 12:57 PM, Christophe Fergeau wrote: >>> + >>> +self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog); >>> +self->priv->foreign_menu = foreign_menu; >> >> I'd g_object_ref it if you need to have it around (together with >> g_clear_object >> in dispose/finalize). > > Okay, fixed. > >>> + >>> +#ifdef HAVE_OVIRT >>> +g_object_class_install_property(object_class, >>> +PROP_OVIRT_FOREIGN_MENU, >>> + >>> g_param_spec_pointer("ovirt-foreign-menu", >> >> This can be a g_param_spec_object, OvirtForeignMenu is a GObject. > > Also fixed. > >> >>> + "oVirt Foreign >>> Menu", >>> + "Object which is >>> used as interface to oVirt", >>> + G_PARAM_READABLE >>> | G_PARAM_STATIC_STRINGS)); >>> +#endif >>> } >>> >>> static void >>> diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c >>> index 867fb86..76fe80f 100644 >>> --- a/src/virt-viewer-window.c >>> +++ b/src/virt-viewer-window.c >>> @@ -1072,11 +1072,25 @@ >>> virt_viewer_window_menu_change_cd_activate(GtkWidget *menu G_GNUC_UNUSED, >>> { >>> VirtViewerWindowPrivate *priv = self->priv; >>> static GtkWidget *dialog = NULL; >>> +GValue foreign_menu = G_VALUE_INIT; >>> >>> if (dialog) >>> return; >>> >>> -dialog = remote_viewer_iso_list_dialog_new(GTK_WINDOW(priv->window)); >>> +g_value_init(_menu, G_TYPE_POINTER); >>> +g_object_get_property(G_OBJECT(priv->app), "ovirt-foreign-menu", >>> _menu); >> >> You can use g_object_get(G_OBJECT(priv->app), "ovirt-foreign_menu", >> _menu, NULL); >> rather than a GValue. >> > > Thanks, fixed too. > Actually I did make it a pointer on purpose, because I did not want to have OvirtForeignMenu type in this file. Maybe I should change it to GObject then? -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ 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 v2 10/11] remote-viewer: Make ovirt-foreign-menu a property
The OvirtForeignMenu pointer is needed by the new ISO list dialog, and we make it acessible via property to avoid interdependency between objects. Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/remote-viewer-iso-list-dialog.c | 25 - src/remote-viewer-iso-list-dialog.h | 2 +- src/remote-viewer.c | 37 + src/virt-viewer-window.c| 14 +- 4 files changed, 71 insertions(+), 7 deletions(-) diff --git a/src/remote-viewer-iso-list-dialog.c b/src/remote-viewer-iso-list-dialog.c index 3ede716..045d601 100644 --- a/src/remote-viewer-iso-list-dialog.c +++ b/src/remote-viewer-iso-list-dialog.c @@ -24,6 +24,7 @@ #include "remote-viewer-iso-list-dialog.h" #include "virt-viewer-util.h" +#include "ovirt-foreign-menu.h" G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, GTK_TYPE_DIALOG) @@ -33,6 +34,7 @@ G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, GTK_TYPE struct _RemoteViewerISOListDialogPrivate { GtkWidget *notebook; +OvirtForeignMenu *foreign_menu; }; enum RemoteViewerIsoListDialogPages @@ -44,6 +46,10 @@ enum RemoteViewerIsoListDialogPages static void remote_viewer_iso_list_dialog_dispose(GObject *object) { +RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object); +RemoteViewerISOListDialogPrivate *priv = self->priv = DIALOG_PRIVATE(self); + +g_clear_object(>foreign_menu); G_OBJECT_CLASS(remote_viewer_iso_list_dialog_parent_class)->dispose(object); } @@ -102,10 +108,19 @@ remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self) } GtkWidget * -remote_viewer_iso_list_dialog_new(GtkWindow *parent) +remote_viewer_iso_list_dialog_new(GtkWindow *parent, GObject *foreign_menu) { -return g_object_new(REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG, -"title", _("Change CD"), -"transient-for", parent, -NULL); +GtkWidget *dialog; +RemoteViewerISOListDialog *self; + +g_return_val_if_fail(foreign_menu != NULL, NULL); + +dialog = g_object_new(REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG, + "title", _("Change CD"), + "transient-for", parent, + NULL); + +self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog); +self->priv->foreign_menu = OVIRT_FOREIGN_MENU(g_object_ref(foreign_menu)); +return dialog; } diff --git a/src/remote-viewer-iso-list-dialog.h b/src/remote-viewer-iso-list-dialog.h index def841b..8b936f5 100644 --- a/src/remote-viewer-iso-list-dialog.h +++ b/src/remote-viewer-iso-list-dialog.h @@ -51,7 +51,7 @@ struct _RemoteViewerISOListDialogClass GType remote_viewer_iso_list_dialog_get_type(void) G_GNUC_CONST; -GtkWidget *remote_viewer_iso_list_dialog_new(GtkWindow *parent); +GtkWidget *remote_viewer_iso_list_dialog_new(GtkWindow *parent, GObject *foreign_menu); G_END_DECLS diff --git a/src/remote-viewer.c b/src/remote-viewer.c index 95130dc..e0cd139 100644 --- a/src/remote-viewer.c +++ b/src/remote-viewer.c @@ -67,6 +67,13 @@ G_DEFINE_TYPE (RemoteViewer, remote_viewer, VIRT_VIEWER_TYPE_APP) #define GET_PRIVATE(o)\ (G_TYPE_INSTANCE_GET_PRIVATE ((o), REMOTE_VIEWER_TYPE, RemoteViewerPrivate)) +enum RemoteViewerProperties { +PROP_0, +#ifdef HAVE_OVIRT +PROP_OVIRT_FOREIGN_MENU, +#endif +}; + #ifdef HAVE_OVIRT static OvirtVm * choose_vm(GtkWindow *main_window, char **vm_name, @@ -213,6 +220,25 @@ end: } static void +remote_viewer_get_property(GObject *object, guint property_id, + GValue *value, GParamSpec *pspec) +{ +RemoteViewer *self = REMOTE_VIEWER(object); +RemoteViewerPrivate *priv = self->priv; + +switch (property_id) { +#ifdef HAVE_OVIRT +case PROP_OVIRT_FOREIGN_MENU: +g_value_set_object(value, priv->ovirt_foreign_menu); +break; +#endif + +default: +G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec); +} +} + +static void remote_viewer_class_init (RemoteViewerClass *klass) { GObjectClass *object_class = G_OBJECT_CLASS (klass); @@ -222,6 +248,7 @@ remote_viewer_class_init (RemoteViewerClass *klass) g_type_class_add_private (klass, sizeof (RemoteViewerPrivate)); +object_class->get_property = remote_viewer_get_property; object_class->dispose = remote_viewer_dispose; g_app_class->local_command_line = remote_viewer_local_command_line; @@ -235,6 +262,16 @@ remote_viewer_class_init (RemoteViewerClass *klass) #else (void) gtk_app_class; #endif + +#ifdef HAVE_OVIRT +g_object_class_install_property(object_class, +
[virt-tools-list] [PATCH virt-viewer v2 07/11] UI: Make 'Change CD' menu item a submenu under 'File' toplevel menu
Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/resources/ui/virt-viewer.ui | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/resources/ui/virt-viewer.ui b/src/resources/ui/virt-viewer.ui index 6e3c5ad..af3ae46 100644 --- a/src/resources/ui/virt-viewer.ui +++ b/src/resources/ui/virt-viewer.ui @@ -1,6 +1,7 @@ + - + False @@ -73,6 +74,13 @@ + +False +_Change CD +True + + + True False @@ -247,14 +255,6 @@ - - -False -False -_Change CD -True - - False -- 2.7.4 ___ 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 04/11] ovirt-foreign-menu: Add accessors for current iso and iso list
On 07/19/2016 12:32 PM, Christophe Fergeau wrote: > On Sun, Jul 17, 2016 at 11:13:04PM -0300, Eduardo Lima (Etrunko) wrote: >> Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> >> --- >> src/ovirt-foreign-menu.c | 49 >> ++-- >> src/ovirt-foreign-menu.h | 5 - >> 2 files changed, 39 insertions(+), 15 deletions(-) >> >> diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c >> index b071e27..2446239 100644 >> --- a/src/ovirt-foreign-menu.c >> +++ b/src/ovirt-foreign-menu.c >> @@ -47,6 +47,7 @@ static void >> ovirt_foreign_menu_fetch_storage_domain_async(OvirtForeignMenu *menu >> static void ovirt_foreign_menu_fetch_vm_cdrom_async(OvirtForeignMenu *menu); >> static void ovirt_foreign_menu_refresh_cdrom_file_async(OvirtForeignMenu >> *menu); >> static void ovirt_foreign_menu_fetch_iso_list_async(OvirtForeignMenu *menu); >> +static void updated_cdrom_cb(GObject *source_object, GAsyncResult *result, >> gpointer user_data); >> >> G_DEFINE_TYPE (OvirtForeignMenu, ovirt_foreign_menu, G_TYPE_OBJECT) >> >> @@ -85,7 +86,7 @@ enum { >> }; >> >> >> -static char * >> +char * >> ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *foreign_menu) >> { >> char *name; >> @@ -100,6 +101,36 @@ >> ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *foreign_menu) >> } >> >> >> +void >> +ovirt_foreign_menu_set_current_iso_name(OvirtForeignMenu *foreign_menu, >> char *name) >> +{ > > For what it's worth, this is a bit misleading as this going to trigger > an async update of the ISO name, and this sets "next_iso_name" more than > "current_iso_name". I think you need to expose this an async method > anyway, so that you can catch failures to change the ISO (ie the REST > API call failed). > Okay, I will work on it. I really did it the laziest way possible, without any error treatment indeed. -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com signature.asc Description: OpenPGP digital signature ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [PATCH] Get rid of deprecated functions to customize widget colors
On 06/28/2016 07:21 AM, Fabiano Fidêncio wrote: > On Tue, Jun 28, 2016 at 12:08 PM, Pavel Grunt <pgr...@redhat.com> wrote: >> Hi Eduardo, >> >> On Mon, 2016-06-27 at 18:00 -0300, Eduardo Lima (Etrunko) wrote: >>> Fixes https://bugs.freedesktop.org/show_bug.cgi?id=94276 >>> >>> Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> >>> --- >>> >>> As a result of commit cc455b7f916110d7cfae6b7af753349e070c9494, setting >>> custom color for background does not work anymore on my Fedora 23 >>> system. The status label still rendered with white color, but with the >>> background with default theme color (usually light grey), it has become >>> hard to read the text from the label. >>> >>> I talked to Fabiano who told me that everything was working as expected >>> with his recently upgraded Fedora 24 system. While trying to track and >>> fix the issue, I noticed that it will only happen if the notebook tabs >>> are hidden. If tabs are shown, the background color will be properly >>> set. >>> >>> I tracked down to Gtk+ some changes to GtkNotebook in recently released >>> version 3.20, which fixed the rendering of the custom background color, >>> with tabs hidden, but those could not be easily backported. Even though >>> it is a change of behavior in virt-viewer, I think it is really a minor >>> issue, and I decided to not spent too much time on this, so I put a >>> check for Gtk+ version to decide whether or not set the custom colors. >>> >>> Some screenshots to illustrate: >>> >>> Gtk+ > 3.20: >>> http://imgur.com/gpuMukA >>> >>> Gtk+ < 3.20: >>> >>> without this patch.: http://imgur.com/RdirSoX >>> with this patch: http://imgur.com/9LJNeNI >> >> I would make it more simple, stick with the system theme >> (ie http://imgur.com/9LJNeNI for all gtk versions) instead of introducing >> some >> css just for 3.20 (is it stable btw ;-) ?). It would simplify the code, imho >> it >> looks better and another gui tool from the family - virt-manager - uses it. >> >> What do you think ? > > Hmm. This solution is quite okay for me. > If the other people agree, I'd say to proceed with your suggestion then. > Oh yes, I am all in favor of keeping the default theme colors. -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [PATCH v2 virt-viewer] Get rid of deprecated functions to customize widget colors
Let's just stick with default theme colors. Fixes https://bugs.freedesktop.org/show_bug.cgi?id=94276 Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/virt-viewer-notebook.c | 13 ++--- src/virt-viewer-window.c | 10 -- 2 files changed, 2 insertions(+), 21 deletions(-) diff --git a/src/virt-viewer-notebook.c b/src/virt-viewer-notebook.c index 420c914..8bb9977 100644 --- a/src/virt-viewer-notebook.c +++ b/src/virt-viewer-notebook.c @@ -71,25 +71,16 @@ static void virt_viewer_notebook_init (VirtViewerNotebook *self) { VirtViewerNotebookPrivate *priv; -GdkRGBA color; self->priv = GET_PRIVATE(self); priv = self->priv; -priv->status = gtk_label_new(""); gtk_notebook_set_show_tabs(GTK_NOTEBOOK(self), FALSE); gtk_notebook_set_show_border(GTK_NOTEBOOK(self), FALSE); + +priv->status = gtk_label_new(""); gtk_widget_show_all(priv->status); gtk_notebook_append_page(GTK_NOTEBOOK(self), priv->status, NULL); -gdk_rgba_parse(, "white"); -/* FIXME: - * This method has been deprecated in 3.16. - * For more details on how to deal with this in the future, please, see: - * https://developer.gnome.org/gtk3/stable/GtkWidget.html#gtk-widget-override-color - * For the bug report about this deprecated function, please, see: - * https://bugs.freedesktop.org/show_bug.cgi?id=94276 - */ -gtk_widget_override_color(priv->status, GTK_STATE_FLAG_NORMAL, ); } void diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c index 1ebb423..c59fff5 100644 --- a/src/virt-viewer-window.c +++ b/src/virt-viewer-window.c @@ -297,7 +297,6 @@ virt_viewer_window_init (VirtViewerWindow *self) { VirtViewerWindowPrivate *priv; GtkWidget *vbox; -GdkRGBA color; GSList *accels; self->priv = GET_PRIVATE(self); @@ -340,15 +339,6 @@ virt_viewer_window_init (VirtViewerWindow *self) virt_viewer_window_toolbar_setup(self); gtk_box_pack_end(GTK_BOX(vbox), GTK_WIDGET(priv->notebook), TRUE, TRUE, 0); -gdk_rgba_parse(, "black"); -/* FIXME: - * This method has been deprecated in 3.16. - * For more details on how to deal with this in the future, please, see: - * https://developer.gnome.org/gtk3/stable/GtkWidget.html#gtk-widget-override-background-color - * For the bug report about this deprecated function, please, see: - * https://bugs.freedesktop.org/show_bug.cgi?id=94276 - */ -gtk_widget_override_background_color(GTK_WIDGET(priv->notebook), GTK_STATE_FLAG_NORMAL, ); priv->window = GTK_WIDGET(gtk_builder_get_object(priv->builder, "viewer")); gtk_window_add_accel_group(GTK_WINDOW(priv->window), priv->accel_group); -- 2.5.5 ___ 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] window: Factor out common code for toolbar items
On 06/28/2016 12:16 PM, Fabiano Fidêncio wrote: > On Tue, Jun 28, 2016 at 5:10 PM, Pavel Gruntwrote: >> Create toolbar widget in the loop > > NACK from my side. > There is any gain on re-factoring a code that will be removed as soon > as we do the release. > Actually, it just makes my life harder in order to rebase Sagar's > patches on top of this change. Agreed, only a small comment below. > >> --- >> src/virt-viewer-window.c | 121 >> --- >> 1 file changed, 83 insertions(+), 38 deletions(-) >> >> diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c >> index 1ebb423..b276ae8 100644 >> --- a/src/virt-viewer-window.c >> +++ b/src/virt-viewer-window.c >> @@ -1062,56 +1062,101 @@ virt_viewer_window_menu_help_about(GtkWidget *menu >> G_GNUC_UNUSED, >> g_object_unref(G_OBJECT(about)); >> } >> >> +typedef struct { >> +GtkWidget *icon; >> +const gchar *icon_name; >> +const gchar *label; >> +const gchar *tooltip; >> +const gboolean sensitive; >> +const gboolean show_label; >> +const GCallback callback; >> +} VirtViewerToolbarButton; >> + >> +static void >> +virt_viewer_window_toolbar_add_button(VirtViewerWindow *self, >> + const VirtViewerToolbarButton >> *tb_button, >> + GtkWidget **dest_widget) >> +{ >> +VirtViewerWindowPrivate *priv = self->priv; >> +GtkToolItem *button = gtk_tool_button_new(tb_button->icon, >> tb_button->label); >> + >> +gtk_tool_button_set_icon_name(GTK_TOOL_BUTTON(button), >> tb_button->icon_name); >> +gtk_tool_item_set_tooltip_text(button, tb_button->tooltip); >> +gtk_tool_item_set_is_important(button, tb_button->show_label); >> +gtk_widget_set_sensitive(GTK_WIDGET(button), tb_button->sensitive); >> +gtk_widget_show_all(GTK_WIDGET(button)); >> +gtk_toolbar_insert(GTK_TOOLBAR(priv->toolbar), button, 0); >> +g_signal_connect(button, "clicked", tb_button->callback, self); >> + >> +if (dest_widget != NULL) >> +*dest_widget = GTK_WIDGET(button); >> +} >> >> static void >> virt_viewer_window_toolbar_setup(VirtViewerWindow *self) >> { >> -GtkWidget *button; >> GtkWidget *overlay; >> VirtViewerWindowPrivate *priv = self->priv; >> +guint i; >> + >> +const struct { >> +const VirtViewerToolbarButton tb_button; >> +GtkWidget **dest_widget; >> +} toolbar_buttons[] = { >> +{ /* Close connection */ >> +{ >> +NULL, >> +"window-close", >> +NULL, >> +_("Disconnect"), >> +TRUE, >> +FALSE, >> +G_CALLBACK(virt_viewer_window_menu_file_quit), >> +}, >> +NULL, >> +},{ /* USB Device selection */ >> +{ >> + >> gtk_image_new_from_resource(VIRT_VIEWER_RESOURCE_PREFIX"/icons/24x24/virt-viewer-usb.png"), >> +NULL, >> +_("USB device selection"), >> +_("USB device selection"), >> +TRUE, >> +FALSE, >> + >> G_CALLBACK(virt_viewer_window_menu_file_usb_device_selection), >> +}, >> +>toolbar_usb_device_selection, >> +},{ /* Send key */ >> +{ >> +NULL, >> +"preferences-desktop-keyboard-shortcuts", >> +NULL, >> +_("Send key combination"), >> +FALSE, >> +FALSE, >> +G_CALLBACK(virt_viewer_window_toolbar_send_key), >> +}, >> +>toolbar_send_key, >> +},{ /* Leave fullscreen */ >> +{ >> +NULL, >> +"view-restore", >> +_("Leave fullscreen"), >> +_("Leave fullscreen"), >> +TRUE, >> +TRUE, >> +G_CALLBACK(virt_viewer_window_toolbar_leave_fullscreen), >> +}, >> +NULL, >> +}, >> +}; In the case we did not have patches in the queue that makes this obsolete, it would be a good idea to have explicit field initializers for the items in the list. Besides being easier to read, you could save some lines of code for the NULL and FALSE ones. Regards, Eduardo. -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ 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 v2 12/12] iso-dialog: Use header bar for buttons
It seems to give more modern look to the dialog, but it requires Gtk+ 3.12, thus I will am keeping this commit separated. Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- configure.ac| 4 ++-- src/remote-viewer-iso-list-dialog.c | 33 - 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/configure.ac b/configure.ac index 7c437a3..7682f42 100644 --- a/configure.ac +++ b/configure.ac @@ -17,8 +17,8 @@ GLIB2_REQUIRED="2.38" GLIB2_ENCODED_VERSION="GLIB_VERSION_2_38" # Keep these two definitions in agreement. -GTK_REQUIRED="3.10" -GTK_ENCODED_VERSION="GDK_VERSION_3_10" +GTK_REQUIRED="3.12" +GTK_ENCODED_VERSION="GDK_VERSION_3_12" LIBXML2_REQUIRED="2.6.0" LIBVIRT_REQUIRED="0.10.0" diff --git a/src/remote-viewer-iso-list-dialog.c b/src/remote-viewer-iso-list-dialog.c index 7fe032b..c6fb5d8 100644 --- a/src/remote-viewer-iso-list-dialog.c +++ b/src/remote-viewer-iso-list-dialog.c @@ -34,6 +34,7 @@ G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, GTK_TYPE struct _RemoteViewerISOListDialogPrivate { +GtkHeaderBar *header_bar; GtkListStore *list_store; GtkWidget *stack; GtkWidget *treeview; @@ -88,6 +89,19 @@ remote_viewer_iso_list_dialog_show_files(RemoteViewerISOListDialog *self) } static void +remote_viewer_iso_list_dialog_set_subtitle(RemoteViewerISOListDialog *self, const char *iso_name) +{ +RemoteViewerISOListDialogPrivate *priv = self->priv; +gchar *subtitle = NULL; + +if (iso_name && strlen(iso_name) != 0) +subtitle = g_strdup_printf(_("Current: %s"), iso_name); + +gtk_header_bar_set_subtitle(priv->header_bar, subtitle); +g_free(subtitle); +} + +static void remote_viewer_iso_list_dialog_foreach(char *name, RemoteViewerISOListDialog *self) { RemoteViewerISOListDialogPrivate *priv = self->priv; @@ -102,6 +116,9 @@ remote_viewer_iso_list_dialog_foreach(char *name, RemoteViewerISOListDialog *sel ISO_NAME, name, FONT_WEIGHT, weight, -1); +if (active) +remote_viewer_iso_list_dialog_set_subtitle(self, current_iso); + free(current_iso); } @@ -134,6 +151,7 @@ remote_viewer_iso_list_dialog_response(GtkDialog *dialog, if (response_id != GTK_RESPONSE_NONE) return; +remote_viewer_iso_list_dialog_set_subtitle(self, NULL); gtk_stack_set_visible_child_full(GTK_STACK(priv->stack), "status", GTK_STACK_TRANSITION_TYPE_SLIDE_LEFT); gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, FALSE); @@ -171,9 +189,13 @@ remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self) RemoteViewerISOListDialogPrivate *priv = self->priv = DIALOG_PRIVATE(self); GtkBuilder *builder = virt_viewer_util_load_ui("remote-viewer-iso-list.ui"); GtkCellRendererToggle *cell_renderer; +GtkWidget *button, *image; gtk_builder_connect_signals(builder, self); +priv->header_bar = GTK_HEADER_BAR(gtk_dialog_get_header_bar(GTK_DIALOG(self))); +gtk_header_bar_set_has_subtitle(priv->header_bar, TRUE); + priv->stack = GTK_WIDGET(gtk_builder_get_object(builder, "stack")); gtk_box_pack_start(GTK_BOX(content), priv->stack, TRUE, TRUE, 0); @@ -184,12 +206,11 @@ remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self) g_object_unref(builder); -gtk_dialog_add_buttons(GTK_DIALOG(self), - _("Refresh"), GTK_RESPONSE_NONE, - _("Close"), GTK_RESPONSE_CLOSE, - NULL); +button = gtk_dialog_add_button(GTK_DIALOG(self), "", GTK_RESPONSE_NONE); +image = gtk_image_new_from_icon_name("view-refresh-symbolic", GTK_ICON_SIZE_BUTTON); +gtk_button_set_image(GTK_BUTTON(button), image); +gtk_button_set_always_show_image(GTK_BUTTON(button), TRUE); -gtk_dialog_set_default_response(GTK_DIALOG(self), GTK_RESPONSE_CLOSE); gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, FALSE); g_signal_connect(self, "response", G_CALLBACK(remote_viewer_iso_list_dialog_response), NULL); } @@ -229,6 +250,7 @@ ovirt_foreign_menu_notify_file(OvirtForeignMenu *foreign_menu, g_free(name); } while (gtk_tree_model_iter_next(model, )); +remote_viewer_iso_list_dialog_set_subtitle(self, current_iso); gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, TRUE); gtk_widget_set_sensitive(priv->treeview, TRUE); @@ -259,6 +281,7 @@ remote_viewer_iso_list_dialog_new(GtkWindow *parent, GObject *foreign_menu) "border-width", 18,
[virt-tools-list] [PATCH virt-viewer v2 03/12] ovirt-foreign-menu: Remove timer used to refresh iso list
With the new ISO dialog, the user triggers the refresh manually. Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/ovirt-foreign-menu.c | 23 +++ 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c index a51f2c9..565ef22 100644 --- a/src/ovirt-foreign-menu.c +++ b/src/ovirt-foreign-menu.c @@ -46,7 +46,7 @@ static void ovirt_foreign_menu_fetch_vm_async(OvirtForeignMenu *menu); static void ovirt_foreign_menu_fetch_storage_domain_async(OvirtForeignMenu *menu); static void ovirt_foreign_menu_fetch_vm_cdrom_async(OvirtForeignMenu *menu); static void ovirt_foreign_menu_refresh_cdrom_file_async(OvirtForeignMenu *menu); -static gboolean ovirt_foreign_menu_refresh_iso_list(gpointer user_data); +static void ovirt_foreign_menu_fetch_iso_list_async(OvirtForeignMenu *menu); G_DEFINE_TYPE (OvirtForeignMenu, ovirt_foreign_menu, G_TYPE_OBJECT) @@ -313,7 +313,7 @@ ovirt_foreign_menu_next_async_step(OvirtForeignMenu *menu, g_warn_if_fail(menu->priv->files != NULL); g_warn_if_fail(menu->priv->cdrom != NULL); -ovirt_foreign_menu_refresh_iso_list(menu); +ovirt_foreign_menu_fetch_iso_list_async(menu); break; default: g_warn_if_reached(); @@ -754,8 +754,6 @@ static void iso_list_fetched_cb(GObject *source_object, files = g_hash_table_get_values(ovirt_collection_get_resources(collection)); ovirt_foreign_menu_set_files(OVIRT_FOREIGN_MENU(user_data), files); g_list_free(files); - -g_timeout_add_seconds(300, ovirt_foreign_menu_refresh_iso_list, user_data); } @@ -765,27 +763,12 @@ static void ovirt_foreign_menu_fetch_iso_list_async(OvirtForeignMenu *menu) return; } +g_debug("Refreshing foreign menu iso list"); ovirt_collection_fetch_async(menu->priv->files, menu->priv->proxy, NULL, iso_list_fetched_cb, menu); } -static gboolean ovirt_foreign_menu_refresh_iso_list(gpointer user_data) -{ -OvirtForeignMenu *menu; - -g_debug("Refreshing foreign menu iso list"); -menu = OVIRT_FOREIGN_MENU(user_data); -ovirt_foreign_menu_fetch_iso_list_async(menu); - -/* ovirt_foreign_menu_fetch_iso_list_async() will schedule a new call to - * that function through iso_list_fetched_cb() when it has finished - * fetching the iso list - */ -return G_SOURCE_REMOVE; -} - - OvirtForeignMenu *ovirt_foreign_menu_new_from_file(VirtViewerFile *file) { OvirtProxy *proxy = NULL; -- 2.7.4 ___ 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 v2 07/12] UI: Make 'Change CD' menu item a submenu under 'File' toplevel menu
Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/resources/ui/virt-viewer.ui | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/resources/ui/virt-viewer.ui b/src/resources/ui/virt-viewer.ui index 6e3c5ad..af3ae46 100644 --- a/src/resources/ui/virt-viewer.ui +++ b/src/resources/ui/virt-viewer.ui @@ -1,6 +1,7 @@ + - + False @@ -73,6 +74,13 @@ + +False +_Change CD +True + + + True False @@ -247,14 +255,6 @@ - - -False -False -_Change CD -True - - False -- 2.7.4 ___ 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 v2 11/12] iso-dialog: Implement functionality provided by oVirt foreign menu
Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/remote-viewer-iso-list-dialog.c| 157 - src/resources/ui/remote-viewer-iso-list.ui | 5 +- 2 files changed, 158 insertions(+), 4 deletions(-) diff --git a/src/remote-viewer-iso-list-dialog.c b/src/remote-viewer-iso-list-dialog.c index 7a84d0c..7fe032b 100644 --- a/src/remote-viewer-iso-list-dialog.c +++ b/src/remote-viewer-iso-list-dialog.c @@ -20,6 +20,7 @@ #include +#include #include #include "remote-viewer-iso-list-dialog.h" @@ -33,7 +34,9 @@ G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, GTK_TYPE struct _RemoteViewerISOListDialogPrivate { +GtkListStore *list_store; GtkWidget *stack; +GtkWidget *treeview; OvirtForeignMenu *foreign_menu; }; @@ -43,13 +46,25 @@ enum RemoteViewerIsoListDialogPages ISO_LIST_PAGE, }; +enum RemoteViewerIsoListDialogModel +{ +ISO_IS_ACTIVE = 0, +ISO_NAME, +FONT_WEIGHT, +}; + +void remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle *cell_renderer, gchar *path, gpointer user_data); + static void remote_viewer_iso_list_dialog_dispose(GObject *object) { RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object); RemoteViewerISOListDialogPrivate *priv = self->priv; -g_clear_object(>foreign_menu); +if (priv->foreign_menu) { +g_signal_handlers_disconnect_by_data(priv->foreign_menu, object); +g_clear_object(>foreign_menu); +} G_OBJECT_CLASS(remote_viewer_iso_list_dialog_parent_class)->dispose(object); } @@ -64,6 +79,51 @@ remote_viewer_iso_list_dialog_class_init(RemoteViewerISOListDialogClass *klass) } static void +remote_viewer_iso_list_dialog_show_files(RemoteViewerISOListDialog *self) +{ +RemoteViewerISOListDialogPrivate *priv = self->priv = DIALOG_PRIVATE(self); +gtk_stack_set_visible_child_full(GTK_STACK(priv->stack), "iso-list", + GTK_STACK_TRANSITION_TYPE_SLIDE_LEFT); +gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, TRUE); +} + +static void +remote_viewer_iso_list_dialog_foreach(char *name, RemoteViewerISOListDialog *self) +{ +RemoteViewerISOListDialogPrivate *priv = self->priv; +char * current_iso = ovirt_foreign_menu_get_current_iso_name(self->priv->foreign_menu); +gboolean active = g_strcmp0(current_iso, name) == 0; +gint weight = active ? PANGO_WEIGHT_BOLD : PANGO_WEIGHT_NORMAL; +GtkTreeIter iter; + +gtk_list_store_append(priv->list_store, ); +gtk_list_store_set(priv->list_store, , + ISO_IS_ACTIVE, active, + ISO_NAME, name, + FONT_WEIGHT, weight, -1); + +free(current_iso); +} + +static void +remote_viewer_iso_list_dialog_refresh_iso_list(RemoteViewerISOListDialog *self, + gboolean refreshing) +{ +RemoteViewerISOListDialogPrivate *priv = self->priv; +GList *iso_list; + +if (refreshing) { +gtk_list_store_clear(priv->list_store); +ovirt_foreign_menu_start(priv->foreign_menu); +return; +} + +iso_list = ovirt_foreign_menu_get_iso_names(priv->foreign_menu); +g_list_foreach(iso_list, (GFunc) remote_viewer_iso_list_dialog_foreach, self); +remote_viewer_iso_list_dialog_show_files(self); +} + +static void remote_viewer_iso_list_dialog_response(GtkDialog *dialog, gint response_id, gpointer user_data G_GNUC_UNUSED) @@ -77,7 +137,31 @@ remote_viewer_iso_list_dialog_response(GtkDialog *dialog, gtk_stack_set_visible_child_full(GTK_STACK(priv->stack), "status", GTK_STACK_TRANSITION_TYPE_SLIDE_LEFT); gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, FALSE); -gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_CLOSE, FALSE); +remote_viewer_iso_list_dialog_refresh_iso_list(self, TRUE); +} + +void +remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle *cell_renderer G_GNUC_UNUSED, + gchar *path, + gpointer user_data) +{ +RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(user_data); +RemoteViewerISOListDialogPrivate *priv = self->priv; +GtkTreeModel *model = GTK_TREE_MODEL(priv->list_store); +GtkTreeIter iter; +gboolean active; +gchar *name; + +gtk_tree_model_get_iter_from_string(model, , path); +gtk_tree_model_get(model, , + ISO_IS_ACTIVE, , + ISO_NAME, , -1); + +gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, FALSE); +gtk_widget_set_sensitive(priv->treeview, FALSE); + +ovirt_foreign_menu_se
[virt-tools-list] [PATCH virt-viewer v2 02/12] ovirt-foreign-menu: Use g_clear_pointer/g_clear_object
Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> Acked-by: Christophe Fergeau <cferg...@redhat.com> --- src/ovirt-foreign-menu.c | 68 1 file changed, 16 insertions(+), 52 deletions(-) diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c index 03dfbe7..a51f2c9 100644 --- a/src/ovirt-foreign-menu.c +++ b/src/ovirt-foreign-menu.c @@ -143,33 +143,23 @@ ovirt_foreign_menu_set_property(GObject *object, guint property_id, switch (property_id) { case PROP_PROXY: -if (priv->proxy != NULL) { -g_object_unref(priv->proxy); -} +g_clear_object(>proxy); priv->proxy = g_value_dup_object(value); break; case PROP_API: -if (priv->api != NULL) { -g_object_unref(priv->api); -} +g_clear_object(>api); priv->api = g_value_dup_object(value); break; case PROP_VM: -if (priv->vm != NULL) { -g_object_unref(priv->vm); -} +g_clear_object(>vm); priv->vm = g_value_dup_object(value); -g_free(priv->vm_guid); -priv->vm_guid = NULL; +g_clear_pointer(>vm_guid, g_free); if (priv->vm != NULL) { g_object_get(G_OBJECT(priv->vm), "guid", >vm_guid, NULL); } break; case PROP_VM_GUID: -if (priv->vm != NULL) { -g_object_unref(priv->vm); -priv->vm = NULL; -} +g_clear_object(>vm); g_free(priv->vm_guid); priv->vm_guid = g_value_dup_string(value); break; @@ -184,44 +174,20 @@ ovirt_foreign_menu_dispose(GObject *obj) { OvirtForeignMenu *self = OVIRT_FOREIGN_MENU(obj); -if (self->priv->proxy) { -g_object_unref(self->priv->proxy); -self->priv->proxy = NULL; -} - -if (self->priv->api != NULL) { -g_object_unref(self->priv->api); -self->priv->api = NULL; -} - -if (self->priv->vm) { -g_object_unref(self->priv->vm); -self->priv->vm = NULL; -} - -g_free(self->priv->vm_guid); -self->priv->vm_guid = NULL; - -if (self->priv->files) { -g_object_unref(self->priv->files); -self->priv->files = NULL; -} - -if (self->priv->cdrom) { -g_object_unref(self->priv->cdrom); -self->priv->cdrom = NULL; -} +g_clear_object(>priv->proxy); +g_clear_object(>priv->api); +g_clear_object(>priv->vm); +g_clear_pointer(>priv->vm_guid, g_free); +g_clear_object(>priv->files); +g_clear_object(>priv->cdrom); if (self->priv->iso_names) { g_list_free_full(self->priv->iso_names, (GDestroyNotify)g_free); self->priv->iso_names = NULL; } -g_free(self->priv->current_iso_name); -self->priv->current_iso_name = NULL; - -g_free(self->priv->next_iso_name); -self->priv->next_iso_name = NULL; +g_clear_pointer(>priv->current_iso_name, g_free); +g_clear_pointer(>priv->next_iso_name, g_free); G_OBJECT_CLASS(ovirt_foreign_menu_parent_class)->dispose(obj); } @@ -408,8 +374,8 @@ static void updated_cdrom_cb(GObject *source_object, current_file?current_file:NULL); g_object_set(foreign_menu->priv->cdrom, "file", current_file, NULL); } -g_free(foreign_menu->priv->next_iso_name); -foreign_menu->priv->next_iso_name = NULL; + +g_clear_pointer(_menu->priv->next_iso_name, g_free); } @@ -544,13 +510,11 @@ static void cdrom_file_refreshed_cb(GObject *source_object, } /* Content of OvirtCdrom is now current */ -g_free(menu->priv->current_iso_name); +g_clear_pointer(>priv->current_iso_name, g_free); if (menu->priv->cdrom != NULL) { g_object_get(G_OBJECT(menu->priv->cdrom), "file", >priv->current_iso_name, NULL); -} else { -menu->priv->current_iso_name = NULL; } g_object_notify(G_OBJECT(menu), "file"); if (menu->priv->cdrom != NULL) { -- 2.7.4 ___ 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 v2 01/12] ovirt-foreign-menu: Rework states logic
Use switch/case instead of lots of conditional blocks Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> Acked-by: Pavel Grunt <pgr...@redhat.com> --- src/ovirt-foreign-menu.c | 46 -- 1 file changed, 20 insertions(+), 26 deletions(-) diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c index 33ff4f1..03dfbe7 100644 --- a/src/ovirt-foreign-menu.c +++ b/src/ovirt-foreign-menu.c @@ -312,51 +312,45 @@ ovirt_foreign_menu_next_async_step(OvirtForeignMenu *menu, g_return_if_fail(current_state >= STATE_0); g_return_if_fail(current_state < STATE_ISOS); -current_state++; - -if (current_state == STATE_API) { +/* Each state will check if the member is initialized, falling directly to + * the next one if so. If not, the callback for the asynchronous call will + * be responsible for calling is function again with the next state as + * argument. + */ +switch (current_state + 1) { +case STATE_API: if (menu->priv->api == NULL) { ovirt_foreign_menu_fetch_api_async(menu); -} else { -current_state++; +break; } -} - -if (current_state == STATE_VM) { +case STATE_VM: if (menu->priv->vm == NULL) { ovirt_foreign_menu_fetch_vm_async(menu); -} else { -current_state++; +break; } -} - -if (current_state == STATE_STORAGE_DOMAIN) { +case STATE_STORAGE_DOMAIN: if (menu->priv->files == NULL) { ovirt_foreign_menu_fetch_storage_domain_async(menu); -} else { -current_state++; +break; } -} - -if (current_state == STATE_VM_CDROM) { +case STATE_VM_CDROM: if (menu->priv->cdrom == NULL) { ovirt_foreign_menu_fetch_vm_cdrom_async(menu); -} else { -current_state++; +break; } -} - -if (current_state == STATE_CDROM_FILE) { +case STATE_CDROM_FILE: ovirt_foreign_menu_refresh_cdrom_file_async(menu); -} - -if (current_state == STATE_ISOS) { +break; +case STATE_ISOS: g_warn_if_fail(menu->priv->api != NULL); g_warn_if_fail(menu->priv->vm != NULL); g_warn_if_fail(menu->priv->files != NULL); g_warn_if_fail(menu->priv->cdrom != NULL); ovirt_foreign_menu_refresh_iso_list(menu); +break; +default: +g_warn_if_reached(); } } -- 2.7.4 ___ 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 v2 06/12] ovirt-foreign-menu: Notify of new files even if nothing changed
When user presses the Refresh button in ISO dialog, the list is cleared, and currently, the only way it is informed of the new list is by the notify signal. The same applies when an error occurs while trying to change the current ISO. Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/ovirt-foreign-menu.c | 41 ++--- 1 file changed, 14 insertions(+), 27 deletions(-) diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c index 493ca47..d8c0553 100644 --- a/src/ovirt-foreign-menu.c +++ b/src/ovirt-foreign-menu.c @@ -375,22 +375,24 @@ static void updated_cdrom_cb(GObject *source_object, g_free(foreign_menu->priv->current_iso_name); foreign_menu->priv->current_iso_name = foreign_menu->priv->next_iso_name; foreign_menu->priv->next_iso_name = NULL; -g_object_notify(G_OBJECT(foreign_menu), "file"); -} else { -/* Reset old state back as we were not successful in switching to - * the new ISO */ -const char *current_file = foreign_menu->priv->current_iso_name; +goto end; +} -if (error != NULL) { -g_warning("failed to update cdrom resource: %s", error->message); -g_clear_error(); -} -g_debug("setting OvirtCdrom:file back to '%s'", -current_file?current_file:NULL); -g_object_set(foreign_menu->priv->cdrom, "file", current_file, NULL); +/* Reset old state back as we were not successful in switching to + * the new ISO */ +if (error != NULL) { +g_warning("failed to update cdrom resource: %s", error->message); +g_clear_error(); } +g_debug("setting OvirtCdrom:file back to '%s'", +foreign_menu->priv->current_iso_name); +g_object_set(foreign_menu->priv->cdrom, + "file", foreign_menu->priv->current_iso_name, + NULL); +end: g_clear_pointer(_menu->priv->next_iso_name, g_free); +g_object_notify(G_OBJECT(foreign_menu), "file"); } @@ -399,7 +401,6 @@ static void ovirt_foreign_menu_set_files(OvirtForeignMenu *menu, { GList *sorted_files = NULL; const GList *it; -GList *it2; for (it = files; it != NULL; it = it->next) { char *name; @@ -416,20 +417,6 @@ static void ovirt_foreign_menu_set_files(OvirtForeignMenu *menu, (GCompareFunc)g_strcmp0); } -for (it = sorted_files, it2 = menu->priv->iso_names; - (it != NULL) && (it2 != NULL); - it = it->next, it2 = it2->next) { -if (g_strcmp0(it->data, it2->data) != 0) { -break; -} -} - -if ((it == NULL) && (it2 == NULL)) { -/* sorted_files and menu->priv->files content was the same */ -g_list_free_full(sorted_files, (GDestroyNotify)g_free); -return; -} - g_list_free_full(menu->priv->iso_names, (GDestroyNotify)g_free); menu->priv->iso_names = sorted_files; g_object_notify(G_OBJECT(menu), "files"); -- 2.7.4 ___ 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 v2 05/12] ovirt-foreign-menu: Remove GtkMenu related functions
With this commit, we finish cleaning up ovirt foreign menu code, which only deals with data, leaving the UI bits to be handled properly in the new ISO list dialog. This patch also updates remote-viewer to reflect the change. Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/ovirt-foreign-menu.c | 79 src/remote-viewer.c | 52 +-- 2 files changed, 8 insertions(+), 123 deletions(-) diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c index eec6bc6..493ca47 100644 --- a/src/ovirt-foreign-menu.c +++ b/src/ovirt-foreign-menu.c @@ -359,22 +359,6 @@ ovirt_foreign_menu_start(OvirtForeignMenu *menu) } -static void -ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, gpointer user_data); - - -static void -menu_item_set_active_no_signal(GtkMenuItem *menuitem, - gboolean active, - GCallback callback, - gpointer user_data) -{ -g_signal_handlers_block_by_func(menuitem, callback, user_data); -gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(menuitem), active); -g_signal_handlers_unblock_by_func(menuitem, callback, user_data); -} - - static void updated_cdrom_cb(GObject *source_object, GAsyncResult *result, gpointer user_data) @@ -410,69 +394,6 @@ static void updated_cdrom_cb(GObject *source_object, } -static void -ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, gpointer user_data) -{ -OvirtForeignMenu *foreign_menu; -const char *iso_name = NULL; -gboolean checked; - -checked = gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(menuitem)); -foreign_menu = OVIRT_FOREIGN_MENU(user_data); -g_return_if_fail(foreign_menu->priv->cdrom != NULL); -g_return_if_fail(foreign_menu->priv->next_iso_name == NULL); - -g_debug("'%s' clicked", gtk_menu_item_get_label(menuitem)); - -/* We only want to move the check mark for the currently selected ISO - * when ovirt_cdrom_update_async() is successful, so for now we move - * the check mark back to where it was before - */ -menu_item_set_active_no_signal(menuitem, !checked, - (GCallback)ovirt_foreign_menu_activate_item_cb, - foreign_menu); - -if (checked) { -iso_name = gtk_menu_item_get_label(menuitem); -} - -ovirt_foreign_menu_set_current_iso_name(foreign_menu, iso_name); -} - - -GtkWidget *ovirt_foreign_menu_get_gtk_menu(OvirtForeignMenu *foreign_menu) -{ -GtkWidget *gtk_menu; -GList *it; -char *current_iso; - -if (foreign_menu->priv->iso_names == NULL) { -g_debug("ISO list is empty, no menu to show"); -return NULL; -} -g_debug("Creating GtkMenu for foreign menu"); -current_iso = ovirt_foreign_menu_get_current_iso_name(foreign_menu); -gtk_menu = gtk_menu_new(); -for (it = foreign_menu->priv->iso_names; it != NULL; it = it->next) { -GtkWidget *menuitem; - -menuitem = gtk_check_menu_item_new_with_label((char *)it->data); -if (g_strcmp0((char *)it->data, current_iso) == 0) { -g_warn_if_fail(g_strcmp0(current_iso, foreign_menu->priv->current_iso_name) == 0); -gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(menuitem), - TRUE); -} -g_signal_connect(menuitem, "activate", - G_CALLBACK(ovirt_foreign_menu_activate_item_cb), - foreign_menu); -gtk_menu_shell_append(GTK_MENU_SHELL(gtk_menu), menuitem); -} -g_free(current_iso); - -return gtk_menu; -} - - static void ovirt_foreign_menu_set_files(OvirtForeignMenu *menu, const GList *files) { diff --git a/src/remote-viewer.c b/src/remote-viewer.c index 6d29bf2..95130dc 100644 --- a/src/remote-viewer.c +++ b/src/remote-viewer.c @@ -734,33 +734,11 @@ authenticate_cb(RestProxy *proxy, G_GNUC_UNUSED RestProxyAuth *auth, static void ovirt_foreign_menu_update(GtkApplication *gtkapp, GtkWindow *gtkwin, G_GNUC_UNUSED gpointer data) { -RemoteViewer *app = REMOTE_VIEWER(gtkapp); +RemoteViewer *self = REMOTE_VIEWER(gtkapp); VirtViewerWindow *win = g_object_get_data(G_OBJECT(gtkwin), "virt-viewer-window"); -GtkWidget *menu = g_object_get_data(G_OBJECT(win), "foreign-menu"); -GtkWidget *submenu; - -if (app->priv->ovirt_foreign_menu == NULL) { -/* nothing to do */ -return; -} - -submenu = ovirt_foreign_menu_get_gtk_menu(app->priv->ovirt_foreign_menu); -if (submenu == NULL) { -/* No items to show, no point in showing the menu */ -if (menu != NULL) -
[virt-tools-list] [PATCH virt-viewer v2 08/12] Introduce ISO List dialog
Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- po/POTFILES.in | 2 + src/Makefile.am| 3 + src/remote-viewer-iso-list-dialog.c| 112 + src/remote-viewer-iso-list-dialog.h| 58 +++ src/resources/ui/remote-viewer-iso-list.ui | 155 + src/resources/virt-viewer.gresource.xml| 1 + 6 files changed, 331 insertions(+) create mode 100644 src/remote-viewer-iso-list-dialog.c create mode 100644 src/remote-viewer-iso-list-dialog.h create mode 100644 src/resources/ui/remote-viewer-iso-list.ui diff --git a/po/POTFILES.in b/po/POTFILES.in index 6775f53..54de445 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -1,9 +1,11 @@ data/remote-viewer.appdata.xml.in data/remote-viewer.desktop.in data/virt-viewer-mime.xml.in +src/remote-viewer-iso-list-dialog.c src/remote-viewer-main.c src/remote-viewer.c [type: gettext/glade] src/resources/ui/remote-viewer-connect.ui +[type: gettext/glade] src/resources/ui/remote-viewer-iso-list.ui [type: gettext/glade] src/resources/ui/virt-viewer-about.ui src/virt-viewer-app.c src/virt-viewer-auth.c diff --git a/src/Makefile.am b/src/Makefile.am index 0c48e40..66a73f8 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -13,6 +13,7 @@ noinst_DATA = \ resources/ui/virt-viewer-vm-connection.ui \ resources/ui/virt-viewer-preferences.ui \ resources/ui/remote-viewer-connect.ui \ + resources/ui/remote-viewer-iso-list.ui \ $(NULL) EXTRA_DIST = \ @@ -96,6 +97,8 @@ if HAVE_OVIRT libvirt_viewer_la_SOURCES += \ ovirt-foreign-menu.h \ ovirt-foreign-menu.c \ + remote-viewer-iso-list-dialog.c \ + remote-viewer-iso-list-dialog.h \ $(NULL) endif diff --git a/src/remote-viewer-iso-list-dialog.c b/src/remote-viewer-iso-list-dialog.c new file mode 100644 index 000..a319adb --- /dev/null +++ b/src/remote-viewer-iso-list-dialog.c @@ -0,0 +1,112 @@ +/* + * Virt Viewer: A virtual machine console viewer + * + * Copyright (C) 2016 Red Hat, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#include + +#include + +#include "remote-viewer-iso-list-dialog.h" +#include "virt-viewer-util.h" + +G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, GTK_TYPE_DIALOG) + +#define DIALOG_PRIVATE(o) \ +(G_TYPE_INSTANCE_GET_PRIVATE((o), REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG, RemoteViewerISOListDialogPrivate)) + +struct _RemoteViewerISOListDialogPrivate +{ +GtkWidget *stack; +}; + +enum RemoteViewerIsoListDialogPages +{ +STATUS_PAGE = 0, +ISO_LIST_PAGE, +}; + +static void +remote_viewer_iso_list_dialog_dispose(GObject *object) +{ + G_OBJECT_CLASS(remote_viewer_iso_list_dialog_parent_class)->dispose(object); +} + +static void +remote_viewer_iso_list_dialog_class_init(RemoteViewerISOListDialogClass *klass) +{ +GObjectClass *object_class = G_OBJECT_CLASS(klass); + +g_type_class_add_private(klass, sizeof(RemoteViewerISOListDialogPrivate)); + +object_class->dispose = remote_viewer_iso_list_dialog_dispose; +} + +static void +remote_viewer_iso_list_dialog_response(GtkDialog *dialog, + gint response_id, + gpointer user_data G_GNUC_UNUSED) +{ +RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog); +RemoteViewerISOListDialogPrivate *priv = self->priv; + +if (response_id != GTK_RESPONSE_NONE) +return; + +gtk_stack_set_visible_child_full(GTK_STACK(priv->stack), "status", + GTK_STACK_TRANSITION_TYPE_SLIDE_LEFT); +gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, FALSE); +gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_CLOSE, FALSE); +} + +static void +remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self) +{ +GtkWidget *content = gtk_dialog_get_content_area(GTK_DIALOG(self)); +RemoteViewerISOListDialogPrivate *priv = self->priv = DIALOG_PRIVATE(self); +GtkBuilder *builder = virt_viewer_util_load_ui("remote-viewer-iso-list.ui"); + +g
[virt-tools-list] [PATCH virt-viewer v2 04/12] ovirt-foreign-menu: Add accessors for current iso and iso list
Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/ovirt-foreign-menu.c | 49 ++-- src/ovirt-foreign-menu.h | 5 - 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c index 565ef22..eec6bc6 100644 --- a/src/ovirt-foreign-menu.c +++ b/src/ovirt-foreign-menu.c @@ -47,6 +47,7 @@ static void ovirt_foreign_menu_fetch_storage_domain_async(OvirtForeignMenu *menu static void ovirt_foreign_menu_fetch_vm_cdrom_async(OvirtForeignMenu *menu); static void ovirt_foreign_menu_refresh_cdrom_file_async(OvirtForeignMenu *menu); static void ovirt_foreign_menu_fetch_iso_list_async(OvirtForeignMenu *menu); +static void updated_cdrom_cb(GObject *source_object, GAsyncResult *result, gpointer user_data); G_DEFINE_TYPE (OvirtForeignMenu, ovirt_foreign_menu, G_TYPE_OBJECT) @@ -85,7 +86,7 @@ enum { }; -static char * +char * ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *foreign_menu) { char *name; @@ -100,6 +101,36 @@ ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *foreign_menu) } +void +ovirt_foreign_menu_set_current_iso_name(OvirtForeignMenu *foreign_menu, char *name) +{ +g_return_if_fail(foreign_menu->priv->cdrom != NULL); +g_return_if_fail(foreign_menu->priv->next_iso_name == NULL); + +if (name) { +g_debug("Updating VM cdrom image to '%s'", name); +foreign_menu->priv->next_iso_name = g_strdup(name); +} else { +g_debug("Removing current cdrom image"); +foreign_menu->priv->next_iso_name = NULL; +} + +g_object_set(foreign_menu->priv->cdrom, + "file", name, + NULL); +ovirt_cdrom_update_async(foreign_menu->priv->cdrom, TRUE, + foreign_menu->priv->proxy, NULL, + updated_cdrom_cb, foreign_menu); +} + + +GList* +ovirt_foreign_menu_get_iso_names(OvirtForeignMenu *foreign_menu) +{ +return foreign_menu->priv->iso_names; +} + + static void ovirt_foreign_menu_get_property(GObject *object, guint property_id, GValue *value, GParamSpec *pspec) @@ -383,7 +414,7 @@ static void ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, gpointer user_data) { OvirtForeignMenu *foreign_menu; -const char *iso_name; +const char *iso_name = NULL; gboolean checked; checked = gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(menuitem)); @@ -403,19 +434,9 @@ ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, gpointer user_data) if (checked) { iso_name = gtk_menu_item_get_label(menuitem); -g_debug("Updating VM cdrom image to '%s'", iso_name); -foreign_menu->priv->next_iso_name = g_strdup(iso_name); -} else { -g_debug("Removing current cdrom image"); -iso_name = NULL; -foreign_menu->priv->next_iso_name = NULL; } -g_object_set(foreign_menu->priv->cdrom, - "file", iso_name, - NULL); -ovirt_cdrom_update_async(foreign_menu->priv->cdrom, TRUE, - foreign_menu->priv->proxy, NULL, - updated_cdrom_cb, foreign_menu); + +ovirt_foreign_menu_set_current_iso_name(foreign_menu, iso_name); } diff --git a/src/ovirt-foreign-menu.h b/src/ovirt-foreign-menu.h index cf18b52..f1a1ddb 100644 --- a/src/ovirt-foreign-menu.h +++ b/src/ovirt-foreign-menu.h @@ -70,7 +70,10 @@ OvirtForeignMenu* ovirt_foreign_menu_new(OvirtProxy *proxy); OvirtForeignMenu *ovirt_foreign_menu_new_from_file(VirtViewerFile *self); void ovirt_foreign_menu_start(OvirtForeignMenu *menu); -GtkWidget *ovirt_foreign_menu_get_gtk_menu(OvirtForeignMenu *foreign_menu); +char *ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *menu); +void ovirt_foreign_menu_set_current_iso_name(OvirtForeignMenu *menu, char *name); + +GList *ovirt_foreign_menu_get_iso_names(OvirtForeignMenu *menu); G_END_DECLS -- 2.7.4 ___ 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 v2 00/12] Replace oVirt foreign menu with dedicated dialog
New version of the ISO list dialog, with changes suggested in first round of reviews and lots of bug fixes. Before sending v1 I had refactored a few bits and it introduced some serious crashes. On the UI side, we now use GtkStack instead of GtkNotebook and the last commit of the series also makes use of more modern looking GtkHeaderBar. It has been kept as a separate commit because it requires newer version of Gtk+ than the one which is currently required. In progress: - Use GtkListBox instead of GtkTreeView. - Rework OvirtForeignMenu/ISOListDialog so that the async API is exposed, making it more robust in case of errors. Eduardo Lima (Etrunko) (12): ovirt-foreign-menu: Rework states logic ovirt-foreign-menu: Use g_clear_pointer/g_clear_object ovirt-foreign-menu: Remove timer used to refresh iso list ovirt-foreign-menu: Add accessors for current iso and iso list ovirt-foreign-menu: Remove GtkMenu related functions ovirt-foreign-menu: Notify of new files even if nothing changed UI: Make 'Change CD' menu item a submenu under 'File' toplevel menu Introduce ISO List dialog Run iso-dialog when 'Change CD' menu is activated remote-viewer: Make ovirt-foreign-menu a property iso-dialog: Implement functionality provided by oVirt foreign menu iso-dialog: Use header bar for buttons configure.ac | 4 +- po/POTFILES.in | 2 + src/Makefile.am| 3 + src/ovirt-foreign-menu.c | 292 src/ovirt-foreign-menu.h | 5 +- src/remote-viewer-iso-list-dialog.c| 301 + src/remote-viewer-iso-list-dialog.h| 58 ++ src/remote-viewer.c| 89 - src/resources/ui/remote-viewer-iso-list.ui | 158 +++ src/resources/ui/virt-viewer.ui| 19 +- src/resources/virt-viewer.gresource.xml| 1 + src/virt-viewer-window.c | 37 12 files changed, 702 insertions(+), 267 deletions(-) create mode 100644 src/remote-viewer-iso-list-dialog.c create mode 100644 src/remote-viewer-iso-list-dialog.h create mode 100644 src/resources/ui/remote-viewer-iso-list.ui -- 2.7.4 ___ 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 v3 00/10] Replace oVirt foreign menu with dedicated dialog
On 08/02/2016 01:17 PM, Christophe Fergeau wrote: > On Fri, Jul 29, 2016 at 06:40:23PM -0300, Eduardo Lima (Etrunko) wrote: >> I have pushed the first two patches of the series because they were >> already acknowledged and were pretty much self-contained. >> >> I tried to replace GtkTreeView in favor of new GtkListBox, but after some >> painful work, I decided to drop it because it is not possible to >> reproduce the exact same behavior of the former with the latter. For >> instance, once we have a GtkRadioButton activated, it is not possible to >> deactivate it. > > For what it's worth, I'm not sure we should keep GtkRadioButtons at all > cost in the list, imo they look not so nice. Dunno if it would be > possibe to have either empty column or check mark instead of a > GtkRadioButton. > Well, I am no UI expert, but it seems to me that radio buttons are the best choice to express the behavior of the list, exactly 1 or 0 item can be active at any given time. With check buttons, user is given the impression that more than once can be selected, which is not the case. All in all it is a one line change to show check buttons instead of radio buttons. >> >> In this version: >> >> * Removed leftover enum when changed from GtkNotebook to GtkStack >> * Some cosmetic fixes (indentation, renaming, etc) >> * UI Tweaks: >>- Added some padding between items of the list. >>- Set tree view selection to current ISO when the list is first loaded >> or refreshed. >>- Handle tree view "activate" signal the same way as radio button >> toggle >>- Removed "Select ISO" label and alignment in header bar patch. >> >> Eduardo Lima (Etrunko) (10): >> ovirt-foreign-menu: Remove timer used to refresh iso list >> ovirt-foreign-menu: Add accessors for current iso and iso list >> ovirt-foreign-menu: Remove GtkMenu related functions >> ovirt-foreign-menu: Notify of new files even if nothing changed >> UI: Make 'Change CD' menu item a submenu under 'File' toplevel menu >> Introduce ISO List dialog >> Run iso-dialog when 'Change CD' menu is activated >> remote-viewer: Make ovirt-foreign-menu a property >> iso-dialog: Implement functionality provided by oVirt foreign menu >> iso-dialog: Use header bar for buttons > > I'm not sure how to approach this series as I don't think > 'ovirt-foreign-menu: Add accessors for current iso and iso list' > 'ovirt-foreign-menu: Notify of new files even if nothing changed' > are needed if you want to have proper error handling in the dialog. > So I don't think I'd have much more to add compared to the first > iteration of the review. Or are there specific changes you want me to > look at? I have been busy with other things at the moment, and I could not finish the work here. The idea was to gather comments about UI and behavior in general, as the new changes will not affect the appearance of the dialog. So it seems it is looking good, apart from the Radio/Check buttons decision. -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com signature.asc Description: OpenPGP digital signature ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
[virt-tools-list] [PATCH] Adjust timer to refresh ovirt foreign menu
This is a temporary solution, as discussed in the bug. We will adjust the timer to refresh the ISO list from 15 seconds to 5 minutes (300 seconds), while reworking in the UI to replace the menu with a dialog, which seems a saner way to display the list. Resolves: rhbz#1347726 Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/ovirt-foreign-menu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c index 2d286fb..33ff4f1 100644 --- a/src/ovirt-foreign-menu.c +++ b/src/ovirt-foreign-menu.c @@ -797,7 +797,7 @@ static void iso_list_fetched_cb(GObject *source_object, ovirt_foreign_menu_set_files(OVIRT_FOREIGN_MENU(user_data), files); g_list_free(files); -g_timeout_add_seconds(15, ovirt_foreign_menu_refresh_iso_list, user_data); +g_timeout_add_seconds(300, ovirt_foreign_menu_refresh_iso_list, user_data); } -- 2.5.5 ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [PATCH] Adjust timer to refresh ovirt foreign menu
On 06/30/2016 09:03 AM, Fabiano Fidêncio wrote: > On Thu, Jun 30, 2016 at 2:01 PM, Eduardo Lima (Etrunko) > <etru...@redhat.com> wrote: >> This is a temporary solution, as discussed in the bug. We will adjust >> the timer to refresh the ISO list from 15 seconds to 5 minutes (300 >> seconds), while reworking in the UI to replace the menu with a dialog, >> which seems a saner way to display the list. >> >> Resolves: rhbz#1347726 >> >> Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> >> --- >> src/ovirt-foreign-menu.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c >> index 2d286fb..33ff4f1 100644 >> --- a/src/ovirt-foreign-menu.c >> +++ b/src/ovirt-foreign-menu.c >> @@ -797,7 +797,7 @@ static void iso_list_fetched_cb(GObject *source_object, >> ovirt_foreign_menu_set_files(OVIRT_FOREIGN_MENU(user_data), files); >> g_list_free(files); >> >> -g_timeout_add_seconds(15, ovirt_foreign_menu_refresh_iso_list, >> user_data); >> +g_timeout_add_seconds(300, ovirt_foreign_menu_refresh_iso_list, >> user_data); >> } >> >> >> -- >> 2.5.5 >> >> ___ >> virt-tools-list mailing list >> virt-tools-list@redhat.com >> https://www.redhat.com/mailman/listinfo/virt-tools-list > > > Acked-by: Fabiano Fidêncio <fiden...@redhat.com> Thanks, pushed. -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [PATCH v2 virt-viewer] Get rid of deprecated functions to customize widget colors
On 06/29/2016 05:08 AM, Pavel Grunt wrote: > Hi, > > Imo the commit message should say that there are issues with the font / > background colour since commit ... in some gtk versions - like you said in the > comment for v1, because that is the reason for this patch. Getting rid of > deprecated functions is a "side effect". > Well, if you notice the previous email, the message was after the line starting with '---' meaning it would be ignored. I wanted to explain why I was doing it only for 3.20 and thought it would not matter in the commit message. Anyway, I will add more information to the commit message and push it. -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [PATCH] timed-revealer: listen to the "grab-notify" signal
On 06/29/2016 10:36 AM, Fabiano Fidêncio wrote: > The "grab-notify" signal lets us know when our widget becomes > shadowed by a Gtk+ grab on another widget, or when it becomes unshadowed > due to a grab being removed. > > That's exactly the case we face when dealing with "usb-redirection" and > "close" items of the fullscreen toolbar. And, currently, when these > widgets get closed the timed-revealer stays there forever. From now on > let's take advantage of the "grab-notify" signal and schedule a timeout > for the revealer when these widgets' windows get closed. > Reading the docs about the 'grab-notify' signal, it indeed seems the right thing to do (TM), but would be nice to have Christophe opinion here too, as he was involved in reviewing the refactor of this widget. Regards, Eduardo. -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ 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] iso-dialog: Avoid crash when closing dialog early
We must take into account that users can close the dialog at anytime, even during an operation of fetch or set ISO has not been finished. This will cause the callbacks for those operations to be invoked with an invalid object, crashing the application. To fix this issue we need to pass a GCancellable to the asynchronous operations, so they can be cancelled if the dialog happens to get closed before they complete. Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- v2: * Move call to g_cancellable_cancel() to response handler. * Don't leak error objects if operation is cancelled. --- src/remote-viewer-iso-list-dialog.c | 42 ++--- 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/src/remote-viewer-iso-list-dialog.c b/src/remote-viewer-iso-list-dialog.c index f23ddb2..aa0ebbb 100644 --- a/src/remote-viewer-iso-list-dialog.c +++ b/src/remote-viewer-iso-list-dialog.c @@ -42,6 +42,7 @@ struct _RemoteViewerISOListDialogPrivate GtkWidget *stack; GtkWidget *tree_view; OvirtForeignMenu *foreign_menu; +GCancellable *cancellable; }; enum RemoteViewerISOListDialogModel @@ -66,6 +67,8 @@ remote_viewer_iso_list_dialog_dispose(GObject *object) RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object); RemoteViewerISOListDialogPrivate *priv = self->priv; +g_clear_object(>cancellable); + if (priv->foreign_menu) { g_signal_handlers_disconnect_by_data(priv->foreign_menu, object); g_clear_object(>foreign_menu); @@ -157,17 +160,24 @@ fetch_iso_names_cb(OvirtForeignMenu *foreign_menu, const gchar *msg = error ? error->message : _("Failed to fetch CD names"); gchar *markup = g_strdup_printf("%s", msg); +g_debug("Error fetching ISO names: %s", msg); +if (error && error->code == G_IO_ERROR_CANCELLED) +goto end; + gtk_label_set_markup(GTK_LABEL(priv->status), markup); gtk_spinner_stop(GTK_SPINNER(priv->spinner)); remote_viewer_iso_list_dialog_show_error(self, msg); gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, TRUE); g_free(markup); -g_clear_error(); -return; +goto end; } +g_clear_object(>cancellable); g_list_foreach(iso_list, (GFunc) remote_viewer_iso_list_dialog_foreach, self); remote_viewer_iso_list_dialog_show_files(self); + +end: +g_clear_error(); } @@ -177,7 +187,10 @@ remote_viewer_iso_list_dialog_refresh_iso_list(RemoteViewerISOListDialog *self) RemoteViewerISOListDialogPrivate *priv = self->priv; gtk_list_store_clear(priv->list_store); -ovirt_foreign_menu_fetch_iso_names_async(priv->foreign_menu, NULL, + +priv->cancellable = g_cancellable_new(); +ovirt_foreign_menu_fetch_iso_names_async(priv->foreign_menu, + priv->cancellable, (GAsyncReadyCallback) fetch_iso_names_cb, self); } @@ -190,8 +203,10 @@ remote_viewer_iso_list_dialog_response(GtkDialog *dialog, RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog); RemoteViewerISOListDialogPrivate *priv = self->priv; -if (response_id != GTK_RESPONSE_NONE) +if (response_id != GTK_RESPONSE_NONE) { +g_cancellable_cancel(priv->cancellable); return; +} gtk_spinner_start(GTK_SPINNER(priv->spinner)); gtk_label_set_markup(GTK_LABEL(priv->status), _("Loading...")); @@ -223,7 +238,9 @@ remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle *cell_renderer G_GNU gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, FALSE); gtk_widget_set_sensitive(priv->tree_view, FALSE); -ovirt_foreign_menu_set_current_iso_name_async(priv->foreign_menu, active ? NULL : name, NULL, +priv->cancellable = g_cancellable_new(); +ovirt_foreign_menu_set_current_iso_name_async(priv->foreign_menu, active ? NULL : name, + priv->cancellable, (GAsyncReadyCallback)ovirt_foreign_menu_iso_name_changed, self); gtk_tree_path_free(tree_path); @@ -308,12 +325,18 @@ ovirt_foreign_menu_iso_name_changed(OvirtForeignMenu *foreign_menu, * change the ISO back to the original, avoiding a possible inconsistency. */ if (!ovirt_foreign_menu_set_current_iso_name_finish(foreign_menu, result, )) { -remote_viewer_iso_list_dialog_show_error(self, error ? error->message : _("Failed to change CD")); -g_clear_error(); +gchar *msg = error ? error->message : _("Fail
Re: [virt-tools-list] [PATCH virt-viewer] iso-dialog: Do not use string directly
Acked-by: Eduardo Lima (Etrunko) <etru...@redhat.com> On 06/02/17 10:02, Pavel Grunt wrote: > Fixes -Werror=format-security used when creating the rpm > --- > see: > https://copr-be.cloud.fedoraproject.org/results/pgrunt/spice-upstream/fedora-25-x86_64/00507878-virt-viewer/build.log.gz > --- > src/remote-viewer-iso-list-dialog.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/remote-viewer-iso-list-dialog.c > b/src/remote-viewer-iso-list-dialog.c > index f23ddb2..2ab5435 100644 > --- a/src/remote-viewer-iso-list-dialog.c > +++ b/src/remote-viewer-iso-list-dialog.c > @@ -286,7 +286,7 @@ > remote_viewer_iso_list_dialog_show_error(RemoteViewerISOListDialog *self, > GTK_DIALOG_DESTROY_WITH_PARENT, > GTK_MESSAGE_ERROR, > GTK_BUTTONS_CLOSE, > -message ? message : _("Unspecified > error")); > +"%s", message ? message : _("Unspecified > error")); > gtk_dialog_run(GTK_DIALOG(dialog)); > gtk_widget_destroy(dialog); > } > -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-viewer PATCH] Show errors generated by connection dialog
On 08/02/17 14:48, Christophe de Dinechin wrote: > When running 'remote-viewer' without arguments, > and selecting a non-supported protocol, e.g. ssh://foo, > the generated error was silently swallowed by the retry loop. > This was introduced in 06b2c382468876a19600374bd59475e66d488af8. > --- > src/remote-viewer.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/src/remote-viewer.c b/src/remote-viewer.c > index 13c6e7c..c9ef4bb 100644 > --- a/src/remote-viewer.c > +++ b/src/remote-viewer.c > @@ -1196,6 +1196,9 @@ cleanup: > type = NULL; > > if (!ret && priv->open_recent_dialog) { > +if (error) { > +virt_viewer_app_simple_message_dialog(>parent, "%s", > error->message); > +} Patch looks good, I personally also like to check for error->message before dereferencing it, but I don't really know what others think about it. Reviewed-by: Eduardo Lima (Etrunko) <etru...@redhat.com> -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ 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] iso-dialog: Avoid crash when closing dialog early
On 03/02/17 14:34, Christophe Fergeau wrote: > Hey, > > On Thu, Jan 26, 2017 at 06:01:23PM -0200, Eduardo Lima (Etrunko) wrote: >> We must take into account that users can close the dialog at anytime, >> even during an operation of fetch or set ISO has not been finished. This >> will cause the callbacks for those operations to be invoked with an >> invalid object, crashing the application. >> >> To fix this issue we need to pass a GCancellable to the asynchronous >> operations, so they can be cancelled if the dialog happens to get closed >> before they complete. >> > > This is going to depend on libgovirt bug > https://bugzilla.gnome.org/show_bug.cgi?id=777808 being fixed, otherwise > we'd be getting a deadlock, is that correct? > Yes, this patch depends on the fix in libgovirt. >> Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> >> --- >> src/remote-viewer-iso-list-dialog.c | 30 +++--- >> 1 file changed, 27 insertions(+), 3 deletions(-) >> >> diff --git a/src/remote-viewer-iso-list-dialog.c >> b/src/remote-viewer-iso-list-dialog.c >> index f23ddb2..a7ac98a 100644 >> --- a/src/remote-viewer-iso-list-dialog.c >> +++ b/src/remote-viewer-iso-list-dialog.c >> @@ -42,6 +42,7 @@ struct _RemoteViewerISOListDialogPrivate >> GtkWidget *stack; >> GtkWidget *tree_view; >> OvirtForeignMenu *foreign_menu; >> +GCancellable *cancellable; >> }; >> >> enum RemoteViewerISOListDialogModel >> @@ -66,10 +67,16 @@ remote_viewer_iso_list_dialog_dispose(GObject *object) >> RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object); >> RemoteViewerISOListDialogPrivate *priv = self->priv; >> >> +if (priv->cancellable) { >> +g_cancellable_cancel(priv->cancellable); >> +g_clear_object(>cancellable); >> +} >> + > > g_cancellable_cancel() can be async, and at this point, the iso dialog > is already dead, are you sure it's safe to cancel here? I am not sure, thinking better about it, it may be safer to cancel on the response signal, or maybe by the close signal. > >> if (priv->foreign_menu) { >> g_signal_handlers_disconnect_by_data(priv->foreign_menu, object); >> g_clear_object(>foreign_menu); >> } >> + > > I'd drop this added blank line Oops, slipped in. > >> >> G_OBJECT_CLASS(remote_viewer_iso_list_dialog_parent_class)->dispose(object); >> } >> >> @@ -157,6 +164,10 @@ fetch_iso_names_cb(OvirtForeignMenu *foreign_menu, >> const gchar *msg = error ? error->message : _("Failed to fetch CD >> names"); >> gchar *markup = g_strdup_printf("%s", msg); >> >> +g_debug("Error fetching ISO names: %s", msg); >> +if (error && error->code == G_IO_ERROR_CANCELLED) >> +return; >> + > > 'error' is leaked. Fixed. > >> gtk_label_set_markup(GTK_LABEL(priv->status), markup); >> gtk_spinner_stop(GTK_SPINNER(priv->spinner)); >> remote_viewer_iso_list_dialog_show_error(self, msg); >> @@ -166,6 +177,7 @@ fetch_iso_names_cb(OvirtForeignMenu *foreign_menu, >> return; >> } >> >> +g_clear_object(>cancellable); >> g_list_foreach(iso_list, (GFunc) remote_viewer_iso_list_dialog_foreach, >> self); >> remote_viewer_iso_list_dialog_show_files(self); >> } >> @@ -177,7 +189,10 @@ >> remote_viewer_iso_list_dialog_refresh_iso_list(RemoteViewerISOListDialog >> *self) >> RemoteViewerISOListDialogPrivate *priv = self->priv; >> >> gtk_list_store_clear(priv->list_store); >> -ovirt_foreign_menu_fetch_iso_names_async(priv->foreign_menu, NULL, >> + >> +priv->cancellable = g_cancellable_new(); >> +ovirt_foreign_menu_fetch_iso_names_async(priv->foreign_menu, >> + priv->cancellable, >> (GAsyncReadyCallback) >> fetch_iso_names_cb, >> self); >> } >> @@ -223,7 +238,9 @@ >> remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle *cell_renderer >> G_GNU >> gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, >> FALSE); >> gtk_widget_set_sensitive(priv->tree_view, FALSE); >> >> -ovirt_foreign_menu_set_cur
Re: [virt-tools-list] [PATCH virt-viewer 1/5] ovirt-foreign-menu: Add accessors for current iso and iso list
On 20/01/17 13:16, Christophe Fergeau wrote: > > Acked-by: Christophe Fergeau <cferg...@redhat.com> Pushed, thanks. > > On Thu, Jan 19, 2017 at 01:42:10PM -0200, Eduardo Lima (Etrunko) wrote: >> Also, to keep consistency around the codebase, changed the return value >> of ovirt_foreign_menu_get_current_iso_name() from char * to gchar *. >> >> Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> >> --- >> src/ovirt-foreign-menu.c | 11 +-- >> src/ovirt-foreign-menu.h | 2 ++ >> 2 files changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c >> index 8a99665..ef3ddd9 100644 >> --- a/src/ovirt-foreign-menu.c >> +++ b/src/ovirt-foreign-menu.c >> @@ -85,10 +85,10 @@ enum { >> }; >> >> >> -static char * >> +gchar * >> ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *foreign_menu) >> { >> -char *name; >> +gchar *name; >> >> if (foreign_menu->priv->cdrom == NULL) { >> return NULL; >> @@ -100,6 +100,13 @@ >> ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *foreign_menu) >> } >> >> >> +GList* >> +ovirt_foreign_menu_get_iso_names(OvirtForeignMenu *foreign_menu) >> +{ >> +return foreign_menu->priv->iso_names; >> +} >> + >> + >> static void >> ovirt_foreign_menu_get_property(GObject *object, guint property_id, >> GValue *value, GParamSpec *pspec) >> diff --git a/src/ovirt-foreign-menu.h b/src/ovirt-foreign-menu.h >> index a864a60..340201f 100644 >> --- a/src/ovirt-foreign-menu.h >> +++ b/src/ovirt-foreign-menu.h >> @@ -88,6 +88,8 @@ gboolean >> ovirt_foreign_menu_set_current_iso_name_finish(OvirtForeignMenu *foreig >> >> >> GtkWidget *ovirt_foreign_menu_get_gtk_menu(OvirtForeignMenu *foreign_menu); >> +gchar *ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *menu); >> +GList *ovirt_foreign_menu_get_iso_names(OvirtForeignMenu *menu); >> >> G_END_DECLS >> >> -- >> 2.9.3 >> >> ___ >> virt-tools-list mailing list >> virt-tools-list@redhat.com >> https://www.redhat.com/mailman/listinfo/virt-tools-list -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com signature.asc Description: OpenPGP digital signature ___ 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 2/5] Introduce ISO List dialog
On 20/01/17 13:20, Christophe Fergeau wrote: > Can you expand a bit in the commit log about what this iso dialog is, > what is your goal for introducing it, .. ? Huge commit with shortlog > does not look good ;) > Alright, it will be done for the next version. > On Thu, Jan 19, 2017 at 01:42:11PM -0200, Eduardo Lima (Etrunko) wrote: >> Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> >> --- >> po/POTFILES.in | 2 + >> src/Makefile.am| 3 + >> src/remote-viewer-iso-list-dialog.c| 365 >> + >> src/remote-viewer-iso-list-dialog.h| 58 + >> src/resources/ui/remote-viewer-iso-list.ui | 158 + >> src/resources/virt-viewer.gresource.xml| 1 + >> 6 files changed, 587 insertions(+) >> create mode 100644 src/remote-viewer-iso-list-dialog.c >> create mode 100644 src/remote-viewer-iso-list-dialog.h >> create mode 100644 src/resources/ui/remote-viewer-iso-list.ui >> >> diff --git a/po/POTFILES.in b/po/POTFILES.in >> index 69d9fef..371c242 100644 >> --- a/po/POTFILES.in >> +++ b/po/POTFILES.in >> @@ -1,9 +1,11 @@ >> data/remote-viewer.appdata.xml.in >> data/remote-viewer.desktop.in >> data/virt-viewer-mime.xml.in >> +src/remote-viewer-iso-list-dialog.c >> src/remote-viewer-main.c >> src/remote-viewer.c >> [type: gettext/glade] src/resources/ui/remote-viewer-connect.ui >> +[type: gettext/glade] src/resources/ui/remote-viewer-iso-list.ui >> [type: gettext/glade] src/resources/ui/virt-viewer-about.ui >> src/virt-viewer-app.c >> src/virt-viewer-auth.c >> diff --git a/src/Makefile.am b/src/Makefile.am >> index 272c4ff..9748277 100644 >> --- a/src/Makefile.am >> +++ b/src/Makefile.am >> @@ -13,6 +13,7 @@ noinst_DATA = \ >> resources/ui/virt-viewer-vm-connection.ui \ >> resources/ui/virt-viewer-preferences.ui \ >> resources/ui/remote-viewer-connect.ui \ >> +resources/ui/remote-viewer-iso-list.ui \ >> resources/ui/virt-viewer-file-transfer-dialog.ui \ >> $(NULL) >> >> @@ -97,6 +98,8 @@ if HAVE_OVIRT >> libvirt_viewer_la_SOURCES += \ >> ovirt-foreign-menu.h \ >> ovirt-foreign-menu.c \ >> +remote-viewer-iso-list-dialog.c \ >> +remote-viewer-iso-list-dialog.h \ >> $(NULL) >> endif >> >> diff --git a/src/remote-viewer-iso-list-dialog.c >> b/src/remote-viewer-iso-list-dialog.c >> new file mode 100644 >> index 000..bf7c6c7 >> --- /dev/null >> +++ b/src/remote-viewer-iso-list-dialog.c >> @@ -0,0 +1,365 @@ >> +/* >> + * Virt Viewer: A virtual machine console viewer >> + * >> + * Copyright (C) 2016 Red Hat, Inc. >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program; if not, write to the Free Software >> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA >> + */ >> + >> +#include >> + >> +#include >> + >> +#include "remote-viewer-iso-list-dialog.h" >> +#include "virt-viewer-util.h" >> +#include "ovirt-foreign-menu.h" >> + >> +static void ovirt_foreign_menu_iso_name_changed(OvirtForeignMenu >> *foreign_menu, GAsyncResult *result, RemoteViewerISOListDialog *self); >> +static void >> remote_viewer_iso_list_dialog_show_error(RemoteViewerISOListDialog *self, >> const gchar *message); >> + >> +G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, >> GTK_TYPE_DIALOG) >> + >> +#define DIALOG_PRIVATE(o) \ >> +(G_TYPE_INSTANCE_GET_PRIVATE((o), >> REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG, RemoteViewerISOListDialogPrivate)) >> + >> +struct _RemoteViewerISOListDialogPrivate >> +{ >> +GtkListStore *list_store; >> +GtkWidget *status; >> +GtkWidget *spinner; >> +GtkWidget *stack; >> +GtkWid
[virt-tools-list] [PATCH virt-viewer] iso-dialog: Avoid crash when closing dialog early
We must take into account that users can close the dialog at anytime, even during an operation of fetch or set ISO has not been finished. This will cause the callbacks for those operations to be invoked with an invalid object, crashing the application. To fix this issue we need to pass a GCancellable to the asynchronous operations, so they can be cancelled if the dialog happens to get closed before they complete. Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/remote-viewer-iso-list-dialog.c | 30 +++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/src/remote-viewer-iso-list-dialog.c b/src/remote-viewer-iso-list-dialog.c index f23ddb2..a7ac98a 100644 --- a/src/remote-viewer-iso-list-dialog.c +++ b/src/remote-viewer-iso-list-dialog.c @@ -42,6 +42,7 @@ struct _RemoteViewerISOListDialogPrivate GtkWidget *stack; GtkWidget *tree_view; OvirtForeignMenu *foreign_menu; +GCancellable *cancellable; }; enum RemoteViewerISOListDialogModel @@ -66,10 +67,16 @@ remote_viewer_iso_list_dialog_dispose(GObject *object) RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object); RemoteViewerISOListDialogPrivate *priv = self->priv; +if (priv->cancellable) { +g_cancellable_cancel(priv->cancellable); +g_clear_object(>cancellable); +} + if (priv->foreign_menu) { g_signal_handlers_disconnect_by_data(priv->foreign_menu, object); g_clear_object(>foreign_menu); } + G_OBJECT_CLASS(remote_viewer_iso_list_dialog_parent_class)->dispose(object); } @@ -157,6 +164,10 @@ fetch_iso_names_cb(OvirtForeignMenu *foreign_menu, const gchar *msg = error ? error->message : _("Failed to fetch CD names"); gchar *markup = g_strdup_printf("%s", msg); +g_debug("Error fetching ISO names: %s", msg); +if (error && error->code == G_IO_ERROR_CANCELLED) +return; + gtk_label_set_markup(GTK_LABEL(priv->status), markup); gtk_spinner_stop(GTK_SPINNER(priv->spinner)); remote_viewer_iso_list_dialog_show_error(self, msg); @@ -166,6 +177,7 @@ fetch_iso_names_cb(OvirtForeignMenu *foreign_menu, return; } +g_clear_object(>cancellable); g_list_foreach(iso_list, (GFunc) remote_viewer_iso_list_dialog_foreach, self); remote_viewer_iso_list_dialog_show_files(self); } @@ -177,7 +189,10 @@ remote_viewer_iso_list_dialog_refresh_iso_list(RemoteViewerISOListDialog *self) RemoteViewerISOListDialogPrivate *priv = self->priv; gtk_list_store_clear(priv->list_store); -ovirt_foreign_menu_fetch_iso_names_async(priv->foreign_menu, NULL, + +priv->cancellable = g_cancellable_new(); +ovirt_foreign_menu_fetch_iso_names_async(priv->foreign_menu, + priv->cancellable, (GAsyncReadyCallback) fetch_iso_names_cb, self); } @@ -223,7 +238,9 @@ remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle *cell_renderer G_GNU gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, FALSE); gtk_widget_set_sensitive(priv->tree_view, FALSE); -ovirt_foreign_menu_set_current_iso_name_async(priv->foreign_menu, active ? NULL : name, NULL, +priv->cancellable = g_cancellable_new(); +ovirt_foreign_menu_set_current_iso_name_async(priv->foreign_menu, active ? NULL : name, + priv->cancellable, (GAsyncReadyCallback)ovirt_foreign_menu_iso_name_changed, self); gtk_tree_path_free(tree_path); @@ -308,10 +325,17 @@ ovirt_foreign_menu_iso_name_changed(OvirtForeignMenu *foreign_menu, * change the ISO back to the original, avoiding a possible inconsistency. */ if (!ovirt_foreign_menu_set_current_iso_name_finish(foreign_menu, result, )) { -remote_viewer_iso_list_dialog_show_error(self, error ? error->message : _("Failed to change CD")); +gchar *msg = error ? error->message : _("Failed to change CD"); +g_debug("Error changing ISO: %s", msg); + +if (error && error->code == G_IO_ERROR_CANCELLED) +return; + +remote_viewer_iso_list_dialog_show_error(self, msg); g_clear_error(); } +g_clear_object(>cancellable); if (!gtk_tree_model_get_iter_first(model, )) return; -- 2.9.3 ___ 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 00/10] Make dialogs use GtkHeaderBar
On 16/02/17 20:57, Daniel P. Berrange wrote: > On Thu, Feb 16, 2017 at 11:05:13AM -0200, Eduardo Lima (Etrunko) wrote: >> On 16/02/17 10:06, Pavel Grunt wrote: >>> Hi, >>> >>> On Thu, 2017-02-09 at 17:22 -0200, Eduardo Lima (Etrunko) wrote: >>>> This whole series is the result of the initial idea of having the >>>> new >>>> iso-dialog to use this widget. Having it done only for the iso- >>>> dialog >>>> would not make much sense, so I went on and ported all other >>>> dialogs. >>>> The widget requires Gtk version 3.12, which should not be a problem >>>> for >>>> any recent distro. >>>> >>>> Using GtkHeaderBar makes dialogs look cleaner and more 'modern', >>>> also >>>> follow the style used by GNOME applications. In the near future, >>>> virt-viewer main window could also make use of it, as we recently >>>> saw in >>>> a series sent to this mailing list. >>> >>> so are we going for the GNOME style for all desktop envs ? In my >>> opinion it does not look nice in Windows and Xfce - simply does not >>> fit it in. iirc it was mentioned that there is a way to have both the >>> classic and the GNOME look in the discussion about Sagar's patches few >>> weeks ago.. >> >> I have no idea of what it looks like in Windows, but it indeed looks >> good in Linux environments. I tested in both GNOME and Enlightenment, >> not sure about other DEs out there. As far as I remember, the discussion >> on that other series was about the classic menu versus the buttons on >> the header bar. > > Please post some screenshots showing the before & after state of this > on GNOME, Linux non-GNOME and Windows, so we can accurately evaluate > what the change is. > > Also, I tried to apply this series to test it myself, but it fails to > apply to git master for some reason You can try my github remote, branch dialogs git remote add etrunko git://github.com/etrunko/virt-viewer -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ 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 00/10] Make dialogs use GtkHeaderBar
On 16/02/17 20:57, Daniel P. Berrange wrote: > On Thu, Feb 16, 2017 at 11:05:13AM -0200, Eduardo Lima (Etrunko) wrote: >> On 16/02/17 10:06, Pavel Grunt wrote: >>> Hi, >>> >>> On Thu, 2017-02-09 at 17:22 -0200, Eduardo Lima (Etrunko) wrote: >>>> This whole series is the result of the initial idea of having the >>>> new >>>> iso-dialog to use this widget. Having it done only for the iso- >>>> dialog >>>> would not make much sense, so I went on and ported all other >>>> dialogs. >>>> The widget requires Gtk version 3.12, which should not be a problem >>>> for >>>> any recent distro. >>>> >>>> Using GtkHeaderBar makes dialogs look cleaner and more 'modern', >>>> also >>>> follow the style used by GNOME applications. In the near future, >>>> virt-viewer main window could also make use of it, as we recently >>>> saw in >>>> a series sent to this mailing list. >>> >>> so are we going for the GNOME style for all desktop envs ? In my >>> opinion it does not look nice in Windows and Xfce - simply does not >>> fit it in. iirc it was mentioned that there is a way to have both the >>> classic and the GNOME look in the discussion about Sagar's patches few >>> weeks ago.. >> >> I have no idea of what it looks like in Windows, but it indeed looks >> good in Linux environments. I tested in both GNOME and Enlightenment, >> not sure about other DEs out there. As far as I remember, the discussion >> on that other series was about the classic menu versus the buttons on >> the header bar. > > Please post some screenshots showing the before & after state of this > on GNOME, Linux non-GNOME and Windows, so we can accurately evaluate > what the change is. Okay, I have uploaded the screenshots of the dialogs to the following url: http://imgur.com/a/WHt8R I ran both before-series and after-series in GNOME and Enlightenment, all dialogs look exactly the same, except for the about dialog. By the way, I realized I should not mess with the About dialog, it does its own magic automatically. That one patch should be dropped. -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ 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 v2] iso-dialog: Avoid crash when closing dialog early
On 09/02/17 17:19, Pavel Grunt wrote: > Hi Eduardo, > > On Thu, 2017-02-09 at 15:13 -0200, Eduardo Lima (Etrunko) wrote: >> We must take into account that users can close the dialog at >> anytime, >> even during an operation of fetch or set ISO has not been finished. >> This >> will cause the callbacks for those operations to be invoked with an >> invalid object, crashing the application. >> >> To fix this issue we need to pass a GCancellable to the asynchronous >> operations, so they can be cancelled if the dialog happens to get >> closed >> before they complete. >> >> Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> >> --- >> v2: >> * Move call to g_cancellable_cancel() to response handler. >> * Don't leak error objects if operation is cancelled. >> --- >> src/remote-viewer-iso-list-dialog.c | 42 >> ++--- >> 1 file changed, 34 insertions(+), 8 deletions(-) >> >> diff --git a/src/remote-viewer-iso-list-dialog.c b/src/remote- >> viewer-iso-list-dialog.c >> index 2ab5435..ed51ffa 100644 >> --- a/src/remote-viewer-iso-list-dialog.c >> +++ b/src/remote-viewer-iso-list-dialog.c >> @@ -42,6 +42,7 @@ struct _RemoteViewerISOListDialogPrivate >> GtkWidget *stack; >> GtkWidget *tree_view; >> OvirtForeignMenu *foreign_menu; >> +GCancellable *cancellable; >> }; >> >> enum RemoteViewerISOListDialogModel >> @@ -66,6 +67,8 @@ remote_viewer_iso_list_dialog_dispose(GObject >> *object) >> RemoteViewerISOListDialog *self = >> REMOTE_VIEWER_ISO_LIST_DIALOG(object); >> RemoteViewerISOListDialogPrivate *priv = self->priv; >> >> +g_clear_object(>cancellable); >> + >> if (priv->foreign_menu) { >> g_signal_handlers_disconnect_by_data(priv->foreign_menu, >> object); >> g_clear_object(>foreign_menu); >> @@ -157,17 +160,24 @@ fetch_iso_names_cb(OvirtForeignMenu >> *foreign_menu, >> const gchar *msg = error ? error->message : _("Failed to >> fetch CD names"); >> gchar *markup = g_strdup_printf("%s", msg); >> >> +g_debug("Error fetching ISO names: %s", msg); >> +if (error && error->code == G_IO_ERROR_CANCELLED) > use g_error_matches if possible > >> +goto end; >> + >> gtk_label_set_markup(GTK_LABEL(priv->status), markup); >> gtk_spinner_stop(GTK_SPINNER(priv->spinner)); >> remote_viewer_iso_list_dialog_show_error(self, msg); >> gtk_dialog_set_response_sensitive(GTK_DIALOG(self), >> GTK_RESPONSE_NONE, TRUE); >> g_free(markup); >> -g_clear_error(); >> -return; >> +goto end; >> } >> >> +g_clear_object(>cancellable); >> g_list_foreach(iso_list, (GFunc) >> remote_viewer_iso_list_dialog_foreach, self); >> remote_viewer_iso_list_dialog_show_files(self); >> + >> +end: >> +g_clear_error(); >> } >> >> >> @@ -177,7 +187,10 @@ >> remote_viewer_iso_list_dialog_refresh_iso_list(RemoteViewerISOListDi >> alog *self) >> RemoteViewerISOListDialogPrivate *priv = self->priv; >> >> gtk_list_store_clear(priv->list_store); >> -ovirt_foreign_menu_fetch_iso_names_async(priv->foreign_menu, >> NULL, >> + >> +priv->cancellable = g_cancellable_new(); >> +ovirt_foreign_menu_fetch_iso_names_async(priv->foreign_menu, >> + priv->cancellable, >> (GAsyncReadyCallback) >> fetch_iso_names_cb, >> self); >> } >> @@ -190,8 +203,10 @@ >> remote_viewer_iso_list_dialog_response(GtkDialog *dialog, >> RemoteViewerISOListDialog *self = >> REMOTE_VIEWER_ISO_LIST_DIALOG(dialog); >> RemoteViewerISOListDialogPrivate *priv = self->priv; >> >> -if (response_id != GTK_RESPONSE_NONE) >> +if (response_id != GTK_RESPONSE_NONE) { >> +g_cancellable_cancel(priv->cancellable); >> return; >> +} >> >> gtk_spinner_start(GTK_SPINNER(priv->spinner)); >> gtk_label_set_markup(GTK_LABEL(priv->status), >> _("Loading...")); >> @@ -223,7 +238,9 @@ >> remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle >> *cell_renderer G_GNU >> gtk_dia
[virt-tools-list] [PATCH virt-viewer 08/10] virt-viewer-auth: Use GtkHeaderBar
The auxiliary text is now presented as the subtitle of the dialog Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/resources/ui/virt-viewer-auth.ui | 148 --- src/virt-viewer-auth.c | 35 + 2 files changed, 86 insertions(+), 97 deletions(-) diff --git a/src/resources/ui/virt-viewer-auth.ui b/src/resources/ui/virt-viewer-auth.ui index 2920780..367678d 100644 --- a/src/resources/ui/virt-viewer-auth.ui +++ b/src/resources/ui/virt-viewer-auth.ui @@ -1,160 +1,146 @@ + - + +350 +1 False -5 -Authentication required -True -center-on-parent -True +12 dialog -True -True - -True + False vertical 2 - -True + False end - -_Cancel -True -True -True -False -True - - -False -False -0 - + - -_OK -True -True -True -True -True -False -True - - -False -False -3 - + False -True -end -0 - - - - -True -False -0 -0 -label -True - - -False -True +False 1 - + True False -2 -2 -6 +True 6 +6 - + True False -1 -Password: +end +Username: -1 -2 +0 +0 - + True False -1 -Username: +end +Password: + +0 +1 + True True +4 +True + 1 -2 +0 True True +4 +True False True + 1 -2 1 -2 +Show password True True -False -Show password +False +True + 1 -2 2 -3 + + + False True -2 +0 - - button-cancel - button-ok - + + +True +False +emblem-ok-symbolic + + +True +False +Authentication required +True +:close + + +True +True +True +True +True +image1 +True + + + +end + + diff --git a/src/virt-viewer-auth.c b/src/virt-viewer-auth.c index 67c770c..73cc707 100644 --- a/src/virt-viewer-auth.c +++ b/src/virt-viewer-auth.c @@ -33,13 +33,23 @@ #include "virt-viewer-auth.h" #include "virt-viewer-util.h" -static void +void show_password(GtkCheckButton *check_button G_GNUC_UNUSED, GtkEntry *entry); +void button_ok_clicked(GtkButton *button G_GNUC_UNUSED, GtkDialog *dialog); + +void show_password(GtkCheckButton *check_button G_GNUC_UNUSED, GtkEntry *entry) { gtk_entry_set_visibility(entry, !gtk_entry_get_visibility(entry)); } +void +button_ok_clicked(GtkButton *button G_GNUC_UNUSED, + GtkDialog *dialog) +{ +gtk_dialog_response(dialog, GTK_RESPONSE_OK); +} + /* NOTE: if username is provided, and *username is non-NULL, the user input * field will be pre-filled wit
Re: [virt-tools-list] [PATCH virt-viewer v2] session-spice: Pass hostname to authentication dialog
On 09/02/17 17:22, Pavel Grunt wrote: > On Thu, 2017-02-09 at 15:32 -0200, Eduardo Lima (Etrunko) wrote: >> With this patch the dialog now shows the host we are connecting to. >> >> Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> > Acked-by: Pavel Grunt <pgr...@redhat.com> Thanks, pushed. >> --- >> v2: Use proper uri if connecting via proxy. >> --- >> src/virt-viewer-session-spice.c | 7 +-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/src/virt-viewer-session-spice.c b/src/virt-viewer- >> session-spice.c >> index c3fce48..9b52ec0 100644 >> --- a/src/virt-viewer-session-spice.c >> +++ b/src/virt-viewer-session-spice.c >> @@ -691,6 +691,7 @@ >> virt_viewer_session_spice_main_channel_event(SpiceChannel *channel, >> case SPICE_CHANNEL_ERROR_AUTH: >> { >> const GError *error = NULL; >> +gchar *host = NULL; >> g_debug("main channel: auth failure (wrong >> username/password?)"); >> >> { >> @@ -717,11 +718,13 @@ >> virt_viewer_session_spice_main_channel_event(SpiceChannel *channel, >> user = g_strdup(g_get_user_name()); >> } >> >> +g_object_get(self->priv->session, "host", , NULL); >> ret = virt_viewer_auth_collect_credentials(self->priv- >>> main_window, >> "SPICE", >> - NULL, >> + host, >> username_require >> d ? : NULL, >> ); >> +g_free(host); >> if (!ret) { >> g_signal_emit_by_name(session, "session-cancelled"); >> } else { >> @@ -750,7 +753,7 @@ >> virt_viewer_session_spice_main_channel_event(SpiceChannel *channel, >> g_warn_if_fail(proxy != NULL); >> >> ret = virt_viewer_auth_collect_credentials(self->priv- >>> main_window, >> - "proxy", >> NULL, >> + "proxy", >> spice_uri_get_hostname(proxy), >> , >> ); >> if (!ret) { >> g_signal_emit_by_name(session, "session- >> cancelled"); -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ virt-tools-list mailing list virt-tools-list@redhat.com https://www.redhat.com/mailman/listinfo/virt-tools-list
Re: [virt-tools-list] [virt-viewer updated PATCH] Show errors generated by connection dialog
On 09/02/17 10:00, Christophe de Dinechin wrote: > When running 'remote-viewer' without arguments, > and selecting a non-supported protocol, e.g. ssh://foo, > the generated error was silently swallowed by the retry loop. > This was introduced in 06b2c382468876a19600374bd59475e66d488af8. > --- > src/remote-viewer.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/src/remote-viewer.c b/src/remote-viewer.c > index 13c6e7c..e4b0909 100644 > --- a/src/remote-viewer.c > +++ b/src/remote-viewer.c > @@ -1196,6 +1196,9 @@ cleanup: > type = NULL; > > if (!ret && priv->open_recent_dialog) { > +if (error != NULL) { > +virt_viewer_app_simple_message_dialog(app, _("Unable to connect: > %s"), error->message); > +} > g_clear_error(); > goto retry_dialog; > } > Sorry for starting this endless discussion about checking for error->message, it has been shown that it is not necessary (TIL). As this version addresses the comments of explicit checking, from Pavel, it's an Ack. Acked-by: Eduardo Lima (Etrunko) <etru...@redhat.com> -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ 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 v2] session-spice: Pass hostname to authentication dialog
With this patch the dialog now shows the host we are connecting to. Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- v2: Use proper uri if connecting via proxy. --- src/virt-viewer-session-spice.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/virt-viewer-session-spice.c b/src/virt-viewer-session-spice.c index c3fce48..9b52ec0 100644 --- a/src/virt-viewer-session-spice.c +++ b/src/virt-viewer-session-spice.c @@ -691,6 +691,7 @@ virt_viewer_session_spice_main_channel_event(SpiceChannel *channel, case SPICE_CHANNEL_ERROR_AUTH: { const GError *error = NULL; +gchar *host = NULL; g_debug("main channel: auth failure (wrong username/password?)"); { @@ -717,11 +718,13 @@ virt_viewer_session_spice_main_channel_event(SpiceChannel *channel, user = g_strdup(g_get_user_name()); } +g_object_get(self->priv->session, "host", , NULL); ret = virt_viewer_auth_collect_credentials(self->priv->main_window, "SPICE", - NULL, + host, username_required ? : NULL, ); +g_free(host); if (!ret) { g_signal_emit_by_name(session, "session-cancelled"); } else { @@ -750,7 +753,7 @@ virt_viewer_session_spice_main_channel_event(SpiceChannel *channel, g_warn_if_fail(proxy != NULL); ret = virt_viewer_auth_collect_credentials(self->priv->main_window, - "proxy", NULL, + "proxy", spice_uri_get_hostname(proxy), , ); if (!ret) { g_signal_emit_by_name(session, "session-cancelled"); -- 2.9.3 ___ 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] iso-dialog: Avoid crash when closing dialog early
Looks like this v2 never made to the list? I am sending it again. On 03/02/17 15:28, Eduardo Lima (Etrunko) wrote: > We must take into account that users can close the dialog at anytime, > even during an operation of fetch or set ISO has not been finished. This > will cause the callbacks for those operations to be invoked with an > invalid object, crashing the application. > > To fix this issue we need to pass a GCancellable to the asynchronous > operations, so they can be cancelled if the dialog happens to get closed > before they complete. > > Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> > --- > v2: > * Move call to g_cancellable_cancel() to response handler. > * Don't leak error objects if operation is cancelled. > > --- > src/remote-viewer-iso-list-dialog.c | 42 > ++--- > 1 file changed, 34 insertions(+), 8 deletions(-) > > diff --git a/src/remote-viewer-iso-list-dialog.c > b/src/remote-viewer-iso-list-dialog.c > index f23ddb2..aa0ebbb 100644 > --- a/src/remote-viewer-iso-list-dialog.c > +++ b/src/remote-viewer-iso-list-dialog.c > @@ -42,6 +42,7 @@ struct _RemoteViewerISOListDialogPrivate > GtkWidget *stack; > GtkWidget *tree_view; > OvirtForeignMenu *foreign_menu; > +GCancellable *cancellable; > }; > > enum RemoteViewerISOListDialogModel > @@ -66,6 +67,8 @@ remote_viewer_iso_list_dialog_dispose(GObject *object) > RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object); > RemoteViewerISOListDialogPrivate *priv = self->priv; > > +g_clear_object(>cancellable); > + > if (priv->foreign_menu) { > g_signal_handlers_disconnect_by_data(priv->foreign_menu, object); > g_clear_object(>foreign_menu); > @@ -157,17 +160,24 @@ fetch_iso_names_cb(OvirtForeignMenu *foreign_menu, > const gchar *msg = error ? error->message : _("Failed to fetch CD > names"); > gchar *markup = g_strdup_printf("%s", msg); > > +g_debug("Error fetching ISO names: %s", msg); > +if (error && error->code == G_IO_ERROR_CANCELLED) > +goto end; > + > gtk_label_set_markup(GTK_LABEL(priv->status), markup); > gtk_spinner_stop(GTK_SPINNER(priv->spinner)); > remote_viewer_iso_list_dialog_show_error(self, msg); > gtk_dialog_set_response_sensitive(GTK_DIALOG(self), > GTK_RESPONSE_NONE, TRUE); > g_free(markup); > -g_clear_error(); > -return; > +goto end; > } > > +g_clear_object(>cancellable); > g_list_foreach(iso_list, (GFunc) remote_viewer_iso_list_dialog_foreach, > self); > remote_viewer_iso_list_dialog_show_files(self); > + > +end: > +g_clear_error(); > } > > > @@ -177,7 +187,10 @@ > remote_viewer_iso_list_dialog_refresh_iso_list(RemoteViewerISOListDialog > *self) > RemoteViewerISOListDialogPrivate *priv = self->priv; > > gtk_list_store_clear(priv->list_store); > -ovirt_foreign_menu_fetch_iso_names_async(priv->foreign_menu, NULL, > + > +priv->cancellable = g_cancellable_new(); > +ovirt_foreign_menu_fetch_iso_names_async(priv->foreign_menu, > + priv->cancellable, > (GAsyncReadyCallback) > fetch_iso_names_cb, > self); > } > @@ -190,8 +203,10 @@ remote_viewer_iso_list_dialog_response(GtkDialog *dialog, > RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog); > RemoteViewerISOListDialogPrivate *priv = self->priv; > > -if (response_id != GTK_RESPONSE_NONE) > +if (response_id != GTK_RESPONSE_NONE) { > +g_cancellable_cancel(priv->cancellable); > return; > +} > > gtk_spinner_start(GTK_SPINNER(priv->spinner)); > gtk_label_set_markup(GTK_LABEL(priv->status), _("Loading...")); > @@ -223,7 +238,9 @@ > remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle *cell_renderer > G_GNU > gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, > FALSE); > gtk_widget_set_sensitive(priv->tree_view, FALSE); > > -ovirt_foreign_menu_set_current_iso_name_async(priv->foreign_menu, active > ? NULL : name, NULL, > +priv->cancellable = g_cancellable_new(); > +ovirt_foreign_menu_set_current_iso_name_async(priv->foreign_menu, active > ? NULL : name, > + priv->cancellable, >
[virt-tools-list] [PATCH virt-viewer v2] iso-dialog: Avoid crash when closing dialog early
We must take into account that users can close the dialog at anytime, even during an operation of fetch or set ISO has not been finished. This will cause the callbacks for those operations to be invoked with an invalid object, crashing the application. To fix this issue we need to pass a GCancellable to the asynchronous operations, so they can be cancelled if the dialog happens to get closed before they complete. Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- v2: * Move call to g_cancellable_cancel() to response handler. * Don't leak error objects if operation is cancelled. --- src/remote-viewer-iso-list-dialog.c | 42 ++--- 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/src/remote-viewer-iso-list-dialog.c b/src/remote-viewer-iso-list-dialog.c index 2ab5435..ed51ffa 100644 --- a/src/remote-viewer-iso-list-dialog.c +++ b/src/remote-viewer-iso-list-dialog.c @@ -42,6 +42,7 @@ struct _RemoteViewerISOListDialogPrivate GtkWidget *stack; GtkWidget *tree_view; OvirtForeignMenu *foreign_menu; +GCancellable *cancellable; }; enum RemoteViewerISOListDialogModel @@ -66,6 +67,8 @@ remote_viewer_iso_list_dialog_dispose(GObject *object) RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object); RemoteViewerISOListDialogPrivate *priv = self->priv; +g_clear_object(>cancellable); + if (priv->foreign_menu) { g_signal_handlers_disconnect_by_data(priv->foreign_menu, object); g_clear_object(>foreign_menu); @@ -157,17 +160,24 @@ fetch_iso_names_cb(OvirtForeignMenu *foreign_menu, const gchar *msg = error ? error->message : _("Failed to fetch CD names"); gchar *markup = g_strdup_printf("%s", msg); +g_debug("Error fetching ISO names: %s", msg); +if (error && error->code == G_IO_ERROR_CANCELLED) +goto end; + gtk_label_set_markup(GTK_LABEL(priv->status), markup); gtk_spinner_stop(GTK_SPINNER(priv->spinner)); remote_viewer_iso_list_dialog_show_error(self, msg); gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, TRUE); g_free(markup); -g_clear_error(); -return; +goto end; } +g_clear_object(>cancellable); g_list_foreach(iso_list, (GFunc) remote_viewer_iso_list_dialog_foreach, self); remote_viewer_iso_list_dialog_show_files(self); + +end: +g_clear_error(); } @@ -177,7 +187,10 @@ remote_viewer_iso_list_dialog_refresh_iso_list(RemoteViewerISOListDialog *self) RemoteViewerISOListDialogPrivate *priv = self->priv; gtk_list_store_clear(priv->list_store); -ovirt_foreign_menu_fetch_iso_names_async(priv->foreign_menu, NULL, + +priv->cancellable = g_cancellable_new(); +ovirt_foreign_menu_fetch_iso_names_async(priv->foreign_menu, + priv->cancellable, (GAsyncReadyCallback) fetch_iso_names_cb, self); } @@ -190,8 +203,10 @@ remote_viewer_iso_list_dialog_response(GtkDialog *dialog, RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog); RemoteViewerISOListDialogPrivate *priv = self->priv; -if (response_id != GTK_RESPONSE_NONE) +if (response_id != GTK_RESPONSE_NONE) { +g_cancellable_cancel(priv->cancellable); return; +} gtk_spinner_start(GTK_SPINNER(priv->spinner)); gtk_label_set_markup(GTK_LABEL(priv->status), _("Loading...")); @@ -223,7 +238,9 @@ remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle *cell_renderer G_GNU gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, FALSE); gtk_widget_set_sensitive(priv->tree_view, FALSE); -ovirt_foreign_menu_set_current_iso_name_async(priv->foreign_menu, active ? NULL : name, NULL, +priv->cancellable = g_cancellable_new(); +ovirt_foreign_menu_set_current_iso_name_async(priv->foreign_menu, active ? NULL : name, + priv->cancellable, (GAsyncReadyCallback)ovirt_foreign_menu_iso_name_changed, self); gtk_tree_path_free(tree_path); @@ -308,12 +325,18 @@ ovirt_foreign_menu_iso_name_changed(OvirtForeignMenu *foreign_menu, * change the ISO back to the original, avoiding a possible inconsistency. */ if (!ovirt_foreign_menu_set_current_iso_name_finish(foreign_menu, result, )) { -remote_viewer_iso_list_dialog_show_error(self, error ? error->message : _("Failed to change CD")); -g_clear_error(); +const gchar *msg = error ? error->message : _("Fail
Re: [virt-tools-list] [PATCH virt-viewer v2] iso-dialog: Avoid crash when closing dialog early
On 09/02/17 15:13, Eduardo Lima (Etrunko) wrote: > We must take into account that users can close the dialog at anytime, > even during an operation of fetch or set ISO has not been finished. This > will cause the callbacks for those operations to be invoked with an > invalid object, crashing the application. > > To fix this issue we need to pass a GCancellable to the asynchronous > operations, so they can be cancelled if the dialog happens to get closed > before they complete. I have modified this commit message with a link to the libgovirt bug: NOTE: This patch triggers a deadlock in libgovirt when the dialog is closed before the operation completes. Bug reported in https://bugzilla.gnome.org/show_bug.cgi?id=777808. We will need to bump libgovirt dependency whenever it has a new release. > > Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> > --- > v2: > * Move call to g_cancellable_cancel() to response handler. > * Don't leak error objects if operation is cancelled. > --- > src/remote-viewer-iso-list-dialog.c | 42 > ++--- > 1 file changed, 34 insertions(+), 8 deletions(-) > > diff --git a/src/remote-viewer-iso-list-dialog.c > b/src/remote-viewer-iso-list-dialog.c > index 2ab5435..ed51ffa 100644 > --- a/src/remote-viewer-iso-list-dialog.c > +++ b/src/remote-viewer-iso-list-dialog.c > @@ -42,6 +42,7 @@ struct _RemoteViewerISOListDialogPrivate > GtkWidget *stack; > GtkWidget *tree_view; > OvirtForeignMenu *foreign_menu; > +GCancellable *cancellable; > }; > > enum RemoteViewerISOListDialogModel > @@ -66,6 +67,8 @@ remote_viewer_iso_list_dialog_dispose(GObject *object) > RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object); > RemoteViewerISOListDialogPrivate *priv = self->priv; > > +g_clear_object(>cancellable); > + > if (priv->foreign_menu) { > g_signal_handlers_disconnect_by_data(priv->foreign_menu, object); > g_clear_object(>foreign_menu); > @@ -157,17 +160,24 @@ fetch_iso_names_cb(OvirtForeignMenu *foreign_menu, > const gchar *msg = error ? error->message : _("Failed to fetch CD > names"); > gchar *markup = g_strdup_printf("%s", msg); > > +g_debug("Error fetching ISO names: %s", msg); > +if (error && error->code == G_IO_ERROR_CANCELLED) > +goto end; > + > gtk_label_set_markup(GTK_LABEL(priv->status), markup); > gtk_spinner_stop(GTK_SPINNER(priv->spinner)); > remote_viewer_iso_list_dialog_show_error(self, msg); > gtk_dialog_set_response_sensitive(GTK_DIALOG(self), > GTK_RESPONSE_NONE, TRUE); > g_free(markup); > -g_clear_error(); > -return; > +goto end; > } > > +g_clear_object(>cancellable); > g_list_foreach(iso_list, (GFunc) remote_viewer_iso_list_dialog_foreach, > self); > remote_viewer_iso_list_dialog_show_files(self); > + > +end: > +g_clear_error(); > } > > > @@ -177,7 +187,10 @@ > remote_viewer_iso_list_dialog_refresh_iso_list(RemoteViewerISOListDialog > *self) > RemoteViewerISOListDialogPrivate *priv = self->priv; > > gtk_list_store_clear(priv->list_store); > -ovirt_foreign_menu_fetch_iso_names_async(priv->foreign_menu, NULL, > + > +priv->cancellable = g_cancellable_new(); > +ovirt_foreign_menu_fetch_iso_names_async(priv->foreign_menu, > + priv->cancellable, > (GAsyncReadyCallback) > fetch_iso_names_cb, > self); > } > @@ -190,8 +203,10 @@ remote_viewer_iso_list_dialog_response(GtkDialog *dialog, > RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog); > RemoteViewerISOListDialogPrivate *priv = self->priv; > > -if (response_id != GTK_RESPONSE_NONE) > +if (response_id != GTK_RESPONSE_NONE) { > +g_cancellable_cancel(priv->cancellable); > return; > +} > > gtk_spinner_start(GTK_SPINNER(priv->spinner)); > gtk_label_set_markup(GTK_LABEL(priv->status), _("Loading...")); > @@ -223,7 +238,9 @@ > remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle *cell_renderer > G_GNU > gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, > FALSE); > gtk_widget_set_sensitive(priv->tree_view, FALSE); > > -ovirt_foreign_menu_set_current_iso_name_async(priv->foreign_menu, active > ? NULL : name, NULL, > +
Re: [virt-tools-list] [PATCH virt-viewer 00/10] Make dialogs use GtkHeaderBar
On 16/02/17 10:06, Pavel Grunt wrote: > Hi, > > On Thu, 2017-02-09 at 17:22 -0200, Eduardo Lima (Etrunko) wrote: >> This whole series is the result of the initial idea of having the >> new >> iso-dialog to use this widget. Having it done only for the iso- >> dialog >> would not make much sense, so I went on and ported all other >> dialogs. >> The widget requires Gtk version 3.12, which should not be a problem >> for >> any recent distro. >> >> Using GtkHeaderBar makes dialogs look cleaner and more 'modern', >> also >> follow the style used by GNOME applications. In the near future, >> virt-viewer main window could also make use of it, as we recently >> saw in >> a series sent to this mailing list. > > so are we going for the GNOME style for all desktop envs ? In my > opinion it does not look nice in Windows and Xfce - simply does not > fit it in. iirc it was mentioned that there is a way to have both the > classic and the GNOME look in the discussion about Sagar's patches few > weeks ago.. I have no idea of what it looks like in Windows, but it indeed looks good in Linux environments. I tested in both GNOME and Enlightenment, not sure about other DEs out there. As far as I remember, the discussion on that other series was about the classic menu versus the buttons on the header bar. -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ 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 3/5] iso-dialog: Use header bar for buttons
On 20/01/17 13:26, Fabiano Fidêncio wrote: > On Fri, Jan 20, 2017 at 4:21 PM, Christophe Fergeau> wrote: >> I haven't looked at this patch at all, as I don't know what the plan is >> for virt-viewer regarding gtk+ versions. It's easy to move last in the >> series though. Would be nice if someone more informed about that chimed >> in on this patch ;) > > All big distros (including the enterprise ones) have a newer version. > So that's not a problem at all. > > I just want to know Daniel's opinion on how it looks on his TWM testing VM :-) > I have some patches locally changing other dialogs to this new look, I will soon publish them on a separate branch so it can be tested easier. -- Eduardo de Barros Lima (Etrunko) Software Engineer - RedHat etru...@redhat.com ___ 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 0/4 v8] Replace oVirt foreign menu with dedicated dialog
In this version, I have addressed the comments on the ISO dialog code, also providing a better message for that commit. Also, put the headerbar patch at the end, as others were already acked and should be ready to merge. Eduardo Lima (Etrunko) (4): Introduce ISO List dialog Run ISO dialog when 'Change CD' menu is activated ovirt-foreign-menu: Remove GtkMenu related functions iso-dialog: Use header bar for buttons configure.ac | 4 +- po/POTFILES.in | 2 + src/Makefile.am| 3 + src/ovirt-foreign-menu.c | 99 src/remote-viewer-iso-list-dialog.c| 386 + src/remote-viewer-iso-list-dialog.h| 58 + src/remote-viewer.c| 124 - src/resources/ui/remote-viewer-iso-list.ui | 135 ++ src/resources/ui/virt-viewer.ui| 19 +- src/resources/virt-viewer.gresource.xml| 1 + src/virt-viewer-window.c | 37 +++ 11 files changed, 677 insertions(+), 191 deletions(-) create mode 100644 src/remote-viewer-iso-list-dialog.c create mode 100644 src/remote-viewer-iso-list-dialog.h create mode 100644 src/resources/ui/remote-viewer-iso-list.ui -- 2.9.3 ___ 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 1/4] Introduce ISO List dialog
The motivation for this dialog started with rhbz #1310624, where it was reported that foreign menu was causing too many debug messages to be printed to the console, because remote viewer had a timeout of 5 seconds to refresh the ISO list automatically. As a workaround, the timeout was adjusted for 5 minutes, but it could cause more problems, such as inconsistencies between what was shown by remote viewer and what the server had configured. Another issue caused by displaying the ISO files as a menu item was that if the list was too long, it would take all the available space on the screen. In the end, a menu item was not the correct choice of UI component for this use case. In order to solve both problems, we now present the ISO list as a dedicated dialog, where the refresh of ISO list is triggered manually by the user and the list is contained within the dialog, by displaying de files in a treeview. Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- po/POTFILES.in | 2 + src/Makefile.am| 3 + src/remote-viewer-iso-list-dialog.c| 365 + src/remote-viewer-iso-list-dialog.h| 58 + src/resources/ui/remote-viewer-iso-list.ui | 158 + src/resources/virt-viewer.gresource.xml| 1 + 6 files changed, 587 insertions(+) create mode 100644 src/remote-viewer-iso-list-dialog.c create mode 100644 src/remote-viewer-iso-list-dialog.h create mode 100644 src/resources/ui/remote-viewer-iso-list.ui diff --git a/po/POTFILES.in b/po/POTFILES.in index 69d9fef..371c242 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -1,9 +1,11 @@ data/remote-viewer.appdata.xml.in data/remote-viewer.desktop.in data/virt-viewer-mime.xml.in +src/remote-viewer-iso-list-dialog.c src/remote-viewer-main.c src/remote-viewer.c [type: gettext/glade] src/resources/ui/remote-viewer-connect.ui +[type: gettext/glade] src/resources/ui/remote-viewer-iso-list.ui [type: gettext/glade] src/resources/ui/virt-viewer-about.ui src/virt-viewer-app.c src/virt-viewer-auth.c diff --git a/src/Makefile.am b/src/Makefile.am index 272c4ff..9748277 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -13,6 +13,7 @@ noinst_DATA = \ resources/ui/virt-viewer-vm-connection.ui \ resources/ui/virt-viewer-preferences.ui \ resources/ui/remote-viewer-connect.ui \ + resources/ui/remote-viewer-iso-list.ui \ resources/ui/virt-viewer-file-transfer-dialog.ui \ $(NULL) @@ -97,6 +98,8 @@ if HAVE_OVIRT libvirt_viewer_la_SOURCES += \ ovirt-foreign-menu.h \ ovirt-foreign-menu.c \ + remote-viewer-iso-list-dialog.c \ + remote-viewer-iso-list-dialog.h \ $(NULL) endif diff --git a/src/remote-viewer-iso-list-dialog.c b/src/remote-viewer-iso-list-dialog.c new file mode 100644 index 000..e27e5fc --- /dev/null +++ b/src/remote-viewer-iso-list-dialog.c @@ -0,0 +1,365 @@ +/* + * Virt Viewer: A virtual machine console viewer + * + * Copyright (C) 2016 Red Hat, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#include + +#include + +#include "remote-viewer-iso-list-dialog.h" +#include "virt-viewer-util.h" +#include "ovirt-foreign-menu.h" + +static void ovirt_foreign_menu_iso_name_changed(OvirtForeignMenu *foreign_menu, GAsyncResult *result, RemoteViewerISOListDialog *self); +static void remote_viewer_iso_list_dialog_show_error(RemoteViewerISOListDialog *self, const gchar *message); + +G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, GTK_TYPE_DIALOG) + +#define DIALOG_PRIVATE(o) \ +(G_TYPE_INSTANCE_GET_PRIVATE((o), REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG, RemoteViewerISOListDialogPrivate)) + +struct _RemoteViewerISOListDialogPrivate +{ +GtkListStore *list_store; +GtkWidget *status; +GtkWidget *spinner; +GtkWidget *stack; +GtkWidget *tree_view; +OvirtForeignMenu *foreign_menu; +}; + +enum RemoteViewerISOListDialogModel +{ +ISO_IS_ACTIVE = 0, +ISO_NAME, +FONT_WEIGHT, +}; + +enum RemoteViewerISOListDialogProperties { +PROP_0, +PROP_FOREIGN_MENU, +}; + + +void remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle *cell_renderer, gch
[virt-tools-list] [PATCH virt-viewer 3/4] ovirt-foreign-menu: Remove GtkMenu related functions
With this commit, we finish cleaning up ovirt foreign menu code, which only deals with data, leaving the UI bits to be handled properly in the new ISO list dialog. This patch also updates remote-viewer to reflect the change. Signed-off-by: Eduardo Lima (Etrunko) <etru...@redhat.com> --- src/ovirt-foreign-menu.c | 99 src/remote-viewer.c | 87 +++--- 2 files changed, 6 insertions(+), 180 deletions(-) diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c index ef3ddd9..2939ae5 100644 --- a/src/ovirt-foreign-menu.c +++ b/src/ovirt-foreign-menu.c @@ -350,22 +350,6 @@ ovirt_foreign_menu_fetch_iso_names_finish(OvirtForeignMenu *foreign_menu, } -static void -ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, gpointer user_data); - - -static void -menu_item_set_active_no_signal(GtkMenuItem *menuitem, - gboolean active, - GCallback callback, - gpointer user_data) -{ -g_signal_handlers_block_by_func(menuitem, callback, user_data); -gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(menuitem), active); -g_signal_handlers_unblock_by_func(menuitem, callback, user_data); -} - - static void iso_name_set_cb(GObject *source_object, GAsyncResult *result, gpointer user_data) @@ -447,88 +431,6 @@ gboolean ovirt_foreign_menu_set_current_iso_name_finish(OvirtForeignMenu *foreig } -static void -ovirt_foreign_menu_iso_name_changed(GObject *source_object, -GAsyncResult *result, -gpointer user_data G_GNUC_UNUSED) -{ -OvirtForeignMenu *foreign_menu = OVIRT_FOREIGN_MENU(source_object); -GError *error = NULL; - -if (!ovirt_foreign_menu_set_current_iso_name_finish(foreign_menu, result, )) { -g_warning(error ? error->message : "Failed to change CD"); -g_clear_error(); -return; -} - -g_object_notify(G_OBJECT(foreign_menu), "file"); -} - - -static void -ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, gpointer user_data) -{ -OvirtForeignMenu *foreign_menu; -const char *iso_name = NULL; -gboolean checked; - -checked = gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(menuitem)); -foreign_menu = OVIRT_FOREIGN_MENU(user_data); -g_return_if_fail(foreign_menu->priv->cdrom != NULL); -g_return_if_fail(foreign_menu->priv->next_iso_name == NULL); - -g_debug("'%s' clicked", gtk_menu_item_get_label(menuitem)); - -/* We only want to move the check mark for the currently selected ISO - * when ovirt_cdrom_update_async() is successful, so for now we move - * the check mark back to where it was before - */ -menu_item_set_active_no_signal(menuitem, !checked, - (GCallback)ovirt_foreign_menu_activate_item_cb, - foreign_menu); - -if (checked) { -iso_name = gtk_menu_item_get_label(menuitem); -} -ovirt_foreign_menu_set_current_iso_name_async(foreign_menu, iso_name, NULL, - ovirt_foreign_menu_iso_name_changed, - menuitem); -} - - -GtkWidget *ovirt_foreign_menu_get_gtk_menu(OvirtForeignMenu *foreign_menu) -{ -GtkWidget *gtk_menu; -GList *it; -char *current_iso; - -if (foreign_menu->priv->iso_names == NULL) { -g_debug("ISO list is empty, no menu to show"); -return NULL; -} -g_debug("Creating GtkMenu for foreign menu"); -current_iso = ovirt_foreign_menu_get_current_iso_name(foreign_menu); -gtk_menu = gtk_menu_new(); -for (it = foreign_menu->priv->iso_names; it != NULL; it = it->next) { -GtkWidget *menuitem; - -menuitem = gtk_check_menu_item_new_with_label((char *)it->data); -if (g_strcmp0((char *)it->data, current_iso) == 0) { -g_warn_if_fail(g_strcmp0(current_iso, foreign_menu->priv->current_iso_name) == 0); -gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(menuitem), - TRUE); -} -g_signal_connect(menuitem, "activate", - G_CALLBACK(ovirt_foreign_menu_activate_item_cb), - foreign_menu); -gtk_menu_shell_append(GTK_MENU_SHELL(gtk_menu), menuitem); -} -g_free(current_iso); - -return gtk_menu; -} - - static void ovirt_foreign_menu_set_files(OvirtForeignMenu *menu, const GList *files) { @@ -594,7 +496,6 @@ static void cdrom_file_refreshed_cb(GObject *source_object, "file", >priv->current_is