[PATCH v2 1/6] storage: refactor qemu-img command line generation

[Ján Tomko <jtomko@xxxxxxxxxx>]

Branch by imgformat first, since we only add new features if
-o backing_fmt is supported.

Switch to virBuffer for generating -o options to make adding
new options easier.

The only change in the generated command line is movement of the
-o/-F options to the end when creating images with backing stores.
---
 src/storage/storage_backend.c | 77 +++++++++++++++++--------------------------
 1 file changed, 31 insertions(+), 46 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index cab72c6..f9e604a 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -669,6 +669,8 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
     bool do_encryption = (vol->target.encryption != NULL);
     unsigned long long int size_arg;
     bool preallocate = false;
+    char *options = NULL;
+    virBuffer buf = VIR_BUFFER_INITIALIZER;
 
     virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, -1);
 
@@ -810,61 +812,44 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
     if (inputvol) {
         virCommandAddArgList(cmd, "convert", "-f", inputType, "-O", type,
                              inputPath, vol->target.path, NULL);
-
-        if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS &&
-            (do_encryption || preallocate)) {
-            virCommandAddArg(cmd, "-o");
-            virCommandAddArgFormat(cmd, "%s%s%s", do_encryption ? "encryption=on" : "",
-                                   (do_encryption && preallocate) ? "," : "",
-                                   preallocate ? "preallocation=metadata" : "");
-        } else if (do_encryption) {
-            virCommandAddArg(cmd, "-e");
-        }
     } else if (vol->backingStore.path) {
+        virCommandAddArgList(cmd, "create", "-f", type, "-b",
+                             vol->backingStore.path, vol->target.path, NULL);
+        virCommandAddArgFormat(cmd, "%lluK", size_arg);
+    } else {
         virCommandAddArgList(cmd, "create", "-f", type,
-                             "-b", vol->backingStore.path, NULL);
+                             vol->target.path, NULL);
+        virCommandAddArgFormat(cmd, "%lluK", size_arg);
+    }
 
-        switch (imgformat) {
-        case QEMU_IMG_BACKING_FORMAT_FLAG:
-            virCommandAddArgList(cmd, "-F", backingType, vol->target.path,
-                                 NULL);
-            virCommandAddArgFormat(cmd, "%lluK", size_arg);
+    if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) {
+        if (do_encryption)
+            virBufferAddLit(&buf, ",encryption=on");
 
-            if (do_encryption)
-                virCommandAddArg(cmd, "-e");
-            break;
+        if (!inputvol && vol->backingStore.path)
+            virBufferAsprintf(&buf, ",backing_fmt=%s", backingType);
+        else if (preallocate)
+            virBufferAddLit(&buf, ",preallocation=metadata");
 
-        case QEMU_IMG_BACKING_FORMAT_OPTIONS:
-            virCommandAddArg(cmd, "-o");
-            virCommandAddArgFormat(cmd, "backing_fmt=%s%s", backingType,
-                                   do_encryption ? ",encryption=on" : "");
-            virCommandAddArg(cmd, vol->target.path);
-            virCommandAddArgFormat(cmd, "%lluK", size_arg);
-            break;
+        if (virBufferError(&buf) > 0) {
+            virReportOOMError();
+            goto cleanup;
+        }
 
-        default:
-            VIR_INFO("Unable to set backing store format for %s with %s",
-                     vol->target.path, create_tool);
+        if ((options = virBufferContentAndReset(&buf)))
+            virCommandAddArgList(cmd, "-o", &(options[1]), NULL);
 
-            virCommandAddArg(cmd, vol->target.path);
-            virCommandAddArgFormat(cmd, "%lluK", size_arg);
-            if (do_encryption)
-                virCommandAddArg(cmd, "-e");
-        }
+        VIR_FREE(options);
     } else {
-        virCommandAddArgList(cmd, "create", "-f", type,
-                             vol->target.path, NULL);
-        virCommandAddArgFormat(cmd, "%lluK", size_arg);
-
-        if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS &&
-            (do_encryption || preallocate)) {
-            virCommandAddArg(cmd, "-o");
-            virCommandAddArgFormat(cmd, "%s%s%s", do_encryption ? "encryption=on" : "",
-                                   (do_encryption && preallocate) ? "," : "",
-                                   preallocate ? "preallocation=metadata" : "");
-        } else if (do_encryption) {
-            virCommandAddArg(cmd, "-e");
+        if (!inputvol && vol->backingStore.path) {
+            if (imgformat == QEMU_IMG_BACKING_FORMAT_FLAG)
+                virCommandAddArgList(cmd, "-F", backingType, NULL);
+            else
+                VIR_INFO("Unable to set backing store format for %s with %s",
+                         vol->target.path, create_tool);
         }
+        if (do_encryption)
+            virCommandAddArg(cmd, "-e");
     }
 
     ret = virStorageBackendCreateExecCommand(pool, vol, cmd);
-- 
1.7.12.4

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