Rewrite qemu-img backing store format handling

When creating qcow2 files with a backing store, it is important
to set an explicit format to prevent QEMU probing. The storage
backend was only doing this if it found a 'kvm-img' binary. This
is wrong because plenty of kvm-img binaries don't support an
explicit format, and plenty of 'qemu-img' binaries do support
a format. The result was that most qcow2 files were not getting
a backing store format.

This patch runs 'qemu-img -h' to check for the two support
argument formats

  '-o backing_format=raw'
  '-F raw'

and use whichever option it finds

* src/storage/storage_backend.c: Query binary to determine
  how to set the backing store format

src/storage/storage_backend.c

@@ -561,6 +561,69 @@ static int virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool,
     return 0;
 }
 
+enum {
+    QEMU_IMG_BACKING_FORMAT_NONE = 0,
+    QEMU_IMG_BACKING_FORMAT_FLAG,
+    QEMU_IMG_BACKING_FORMAT_OPTIONS,
+};
+
+static int virStorageBackendQEMUImgBackingFormat(const char *qemuimg)
+{
+    const char *const qemuarg[] = { qemuimg, "-h", NULL };
+    const char *const qemuenv[] = { "LC_ALL=C", NULL };
+    pid_t child = 0;
+    int status;
+    int newstdout = -1;
+    char *help = NULL;
+    enum { MAX_HELP_OUTPUT_SIZE = 1024*8 };
+    int len;
+    char *start;
+    char *end;
+    char *tmp;
+    int ret = -1;
+
+    if (virExec(qemuarg, qemuenv, NULL,
+                &child, -1, &newstdout, NULL, VIR_EXEC_CLEAR_CAPS) < 0)
+        goto cleanup;
+
+    if ((len = virFileReadLimFD(newstdout, MAX_HELP_OUTPUT_SIZE, &help)) < 0) {
+        virReportSystemError(errno,
+                             _("Unable to read '%s -h' output"),
+                             qemuimg);
+        goto cleanup;
+    }
+
+    start = strstr(help, " create ");
+    end = strstr(start, "\n");
+    if ((tmp = strstr(start, "-F fmt")) && tmp < end)
+        ret = QEMU_IMG_BACKING_FORMAT_FLAG;
+    else if ((tmp = strstr(start, "[-o options]")) && tmp < end)
+        ret = QEMU_IMG_BACKING_FORMAT_OPTIONS;
+    else
+        ret = QEMU_IMG_BACKING_FORMAT_NONE;
+
+cleanup:
+    VIR_FREE(help);
+    close(newstdout);
+rewait:
+    if (child) {
+        if (waitpid(child, &status, 0) != child) {
+            if (errno == EINTR)
+                goto rewait;
+
+            VIR_ERROR(_("Unexpected exit status from qemu %d pid %lu"),
+                      WEXITSTATUS(status), (unsigned long)child);
+        }
+        if (WEXITSTATUS(status) != 0) {
+            VIR_WARN("Unexpected exit status '%d', qemu probably failed",
+                     WEXITSTATUS(status));
+        }
+    }
+
+    return ret;
+}
+
+
 static int
 virStorageBackendCreateQemuImg(virConnectPtr conn,
                                virStoragePoolObjPtr pool,
@@ -568,10 +631,9 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
                                virStorageVolDefPtr inputvol,
                                unsigned int flags ATTRIBUTE_UNUSED)
 {
-    int ret;
+    int ret = -1;
     char size[100];
     char *create_tool;
-    short use_kvmimg;
 
     const char *type = virStorageFileFormatTypeToString(vol->target.format);
     const char *backingType = vol->backingStore.path ?
@@ -582,41 +644,10 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
     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) :
+        virStorageFileFormatTypeToString(inputvol->type == VIR_STORAGE_VOL_BLOCK ?
-                            NULL;
+                                         VIR_STORAGE_FILE_RAW :
-
+                                         inputvol->target.format) :
-    const char **imgargv;
+        NULL;
-    /* The extra NULL field is for indicating encryption (-e). */
-    const char *imgargvnormal[] = {
-        NULL, "create",
-        "-f", type,
-        vol->target.path,
-        size,
-        NULL,
-        NULL
-    };
-    /* Extra NULL fields are for including "backingType" when using
-     * kvm-img (-F backingType), and for indicating encryption (-e).
-     */
-    const char *imgargvbacking[] = {
-        NULL, "create",
-        "-f", type,
-        "-b", vol->backingStore.path,
-        vol->target.path,
-        size,
-        NULL,
-        NULL,
-        NULL,
-        NULL
-    };
-    const char *convargv[] = {
-        NULL, "convert",
-        "-f", inputType,
-        "-O", type,
-        inputPath,
-        vol->target.path,
-        NULL,
-    };
 
     if (type == NULL) {
         virStorageReportError(VIR_ERR_INTERNAL_ERROR,
@@ -690,44 +721,103 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
         }
     }
 
