On Fri, Jan 13, 2017 at 07:11:05PM -0200, Eduardo Lima (Etrunko) wrote:
> Similar to the previous commit, the ISO dialog will fetch the result
> asynchronously, rather than relying on the "notify::files" signal from
> OvirtForeignMenu object. It also enables error to be shown if anything
> goes wrong.
>
> Signed-off-by: Eduardo Lima (Etrunko) <etrunko@xxxxxxxxxx>
> ---
> src/ovirt-foreign-menu.c | 159 +++++++++++++++++++++++++----------------------
> src/ovirt-foreign-menu.h | 9 ++-
> src/remote-viewer.c | 45 ++++++++++++--
> 3 files changed, 135 insertions(+), 78 deletions(-)
>
> diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
> index 366259a..6892f0d 100644
> --- a/src/ovirt-foreign-menu.c
> +++ b/src/ovirt-foreign-menu.c
> @@ -40,13 +40,13 @@ typedef enum {
> 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 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_next_async_step(OvirtForeignMenu *menu, GTask *task, OvirtForeignMenuState state);
> +static void ovirt_foreign_menu_fetch_api_async(OvirtForeignMenu *menu, GTask *task);
> +static void ovirt_foreign_menu_fetch_vm_async(OvirtForeignMenu *menu, GTask *task);
> +static void ovirt_foreign_menu_fetch_storage_domain_async(OvirtForeignMenu *menu, GTask *task);
> +static void ovirt_foreign_menu_fetch_vm_cdrom_async(OvirtForeignMenu *menu, GTask *task);
> +static void ovirt_foreign_menu_refresh_cdrom_file_async(OvirtForeignMenu *menu, GTask *task);
> +static void ovirt_foreign_menu_fetch_iso_list_async(OvirtForeignMenu *menu, GTask *task);
Wondering if it would make sense to pass the GTask around as an
OvirtForeignMenu member?
>
> G_DEFINE_TYPE (OvirtForeignMenu, ovirt_foreign_menu, G_TYPE_OBJECT)
>
> @@ -273,11 +273,9 @@ OvirtForeignMenu* ovirt_foreign_menu_new(OvirtProxy *proxy)
>
> static void
> ovirt_foreign_menu_next_async_step(OvirtForeignMenu *menu,
> + GTask *task,
> OvirtForeignMenuState current_state)
> {
> - g_return_if_fail(current_state >= STATE_0);
> - g_return_if_fail(current_state < STATE_ISOS);
> -
Why drop this?
> /* 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
> @@ -286,26 +284,26 @@ ovirt_foreign_menu_next_async_step(OvirtForeignMenu *menu,
> switch (current_state + 1) {
> case STATE_API:
> if (menu->priv->api == NULL) {
> - ovirt_foreign_menu_fetch_api_async(menu);
> + ovirt_foreign_menu_fetch_api_async(menu, task);
> break;
> }
> case STATE_VM:
> if (menu->priv->vm == NULL) {
> - ovirt_foreign_menu_fetch_vm_async(menu);
> + ovirt_foreign_menu_fetch_vm_async(menu, task);
> break;
> }
> case STATE_STORAGE_DOMAIN:
> if (menu->priv->files == NULL) {
> - ovirt_foreign_menu_fetch_storage_domain_async(menu);
> + ovirt_foreign_menu_fetch_storage_domain_async(menu, task);
> break;
> }
> case STATE_VM_CDROM:
> if (menu->priv->cdrom == NULL) {
> - ovirt_foreign_menu_fetch_vm_cdrom_async(menu);
> + ovirt_foreign_menu_fetch_vm_cdrom_async(menu, task);
> break;
> }
> case STATE_CDROM_FILE:
> - ovirt_foreign_menu_refresh_cdrom_file_async(menu);
> + ovirt_foreign_menu_refresh_cdrom_file_async(menu, task);
> break;
> case STATE_ISOS:
> g_warn_if_fail(menu->priv->api != NULL);
> @@ -313,18 +311,35 @@ 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, task);
> break;
> default:
> g_warn_if_reached();
> + g_task_return_new_error(task, OVIRT_ERROR, OVIRT_ERROR_FAILED,
> + "Invalid state: %d", current_state);
> + g_object_unref(task);
> }
> }
>
>
> void
> -ovirt_foreign_menu_start(OvirtForeignMenu *menu)
> +ovirt_foreign_menu_fetch_iso_names_async(OvirtForeignMenu *menu,
> + GCancellable *cancellable,
> + GAsyncReadyCallback callback,
> + gpointer user_data)
> {
> - ovirt_foreign_menu_next_async_step(menu, STATE_0);
> + GTask *task = g_task_new(menu, cancellable, callback, user_data);
> + ovirt_foreign_menu_next_async_step(menu, task, STATE_0);
> +}
> +
> +
> +GList *
> +ovirt_foreign_menu_fetch_iso_names_finish(OvirtForeignMenu *foreign_menu,
> + GAsyncResult *result,
> + GError **error)
> +{
> + g_return_val_if_fail(OVIRT_IS_FOREIGN_MENU(foreign_menu), NULL);
> + return g_task_propagate_pointer(G_TASK(result), error);
> }
>
>
> @@ -545,7 +560,6 @@ static void ovirt_foreign_menu_set_files(OvirtForeignMenu *menu,
>
> g_list_free_full(menu->priv->iso_names, (GDestroyNotify)g_free);
> menu->priv->iso_names = sorted_files;
> - g_object_notify(G_OBJECT(menu), "files");
> }
>
>
> @@ -553,14 +567,16 @@ static void cdrom_file_refreshed_cb(GObject *source_object,
> GAsyncResult *result,
> gpointer user_data)
> {
> + GTask *task = G_TASK(user_data);
> + OvirtForeignMenu *menu = OVIRT_FOREIGN_MENU(g_task_get_source_object(task));
> OvirtResource *cdrom = OVIRT_RESOURCE(source_object);
> - OvirtForeignMenu *menu = OVIRT_FOREIGN_MENU(user_data);
> GError *error = NULL;
>
> ovirt_resource_refresh_finish(cdrom, result, &error);
> if (error != NULL) {
> g_warning("failed to refresh cdrom content: %s", error->message);
> - g_clear_error(&error);
> + g_task_return_error(task, error);
> + g_object_unref(task);
> return;
> }
>
> @@ -571,22 +587,22 @@ static void cdrom_file_refreshed_cb(GObject *source_object,
> "file", &menu->priv->current_iso_name,
> NULL);
> }
> - g_object_notify(G_OBJECT(menu), "file");
Are you sure this should be removed in this patch?
> if (menu->priv->cdrom != NULL) {
> - ovirt_foreign_menu_next_async_step(menu, STATE_CDROM_FILE);
> + ovirt_foreign_menu_next_async_step(menu, task, STATE_CDROM_FILE);
> } else {
> g_debug("Could not find VM cdrom through oVirt REST API");
I think you need a g_task_return_error() here.
> }
> }
>
>
> -static void ovirt_foreign_menu_refresh_cdrom_file_async(OvirtForeignMenu *menu)
> +static void ovirt_foreign_menu_refresh_cdrom_file_async(OvirtForeignMenu *menu,
> + GTask *task)
> {
> g_return_if_fail(OVIRT_IS_RESOURCE(menu->priv->cdrom));
>
> ovirt_resource_refresh_async(OVIRT_RESOURCE(menu->priv->cdrom),
> menu->priv->proxy, NULL,
> - cdrom_file_refreshed_cb, menu);
> + cdrom_file_refreshed_cb, task);
> }
>
>
> @@ -596,7 +612,8 @@ static void cdroms_fetched_cb(GObject *source_object,
> {
> GHashTable *cdroms;
> OvirtCollection *cdrom_collection = OVIRT_COLLECTION(source_object);
> - OvirtForeignMenu *menu = OVIRT_FOREIGN_MENU(user_data);
> + GTask *task = G_TASK(user_data);
> + OvirtForeignMenu *menu = OVIRT_FOREIGN_MENU(g_task_get_source_object(task));
> GHashTableIter iter;
> OvirtCdrom *cdrom;
> GError *error = NULL;
> @@ -604,7 +621,8 @@ static void cdroms_fetched_cb(GObject *source_object,
> ovirt_collection_fetch_finish(cdrom_collection, result, &error);
> if (error != NULL) {
> g_warning("failed to fetch cdrom collection: %s", error->message);
> - g_clear_error(&error);
> + g_task_return_error(task, error);
> + g_object_unref(task);
> return;
> }
>
> @@ -626,20 +644,21 @@ static void cdroms_fetched_cb(GObject *source_object,
> }
>
> if (menu->priv->cdrom != NULL) {
> - ovirt_foreign_menu_next_async_step(menu, STATE_VM_CDROM);
> + ovirt_foreign_menu_next_async_step(menu, task, STATE_VM_CDROM);
> } else {
> g_debug("Could not find VM cdrom through oVirt REST API");
I think you need a g_task_return_error() here too.
> }
> }
>
>
> -static void ovirt_foreign_menu_fetch_vm_cdrom_async(OvirtForeignMenu *menu)
> +static void ovirt_foreign_menu_fetch_vm_cdrom_async(OvirtForeignMenu *menu,
> + GTask *task)
> {
> OvirtCollection *cdrom_collection;
>
> cdrom_collection = ovirt_vm_get_cdroms(menu->priv->vm);
> ovirt_collection_fetch_async(cdrom_collection, menu->priv->proxy, NULL,
> - cdroms_fetched_cb, menu);
> + cdroms_fetched_cb, task);
> }
>
>
> @@ -648,7 +667,8 @@ static void storage_domains_fetched_cb(GObject *source_object,
> gpointer user_data)
> {
> GError *error = NULL;
> - OvirtForeignMenu *menu = OVIRT_FOREIGN_MENU(user_data);
> + GTask *task = G_TASK(user_data);
> + OvirtForeignMenu *menu = OVIRT_FOREIGN_MENU(g_task_get_source_object(task));
> OvirtCollection *collection = OVIRT_COLLECTION(source_object);
> GHashTableIter iter;
> OvirtStorageDomain *domain;
> @@ -656,7 +676,8 @@ static void storage_domains_fetched_cb(GObject *source_object,
> ovirt_collection_fetch_finish(collection, result, &error);
> if (error != NULL) {
> g_warning("failed to fetch storage domains: %s", error->message);
> - g_clear_error(&error);
> + g_task_return_error(task, error);
> + g_object_unref(task);
> return;
> }
>
> @@ -687,21 +708,21 @@ static void storage_domains_fetched_cb(GObject *source_object,
> }
>
> if (menu->priv->files != NULL) {
> - ovirt_foreign_menu_next_async_step(menu, STATE_STORAGE_DOMAIN);
> + ovirt_foreign_menu_next_async_step(menu, task, STATE_STORAGE_DOMAIN);
> } else {
> g_debug("Could not find iso file collection");
g_task_return_error()
> }
> }
>
>
> -static void ovirt_foreign_menu_fetch_storage_domain_async(OvirtForeignMenu *menu)
> +static void ovirt_foreign_menu_fetch_storage_domain_async(OvirtForeignMenu *menu,
> + GTask *task)
> {
> - OvirtCollection *collection;
> + OvirtCollection *collection = ovirt_api_get_storage_domains(menu->priv->api);
>
> g_debug("Start fetching oVirt REST collection");
> - collection = ovirt_api_get_storage_domains(menu->priv->api);
> ovirt_collection_fetch_async(collection, menu->priv->proxy, NULL,
> - storage_domains_fetched_cb, menu);
> + storage_domains_fetched_cb, task);
> }
>
>
> @@ -710,16 +731,17 @@ static void vms_fetched_cb(GObject *source_object,
> gpointer user_data)
> {
> GError *error = NULL;
> - OvirtForeignMenu *menu = OVIRT_FOREIGN_MENU(user_data);
> - OvirtCollection *collection;
> + GTask *task = G_TASK(user_data);
> + OvirtForeignMenu *menu = OVIRT_FOREIGN_MENU(g_task_get_source_object(task));
> + OvirtCollection *collection = OVIRT_COLLECTION(source_object);
> GHashTableIter iter;
> OvirtVm *vm;
>
> - 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);
> + g_task_return_error(task, error);
> + g_object_unref(task);
> return;
> }
>
> @@ -736,14 +758,15 @@ static void vms_fetched_cb(GObject *source_object,
> g_free(guid);
> }
> if (menu->priv->vm != NULL) {
> - ovirt_foreign_menu_next_async_step(menu, STATE_VM);
> + ovirt_foreign_menu_next_async_step(menu, task, STATE_VM);
> } else {
> g_warning("failed to find a VM with guid \"%s\"", menu->priv->vm_guid);
g_task_return_error()
Reviewed-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
