----- Original Message -----
> From: "Christophe Fergeau" <cfergeau@xxxxxxxxxx>
> To: virt-tools-list@xxxxxxxxxx
> Sent: Wednesday, April 16, 2014 11:59:53 AM
> Subject: [virt-viewer 6/7] Implement ovirt_foreign_menu_new_from_file()
>
> This will create an OvirtForeignMenu instance from a .vv file. Contrary to
> the ovirt:// case when we already have an OvirtAPI and OvirtVm instance,
> when working from a .vv file, we'll need to get them from the REST API.
> Authentication should happen through the JSESSIONID cookie, if that fails
> we want to give up on using the foreign menu, so we don't need to set up
> authentication callbacks.
> ---
> src/ovirt-foreign-menu.c | 174
> +++++++++++++++++++++++++++++++++++++++++++++++
> src/ovirt-foreign-menu.h | 2 +
> 2 files changed, 176 insertions(+)
>
> diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
> index 642c0ef..57ffd60 100644
> --- a/src/ovirt-foreign-menu.c
> +++ b/src/ovirt-foreign-menu.c
> @@ -25,6 +25,8 @@
>
> #include <config.h>
>
> +#include <string.h>
> +
> #include "ovirt-foreign-menu.h"
> #include "virt-glib-compat.h"
>
> @@ -35,12 +37,16 @@
>
> typedef enum {
> STATE_0,
> + STATE_API,
> + STATE_VM,
> STATE_STORAGE_DOMAIN,
> STATE_VM_CDROM,
> STATE_ISOS
> } OvirtForeignMenuState;
>
> static void ovirt_foreign_menu_next_async_step(OvirtForeignMenu *menu,
> OvirtForeignMenuState state);
> +static void ovirt_foreign_menu_fetch_api_async(OvirtForeignMenu *menu);
> +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 gboolean ovirt_foreign_menu_refresh_iso_list(gpointer user_data);
> @@ -52,11 +58,13 @@ struct _OvirtForeignMenuPrivate {
> OvirtProxy *proxy;
> OvirtApi *api;
> OvirtVm *vm;
> + char *vm_guid;
>
> OvirtCollection *files;
> OvirtCdrom *cdrom;
>
> GList *iso_names;
> +
> };
>
>
> @@ -69,6 +77,7 @@ enum {
> PROP_API,
> PROP_VM,
> PROP_FILES,
> + PROP_VM_GUID,
> };
>
> static void
> @@ -90,6 +99,10 @@ ovirt_foreign_menu_get_property(GObject *object, guint
> property_id,
> break;
> case PROP_FILES:
> g_value_set_pointer(value, priv->iso_names);
> + break;
Looks like there was a missing break here before? I guess it's not a huge deal, since you'll just get an extraneous invalid property warning when you get the 'files' property, but it might be worth fixing that in the patch 2 where you introduced this class.
> + case PROP_VM_GUID:
> + g_value_set_string(value, priv->vm_guid);
> + break;
>
> default:
> G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
> @@ -122,6 +135,15 @@ ovirt_foreign_menu_set_property(GObject *object, guint
> property_id,
> g_object_unref(priv->vm);
> }
> priv->vm = g_value_dup_object(value);
> + g_free(priv->vm_guid);
> + priv->vm_guid = NULL;
> + if (priv->vm != NULL) {
> + g_object_get(G_OBJECT(priv->vm), "guid", &priv->vm_guid, NULL);
> + }
> + break;
> + case PROP_VM_GUID:
> + g_free(priv->vm_guid);
> + priv->vm_guid = g_value_dup_string(value);
Is it valid to set the vm-guid property when priv->vm is non-null? Should we issue a warning if priv->vm is already set?
> break;
> default:
> G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
> @@ -150,6 +172,9 @@ ovirt_foreign_menu_dispose(GObject *obj)
> 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;
> @@ -212,6 +237,14 @@ ovirt_foreign_menu_class_init(OvirtForeignMenuClass
> *klass)
> "GSList of ISO
> names for this
> oVirt VM",
> G_PARAM_READABLE |
> G_PARAM_STATIC_STRINGS));
> + g_object_class_install_property(oclass,
> + PROP_VM_GUID,
> + g_param_spec_string("vm-guid",
> + "VM GUID",
> + "GUID of the
> virtual machine to provide a foreign menu for",
> + NULL,
> + G_PARAM_READABLE |
> +
> G_PARAM_STATIC_STRINGS));
> }
>
>
> @@ -236,6 +269,22 @@ ovirt_foreign_menu_next_async_step(OvirtForeignMenu
> *menu,
> {
> current_state++;
>
> + if (current_state == STATE_API) {
> + if (menu->priv->api == NULL) {
> + ovirt_foreign_menu_fetch_api_async(menu);
> + } else {
> + current_state++;
> + }
> + }
> +
> + if (current_state == STATE_VM) {
> + if (menu->priv->vm == NULL) {
> + ovirt_foreign_menu_fetch_vm_async(menu);
> + } else {
> + current_state++;
> + }
> + }
> +
> if (current_state == STATE_STORAGE_DOMAIN) {
> if (menu->priv->files == NULL) {
> ovirt_foreign_menu_fetch_storage_domain_async(menu);
> @@ -530,6 +579,88 @@ static void
> ovirt_foreign_menu_fetch_storage_domain_async(OvirtForeignMenu *menu
> }
>
>
> +static void vms_fetched_cb(GObject *source_object,
> + GAsyncResult *result,
> + gpointer user_data)
> +{
> + GError *error = NULL;
> + OvirtForeignMenu *menu = OVIRT_FOREIGN_MENU(user_data);
> + OvirtCollection *collection;
> + GList *vms;
> + GList *it;
> +
> + collection = OVIRT_COLLECTION(source_object);
> + ovirt_collection_fetch_finish(collection, result, &error);
> + if (error != NULL) {
> + g_debug("failed to fetch VM list: %s", error->message);
> + g_clear_error(&error);
> + return;
> + }
> +
> + vms =
> g_hash_table_get_values(ovirt_collection_get_resources(collection));
GHashTableIter?
> + for (it = vms; it != NULL; it = it->next) {
> + char *guid;
> +
> + g_object_get(G_OBJECT(it), "guid", &guid, NULL);
'it' is GList*, I assume you want it->data (assuming you don't change to GHashTableIter...)
> + if (g_strcmp0(guid, menu->priv->vm_guid) == 0) {
> + menu->priv->vm = g_object_ref(it);
same issue here. Also, I assume you can simply break out of the loop here after you find the matching GUID?
> + }
> + }
> + g_list_free(vms);
> + if (menu->priv->vm != NULL) {
> + ovirt_foreign_menu_next_async_step(menu, STATE_VM);
> + } else {
> + g_debug("failed to find a VM with guid \"%s\"",
> menu->priv->vm_guid);
warning?
> + }
> +}
> +
> +
> +static void ovirt_foreign_menu_fetch_vm_async(OvirtForeignMenu *menu)
> +{
> + OvirtCollection *vms;
> +
> + g_return_if_fail(OVIRT_IS_FOREIGN_MENU(menu));
> + g_return_if_fail(OVIRT_IS_PROXY(menu->priv->proxy));
> + g_return_if_fail(OVIRT_IS_API(menu->priv->api));
> +
> + vms = ovirt_api_get_vms(menu->priv->api);
> + ovirt_collection_fetch_async(vms, menu->priv->proxy,
> + NULL, vms_fetched_cb, menu);
> +}
> +
> +
> +static void api_fetched_cb(GObject *source_object,
> + GAsyncResult *result,
> + gpointer user_data)
> +{
> + GError *error = NULL;
> + OvirtProxy *proxy;
> + OvirtForeignMenu *menu = OVIRT_FOREIGN_MENU(user_data);
> +
> + proxy = OVIRT_PROXY(source_object);
> + menu->priv->api = ovirt_proxy_fetch_api_finish(proxy, result, &error);
> + if (error != NULL) {
> + g_debug("failed to fetch toplevel API object: %s", error->message);
> + g_clear_error(&error);
> + return;
> + }
> + g_return_if_fail(OVIRT_IS_API(menu->priv->api));
> +
> + ovirt_foreign_menu_next_async_step(menu, STATE_API);
> +}
> +
> +
> +static void ovirt_foreign_menu_fetch_api_async(OvirtForeignMenu *menu)
> +{
> + g_debug("Start fetching oVirt main entry point");
> +
> + g_return_if_fail(OVIRT_IS_FOREIGN_MENU(menu));
> + g_return_if_fail(OVIRT_IS_PROXY(menu->priv->proxy));
> +
> + ovirt_proxy_fetch_api_async(menu->priv->proxy, NULL, api_fetched_cb,
> menu);
> +}
> +
> +
> static void iso_list_fetched_cb(GObject *source_object,
> GAsyncResult *result,
> gpointer user_data)
> @@ -578,3 +709,46 @@ static gboolean
> ovirt_foreign_menu_refresh_iso_list(gpointer user_data)
> */
> return G_SOURCE_REMOVE;
> }
> +
> +
> +OvirtForeignMenu *ovirt_foreign_menu_new_from_file(VirtViewerFile *file)
> +{
> + OvirtProxy *proxy = NULL;
> + OvirtForeignMenu *menu = NULL;
> + char *ca_str = NULL;
> + char *jsessionid = NULL;
> + char *url = NULL;
> + char *vm_guid = NULL;
> + GByteArray *ca = NULL;
> +
> + url = virt_viewer_file_get_ovirt_host(file);
> + vm_guid = virt_viewer_file_get_ovirt_vm_guid(file);
> + jsessionid = virt_viewer_file_get_ovirt_jsessionid(file);
> + ca_str = virt_viewer_file_get_ovirt_ca(file);
> +
> + if ((url == NULL) || (vm_guid == NULL))
> + goto end;
> +
> + proxy = ovirt_proxy_new(url);
> + if (proxy == NULL)
> + goto end;
> +
> + if (ca_str != NULL) {
> + ca = g_byte_array_new_take((guint8 *)ca_str, strlen(ca_str) + 1);
> + ca_str = NULL;
> + }
> +
> + g_object_set(G_OBJECT(proxy),
> + "session-id", jsessionid,
> + "ca-cert", ca,
> + NULL);
> + menu = ovirt_foreign_menu_new(proxy);
> + g_object_set(G_OBJECT(menu), "guid", vm_guid, NULL);
> +end:
> + g_free(url);
> + g_free(vm_guid);
> + g_free(jsessionid);
> + g_free(ca_str);
'ca' byte array is leaked?
> +
> + return menu;
> +}
> diff --git a/src/ovirt-foreign-menu.h b/src/ovirt-foreign-menu.h
> index ffadd46..4ba77a3 100644
> --- a/src/ovirt-foreign-menu.h
> +++ b/src/ovirt-foreign-menu.h
> @@ -29,6 +29,7 @@
> #include <govirt/govirt.h>
> #include <gtk/gtk.h>
>
> +#include "virt-viewer-file.h"
>
> G_BEGIN_DECLS
>
> @@ -66,6 +67,7 @@ struct _OvirtForeignMenuClass {
> GType ovirt_foreign_menu_get_type(void);
>
> 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);
> --
> 1.9.0
>
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/virt-tools-list
>
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
