On 18/01/17 15:45, Christophe Fergeau wrote:
> 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@xxxxxxxxxx>
>> ---
>> 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@xxxxxxxxxx>
Pushed, thanks
--
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etrunko@xxxxxxxxxx
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
