Re: [PATCH virt-viewer] ovirt-foreign-menu: Factor out code to set file collection

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Fri, Aug 23, 2019 at 11:38:05AM -0300, Eduardo Lima (Etrunko) wrote:
> Signed-off-by: Eduardo Lima (Etrunko) <etrunko@xxxxxxxxxx>
> ---
>  src/ovirt-foreign-menu.c | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
> index c2f43e6..190bb3b 100644
> --- a/src/ovirt-foreign-menu.c
> +++ b/src/ovirt-foreign-menu.c
> @@ -674,6 +674,18 @@ static gboolean storage_domain_validate(OvirtForeignMenu *menu G_GNUC_UNUSED,
>      return ret;
>  }
>  
> +static gboolean ovirt_foreign_menu_set_file_collection(OvirtForeignMenu *menu, OvirtCollection *file_collection)
> +{
> +    g_return_val_if_fail(file_collection != NULL, FALSE);

I think this is the only additional behavior, if
ovirt_storage_domain_get_files() returns NULL, we will issue a
critical warning.

If that's not intended, just return normally.

IMHO, no need to send a v2,
Acked-by: Victor Toso <victortoso@xxxxxxxxxx>

> +
> +    if (menu->priv->files) {
> +        g_object_unref(G_OBJECT(menu->priv->files));
> +    }
> +    menu->priv->files = g_object_ref(G_OBJECT(file_collection));
> +    g_debug("Set VM files to %p", menu->priv->files);
> +    return TRUE;
> +}
> +
>  static void storage_domains_fetched_cb(GObject *source_object,
>                                         GAsyncResult *result,
>                                         gpointer user_data)
> @@ -705,14 +717,11 @@ static void storage_domains_fetched_cb(GObject *source_object,
>              domain_valid = TRUE;
>  
>          file_collection = ovirt_storage_domain_get_files(domain);
> -        if (file_collection != NULL) {
> -            if (menu->priv->files) {
> -                g_object_unref(G_OBJECT(menu->priv->files));
> -            }
> -            menu->priv->files = g_object_ref(G_OBJECT(file_collection));
> -            g_debug("Set VM files to %p", menu->priv->files);
> -            break;
> -        }
> +        if (!ovirt_foreign_menu_set_file_collection(menu, file_collection))
> +            continue;
> +
> +        break; /* There can only be one valid storage domain at a time,
> +                  no need to iterate more on the list */
>      }
>  
>      if (menu->priv->files != NULL) {
> -- 
> 2.21.0
> 
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/virt-tools-list

Attachment: signature.asc
Description: PGP signature

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

[Index of Archives]     [Linux Virtualization]     [KVM Development]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]     [Video 4 Linux]

  Powered by Linux