[libvirt] [PATCH 0/5] storage: add tests for qemu-img command line

This series adds tests to check for changes in the generated qemu-img command line and moves the options in front of positional arguments. Ján Tomko (5): storage: move flag setting after declarations storage: separate qemu-img command generation and execution storage: add test for qemu-img command line generation storage: move qemu-img options before positional arguments storage: qemu-img: change INFO to DEBUG src/storage/storage_backend.c | 131 ++++++++------ src/storage/storage_backend.h | 8 + tests/Makefile.am | 9 + tests/storagevolxml2argvdata/pool-dir.xml | 18 ++ tests/storagevolxml2argvdata/qcow2-flag.argv | 1 + .../qcow2-nobacking-convert-flag.argv | 1 + .../qcow2-nobacking-convert-none.argv | 1 + .../qcow2-nobacking-convert-prealloc.argv | 1 + .../qcow2-nobacking-flag.argv | 1 + .../qcow2-nobacking-none.argv | 1 + .../qcow2-nobacking-prealloc.argv | 1 + tests/storagevolxml2argvdata/qcow2-none.argv | 1 + tests/storagevolxml2argvdata/qcow2.argv | 1 + tests/storagevolxml2argvdata/vol-file.xml | 20 +++ .../storagevolxml2argvdata/vol-qcow2-nobacking.xml | 21 +++ tests/storagevolxml2argvdata/vol-qcow2.xml | 31 ++++ tests/storagevolxml2argvtest.c | 189 +++++++++++++++++++++ 17 files changed, 380 insertions(+), 56 deletions(-) create mode 100644 tests/storagevolxml2argvdata/pool-dir.xml create mode 100644 tests/storagevolxml2argvdata/qcow2-flag.argv create mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-convert-flag.argv create mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-convert-none.argv create mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc.argv create mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-flag.argv create mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-none.argv create mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-prealloc.argv create mode 100644 tests/storagevolxml2argvdata/qcow2-none.argv create mode 100644 tests/storagevolxml2argvdata/qcow2.argv create mode 100644 tests/storagevolxml2argvdata/vol-file.xml create mode 100644 tests/storagevolxml2argvdata/vol-qcow2-nobacking.xml create mode 100644 tests/storagevolxml2argvdata/vol-qcow2.xml create mode 100644 tests/storagevolxml2argvtest.c -- 1.7.12.4

