[virt-tools-list] [PATCH virt-viewer 2/9] ovirt-foreign-menu: Fetch ISO names using GTask API

[Christophe Fergeau]

On Wed, Jan 18, 2017 at 12:16:53PM -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 at redhat.com>
> ---
>  src/ovirt-foreign-menu.c | 170 +++++++++++++++++++++++++++--------------------
>  src/ovirt-foreign-menu.h |   9 ++-
>  src/remote-viewer.c      |  45 +++++++++++--
>  3 files changed, 147 insertions(+), 77 deletions(-)
> 
> diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
> index 366259a..50a8ea6 100644
> --- a/src/ovirt-foreign-menu.c
> +++ b/src/ovirt-foreign-menu.c
> -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);

Since we have a GTask, we could pass the GCancellable it contains when
we make ovirt_*_async() calls. Since we don't use the GCancellable
functionality, that's probably not worth it ;)


> @@ -794,6 +832,8 @@ static void iso_list_fetched_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));
>      OvirtCollection *collection = OVIRT_COLLECTION(source_object);
>      GError *error = NULL;
>      GList *files;
> @@ -802,42 +842,28 @@ static void iso_list_fetched_cb(GObject *source_object,
>      if (error != NULL) {
>          g_warning("failed to fetch files for ISO storage domain: %s",
>                     error->message);
> -        g_clear_error(&error);
> +        g_task_return_error(task, error);
> +        g_object_unref(task);
>          return;
>      }
>  
>      files = g_hash_table_get_values(ovirt_collection_get_resources(collection));
> -    ovirt_foreign_menu_set_files(OVIRT_FOREIGN_MENU(user_data), files);
> +    ovirt_foreign_menu_set_files(menu, files);
>      g_list_free(files);
> -
> -    g_timeout_add_seconds(300, ovirt_foreign_menu_refresh_iso_list, user_data);
> +    g_task_return_pointer(task, menu->priv->iso_names, NULL);

Fwiw, there are probably some corner cases where this could be racy,
with ovirt_foreign_menu_set_files() invalidating menu->priv->iso_names
while this GTask still contains a pointer to it. I don't think this can
occur with the way we use that API though, so probably not worth making
a deep copy of it.

Acked-by: Christophe Fergeau <cfergeau at redhat.com>

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20170118/d67b93c4/attachment.sig>