[virt-tools-list] [PATCH virt-viewer 3/3] foreign-menu: Check if storage domain is active for data center

Christophe Fergeau cfergeau at redhat.com
Fri Sep 8 12:49:38 UTC 2017


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




More information about the virt-tools-list mailing list