--- src/storage/storage_backend.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 87eca5e..295fd32 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -652,10 +652,6 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, unsigned long long int size_arg; bool preallocate = false; - virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, -1); - - preallocate = !!(flags & VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA); - const char *type = virStorageFileFormatTypeToString(vol->target.format); const char *backingType = vol->backingStore.path ? virStorageFileFormatTypeToString(vol->backingStore.format) : NULL; @@ -670,6 +666,10 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, inputvol->target.format) : NULL; + virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL); + + preallocate = !!(flags & VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA); + if (type == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown storage vol type %d"), -- 1.7.12.4

This allows us to create a test for the generated command line. --- src/storage/storage_backend.c | 98 ++++++++++++++++++++++++++----------------- src/storage/storage_backend.h | 8 ++++ 2 files changed, 67 insertions(+), 39 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 295fd32..bd7e741 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -636,17 +636,15 @@ cleanup: return ret; } - -static int -virStorageBackendCreateQemuImg(virConnectPtr conn, - virStoragePoolObjPtr pool, - virStorageVolDefPtr vol, - virStorageVolDefPtr inputvol, - unsigned int flags) +virCommandPtr +virStorageBackendCreateQemuImgCmd(virConnectPtr conn, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + virStorageVolDefPtr inputvol, + unsigned int flags, + const char *create_tool, + int imgformat) { - int ret = -1; - char *create_tool; - int imgformat = -1; virCommandPtr cmd = NULL; bool do_encryption = (vol->target.encryption != NULL); unsigned long long int size_arg; @@ -674,18 +672,18 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown storage vol type %d"), vol->target.format); - return -1; + return NULL; } if (inputvol && inputType == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown storage vol type %d"), inputvol->target.format); - return -1; + return NULL; } if (preallocate && vol->target.format != VIR_STORAGE_FILE_QCOW2) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("metadata preallocation only available with qcow2")); - return -1; + return NULL; } if (vol->backingStore.path) { @@ -696,7 +694,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("metadata preallocation conflicts with backing" " store")); - return -1; + return NULL; } /* XXX: Not strictly required: qemu-img has an option a different @@ -709,14 +707,14 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("a different backing store cannot " "be specified.")); - return -1; + return NULL; } if (backingType == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown storage vol backing store type %d"), vol->backingStore.format); - return -1; + return NULL; } /* Convert relative backing store paths to absolute paths for access @@ -726,7 +724,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, virAsprintf(&absolutePath, "%s/%s", pool->def->target.path, vol->backingStore.path) < 0) { virReportOOMError(); - return -1; + return NULL; } accessRetCode = access(absolutePath ? absolutePath : vol->backingStore.path, R_OK); @@ -735,7 +733,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, virReportSystemError(errno, _("inaccessible backing store volume %s"), vol->backingStore.path); - return -1; + return NULL; } } @@ -747,7 +745,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("qcow volume encryption unsupported with " "volume format %s"), type); - return -1; + return NULL; } enc = vol->target.encryption; if (enc->format != VIR_STORAGE_ENCRYPTION_FORMAT_QCOW && @@ -755,38 +753,23 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported volume encryption format %d"), vol->target.encryption->format); - return -1; + return NULL; } if (enc->nsecrets > 1) { virReportError(VIR_ERR_XML_ERROR, "%s", _("too many secrets for qcow encryption")); - return -1; + return NULL; } if (enc->format == VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT || enc->nsecrets == 0) { if (virStorageGenerateQcowEncryption(conn, vol) < 0) - return -1; + return NULL; } } /* Size in KB */ size_arg = VIR_DIV_UP(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) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("unable to find kvm-img or qemu-img")); - return -1; - } - - imgformat = virStorageBackendQEMUImgBackingFormat(create_tool); - if (imgformat < 0) - goto cleanup; - cmd = virCommandNew(create_tool); if (inputvol) { @@ -849,11 +832,48 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, } } + return cmd; +} + +static int +virStorageBackendCreateQemuImg(virConnectPtr conn, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + virStorageVolDefPtr inputvol, + unsigned int flags) +{ + int ret = -1; + const char *create_tool; + int imgformat; + virCommandPtr cmd; + + virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, -1); + + /* 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) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("unable to find kvm-img or qemu-img")); + return -1; + } + + imgformat = virStorageBackendQEMUImgBackingFormat(create_tool); + if (imgformat < 0) + goto cleanup; + + cmd = virStorageBackendCreateQemuImgCmd(conn, pool, vol, inputvol, flags, + create_tool, imgformat); + if (!cmd) + goto cleanup; + ret = virStorageBackendCreateExecCommand(pool, vol, cmd); + + virCommandFree(cmd); cleanup: VIR_FREE(create_tool); - virCommandFree(cmd); - return ret; } diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 56b6797..3dc9c64 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -156,5 +156,13 @@ int virStorageBackendRunProgNul(virStoragePoolObjPtr pool, virStorageBackendListVolNulFunc func, void *data); +virCommandPtr +virStorageBackendCreateQemuImgCmd(virConnectPtr conn, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + virStorageVolDefPtr inputvol, + unsigned int flags, + const char *create_tool, + int imgformat); #endif /* __VIR_STORAGE_BACKEND_H__ */ -- 1.7.12.4

On 02/18/2013 09:27 AM, Ján Tomko wrote:
This allows us to create a test for the generated command line. --- src/storage/storage_backend.c | 98 ++++++++++++++++++++++++++----------------- src/storage/storage_backend.h | 8 ++++ 2 files changed, 67 insertions(+), 39 deletions(-)
ACK John

