On 8/12/19 6:38 AM, Victor Toso wrote: > Hi, > > On Fri, Aug 09, 2019 at 04:51:07PM -0300, Eduardo Lima (Etrunko) wrote: >> Instead of fetching toplevel REST API query, we use the one relative >> from the data center, which returns more detailed information, >> especially the status of the storage domain. >> >> This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1427467 > > >> Signed-off-by: Eduardo Lima (Etrunko) <etrunko@xxxxxxxxxx> >> --- >> src/ovirt-foreign-menu.c | 24 +++++++++++++++++++----- >> 1 file changed, 19 insertions(+), 5 deletions(-) >> >> diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c >> index 59c3d48..6fc05e7 100644 >> --- a/src/ovirt-foreign-menu.c >> +++ b/src/ovirt-foreign-menu.c >> @@ -683,6 +683,7 @@ static void storage_domains_fetched_cb(GObject *source_object, >> OvirtCollection *collection = OVIRT_COLLECTION(source_object); >> GHashTableIter iter; >> OvirtStorageDomain *domain; >> + gboolean domain_valid = FALSE; >> >> ovirt_collection_fetch_finish(collection, result, &error); >> if (error != NULL) { >> @@ -699,6 +700,7 @@ static void storage_domains_fetched_cb(GObject *source_object, >> if (!storage_domain_validate(menu, domain)) >> continue; >> >> + domain_valid = TRUE; >> file_collection = ovirt_storage_domain_get_files(domain); >> if (file_collection != NULL) { >> if (menu->priv->files) { >> @@ -713,9 +715,11 @@ static void storage_domains_fetched_cb(GObject *source_object, >> if (menu->priv->files != NULL) { >> ovirt_foreign_menu_next_async_step(menu, task, STATE_STORAGE_DOMAIN); >> } else { >> - g_debug("Could not find iso file collection"); >> - g_task_return_new_error(task, OVIRT_ERROR, OVIRT_ERROR_FAILED, >> - "Could not find ISO file collection"); >> + const char *msg = domain_valid ? "Could not find ISO file collection" >> + : "Could not find valid ISO storage domain"; >> + >> + g_debug(msg); >> + g_task_return_new_error(task, OVIRT_ERROR, OVIRT_ERROR_FAILED, msg); >> g_object_unref(task); >> } >> } >> @@ -724,9 +728,19 @@ static void storage_domains_fetched_cb(GObject *source_object, >> static void ovirt_foreign_menu_fetch_storage_domain_async(OvirtForeignMenu *menu, >> GTask *task) >> { >> - OvirtCollection *collection = ovirt_api_get_storage_domains(menu->priv->api); >> + OvirtCollection *collection = NULL; >> + >> +#ifdef HAVE_OVIRT_DATA_CENTER >> + 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_DATA_CENTER(menu->priv->data_center)); >> + >> + collection = ovirt_data_center_get_storage_domains(menu->priv->data_center); >> +#else >> + collection = ovirt_api_get_storage_domains(menu->priv->api); >> +#endif >> >> - g_debug("Start fetching oVirt REST collection"); >> + g_debug("Start fetching iso file collection"); > > Because HAVE_OVIRT_DATA_CENTER depends on the ovirt's API > existing or not, maybe it could help adding some prefix like > "ovirt-data-center=%s", has_data_center ? "yes" : "no" kind of > thing. > > If you find trivial to know this info with some ovirt environment > variable for instance, feel free to ignore > Yes, this is actually done with the REST_DEBUG enviroment variable. I really don't see much value in signaling that we are using the ovirt_data_center APIs here, other than the error messages. > Acked-by: Victor Toso <victortoso@xxxxxxxxxx> > Thanks, I sent a new version with a simple improvement, and would like to push that one. -- Eduardo de Barros Lima (Etrunko) Software Engineer - Red Hat 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
