[virt-tools-list] [PATCH virt-viewer] ovirt-foreign-menu: Fix endpoint for storage domains query

[Eduardo Lima (Etrunko)]

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 at redhat.com>
>> ---
>>  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 at redhat.com>
> 

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 at redhat.com

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20190812/e4c81a92/attachment.sig>