Hi,
On Mon, Aug 26, 2019 at 09:38:03AM -0300, Eduardo Lima (Etrunko) wrote:
> On 8/26/19 4:15 AM, Victor Toso wrote:
> > 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.
>
> This was intended yes, do you think it needs to be silent?
Up to you. I don't have the proper setup to check how verbose
this could get or not. I'm asking just in case you might have
added it out of habit of checking arguments, etc.
> > 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
>
>
> --
> Eduardo de Barros Lima (Etrunko)
> Software Engineer - Red Hat
> etrunko@xxxxxxxxxx
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