--- tests/Makefile.am | 9 + tests/storagevolxml2argvdata/pool-dir.xml | 18 ++ tests/storagevolxml2argvdata/qcow2-flag.argv | 1 + .../qcow2-nobacking-convert-flag.argv | 1 + .../qcow2-nobacking-convert-none.argv | 1 + .../qcow2-nobacking-convert-prealloc.argv | 1 + .../qcow2-nobacking-flag.argv | 1 + .../qcow2-nobacking-none.argv | 1 + .../qcow2-nobacking-prealloc.argv | 1 + tests/storagevolxml2argvdata/qcow2-none.argv | 1 + tests/storagevolxml2argvdata/qcow2.argv | 1 + tests/storagevolxml2argvdata/vol-file.xml | 20 +++ .../storagevolxml2argvdata/vol-qcow2-nobacking.xml | 21 +++ tests/storagevolxml2argvdata/vol-qcow2.xml | 31 ++++ tests/storagevolxml2argvtest.c | 189 +++++++++++++++++++++ 15 files changed, 297 insertions(+) create mode 100644 tests/storagevolxml2argvdata/pool-dir.xml create mode 100644 tests/storagevolxml2argvdata/qcow2-flag.argv create mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-convert-flag.argv create mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-convert-none.argv create mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc.argv create mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-flag.argv create mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-none.argv create mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-prealloc.argv create mode 100644 tests/storagevolxml2argvdata/qcow2-none.argv create mode 100644 tests/storagevolxml2argvdata/qcow2.argv create mode 100644 tests/storagevolxml2argvdata/vol-file.xml create mode 100644 tests/storagevolxml2argvdata/vol-qcow2-nobacking.xml create mode 100644 tests/storagevolxml2argvdata/vol-qcow2.xml create mode 100644 tests/storagevolxml2argvtest.c diff --git a/tests/Makefile.am b/tests/Makefile.am index cafdae0..0304829 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -74,6 +74,7 @@ EXTRA_DIST = \ storagepoolschematest \ storagepoolxml2xmlin \ storagepoolxml2xmlout \ + storagevolxml2argvdata \ storagevolschematest \ storagevolxml2xmlin \ storagevolxml2xmlout \ @@ -170,6 +171,8 @@ endif test_programs += nwfilterxml2xmltest +test_programs += storagevolxml2argvtest + test_programs += storagevolxml2xmltest storagepoolxml2xmltest test_programs += nodedevxml2xmltest @@ -470,6 +473,12 @@ nwfilterxml2xmltest_SOURCES = \ testutils.c testutils.h nwfilterxml2xmltest_LDADD = $(LDADDS) +storagevolxml2argvtest_SOURCES = \ + storagevolxml2argvtest.c \ + testutils.c testutils.h +storagevolxml2argvtest_LDADD = \ + ../src/libvirt_driver_storage_impl.la $(LDADDS) + storagevolxml2xmltest_SOURCES = \ storagevolxml2xmltest.c \ testutils.c testutils.h diff --git a/tests/storagevolxml2argvdata/pool-dir.xml b/tests/storagevolxml2argvdata/pool-dir.xml new file mode 100644 index 0000000..e10ccb7 --- /dev/null +++ b/tests/storagevolxml2argvdata/pool-dir.xml @@ -0,0 +1,18 @@ +<pool type='dir'> + <name>virtimages</name> + <uuid>70a7eb15-6c34-ee9c-bf57-69e8e5ff3fb2</uuid> + <capacity>0</capacity> + <allocation>0</allocation> + <available>0</available> + <source> + </source> + <target> + <path>///var/////lib/libvirt/images//</path> + <permissions> + <mode>0700</mode> + <owner>-1</owner> + <group>-1</group> + <label>some_label_t</label> + </permissions> + </target> +</pool> diff --git a/tests/storagevolxml2argvdata/qcow2-flag.argv b/tests/storagevolxml2argvdata/qcow2-flag.argv new file mode 100644 index 0000000..3ac9010 --- /dev/null +++ b/tests/storagevolxml2argvdata/qcow2-flag.argv @@ -0,0 +1 @@ +qemu-img create -f qcow2 -b /dev/null -F raw /var/lib/libvirt/images/OtherDemo.img 5242880K -e diff --git a/tests/storagevolxml2argvdata/qcow2-nobacking-convert-flag.argv b/tests/storagevolxml2argvdata/qcow2-nobacking-convert-flag.argv new file mode 100644 index 0000000..2d37c50 --- /dev/null +++ b/tests/storagevolxml2argvdata/qcow2-nobacking-convert-flag.argv @@ -0,0 +1 @@ +qemu-img convert -f raw -O qcow2 /var/lib/libvirt/images/sparse.img /var/lib/libvirt/images/OtherDemo.img -e diff --git a/tests/storagevolxml2argvdata/qcow2-nobacking-convert-none.argv b/tests/storagevolxml2argvdata/qcow2-nobacking-convert-none.argv new file mode 100644 index 0000000..2d37c50 --- /dev/null +++ b/tests/storagevolxml2argvdata/qcow2-nobacking-convert-none.argv @@ -0,0 +1 @@ +qemu-img convert -f raw -O qcow2 /var/lib/libvirt/images/sparse.img /var/lib/libvirt/images/OtherDemo.img -e diff --git a/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc.argv b/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc.argv new file mode 100644 index 0000000..18e8f64 --- /dev/null +++ b/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc.argv @@ -0,0 +1 @@ +qemu-img convert -f raw -O qcow2 /var/lib/libvirt/images/sparse.img /var/lib/libvirt/images/OtherDemo.img -o encryption=on,preallocation=metadata diff --git a/tests/storagevolxml2argvdata/qcow2-nobacking-flag.argv b/tests/storagevolxml2argvdata/qcow2-nobacking-flag.argv new file mode 100644 index 0000000..8980cc4 --- /dev/null +++ b/tests/storagevolxml2argvdata/qcow2-nobacking-flag.argv @@ -0,0 +1 @@ +qemu-img create -f qcow2 /var/lib/libvirt/images/OtherDemo.img 5242880K -e diff --git a/tests/storagevolxml2argvdata/qcow2-nobacking-none.argv b/tests/storagevolxml2argvdata/qcow2-nobacking-none.argv new file mode 100644 index 0000000..8980cc4 --- /dev/null +++ b/tests/storagevolxml2argvdata/qcow2-nobacking-none.argv @@ -0,0 +1 @@ +qemu-img create -f qcow2 /var/lib/libvirt/images/OtherDemo.img 5242880K -e diff --git a/tests/storagevolxml2argvdata/qcow2-nobacking-prealloc.argv b/tests/storagevolxml2argvdata/qcow2-nobacking-prealloc.argv new file mode 100644 index 0000000..828f5fc --- /dev/null +++ b/tests/storagevolxml2argvdata/qcow2-nobacking-prealloc.argv @@ -0,0 +1 @@ +qemu-img create -f qcow2 /var/lib/libvirt/images/OtherDemo.img 5242880K -o encryption=on,preallocation=metadata diff --git a/tests/storagevolxml2argvdata/qcow2-none.argv b/tests/storagevolxml2argvdata/qcow2-none.argv new file mode 100644 index 0000000..f2dfd15 --- /dev/null +++ b/tests/storagevolxml2argvdata/qcow2-none.argv @@ -0,0 +1 @@ +qemu-img create -f qcow2 -b /dev/null /var/lib/libvirt/images/OtherDemo.img 5242880K -e diff --git a/tests/storagevolxml2argvdata/qcow2.argv b/tests/storagevolxml2argvdata/qcow2.argv new file mode 100644 index 0000000..d6f4cb6 --- /dev/null +++ b/tests/storagevolxml2argvdata/qcow2.argv @@ -0,0 +1 @@ +qemu-img create -f qcow2 -b /dev/null -o backing_fmt=raw,encryption=on /var/lib/libvirt/images/OtherDemo.img 5242880K diff --git a/tests/storagevolxml2argvdata/vol-file.xml b/tests/storagevolxml2argvdata/vol-file.xml new file mode 100644 index 0000000..d3f65f6 --- /dev/null +++ b/tests/storagevolxml2argvdata/vol-file.xml @@ -0,0 +1,20 @@ +<volume> + <name>sparse.img</name> + <source/> + <capacity unit="TiB">1</capacity> + <allocation unit="bytes">0</allocation> + <target> + <path>/var/lib/libvirt/images/sparse.img</path> + <permissions> + <mode>0</mode> + <owner>0744</owner> + <group>0</group> + <label>virt_image_t</label> + </permissions> + <timestamps> + <atime>1341933637.273190990</atime> + <mtime>1341930622.047245868</mtime> + <ctime>1341930622.047245868</ctime> + </timestamps> + </target> +</volume> diff --git a/tests/storagevolxml2argvdata/vol-qcow2-nobacking.xml b/tests/storagevolxml2argvdata/vol-qcow2-nobacking.xml new file mode 100644 index 0000000..6a6bd5b --- /dev/null +++ b/tests/storagevolxml2argvdata/vol-qcow2-nobacking.xml @@ -0,0 +1,21 @@ +<volume> + <name>OtherDemo.img</name> + <key>/var/lib/libvirt/images/OtherDemo.img</key> + <source> + </source> + <capacity unit="G">5</capacity> + <allocation>294912</allocation> + <target> + <path>/var/lib/libvirt/images/OtherDemo.img</path> + <format type='qcow2'/> + <permissions> + <mode>0644</mode> + <owner>0</owner> + <group>0</group> + <label>unconfined_u:object_r:virt_image_t:s0</label> + </permissions> + <encryption format='qcow'> + <secret type='passphrase' uuid='e78d4b51-a2af-485f-b0f5-afca709a80f4'/> + </encryption> + </target> +</volume> diff --git a/tests/storagevolxml2argvdata/vol-qcow2.xml b/tests/storagevolxml2argvdata/vol-qcow2.xml new file mode 100644 index 0000000..49a7de3 --- /dev/null +++ b/tests/storagevolxml2argvdata/vol-qcow2.xml @@ -0,0 +1,31 @@ +<volume> + <name>OtherDemo.img</name> + <key>/var/lib/libvirt/images/OtherDemo.img</key> + <source> + </source> + <capacity unit="G">5</capacity> + <allocation>294912</allocation> + <target> + <path>/var/lib/libvirt/images/OtherDemo.img</path> + <format type='qcow2'/> + <permissions> + <mode>0644</mode> + <owner>0</owner> + <group>0</group> + <label>unconfined_u:object_r:virt_image_t:s0</label> + </permissions> + <encryption format='qcow'> + <secret type='passphrase' uuid='e78d4b51-a2af-485f-b0f5-afca709a80f4'/> + </encryption> + </target> + <backingStore> + <path>/dev/null</path> + <format type='raw'/> + <permissions> + <mode>0644</mode> + <owner>0</owner> + <group>0</group> + <label>unconfined_u:object_r:virt_image_t:s0</label> + </permissions> + </backingStore> +</volume> diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c new file mode 100644 index 0000000..416bb6f --- /dev/null +++ b/tests/storagevolxml2argvtest.c @@ -0,0 +1,189 @@ +#include <config.h> + +#include "internal.h" +#include "testutils.h" +#include "datatypes.h" +#include "storage/storage_backend.h" +#include "testutilsqemu.h" + +const char create_tool[] = "qemu-img"; + +static int +testCompareXMLToArgvFiles(bool shouldFail, + const char *poolxml, + const char *volxml, + const char *inputvolxml, + const char *cmdline, + unsigned int flags, + int imgformat) +{ + char *volXmlData = NULL; + char *poolXmlData = NULL; + char *inputvolXmlData = NULL; + char *expectedCmdline = NULL; + char *actualCmdline = NULL; + int ret = -1; + + int len; + + virCommandPtr cmd = NULL; + virConnectPtr conn; + + virStorageVolDefPtr vol = NULL, inputvol = NULL; + virStoragePoolDefPtr pool = NULL; + virStoragePoolObj poolobj = {.def = NULL }; + + + if (!(conn = virGetConnect())) + goto cleanup; + + if (virtTestLoadFile(poolxml, &poolXmlData) < 0) + goto cleanup; + if (virtTestLoadFile(volxml, &volXmlData) < 0) + goto cleanup; + if (inputvolxml && + virtTestLoadFile(inputvolxml, &inputvolXmlData) < 0) + goto cleanup; + + if (!(pool = virStoragePoolDefParseString(poolXmlData))) + goto cleanup; + + poolobj.def = pool; + + if (!(vol = virStorageVolDefParseString(pool, volXmlData))) + goto cleanup; + + if (inputvolxml && + !(inputvol = virStorageVolDefParseString(pool, inputvolXmlData))) + goto cleanup; + + cmd = virStorageBackendCreateQemuImgCmd(conn, &poolobj, vol, inputvol, + flags, create_tool, imgformat); + + actualCmdline = virCommandToString(cmd); + if (!actualCmdline) { + if (shouldFail) { + virResetLastError(); + ret = 0; + } + goto cleanup; + } + + len = virtTestLoadFile(cmdline, &expectedCmdline); + if (len < 0) + goto cleanup; + if (len && expectedCmdline[len-1] == '\n') + expectedCmdline[len-1] = '\0'; + + if (STRNEQ_NULLABLE(expectedCmdline, actualCmdline)) { + virtTestDifference(stderr, expectedCmdline, actualCmdline); + goto cleanup; + } + + ret = 0; + +cleanup: + virStoragePoolDefFree(pool); + virStorageVolDefFree(vol); + virStorageVolDefFree(inputvol); + virCommandFree(cmd); + VIR_FREE(actualCmdline); + VIR_FREE(expectedCmdline); + return ret; +} + +struct testInfo { + bool shouldFail; + const char *pool; + const char *vol; + const char *inputvol; + const char *cmdline; + unsigned int flags; + int imgformat; +}; + +static int +testCompareXMLToArgvHelper(const void *data) +{ + int result = -1; + const struct testInfo *info = data; + char *poolxml = NULL; + char *volxml = NULL; + char *inputvolxml = NULL; + char *cmdline = NULL; + + if (info->inputvol && + virAsprintf(&inputvolxml, "%s/storagevolxml2argvdata/%s.xml", + abs_srcdir, info->inputvol) < 0) + goto cleanup; + if (virAsprintf(&poolxml, "%s/storagevolxml2argvdata/%s.xml", + abs_srcdir, info->pool) < 0 || + virAsprintf(&volxml, "%s/storagevolxml2argvdata/%s.xml", + abs_srcdir, info->vol) < 0) { + goto cleanup; + } + if (virAsprintf(&cmdline, "%s/storagevolxml2argvdata/%s.argv", + abs_srcdir, info->cmdline) < 0 && !info->shouldFail) + goto cleanup; + + result = testCompareXMLToArgvFiles(info->shouldFail, poolxml, volxml, + inputvolxml, cmdline, info->flags, + info->imgformat); + +cleanup: + VIR_FREE(poolxml); + VIR_FREE(volxml); + VIR_FREE(inputvolxml); + VIR_FREE(cmdline); + + return result; +} + +enum { + FMT_NONE = 0, + FMT_FLAG, + FMT_OPTIONS, +}; + + + +static int +mymain(void) +{ + int ret = 0; + unsigned int flags = VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA; + +#define DO_TEST(shouldFail, pool, vol, inputvol, cmdline, flags, imgformat) \ + do { \ + struct testInfo info = { shouldFail, pool, vol, inputvol, cmdline, \ + flags, imgformat }; \ + if (virtTestRun("Storage Vol XML-2-argv " cmdline, \ + 1, testCompareXMLToArgvHelper, &info) < 0) \ + ret = -1; \ + } \ + while (0); + + DO_TEST(false, "pool-dir", "vol-qcow2", NULL, "qcow2", 0, FMT_OPTIONS); + DO_TEST(true, "pool-dir", "vol-qcow2", NULL, "qcow2-prealloc", flags, + FMT_OPTIONS); + DO_TEST(false, "pool-dir", "vol-qcow2-nobacking", NULL, + "qcow2-nobacking-prealloc", flags, FMT_OPTIONS); + DO_TEST(false, "pool-dir", "vol-qcow2-nobacking", "vol-file", + "qcow2-nobacking-convert-prealloc", flags, FMT_OPTIONS); + DO_TEST(true, "pool-dir", "vol-qcow2", "vol-file", + "qcow2-convert-prealloc", flags, FMT_OPTIONS); + DO_TEST(false, "pool-dir", "vol-qcow2", NULL, "qcow2-flag", 0, FMT_FLAG); + DO_TEST(false, "pool-dir", "vol-qcow2-nobacking", NULL, + "qcow2-nobacking-flag", 0, FMT_FLAG); + DO_TEST(false, "pool-dir", "vol-qcow2-nobacking", "vol-file", + "qcow2-nobacking-convert-flag", 0, FMT_FLAG); + DO_TEST(false, "pool-dir", "vol-qcow2", NULL, "qcow2-none", 0, FMT_NONE); + DO_TEST(false, "pool-dir", "vol-qcow2-nobacking", NULL, + "qcow2-nobacking-none", 0, FMT_NONE); + DO_TEST(false, "pool-dir", "vol-qcow2-nobacking", "vol-file", + "qcow2-nobacking-convert-none", 0, FMT_NONE); + + return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIRT_TEST_MAIN(mymain) -- 1.7.12.4

