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

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

 



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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]