[libvirt] [PATCHv4 3/6] vbox: Add support for virConnectListAllDomains()

[Peter Krempa]

On 06/28/12 14:58, Michal Privoznik wrote:
> On 20.06.2012 15:33, Peter Krempa wrote:
>> VirtualBox doesn't use the common virDomainObj implementation so this
>> patch adds a separate implementation using the VirtualBox API.
>>
>> This driver implementation supports all currently defined flags. As
>> VirtualBox does not support transient guests, managed save images and
>> autostarting we assume all guests are persistent, don't have a managed
>> save image and are not autostarted. Filtering for existence of those
>> properities results in empty list.
>> ---

...

>> +
>> +                machine->vtbl->GetName(machine, &machineNameUtf16);
>> +                VBOX_UTF16_TO_UTF8(machineNameUtf16, &machineNameUtf8);
>> +                machine->vtbl->GetId(machine, &iid.value);
>> +                vboxIIDToUUID(&iid, uuid);
>> +                vboxIIDUnalloc(&iid);
>> +
>> +                dom = virGetDomain(conn, machineNameUtf8, uuid);
>> +
>> +                if (machineNameUtf8) {
>> +                    VBOX_UTF8_FREE(machineNameUtf8);
>> +                    machineNameUtf8 = NULL;
>> +                }
>> +
>> +                if (machineNameUtf16) {
>> +                    VBOX_COM_UNALLOC_MEM(machineNameUtf16);
>> +                    machineNameUtf16 = NULL;
>> +                }
>
> These two checks for !NULL are useless.
>

Indeed. I verified that in the Vbox XPCOM code.

>> +
>> +                if (!dom)
>> +                    goto no_memory;
>> +
>> +                if (active)

...

>> +        ignore_value(VIR_REALLOC_N(doms, count + 1));
>> +        *domains = doms;
>> +    }
>> +    doms = NULL;
>
> This assignment can be moved into the body of if statement. But that's
> just small optimization.
>
>> +    ret = count;
>> +
>> +cleanup:
>> +    if (doms) {
>> +        for (i = 0; i < count; i++) {
>> +            if (doms[i])
>>
>
> ACK with those nits fixed.
>
> Michal
>

I've fix them and pushed. Thanks for the reveiw.

Peter