[virt-tools-list] [PATCH virt-viewer 2/2] Report more detail about oVirt connection failures

Jonathon Jongsma jjongsma at redhat.com
Thu Aug 7 15:20:56 UTC 2014


Previously, when a user tried to connect to an ovirt vm using an
ovirt:// URI, all connection failures were reported to the user with a
simple "Couldn't open oVirt Session" error dialog. Now we report a
slightly more detailed error message to the user (e.g. "Unauthorized",
"No vm specified", etc.

This change also catches an error case that wasn't handled before: an
empty string is no longer a valid vm name.
---
 src/remote-viewer.c | 95 +++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 67 insertions(+), 28 deletions(-)

diff --git a/src/remote-viewer.c b/src/remote-viewer.c
index b2cf748..30f8444 100644
--- a/src/remote-viewer.c
+++ b/src/remote-viewer.c
@@ -624,7 +624,7 @@ remote_viewer_window_added(VirtViewerApp *app,
 
 #ifdef HAVE_OVIRT
 static gboolean
-parse_ovirt_uri(const gchar *uri_str, char **rest_uri, char **name, char **username)
+parse_ovirt_uri(const gchar *uri_str, char **rest_uri, char **name, char **username, GError** error)
 {
     char *vm_name = NULL;
     char *rel_path;
@@ -637,16 +637,29 @@ parse_ovirt_uri(const gchar *uri_str, char **rest_uri, char **name, char **usern
     g_return_val_if_fail(name != NULL, FALSE);
 
     uri = xmlParseURI(uri_str);
-    if (uri == NULL)
+    if (uri == NULL) {
+        if (error)
+            *error = g_error_new_literal(VIRT_VIEWER_ERROR,
+                                         VIRT_VIEWER_ERROR_FAILED,
+                                         _("URI is not valid"));
         return FALSE;
+    }
 
     if (g_strcmp0(uri->scheme, "ovirt") != 0) {
         xmlFreeURI(uri);
+        if (error)
+            *error = g_error_new_literal(VIRT_VIEWER_ERROR,
+                                         VIRT_VIEWER_ERROR_FAILED,
+                                         _("URI is not an ovirt URI"));
         return FALSE;
     }
 
     if (uri->path == NULL) {
         xmlFreeURI(uri);
+        if (error)
+            *error = g_error_new_literal(VIRT_VIEWER_ERROR,
+                                         VIRT_VIEWER_ERROR_FAILED,
+                                         _("No virtual machine specified"));
         return FALSE;
     }
 
@@ -654,12 +667,19 @@ parse_ovirt_uri(const gchar *uri_str, char **rest_uri, char **name, char **usern
     path_elements = g_strsplit(uri->path, "/", -1);
 
     element_count = g_strv_length(path_elements);
-    if (element_count == 0) {
+    if (element_count > 0)
+        vm_name = path_elements[element_count - 1];
+
+    if (vm_name == NULL || *vm_name == '\0') {
         g_strfreev(path_elements);
         xmlFreeURI(uri);
+        if (error)
+            *error = g_error_new_literal(VIRT_VIEWER_ERROR,
+                                         VIRT_VIEWER_ERROR_FAILED,
+                                         _("No virtual machine specified"));
         return FALSE;
     }
-    vm_name = path_elements[element_count-1];
+
     path_elements[element_count-1] = NULL;
 
     if (username && uri->user)
@@ -712,7 +732,7 @@ authenticate_cb(RestProxy *proxy, G_GNUC_UNUSED RestProxyAuth *auth,
 
 
 static gboolean
-create_ovirt_session(VirtViewerApp *app, const char *uri)
+create_ovirt_session(VirtViewerApp *app, const char *uri, GError **error)
 {
     OvirtProxy *proxy = NULL;
     OvirtApi *api = NULL;
@@ -720,7 +740,6 @@ create_ovirt_session(VirtViewerApp *app, const char *uri)
     OvirtVm *vm = NULL;
     OvirtVmDisplay *display = NULL;
     OvirtVmState state;
-    GError *error = NULL;
     char *rest_uri = NULL;
     char *vm_name = NULL;
     char *username = NULL;
@@ -729,6 +748,7 @@ create_ovirt_session(VirtViewerApp *app, const char *uri)
     guint secure_port;
     OvirtVmDisplayType type;
     const char *session_type;
+    GError* e = NULL;
 
     gchar *gport = NULL;
     gchar *gtlsport = NULL;
@@ -738,12 +758,10 @@ create_ovirt_session(VirtViewerApp *app, const char *uri)
 
     g_return_val_if_fail(VIRT_VIEWER_IS_APP(app), FALSE);
 
-    if (!parse_ovirt_uri(uri, &rest_uri, &vm_name, &username))
-        goto error;
-    proxy = ovirt_proxy_new(rest_uri);
-    if (proxy == NULL)
+    if (!parse_ovirt_uri(uri, &rest_uri, &vm_name, &username, &e))
         goto error;
 
+    proxy = ovirt_proxy_new(rest_uri);
     g_object_set(proxy,
                  "username", username,
                  NULL);
@@ -751,32 +769,43 @@ create_ovirt_session(VirtViewerApp *app, const char *uri)
     g_signal_connect(G_OBJECT(proxy), "authenticate",
                      G_CALLBACK(authenticate_cb), app);
 
-    api = ovirt_proxy_fetch_api(proxy, &error);
-    if (error != NULL) {
-        g_debug("failed to get oVirt 'api' collection: %s", error->message);
+    api = ovirt_proxy_fetch_api(proxy, &e);
+    if (e) {
+        g_debug("failed to get oVirt 'api' collection");
         goto error;
     }
     vms = ovirt_api_get_vms(api);
-    ovirt_collection_fetch(vms, proxy, &error);
-    if (error != NULL) {
-        g_debug("failed to lookup %s: %s", vm_name, error->message);
+    if (!ovirt_collection_fetch(vms, proxy, &e)) {
+        g_debug("failed to lookup %s", vm_name);
         goto error;
     }
     vm = OVIRT_VM(ovirt_collection_lookup_resource(vms, vm_name));
-    g_return_val_if_fail(vm != NULL, FALSE);
+    if (!vm) {
+        e = g_error_new(VIRT_VIEWER_ERROR,
+                        VIRT_VIEWER_ERROR_FAILED,
+                        "Unable to find virtual machine '%s'",
+                        vm_name);
+        goto error;
+    }
     g_object_get(G_OBJECT(vm), "state", &state, NULL);
     if (state != OVIRT_VM_STATE_UP) {
-        g_debug("oVirt VM %s is not running", vm_name);
+        e = g_error_new(VIRT_VIEWER_ERROR,
+                        VIRT_VIEWER_ERROR_FAILED,
+                        "oVirt VM %s is not running",
+                        vm_name);
         goto error;
     }
 
-    if (!ovirt_vm_get_ticket(vm, proxy, &error)) {
-        g_debug("failed to get ticket for %s: %s", vm_name, error->message);
+    if (!ovirt_vm_get_ticket(vm, proxy, &e)) {
+        g_debug("failed to get ticket for %s", vm_name);
         goto error;
     }
 
     g_object_get(G_OBJECT(vm), "display", &display, NULL);
     if (display == NULL) {
+        e = g_error_new_literal(VIRT_VIEWER_ERROR,
+                                VIRT_VIEWER_ERROR_FAILED,
+                                _("Unable to get the display for the requested virtual machine"));
         goto error;
     }
 
@@ -796,15 +825,22 @@ create_ovirt_session(VirtViewerApp *app, const char *uri)
     } else if (type == OVIRT_VM_DISPLAY_VNC) {
         session_type = "vnc";
     } else {
-        g_debug("Unknown display type: %d", type);
+        e = g_error_new(VIRT_VIEWER_ERROR,
+                        VIRT_VIEWER_ERROR_FAILED,
+                        "Unknown display type: %d",
+                        type);
         goto error;
     }
 
     virt_viewer_app_set_connect_info(app, NULL, ghost, gport, gtlsport,
                                      session_type, NULL, NULL, 0, NULL);
 
-    if (virt_viewer_app_create_session(app, session_type) < 0)
+    if (virt_viewer_app_create_session(app, session_type) < 0) {
+        e = g_error_new_literal(VIRT_VIEWER_ERROR,
+                                VIRT_VIEWER_ERROR_FAILED,
+                                _("Unable to create session"));
         goto error;
+    }
 
 #ifdef HAVE_SPICE_GTK
     if (type == OVIRT_VM_DISPLAY_SPICE) {
@@ -826,8 +862,6 @@ create_ovirt_session(VirtViewerApp *app, const char *uri)
     }
 #endif
 
-    success = TRUE;
-
 error:
     g_free(username);
     g_free(rest_uri);
@@ -838,8 +872,6 @@ error:
     g_free(ghost);
     g_free(host_subject);
 
-    if (error != NULL)
-        g_error_free(error);
     if (display != NULL)
         g_object_unref(display);
     if (vm != NULL)
@@ -849,6 +881,12 @@ error:
     if (proxy != NULL)
         g_object_unref(proxy);
 
+    success = (e == NULL);
+    if (error)
+        *error = e;
+    else
+        g_clear_error(&e);
+
     return success;
 }
 
@@ -1075,8 +1113,9 @@ retry_dialog:
         }
 #ifdef HAVE_OVIRT
         if (g_strcmp0(type, "ovirt") == 0) {
-            if (!create_ovirt_session(app, guri)) {
-                virt_viewer_app_simple_message_dialog(app, _("Couldn't open oVirt session"));
+            if (!create_ovirt_session(app, guri, &error)) {
+                virt_viewer_app_simple_message_dialog(app, _("Couldn't open oVirt session: %s"), error->message);
+                g_clear_error(&error);
                 goto cleanup;
             }
         } else
-- 
1.9.3




More information about the virt-tools-list mailing list