On 02/18/2013 09:27 AM, Ján Tomko wrote:
--- tests/Makefile.am | 9 + tests/storagevolxml2argvdata/pool-dir.xml | 18 ++ tests/storagevolxml2argvdata/qcow2-flag.argv | 1 + .../qcow2-nobacking-convert-flag.argv | 1 + .../qcow2-nobacking-convert-none.argv | 1 + .../qcow2-nobacking-convert-prealloc.argv | 1 + .../qcow2-nobacking-flag.argv | 1 + .../qcow2-nobacking-none.argv | 1 + .../qcow2-nobacking-prealloc.argv | 1 + tests/storagevolxml2argvdata/qcow2-none.argv | 1 + tests/storagevolxml2argvdata/qcow2.argv | 1 + tests/storagevolxml2argvdata/vol-file.xml | 20 +++ .../storagevolxml2argvdata/vol-qcow2-nobacking.xml | 21 +++ tests/storagevolxml2argvdata/vol-qcow2.xml | 31 ++++ tests/storagevolxml2argvtest.c | 189 +++++++++++++++++++++ 15 files changed, 297 insertions(+) create mode 100644 tests/storagevolxml2argvdata/pool-dir.xml create mode 100644 tests/storagevolxml2argvdata/qcow2-flag.argv create mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-convert-flag.argv create mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-convert-none.argv create mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc.argv create mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-flag.argv create mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-none.argv create mode 100644 tests/storagevolxml2argvdata/qcow2-nobacking-prealloc.argv create mode 100644 tests/storagevolxml2argvdata/qcow2-none.argv create mode 100644 tests/storagevolxml2argvdata/qcow2.argv create mode 100644 tests/storagevolxml2argvdata/vol-file.xml create mode 100644 tests/storagevolxml2argvdata/vol-qcow2-nobacking.xml create mode 100644 tests/storagevolxml2argvdata/vol-qcow2.xml create mode 100644 tests/storagevolxml2argvtest.c
ACK, seems reasonable John

