Re: [libvirt] [PATCHv4 3/6] vbox: Add support for virConnectListAllDomains()
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