[PATCH 3/7] vmx: Rework disk def allocation

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

 



The way we parse VMX configuration is rather unfortunate,
especially when it comes to disks. We allocate an array that can
handle all possible disks but leave the array counter (ndisks) at
zero and increase it only after successful parsing. But, we never
size the array down to release unneeded chunks of memory.

We can do better: we can use VIR_APPEND_ELEMENT() to allocate
array as needed.

Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
 src/vmx/vmx.c | 84 ++++++++++++++++++++++++++-------------------------
 1 file changed, 43 insertions(+), 41 deletions(-)

diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index c3c00a5c7e..7a934e067d 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -1719,10 +1719,6 @@ virVMXParseConfig(virVMXContext *ctx,
     if (def->graphics[def->ngraphics] != NULL)
         ++def->ngraphics;
 
-    /* def:disks: 4 * 15 scsi + 4 * 30 sata + 2 * 2 ide + 2 floppy = 186 */
-    def->disks = g_new0(virDomainDiskDef *, 186);
-    def->ndisks = 0;
-
     /* def:disks (scsi) */
     for (controller = 0; controller < 4; ++controller) {
         if (virVMXParseSCSIController(conf, controller, &present,
@@ -1734,6 +1730,8 @@ virVMXParseConfig(virVMXContext *ctx,
             continue;
 
         for (unit = 0; unit < 16; ++unit) {
+            g_autoptr(virDomainDiskDef) disk = NULL;
+
             if (unit == 7) {
                 /*
                  * SCSI unit 7 is assigned to the SCSI controller and cannot be
@@ -1744,25 +1742,23 @@ virVMXParseConfig(virVMXContext *ctx,
 
             if (virVMXParseDisk(ctx, xmlopt, conf, VIR_DOMAIN_DISK_DEVICE_DISK,
                                 VIR_DOMAIN_DISK_BUS_SCSI, controller, unit,
-                                &def->disks[def->ndisks], def) < 0) {
+                                &disk, def) < 0) {
                 goto cleanup;
             }
 
-            if (def->disks[def->ndisks] != NULL) {
-                ++def->ndisks;
+            if (!disk &&
+                virVMXParseDisk(ctx, xmlopt, conf, VIR_DOMAIN_DISK_DEVICE_CDROM,
+                                VIR_DOMAIN_DISK_BUS_SCSI, controller, unit,
+                                &disk, def) < 0) {
+                goto cleanup;
+            }
+
+            if (!disk)
                 continue;
-            }
 
-            if (virVMXParseDisk(ctx, xmlopt, conf, VIR_DOMAIN_DISK_DEVICE_CDROM,
-                                 VIR_DOMAIN_DISK_BUS_SCSI, controller, unit,
-                                 &def->disks[def->ndisks], def) < 0) {
+            if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0)
                 goto cleanup;
-            }
-
-            if (def->disks[def->ndisks] != NULL)
-                ++def->ndisks;
         }
-
     }
 
     /* add all the SCSI controllers we've seen, up until the last one that is
@@ -1787,27 +1783,27 @@ virVMXParseConfig(virVMXContext *ctx,
             continue;
 
         for (unit = 0; unit < 30; ++unit) {
+            g_autoptr(virDomainDiskDef) disk = NULL;
+
             if (virVMXParseDisk(ctx, xmlopt, conf, VIR_DOMAIN_DISK_DEVICE_DISK,
                                 VIR_DOMAIN_DISK_BUS_SATA, controller, unit,
-                                &def->disks[def->ndisks], def) < 0) {
+                                &disk, def) < 0) {
                 goto cleanup;
             }
 
-            if (def->disks[def->ndisks] != NULL) {
-                ++def->ndisks;
-                continue;
-            }
-
-            if (virVMXParseDisk(ctx, xmlopt, conf, VIR_DOMAIN_DISK_DEVICE_CDROM,
+            if (!disk &&
+                virVMXParseDisk(ctx, xmlopt, conf, VIR_DOMAIN_DISK_DEVICE_CDROM,
                                  VIR_DOMAIN_DISK_BUS_SATA, controller, unit,
-                                 &def->disks[def->ndisks], def) < 0) {
+                                 &disk, def) < 0) {
                 goto cleanup;
             }
 
-            if (def->disks[def->ndisks] != NULL)
-                ++def->ndisks;
+            if (!disk)
+                continue;
+
+            if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0)
+                goto cleanup;
         }
-
     }
 
     /* add all the SATA controllers we've seen, up until the last one that is
@@ -1824,38 +1820,44 @@ virVMXParseConfig(virVMXContext *ctx,
     /* def:disks (ide) */
     for (bus = 0; bus < 2; ++bus) {
         for (unit = 0; unit < 2; ++unit) {
+            g_autoptr(virDomainDiskDef) disk = NULL;
+
             if (virVMXParseDisk(ctx, xmlopt, conf, VIR_DOMAIN_DISK_DEVICE_DISK,
                                 VIR_DOMAIN_DISK_BUS_IDE, bus, unit,
-                                &def->disks[def->ndisks], def) < 0) {
+                                &disk, def) < 0) {
                 goto cleanup;
             }
 
-            if (def->disks[def->ndisks] != NULL) {
-                ++def->ndisks;
+            if (!disk &&
+                virVMXParseDisk(ctx, xmlopt, conf, VIR_DOMAIN_DISK_DEVICE_CDROM,
+                                VIR_DOMAIN_DISK_BUS_IDE, bus, unit,
+                                &disk, def) < 0) {
+                goto cleanup;
+            }
+
+            if (!disk)
                 continue;
-            }
 
-            if (virVMXParseDisk(ctx, xmlopt, conf, VIR_DOMAIN_DISK_DEVICE_CDROM,
-                                VIR_DOMAIN_DISK_BUS_IDE, bus, unit,
-                                &def->disks[def->ndisks], def) < 0) {
+            if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0)
                 goto cleanup;
-            }
-
-            if (def->disks[def->ndisks] != NULL)
-                ++def->ndisks;
         }
     }
 
     /* def:disks (floppy) */
     for (unit = 0; unit < 2; ++unit) {
+        g_autoptr(virDomainDiskDef) disk = NULL;
+
         if (virVMXParseDisk(ctx, xmlopt, conf, VIR_DOMAIN_DISK_DEVICE_FLOPPY,
                             VIR_DOMAIN_DISK_BUS_FDC, 0, unit,
-                            &def->disks[def->ndisks], def) < 0) {
+                            &disk, def) < 0) {
             goto cleanup;
         }
 
-        if (def->disks[def->ndisks] != NULL)
-            ++def->ndisks;
+        if (!disk)
+            continue;
+
+        if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0)
+            goto cleanup;
     }
 
     /* def:fss */
-- 
2.31.1




[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]

  Powered by Linux