Hey,

Same comment as on the last patch regarding the lack of an explanation
of what we are trying to achieve with this patch/why we need it.

On Fri, Aug 04, 2017 at 06:53:49PM -0300, Eduardo Lima (Etrunko) wrote:
> diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
> index 32c6452..6403a0b 100644
> --- a/src/ovirt-foreign-menu.c
> +++ b/src/ovirt-foreign-menu.c
> @@ -29,6 +29,7 @@
>  
>  #include "ovirt-foreign-menu.h"
>  #include "virt-viewer-util.h"
> +#include "glib-compat.h"
>  
>  typedef enum {
>      STATE_0,
> @@ -612,6 +613,23 @@ static void 
> ovirt_foreign_menu_fetch_vm_cdrom_async(OvirtForeignMenu *menu,
>  }
>  
>  
> +static gboolean storage_domain_active_for_data_center(OvirtStorageDomain 
> *domain,
> +                                                      OvirtDataCenter 
> *data_center)

I find this a bit misleading, the 'active' part was tested with
    if (state != OVIRT_STORAGE_DOMAIN_STATE_ACTIVE) {
        continue;
    }
Maybe storage_domain_is_attached_to_data_center() instead?

Apart from this, looks good to me.

Christophe

_______________________________________________
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

Reply via email to