On 8/26/19 9:47 AM, Victor Toso wrote:
> 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.
Thanks, it has been pushed without modifications.
>
>>> 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
--
Eduardo de Barros Lima (Etrunko)
Software Engineer - Red Hat
etrunko@xxxxxxxxxx
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
