From 2ba8625d6d148fa489586efabdfaf2ef20903762 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/11] 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(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c

index aba8937..c185693 100644

--- a/src/storage/storage_backend.c

+++ b/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) :

-                            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(VIR_ERR_INTERNAL_ERROR,

@@ -690,44 +721,103 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,

         }

     }

-    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(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(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(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();

+                goto cleanup;

+            }

+            imgargv[6] = "-o";

+            imgargv[7] = optflag;

+            imgargv[8] = vol->target.path;

+            imgargv[9] = size;

             if (vol->target.encryption != NULL)

-                imgargvbacking[10] = "-e";

-        } else if (vol->target.encryption != NULL)

-            imgargvbacking[8] = "-e";

-        imgargv = imgargvbacking;

+                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;

-        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";

-    }

+        ret = virStorageBackendCreateExecCommand(pool, vol, imgargv);

+    }

-    /* Size in KB */

-    snprintf(size, sizeof(size), "%lluK", vol->capacity/1024);

-

-    ret = virStorageBackendCreateExecCommand(pool, vol, imgargv);

-    VIR_FREE(imgargv[0]);

+    cleanup:

+    VIR_FREE(create_tool);

     return ret;

 }

-- 

1.7.1.1