>From 6883117e3c6a6cabd760b6e6a468b69ad7b02839 Mon Sep 17 00:00:00 2001
From: Daniel P. Berrange <berrange@redhat.com>
Date: Wed, 16 Jun 2010 14:14:05 +0100
Subject: [PATCH 10/10] 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 |  214 +++++++++++++++++++++++++++++------------
 1 files changed, 152 insertions(+), 62 deletions(-)

Index: libvirt-0.7.6/src/storage/storage_backend.c
===================================================================
--- libvirt-0.7.6.orig/src/storage/storage_backend.c
+++ libvirt-0.7.6/src/storage/storage_backend.c
@@ -563,6 +563,70 @@ static int virStorageBackendCreateExecCo
     return 0;
 }
 
+enum {
+    QEMU_IMG_BACKING_FORMAT_NONE = 0,
+    QEMU_IMG_BACKING_FORMAT_FLAG,
+    QEMU_IMG_BACKING_FORMAT_OPTIONS,
+};
+
+static int virStorageBackendQEMUImgBackingFormat(virConnectPtr conn,
+                                                 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(conn, 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(conn, 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,
@@ -570,10 +634,9 @@ virStorageBackendCreateQemuImg(virConnec
                                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 ?
@@ -584,41 +647,10 @@ virStorageBackendCreateQemuImg(virConnec
     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;
-
-    const char **imgargv;
-    /* 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,
-    };
+        virStorageFileFormatTypeToString(inputvol->type == VIR_STORAGE_VOL_BLOCK ?
+                                         VIR_STORAGE_FILE_RAW :
+                                         inputvol->target.format) :
+        NULL;
 
     if (type == NULL) {
         virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
@@ -692,44 +724,103 @@ virStorageBackendCreateQemuImg(virConnec
         }
     }
 
-    if ((create_tool = virFindFileInPath("kvm-img")) != NULL)
-        use_kvmimg = 1;
-    else if ((create_tool = virFindFileInPath("qemu-img")) != NULL)
-        use_kvmimg = 0;
-    else {
+    /* Size in KB */
+    snprintf(size, sizeof(size), "%lluK", vol->capacity/1024);
+
+    /* KVM is usually ahead of qemu on features, so try that first */
+    create_tool = virFindFileInPath("kvm-img");
+    if (!create_tool)
+        create_tool = virFindFileInPath("qemu-img");
+
+    if (!create_tool) {
         virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
                               "%s", _("unable to find kvm-img or qemu-img"));
         return -1;
     }
 
     if (inputvol) {
-        convargv[0] = create_tool;
-        imgargv = convargv;
+        const char *imgargv[] = {
+            create_tool,
+            "convert",
+            "-f", inputType,
+            "-O", type,
+            inputPath,
+            vol->target.path,
+            NULL,
+        };
+
+        ret = virStorageBackendCreateExecCommand(conn, pool, vol, imgargv);
     } else if (vol->backingStore.path) {
-        imgargvbacking[0] = create_tool;
-        if (use_kvmimg) {
-            imgargvbacking[6] = "-F";
-            imgargvbacking[7] = backingType;
-            imgargvbacking[8] = vol->target.path;
-            imgargvbacking[9] = size;
+        const char *imgargv[] = {
+            create_tool,
+            "create",
+            "-f", type,
+            "-b", vol->backingStore.path,
+            NULL,
+            NULL,
+            NULL,
+            NULL,
+            NULL,
+            NULL
+        };
+        int imgformat = virStorageBackendQEMUImgBackingFormat(conn, 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)
+                imgargv[10] = "-e";
+            break;
+
+        case QEMU_IMG_BACKING_FORMAT_OPTIONS:
+            if (virAsprintf(&optflag, "backing_fmt=%s", backingType) < 0) {
+                virReportOOMError(conn);
+                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)
-                imgargvbacking[10] = "-e";
-        } else if (vol->target.encryption != NULL)
-            imgargvbacking[8] = "-e";
-        imgargv = imgargvbacking;
+                imgargv[8] = "-e";
+        }
+
+        ret = virStorageBackendCreateExecCommand(conn, pool, vol, imgargv);
+        VIR_FREE(optflag);
     } else {
-        imgargvnormal[0] = create_tool;
-        imgargv = imgargvnormal;
+        /* The extra NULL field is for indicating encryption (-e). */
+        const char *imgargv[] = {
+            create_tool,
+            "create",
+            "-f", type,
+            vol->target.path,
+            size,
+            NULL,
+            NULL
+        };
         if (vol->target.encryption != NULL)
             imgargv[6] = "-e";
-    }
-
 
-    /* Size in KB */
-    snprintf(size, sizeof(size), "%lluK", vol->capacity/1024);
+        ret = virStorageBackendCreateExecCommand(conn, pool, vol, imgargv);
+    }
 
-    ret = virStorageBackendCreateExecCommand(conn, pool, vol, imgargv);
-    VIR_FREE(imgargv[0]);
+    cleanup:
+    VIR_FREE(create_tool);
 
     return ret;
 }
