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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list