Hi,
Some small nitpicks inline...
On 03/07/2013 08:55 PM, Marc-André Lureau wrote:
---
src/remote-viewer.c | 31 ++++++++++++++++++-----------
src/virt-viewer-app.c | 44 ++++++++++++++++++++---------------------
src/virt-viewer-app.h | 8 ++++----
src/virt-viewer-file.c | 2 ++
src/virt-viewer-session-spice.c | 4 ++--
src/virt-viewer-session-vnc.c | 5 +++--
src/virt-viewer-session.c | 4 ++--
src/virt-viewer-session.h | 4 ++--
src/virt-viewer.c | 43 +++++++++++++++++++++++-----------------
9 files changed, 82 insertions(+), 63 deletions(-)
diff --git a/src/remote-viewer.c b/src/remote-viewer.c
index fa17edd..216d0d8 100644
--- a/src/remote-viewer.c
+++ b/src/remote-viewer.c
@@ -66,7 +66,7 @@ enum {
static gboolean remote_viewer_start(VirtViewerApp *self);
#if HAVE_SPICE_GTK
-static int remote_viewer_activate(VirtViewerApp *self);
+static gboolean remote_viewer_activate(VirtViewerApp *self, GError **error);
static void remote_viewer_window_added(VirtViewerApp *self, VirtViewerWindow *win);
static void spice_foreign_menu_updated(RemoteViewer *self);
#endif
@@ -236,8 +236,13 @@ static void
spice_ctrl_do_connect(SpiceCtrlController *ctrl G_GNUC_UNUSED,
VirtViewerApp *self)
{
- if (virt_viewer_app_initial_connect(self) < 0) {
- virt_viewer_app_simple_message_dialog(self, _("Failed to initiate connection"));
+ GError *error = NULL;
+
+ if (!virt_viewer_app_initial_connect(self, &error)) {
+ const gchar *msg = error ? error->message :
+ _("Failed to initiate connection");
+ virt_viewer_app_simple_message_dialog(self, msg);
+ g_clear_error(&error);
}
}
@@ -558,19 +563,19 @@ spice_ctrl_listen_async_cb(GObject *object,
}
-static int
-remote_viewer_activate(VirtViewerApp *app)
+static gboolean
+remote_viewer_activate(VirtViewerApp *app, GError **error)
{
- g_return_val_if_fail(REMOTE_VIEWER_IS(app), -1);
+ g_return_val_if_fail(REMOTE_VIEWER_IS(app), FALSE);
RemoteViewer *self = REMOTE_VIEWER(app);
- int ret = -1;
+ gboolean ret = FALSE;
if (self->priv->controller) {
SpiceSession *session = remote_viewer_get_spice_session(self);
ret = spice_session_connect(session);
g_object_unref(session);
} else {
- ret = VIRT_VIEWER_APP_CLASS(remote_viewer_parent_class)->activate(app);
+ ret = VIRT_VIEWER_APP_CLASS(remote_viewer_parent_class)->activate(app, error);
}
return ret;
@@ -599,6 +604,7 @@ remote_viewer_start(VirtViewerApp *app)
gboolean ret = FALSE;
gchar *guri = NULL;
gchar *type = NULL;
+ GError *error = NULL;
#if HAVE_SPICE_GTK
g_signal_connect(app, "notify", G_CALLBACK(app_notified), self);
@@ -631,7 +637,6 @@ remote_viewer_start(VirtViewerApp *app)
file = g_file_new_for_commandline_arg(guri);
if (g_file_query_exists(file, NULL)) {
- GError *error = NULL;
gchar *path = g_file_get_path(file);
vvfile = virt_viewer_file_new(path, &error);
g_free(path);
@@ -654,8 +659,12 @@ remote_viewer_start(VirtViewerApp *app)
virt_viewer_session_set_file(virt_viewer_app_get_session(app), vvfile);
- if (virt_viewer_app_initial_connect(app) < 0) {
- virt_viewer_app_simple_message_dialog(app, _("Failed to initiate connection"));
+ if (!virt_viewer_app_initial_connect(app, &error)) {
+ const gchar *msg = error ? error->message :
+ _("Failed to initiate connection");
+
+ virt_viewer_app_simple_message_dialog(app, msg);
+ g_clear_error(&error);
goto cleanup;
}
#if HAVE_SPICE_GTK
diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
index 25df5c8..2e6c76d 100644
--- a/src/virt-viewer-app.c
+++ b/src/virt-viewer-app.c
@@ -860,14 +860,14 @@ virt_viewer_app_channel_open(VirtViewerSession *session G_GNUC_UNUSED,
}
#endif
-static int
-virt_viewer_app_default_activate(VirtViewerApp *self)
+static gboolean
+virt_viewer_app_default_activate(VirtViewerApp *self, GError **error)
{
VirtViewerAppPrivate *priv = self->priv;
int fd = -1;
if (!virt_viewer_app_open_connection(self, &fd))
- return -1;
+ return FALSE;
DEBUG_LOG("After open connection callback fd=%d", fd);
@@ -897,12 +897,12 @@ virt_viewer_app_default_activate(VirtViewerApp *self)
if ((fd = virt_viewer_app_open_tunnel_ssh(priv->host, priv->port,
priv->user, priv->ghost,
priv->gport, priv->unixsock)) < 0)
- return -1;
+ return FALSE;
} else if (priv->unixsock && fd == -1) {
virt_viewer_app_trace(self, "Opening direct UNIX connection to display at %s",
priv->unixsock);
if ((fd = virt_viewer_app_open_unix_sock(priv->unixsock)) < 0)
- return -1;
+ return FALSE;
}
#endif
@@ -910,7 +910,7 @@ virt_viewer_app_default_activate(VirtViewerApp *self)
return virt_viewer_session_open_fd(VIRT_VIEWER_SESSION(priv->session), fd);
} else if (priv->guri) {
virt_viewer_app_trace(self, "Opening connection to display at %s", priv->guri);
- return virt_viewer_session_open_uri(VIRT_VIEWER_SESSION(priv->session), priv->guri);
+ return virt_viewer_session_open_uri(VIRT_VIEWER_SESSION(priv->session), priv->guri, error);
} else {
virt_viewer_app_trace(self, "Opening direct TCP connection to display at %s:%s:%s",
priv->ghost, priv->gport, priv->gtlsport ? priv->gtlsport : "-1");
@@ -918,24 +918,24 @@ virt_viewer_app_default_activate(VirtViewerApp *self)
priv->ghost, priv->gport, priv->gtlsport);
}
- return -1;
+ return FALSE;
}
-int
-virt_viewer_app_activate(VirtViewerApp *self)
+gboolean
+virt_viewer_app_activate(VirtViewerApp *self, GError **error)
{
VirtViewerAppPrivate *priv;
int ret;
ret should be changed into a gboolean here.
- g_return_val_if_fail(VIRT_VIEWER_IS_APP(self), -1);
+ g_return_val_if_fail(VIRT_VIEWER_IS_APP(self), FALSE);
priv = self->priv;
if (priv->active)
- return -1;
+ return FALSE;
- ret = VIRT_VIEWER_APP_GET_CLASS(self)->activate(self);
+ ret = VIRT_VIEWER_APP_GET_CLASS(self)->activate(self, error);
- if (ret == -1) {
+ if (ret == FALSE) {
priv->connected = FALSE;
} else {
virt_viewer_app_show_status(self, _("Connecting to graphic server"));
@@ -1004,21 +1004,21 @@ static void virt_viewer_app_bell(VirtViewerSession *session G_GNUC_UNUSED,
}
-static int
-virt_viewer_app_default_initial_connect(VirtViewerApp *self)
+static gboolean
+virt_viewer_app_default_initial_connect(VirtViewerApp *self, GError **error)
{
- return virt_viewer_app_activate(self);
+ return virt_viewer_app_activate(self, error);
}
-int
-virt_viewer_app_initial_connect(VirtViewerApp *self)
+gboolean
+virt_viewer_app_initial_connect(VirtViewerApp *self, GError **error)
{
VirtViewerAppClass *klass;
- g_return_val_if_fail(VIRT_VIEWER_IS_APP(self), -1);
+ g_return_val_if_fail(VIRT_VIEWER_IS_APP(self), FALSE);
klass = VIRT_VIEWER_APP_GET_CLASS(self);
- return klass->initial_connect(self);
+ return klass->initial_connect(self, error);
}
static gboolean
@@ -1026,7 +1026,7 @@ virt_viewer_app_retryauth(gpointer opaque)
{
VirtViewerApp *self = opaque;
- virt_viewer_app_initial_connect(self);
+ virt_viewer_app_initial_connect(self, NULL);
return FALSE;
}
@@ -1040,7 +1040,7 @@ virt_viewer_app_connect_timer(void *opaque)
DEBUG_LOG("Connect timer fired");
if (!priv->active &&
- virt_viewer_app_initial_connect(self) < 0)
+ virt_viewer_app_initial_connect(self, NULL) < 0)
gtk_main_quit();
if (priv->active) {
diff --git a/src/virt-viewer-app.h b/src/virt-viewer-app.h
index 663f7a1..6fc523b 100644
--- a/src/virt-viewer-app.h
+++ b/src/virt-viewer-app.h
@@ -52,8 +52,8 @@ typedef struct {
/*< private >*/
gboolean (*start) (VirtViewerApp *self);
- int (*initial_connect) (VirtViewerApp *self);
- int (*activate) (VirtViewerApp *self);
+ gboolean (*initial_connect) (VirtViewerApp *self, GError **error);
+ gboolean (*activate) (VirtViewerApp *self, GError **error);
void (*deactivated) (VirtViewerApp *self);
gboolean (*open_connection)(VirtViewerApp *self, int *fd);
} VirtViewerAppClass;
@@ -71,8 +71,8 @@ void virt_viewer_app_simple_message_dialog(VirtViewerApp *self, const char *fmt,
gboolean virt_viewer_app_is_active(VirtViewerApp *app);
void virt_viewer_app_free_connect_info(VirtViewerApp *self);
int virt_viewer_app_create_session(VirtViewerApp *self, const gchar *type);
-int virt_viewer_app_activate(VirtViewerApp *self);
-int virt_viewer_app_initial_connect(VirtViewerApp *self);
+gboolean virt_viewer_app_activate(VirtViewerApp *self, GError **error);
+gboolean virt_viewer_app_initial_connect(VirtViewerApp *self, GError **error);
void virt_viewer_app_start_reconnect_poll(VirtViewerApp *self);
void virt_viewer_app_set_zoom_level(VirtViewerApp *self, gint zoom_level);
void virt_viewer_app_set_direct(VirtViewerApp *self, gboolean direct);
diff --git a/src/virt-viewer-file.c b/src/virt-viewer-file.c
index 7271156..33b9666 100644
--- a/src/virt-viewer-file.c
+++ b/src/virt-viewer-file.c
@@ -20,6 +20,8 @@
*/
#include <config.h>
+#include <glib/gi18n.h>
+
Hmm, unrelated / unneeded change ?
#include "virt-viewer-util.h"
#include "virt-viewer-file.h"
diff --git a/src/virt-viewer-session-spice.c b/src/virt-viewer-session-spice.c
index 80a3f34..cfe31ac 100644
--- a/src/virt-viewer-session-spice.c
+++ b/src/virt-viewer-session-spice.c
@@ -64,7 +64,7 @@ enum {
static void virt_viewer_session_spice_close(VirtViewerSession *session);
static gboolean virt_viewer_session_spice_open_fd(VirtViewerSession *session, int fd);
static gboolean virt_viewer_session_spice_open_host(VirtViewerSession *session, const gchar *host, const gchar *port, const gchar *tlsport);
-static gboolean virt_viewer_session_spice_open_uri(VirtViewerSession *session, const gchar *uri);
+static gboolean virt_viewer_session_spice_open_uri(VirtViewerSession *session, const gchar *uri, GError **error);
static gboolean virt_viewer_session_spice_channel_open_fd(VirtViewerSession *session, VirtViewerSessionChannel *channel, int fd);
static gboolean virt_viewer_session_spice_has_usb(VirtViewerSession *session);
static void virt_viewer_session_spice_usb_device_selection(VirtViewerSession *session, GtkWindow *parent);
@@ -342,7 +342,7 @@ fill_session(VirtViewerFile *file, SpiceSession *session)
static gboolean
virt_viewer_session_spice_open_uri(VirtViewerSession *session,
- const gchar *uri)
+ const gchar *uri, GError **error)
{
VirtViewerSessionSpice *self = VIRT_VIEWER_SESSION_SPICE(session);
VirtViewerFile *file = virt_viewer_session_get_file(session);
diff --git a/src/virt-viewer-session-vnc.c b/src/virt-viewer-session-vnc.c
index 3abd4e4..bbebc7e 100644
--- a/src/virt-viewer-session-vnc.c
+++ b/src/virt-viewer-session-vnc.c
@@ -44,7 +44,7 @@ struct _VirtViewerSessionVncPrivate {
static void virt_viewer_session_vnc_close(VirtViewerSession* session);
static gboolean virt_viewer_session_vnc_open_fd(VirtViewerSession* session, int fd);
static gboolean virt_viewer_session_vnc_open_host(VirtViewerSession* session, const gchar *host, const gchar *port, const gchar *tlsport);
-static gboolean virt_viewer_session_vnc_open_uri(VirtViewerSession* session, const gchar *uri);
+static gboolean virt_viewer_session_vnc_open_uri(VirtViewerSession* session, const gchar *uri, GError **error);
static gboolean virt_viewer_session_vnc_channel_open_fd(VirtViewerSession* session,
VirtViewerSessionChannel* channel, int fd);
@@ -198,7 +198,8 @@ virt_viewer_session_vnc_open_host(VirtViewerSession* session,
static gboolean
virt_viewer_session_vnc_open_uri(VirtViewerSession* session,
- const gchar *uristr)
+ const gchar *uristr,
+ GError **error)
{
VirtViewerSessionVnc *self = VIRT_VIEWER_SESSION_VNC(session);
VirtViewerFile *file = virt_viewer_session_get_file(session);
diff --git a/src/virt-viewer-session.c b/src/virt-viewer-session.c
index 16c878a..07e7226 100644
--- a/src/virt-viewer-session.c
+++ b/src/virt-viewer-session.c
@@ -374,7 +374,7 @@ gboolean virt_viewer_session_open_host(VirtViewerSession *session, const gchar *
return klass->open_host(session, host, port, tlsport);
}
-gboolean virt_viewer_session_open_uri(VirtViewerSession *session, const gchar *uri)
+gboolean virt_viewer_session_open_uri(VirtViewerSession *session, const gchar *uri, GError **error)
{
VirtViewerSessionClass *klass;
@@ -385,7 +385,7 @@ gboolean virt_viewer_session_open_uri(VirtViewerSession *session, const gchar *u
session->priv->uri = g_strdup(uri);
- return klass->open_uri(session, uri);
+ return klass->open_uri(session, uri, error);
}
const gchar* virt_viewer_session_mime_type(VirtViewerSession *self)
diff --git a/src/virt-viewer-session.h b/src/virt-viewer-session.h
index f337937..5aa748c 100644
--- a/src/virt-viewer-session.h
+++ b/src/virt-viewer-session.h
@@ -68,7 +68,7 @@ struct _VirtViewerSessionClass {
void (* close) (VirtViewerSession* session);
gboolean (* open_fd) (VirtViewerSession* session, int fd);
gboolean (* open_host) (VirtViewerSession* session, const gchar *host, const gchar *port, const gchar *tlsport);
- gboolean (* open_uri) (VirtViewerSession* session, const gchar *uri);
+ gboolean (* open_uri) (VirtViewerSession* session, const gchar *uri, GError **error);
gboolean (* channel_open_fd) (VirtViewerSession* session, VirtViewerSessionChannel *channel, int fd);
gboolean (* has_usb) (VirtViewerSession* session);
void (* usb_device_selection) (VirtViewerSession* session, GtkWindow *parent);
@@ -114,7 +114,7 @@ gboolean virt_viewer_session_open_host(VirtViewerSession* session, const gchar *
GObject* virt_viewer_session_get(VirtViewerSession* session);
gboolean virt_viewer_session_channel_open_fd(VirtViewerSession* session,
VirtViewerSessionChannel* channel, int fd);
-gboolean virt_viewer_session_open_uri(VirtViewerSession *session, const gchar *uri);
+gboolean virt_viewer_session_open_uri(VirtViewerSession *session, const gchar *uri, GError **error);
void virt_viewer_session_set_auto_usbredir(VirtViewerSession* session, gboolean auto_usbredir);
gboolean virt_viewer_session_get_auto_usbredir(VirtViewerSession* session);
diff --git a/src/virt-viewer.c b/src/virt-viewer.c
index aedbf73..8d4ec62 100644
--- a/src/virt-viewer.c
+++ b/src/virt-viewer.c
@@ -63,7 +63,7 @@ G_DEFINE_TYPE (VirtViewer, virt_viewer, VIRT_VIEWER_TYPE_APP)
#define GET_PRIVATE(o) \
(G_TYPE_INSTANCE_GET_PRIVATE ((o), VIRT_VIEWER_TYPE, VirtViewerPrivate))
-static int virt_viewer_initial_connect(VirtViewerApp *self);
+static gboolean virt_viewer_initial_connect(VirtViewerApp *self, GError **error);
static gboolean virt_viewer_open_connection(VirtViewerApp *self, int *fd);
static void virt_viewer_deactivated(VirtViewerApp *self);
static gboolean virt_viewer_start(VirtViewerApp *self);
@@ -406,7 +406,7 @@ virt_viewer_extract_connect_info(VirtViewer *self,
return retval;
}
-static int
+static gboolean
virt_viewer_update_display(VirtViewer *self, virDomainPtr dom)
{
VirtViewerPrivate *priv = self->priv;
@@ -424,10 +424,10 @@ virt_viewer_update_display(VirtViewer *self, virDomainPtr dom)
if (!virt_viewer_app_has_session(app)) {
if (!virt_viewer_extract_connect_info(self, dom))
- return -1;
+ return FALSE;
}
- return 0;
+ return TRUE;
}
static gboolean
@@ -469,6 +469,7 @@ virt_viewer_domain_event(virConnectPtr conn G_GNUC_UNUSED,
{
VirtViewer *self = opaque;
VirtViewerApp *app = VIRT_VIEWER_APP(self);
+ GError *error = NULL;
DEBUG_LOG("Got domain event %d %d", event, detail);
@@ -482,7 +483,13 @@ virt_viewer_domain_event(virConnectPtr conn G_GNUC_UNUSED,
case VIR_DOMAIN_EVENT_STARTED:
virt_viewer_update_display(self, dom);
- virt_viewer_app_activate(app);
+ virt_viewer_app_activate(app, &error);
+ if (error) {
+ /* we may want to consolidate error reporting in
+ app_activate() instead */
+ g_warning("%s", error->message);
+ g_clear_error(&error);
+ }
break;
}
@@ -508,16 +515,15 @@ virt_viewer_conn_event(virConnectPtr conn G_GNUC_UNUSED,
static int virt_viewer_connect(VirtViewerApp *app);
-static int
-virt_viewer_initial_connect(VirtViewerApp *app)
+static gboolean
+virt_viewer_initial_connect(VirtViewerApp *app, GError **error)
{
virDomainPtr dom = NULL;
virDomainInfo info;
- int ret = -1;
+ gboolean ret = FALSE;
VirtViewer *self = VIRT_VIEWER(app);
VirtViewerPrivate *priv = self->priv;
-
DEBUG_LOG("initial connect");
if (!priv->conn &&
@@ -552,9 +558,9 @@ virt_viewer_initial_connect(VirtViewerApp *app)
virt_viewer_app_show_status(app, _("Waiting for guest domain to start"));
} else {
ret = virt_viewer_update_display(self, dom);
- if (ret >= 0)
- ret = VIRT_VIEWER_APP_CLASS(virt_viewer_parent_class)->initial_connect(app);
- if (ret < 0) {
+ if (ret)
+ ret = VIRT_VIEWER_APP_CLASS(virt_viewer_parent_class)->initial_connect(app, error);
+ if (!ret) {
if (priv->waitvm) {
virt_viewer_app_show_status(app, _("Waiting for guest domain to start server"));
virt_viewer_app_trace(app, "Guest %s has not activated its display yet, waiting for it to start",
@@ -563,15 +569,11 @@ virt_viewer_initial_connect(VirtViewerApp *app)
DEBUG_LOG("Failed to activate viewer");
goto cleanup;
}
- } else if (ret == 0) {
- DEBUG_LOG("Failed to activate viewer");
- ret = -1;
- goto cleanup;
}
}
done:
- ret = 0;
+ ret = TRUE;
cleanup:
if (dom)
virDomainFree(dom);
@@ -660,6 +662,7 @@ virt_viewer_connect(VirtViewerApp *app)
.cbdata = app,
};
int oflags = 0;
+ GError *error = NULL;
if (!virt_viewer_app_get_attach(app))
oflags |= VIR_CONNECT_RO;
@@ -678,8 +681,12 @@ virt_viewer_connect(VirtViewerApp *app)
return -1;
}
- if (virt_viewer_app_initial_connect(app) < 0)
+ if (!virt_viewer_app_initial_connect(app, &error)) {
+ if (error)
+ g_warning(error->message);
+ g_clear_error(&error);
return -1;
+ }
if (virConnectDomainEventRegister(priv->conn,
virt_viewer_domain_event,
Regards,
Hans
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