Modify the expected output of storagevolxml2argv tests as well. --- src/storage/storage_backend.c | 21 ++++++++++----------- tests/storagevolxml2argvdata/qcow2-flag.argv | 2 +- .../qcow2-nobacking-convert-flag.argv | 2 +- .../qcow2-nobacking-convert-none.argv | 2 +- .../qcow2-nobacking-convert-prealloc.argv | 2 +- .../qcow2-nobacking-flag.argv | 2 +- .../qcow2-nobacking-none.argv | 2 +- .../qcow2-nobacking-prealloc.argv | 2 +- tests/storagevolxml2argvdata/qcow2-none.argv | 2 +- 9 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index bd7e741..b32c7ef 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -773,8 +773,7 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn, cmd = virCommandNew(create_tool); if (inputvol) { - virCommandAddArgList(cmd, "convert", "-f", inputType, "-O", type, - inputPath, vol->target.path, NULL); + virCommandAddArgList(cmd, "convert", "-f", inputType, "-O", type, NULL); if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS && (do_encryption || preallocate)) { @@ -785,18 +784,18 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn, } else if (do_encryption) { virCommandAddArg(cmd, "-e"); } + virCommandAddArgList(cmd, inputPath, vol->target.path, NULL); } else if (vol->backingStore.path) { virCommandAddArgList(cmd, "create", "-f", type, "-b", vol->backingStore.path, NULL); switch (imgformat) { case QEMU_IMG_BACKING_FORMAT_FLAG: - virCommandAddArgList(cmd, "-F", backingType, vol->target.path, - NULL); - virCommandAddArgFormat(cmd, "%lluK", size_arg); - + virCommandAddArgList(cmd, "-F", backingType, NULL); if (do_encryption) virCommandAddArg(cmd, "-e"); + virCommandAddArg(cmd, vol->target.path); + virCommandAddArgFormat(cmd, "%lluK", size_arg); break; case QEMU_IMG_BACKING_FORMAT_OPTIONS: @@ -811,15 +810,13 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn, VIR_INFO("Unable to set backing store format for %s with %s", vol->target.path, create_tool); - virCommandAddArg(cmd, vol->target.path); - virCommandAddArgFormat(cmd, "%lluK", size_arg); if (do_encryption) virCommandAddArg(cmd, "-e"); + virCommandAddArg(cmd, vol->target.path); + virCommandAddArgFormat(cmd, "%lluK", size_arg); } } else { - virCommandAddArgList(cmd, "create", "-f", type, - vol->target.path, NULL); - virCommandAddArgFormat(cmd, "%lluK", size_arg); + virCommandAddArgList(cmd, "create", "-f", type, NULL); if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS && (do_encryption || preallocate)) { @@ -830,6 +827,8 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn, } else if (do_encryption) { virCommandAddArg(cmd, "-e"); } + virCommandAddArg(cmd, vol->target.path); + virCommandAddArgFormat(cmd, "%lluK", size_arg); } return cmd; diff --git a/tests/storagevolxml2argvdata/qcow2-flag.argv b/tests/storagevolxml2argvdata/qcow2-flag.argv index 3ac9010..2be10a2 100644 --- a/tests/storagevolxml2argvdata/qcow2-flag.argv +++ b/tests/storagevolxml2argvdata/qcow2-flag.argv @@ -1 +1 @@ -qemu-img create -f qcow2 -b /dev/null -F raw /var/lib/libvirt/images/OtherDemo.img 5242880K -e +qemu-img create -f qcow2 -b /dev/null -F raw -e /var/lib/libvirt/images/OtherDemo.img 5242880K diff --git a/tests/storagevolxml2argvdata/qcow2-nobacking-convert-flag.argv b/tests/storagevolxml2argvdata/qcow2-nobacking-convert-flag.argv index 2d37c50..7add158 100644 --- a/tests/storagevolxml2argvdata/qcow2-nobacking-convert-flag.argv +++ b/tests/storagevolxml2argvdata/qcow2-nobacking-convert-flag.argv @@ -1 +1 @@ -qemu-img convert -f raw -O qcow2 /var/lib/libvirt/images/sparse.img /var/lib/libvirt/images/OtherDemo.img -e +qemu-img convert -f raw -O qcow2 -e /var/lib/libvirt/images/sparse.img /var/lib/libvirt/images/OtherDemo.img diff --git a/tests/storagevolxml2argvdata/qcow2-nobacking-convert-none.argv b/tests/storagevolxml2argvdata/qcow2-nobacking-convert-none.argv index 2d37c50..7add158 100644 --- a/tests/storagevolxml2argvdata/qcow2-nobacking-convert-none.argv +++ b/tests/storagevolxml2argvdata/qcow2-nobacking-convert-none.argv @@ -1 +1 @@ -qemu-img convert -f raw -O qcow2 /var/lib/libvirt/images/sparse.img /var/lib/libvirt/images/OtherDemo.img -e +qemu-img convert -f raw -O qcow2 -e /var/lib/libvirt/images/sparse.img /var/lib/libvirt/images/OtherDemo.img diff --git a/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc.argv b/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc.argv index 18e8f64..037e9f6 100644 --- a/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc.argv +++ b/tests/storagevolxml2argvdata/qcow2-nobacking-convert-prealloc.argv @@ -1 +1 @@ -qemu-img convert -f raw -O qcow2 /var/lib/libvirt/images/sparse.img /var/lib/libvirt/images/OtherDemo.img -o encryption=on,preallocation=metadata +qemu-img convert -f raw -O qcow2 -o encryption=on,preallocation=metadata /var/lib/libvirt/images/sparse.img /var/lib/libvirt/images/OtherDemo.img diff --git a/tests/storagevolxml2argvdata/qcow2-nobacking-flag.argv b/tests/storagevolxml2argvdata/qcow2-nobacking-flag.argv index 8980cc4..62ad85f 100644 --- a/tests/storagevolxml2argvdata/qcow2-nobacking-flag.argv +++ b/tests/storagevolxml2argvdata/qcow2-nobacking-flag.argv @@ -1 +1 @@ -qemu-img create -f qcow2 /var/lib/libvirt/images/OtherDemo.img 5242880K -e +qemu-img create -f qcow2 -e /var/lib/libvirt/images/OtherDemo.img 5242880K diff --git a/tests/storagevolxml2argvdata/qcow2-nobacking-none.argv b/tests/storagevolxml2argvdata/qcow2-nobacking-none.argv index 8980cc4..62ad85f 100644 --- a/tests/storagevolxml2argvdata/qcow2-nobacking-none.argv +++ b/tests/storagevolxml2argvdata/qcow2-nobacking-none.argv @@ -1 +1 @@ -qemu-img create -f qcow2 /var/lib/libvirt/images/OtherDemo.img 5242880K -e +qemu-img create -f qcow2 -e /var/lib/libvirt/images/OtherDemo.img 5242880K diff --git a/tests/storagevolxml2argvdata/qcow2-nobacking-prealloc.argv b/tests/storagevolxml2argvdata/qcow2-nobacking-prealloc.argv index 828f5fc..ebeabc8 100644 --- a/tests/storagevolxml2argvdata/qcow2-nobacking-prealloc.argv +++ b/tests/storagevolxml2argvdata/qcow2-nobacking-prealloc.argv @@ -1 +1 @@ -qemu-img create -f qcow2 /var/lib/libvirt/images/OtherDemo.img 5242880K -o encryption=on,preallocation=metadata +qemu-img create -f qcow2 -o encryption=on,preallocation=metadata /var/lib/libvirt/images/OtherDemo.img 5242880K diff --git a/tests/storagevolxml2argvdata/qcow2-none.argv b/tests/storagevolxml2argvdata/qcow2-none.argv index f2dfd15..10b7175 100644 --- a/tests/storagevolxml2argvdata/qcow2-none.argv +++ b/tests/storagevolxml2argvdata/qcow2-none.argv @@ -1 +1 @@ -qemu-img create -f qcow2 -b /dev/null /var/lib/libvirt/images/OtherDemo.img 5242880K -e +qemu-img create -f qcow2 -b /dev/null -e /var/lib/libvirt/images/OtherDemo.img 5242880K -- 1.7.12.4