-    if ((create_tool = virFindFileInPath("kvm-img")) != NULL)
+    /* Size in KB */
-        use_kvmimg = 1;
+    snprintf(size, sizeof(size), "%lluK", vol->capacity/1024);
-    else if ((create_tool = virFindFileInPath("qemu-img")) != NULL)
+
-        use_kvmimg = 0;
+    /* KVM is usually ahead of qemu on features, so try that first */
-    else {
+    create_tool = virFindFileInPath("kvm-img");
+    if (!create_tool)
+        create_tool = virFindFileInPath("qemu-img");
+
+    if (!create_tool) {
         virStorageReportError(VIR_ERR_INTERNAL_ERROR,
                               "%s", _("unable to find kvm-img or qemu-img"));
         return -1;
     }
 
     if (inputvol) {
-        convargv[0] = create_tool;
+        const char *imgargv[] = {
-        imgargv = convargv;
+            create_tool,
+            "convert",
+            "-f", inputType,
+            "-O", type,
+            inputPath,
+            vol->target.path,
+            NULL,
+        };
+
+        ret = virStorageBackendCreateExecCommand(pool, vol, imgargv);
     } else if (vol->backingStore.path) {
-        imgargvbacking[0] = create_tool;
+        const char *imgargv[] = {
-        if (use_kvmimg) {
+            create_tool,
-            imgargvbacking[6] = "-F";
+            "create",
-            imgargvbacking[7] = backingType;
+            "-f", type,
-            imgargvbacking[8] = vol->target.path;
+            "-b", vol->backingStore.path,
-            imgargvbacking[9] = size;
+            NULL,
+            NULL,
+            NULL,
+            NULL,
+            NULL,
+            NULL
+        };
+        int imgformat = virStorageBackendQEMUImgBackingFormat(create_tool);
+        char *optflag = NULL;
+        if (imgformat < 0)
+            goto cleanup;
+
+        switch (imgformat) {
+        case QEMU_IMG_BACKING_FORMAT_FLAG:
+            imgargv[6] = "-F";
+            imgargv[7] = backingType;
+            imgargv[8] = vol->target.path;
+            imgargv[9] = size;
             if (vol->target.encryption != NULL)
-                imgargvbacking[10] = "-e";
+                imgargv[10] = "-e";
-        } else if (vol->target.encryption != NULL)
+            break;
-            imgargvbacking[8] = "-e";
+
-        imgargv = imgargvbacking;
+        case QEMU_IMG_BACKING_FORMAT_OPTIONS:
+            if (virAsprintf(&optflag, "backing_fmt=%s", backingType) < 0) {
+                virReportOOMError();
+                goto cleanup;
+            }
+            imgargv[6] = "-o";
+            imgargv[7] = optflag;
+            imgargv[8] = vol->target.path;
+            imgargv[9] = size;
+            if (vol->target.encryption != NULL)
+                imgargv[10] = "-e";
+            break;
+
+        default:
+            VIR_INFO("Unable to set backing store format for %s with %s",
+                     vol->target.path, create_tool);
+            imgargv[6] = vol->target.path;
+            imgargv[7] = size;
+            if (vol->target.encryption != NULL)
+                imgargv[8] = "-e";
+        }
+
+        ret = virStorageBackendCreateExecCommand(pool, vol, imgargv);
+        VIR_FREE(optflag);
     } else {
-        imgargvnormal[0] = create_tool;
+        /* The extra NULL field is for indicating encryption (-e). */
-        imgargv = imgargvnormal;
+        const char *imgargv[] = {
+            create_tool,
+            "create",
+            "-f", type,
+            vol->target.path,
+            size,
+            NULL,
+            NULL
+        };
         if (vol->target.encryption != NULL)
             imgargv[6] = "-e";
+
+        ret = virStorageBackendCreateExecCommand(pool, vol, imgargv);
     }
 
-
+    cleanup:
-    /* Size in KB */
+    VIR_FREE(create_tool);
-    snprintf(size, sizeof(size), "%lluK", vol->capacity/1024);
-
-    ret = virStorageBackendCreateExecCommand(pool, vol, imgargv);
-    VIR_FREE(imgargv[0]);
 
     return ret;
 }