[virt-tools-list] [virt-viewer] ovirt: Only use active ISO domains for foreign menu

[Eduardo Lima (Etrunko)]

On 03/01/2016 01:18 PM, Christophe Fergeau wrote:
> On Tue, Mar 01, 2016 at 12:55:48PM -0300, Eduardo Lima (Etrunko) wrote:
>> On 03/01/2016 10:36 AM, Christophe Fergeau wrote:
>>> oVirt storage domains can be in various states (inactive, in
>>> maintainance, ...). We only want to show the ISOs it contains in the
>>> foreign menu when the storage domain is actually active, not in the
>>> other states.
>>> ---
>>>  src/ovirt-foreign-menu.c | 7 ++++++-
>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
>>> index 9859439..82f0f2a 100644
>>> --- a/src/ovirt-foreign-menu.c
>>> +++ b/src/ovirt-foreign-menu.c
>>> @@ -650,12 +650,17 @@ static void storage_domains_fetched_cb(GObject *source_object,
>>>      while (g_hash_table_iter_next(&iter, NULL, (gpointer *)&domain)) {
>>>          OvirtCollection *file_collection;
>>>          int type;
>>> +        int state;
>>>  
>>> -        g_object_get(domain, "type", &type, NULL);
>>> +        g_object_get(domain, "type", &type, "state", &state, NULL);
>>>          if (type != OVIRT_STORAGE_DOMAIN_TYPE_ISO) {
>>>              continue;
>>>          }
>>>  
>>> +        if (state != OVIRT_STORAGE_DOMAIN_STATE_ACTIVE) {
>>> +            continue;
>>> +        }
>>> +
>>
>> Maybe put this test together with the previous one with || ?
> 
> I considered this, but as the line would be quite long, I'd split the
> condition on 2 lines anyway. 2 separate 'if's seemed more readable to
> me. No strong feeling on this though, I'm ok with changing it you think
> it's better.
> 

I also don't have a strong feeling, I just think it is readable enough
to be in the same clause... just a matter of conciseness.  I would also
join the variable declarations above in the same line.

> Christophe
> 


-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etrunko at redhat.com