On 02/18/2013 09:27 AM, Ján Tomko wrote:
Modify the expected output of storagevolxml2argv tests as well. --- src/storage/storage_backend.c | 21 ++++++++++----------- tests/storagevolxml2argvdata/qcow2-flag.argv | 2 +- .../qcow2-nobacking-convert-flag.argv | 2 +- .../qcow2-nobacking-convert-none.argv | 2 +- .../qcow2-nobacking-convert-prealloc.argv | 2 +- .../qcow2-nobacking-flag.argv | 2 +- .../qcow2-nobacking-none.argv | 2 +- .../qcow2-nobacking-prealloc.argv | 2 +- tests/storagevolxml2argvdata/qcow2-none.argv | 2 +- 9 files changed, 18 insertions(+), 19 deletions(-)
ACK John

For really old qemu-img binaries which do not support specifying the format of the backing file, display a DEBUG message instead of INFO that this can't be done. --- src/storage/storage_backend.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index b32c7ef..460b792 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -807,8 +807,8 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn, break; default: - VIR_INFO("Unable to set backing store format for %s with %s", - vol->target.path, create_tool); + VIR_DEBUG("Unable to set backing store format for %s with %s", + vol->target.path, create_tool); if (do_encryption) virCommandAddArg(cmd, "-e"); -- 1.7.12.4

On 02/18/2013 09:27 AM, Ján Tomko wrote:
For really old qemu-img binaries which do not support specifying the format of the backing file, display a DEBUG message instead of INFO that this can't be done. --- src/storage/storage_backend.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
ACK John

On 02/22/13 14:28, John Ferlan wrote:
On 02/18/2013 09:27 AM, Ján Tomko wrote:
For really old qemu-img binaries which do not support specifying the format of the backing file, display a DEBUG message instead of INFO that this can't be done. --- src/storage/storage_backend.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
ACK
John
Thanks. I've pushed the series. Jan
participants (2)
-
John Ferlan
-
Ján Tomko