[PATCH 3/4] storage: Avoid unnecessary ternary operators and refactor the code

[Peter Krempa <pkrempa@xxxxxxxxxx>]

Setting of local variables in virStorageBackendCreateQemuImgCmd was
unnecessarily cluttered with ternary operators and repeated testing of
of conditions.

This patch refactors the function to use if statements and optimizes the
code flow resulting in a line count reduction.
---
 src/storage/storage_backend.c | 80 +++++++++++++++++++------------------------
 1 file changed, 35 insertions(+), 45 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index ace9cae..4846210 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -649,53 +649,56 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn,
     virCommandPtr cmd = NULL;
     bool do_encryption = (vol->target.encryption != NULL);
     unsigned long long int size_arg;
-    bool preallocate = false;
-
-    /* Treat output block devices as 'raw' format */
-    const char *type =
-        virStorageFileFormatTypeToString(vol->type == VIR_STORAGE_VOL_BLOCK ?
-                                         VIR_STORAGE_FILE_RAW :
-                                         vol->target.format);
-
-    const char *backingType = vol->backingStore.path ?
-        virStorageFileFormatTypeToString(vol->backingStore.format) : NULL;
-
-    const char *inputBackingPath = (inputvol ? inputvol->backingStore.path
-                                             : NULL);
-    const char *inputPath = inputvol ? inputvol->target.path : NULL;
-    /* Treat input block devices as 'raw' format */
-    const char *inputType = inputPath ?
-        virStorageFileFormatTypeToString(inputvol->type == VIR_STORAGE_VOL_BLOCK ?
-                                         VIR_STORAGE_FILE_RAW :
-                                         inputvol->target.format) :
-        NULL;
+    bool preallocate = !!(flags & VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA);
+    const char *type;
+    const char *backingType = NULL;
+    const char *inputPath = NULL;
+    const char *inputType = NULL;

     virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL);

-    preallocate = !!(flags & VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA);
+    /* Treat output block devices as 'raw' format */
+    type = virStorageFileFormatTypeToString(vol->type == VIR_STORAGE_VOL_BLOCK ?
+                                            VIR_STORAGE_FILE_RAW :
+                                            vol->target.format);

-    if (type == NULL) {
+    if (!type) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("unknown storage vol type %d"),
                        vol->target.format);
         return NULL;
     }
-    if (inputvol && inputType == NULL) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("unknown storage vol type %d"),
-                       inputvol->target.format);
-        return NULL;
-    }
-    if (preallocate && vol->target.format != VIR_STORAGE_FILE_QCOW2) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                       _("metadata preallocation only available with qcow2"));
-        return NULL;
+
+    if (inputvol) {
+        inputPath = inputvol->target.path;
+        inputType = virStorageFileFormatTypeToString(inputvol->type == VIR_STORAGE_VOL_BLOCK ?
+                                                     VIR_STORAGE_FILE_RAW :
+                                                     inputvol->target.format);
+
+        if (!inputType) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("unknown storage vol type %d"),
+                           inputvol->target.format);
+            return NULL;
+        }
+
+        /* XXX: Not strictly required: qemu-img has an option a different
+         * backing store, not really sure what use it serves though, and it
+         * may cause issues with lvm. Untested essentially.
+         */
+        if (STRNEQ_NULLABLE(inputvol->backingStore.path, vol->backingStore.path)) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("a different backing store cannot be specified."));
+            return NULL;
+        }
     }

     if (vol->backingStore.path) {
         int accessRetCode = -1;
         char *absolutePath = NULL;

+        backingType = virStorageFileFormatTypeToString(vol->backingStore.format);
+
         if (preallocate) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                            _("metadata preallocation conflicts with backing"
@@ -703,19 +706,6 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn,
             return NULL;
         }

-        /* XXX: Not strictly required: qemu-img has an option a different
-         * backing store, not really sure what use it serves though, and it
-         * may cause issues with lvm. Untested essentially.
-         */
-        if (inputvol &&
-            (!inputBackingPath ||
-             STRNEQ(inputBackingPath, vol->backingStore.path))) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           "%s", _("a different backing store cannot "
-                                   "be specified."));
-            return NULL;
-        }
-
         if (backingType == NULL) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("unknown storage vol backing store type %d"),
-- 
1.8.2.1

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