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

Christophe Fergeau cfergeau at redhat.com
Wed Mar 2 07:53:25 UTC 2016


On Tue, Mar 01, 2016 at 01:59:38PM -0300, Eduardo Lima (Etrunko) wrote:
> 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.

I'll send a v2 with the 2 ifs() together. I prefer variables
declarations on their own line as otherwise this gets messy when you initialize
them at the same as you declare them.

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


More information about the virt-tools-list mailing list