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.
---
I could have sworn that I had already sent this revised patch to the list, but
I can't find any evidence of having sent it. Fixed issues from Christophe's
earlier review.
src/remote-viewer.c | 82 ++++++++++++++++++++++++++++++++++++++++-------------
1 file changed, 62 insertions(+), 20 deletions(-)
diff --git a/src/remote-viewer.c b/src/remote-viewer.c
index cb43365..03998bf 100644
--- a/src/remote-viewer.c
+++ b/src/remote-viewer.c
@@ -670,7 +670,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;
@@ -683,16 +683,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(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(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(VIRT_VIEWER_ERROR,
+ VIRT_VIEWER_ERROR_FAILED,
+ _("No virtual machine specified"));
return FALSE;
}
g_return_val_if_fail(*uri->path == '/', FALSE);
@@ -701,12 +714,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(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)
@@ -789,7 +809,7 @@ virt_viewer_app_set_ovirt_foreign_menu(VirtViewerApp *app,
static gboolean
-create_ovirt_session(VirtViewerApp *app, const char *uri)
+create_ovirt_session(VirtViewerApp *app, const char *uri, GError **err)
{
OvirtProxy *proxy = NULL;
OvirtApi *api = NULL;
@@ -815,12 +835,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, &error))
goto error;
+ proxy = ovirt_proxy_new(rest_uri);
g_object_set(proxy,
"username", username,
NULL);
@@ -830,19 +848,29 @@ create_ovirt_session(VirtViewerApp *app, const char *uri)
api = ovirt_proxy_fetch_api(proxy, &error);
if (error != NULL) {
- g_debug("failed to get oVirt 'api' collection: %s", error->message);
+ g_debug("Failed to get oVirt 'api' collection: %s", error->message);
goto error;
}
vms = ovirt_api_get_vms(api);
- ovirt_collection_fetch(vms, proxy, &error);
- if (error != NULL) {
+ if (!ovirt_collection_fetch(vms, proxy, &error)) {
g_debug("failed to lookup %s: %s", vm_name, error->message);
goto error;
}
vm = OVIRT_VM(ovirt_collection_lookup_resource(vms, vm_name));
- g_return_val_if_fail(vm != NULL, FALSE);
+ if (!vm) {
+ error = g_error_new(VIRT_VIEWER_ERROR,
+ VIRT_VIEWER_ERROR_FAILED,
+ "Unable to find virtual machine '%s'",
+ vm_name);
+ g_debug("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) {
+ error = g_error_new(VIRT_VIEWER_ERROR,
+ VIRT_VIEWER_ERROR_FAILED,
+ "oVirt VM %s is not running",
+ vm_name);
g_debug("oVirt VM %s is not running", vm_name);
goto error;
}
@@ -854,6 +882,9 @@ create_ovirt_session(VirtViewerApp *app, const char *uri)
g_object_get(G_OBJECT(vm), "display", &display, NULL);
if (display == NULL) {
+ error = g_error_new(VIRT_VIEWER_ERROR,
+ VIRT_VIEWER_ERROR_FAILED,
+ _("Unable to get the display for the requested virtual machine"));
goto error;
}
@@ -873,6 +904,10 @@ create_ovirt_session(VirtViewerApp *app, const char *uri)
} else if (type == OVIRT_VM_DISPLAY_VNC) {
session_type = "vnc";
} else {
+ error = g_error_new(VIRT_VIEWER_ERROR,
+ VIRT_VIEWER_ERROR_FAILED,
+ "Unknown display type: %d",
+ type);
g_debug("Unknown display type: %d", type);
goto error;
}
@@ -886,8 +921,12 @@ create_ovirt_session(VirtViewerApp *app, const char *uri)
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) {
+ error = g_error_new(VIRT_VIEWER_ERROR,
+ VIRT_VIEWER_ERROR_FAILED,
+ _("Unable to create session"));
goto error;
+ }
#ifdef HAVE_SPICE_GTK
if (type == OVIRT_VM_DISPLAY_SPICE) {
@@ -909,8 +948,6 @@ create_ovirt_session(VirtViewerApp *app, const char *uri)
}
#endif
- success = TRUE;
-
error:
g_free(username);
g_free(rest_uri);
@@ -921,8 +958,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)
@@ -932,6 +967,12 @@ error:
if (proxy != NULL)
g_object_unref(proxy);
+ success = (error == NULL);
+ if (err)
+ *err = error;
+ else
+ g_clear_error(&error);
+
return success;
}
@@ -1158,8 +1199,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
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
