[libvirt] [PATCH 0/3] storage: drop support for qcow-create, kvm-img

I don't think we need to try and support qcow-create or kvm-img binaries anymore; everywhere we care about should have a /usr/bin/qemu-img. See patches for more details Cole Robinson (3): storage: remove support for /usr/bin/qcow-create storage: remove support for /usr/bin/kvm-img storage: drop the plumbing needed for kvm-img/qcow-create src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 4 +- src/storage/storage_backend.c | 131 ++------------------------------------- src/storage/storage_backend.h | 9 ++- src/storage/storage_backend_fs.c | 21 ++----- src/util/virfile.c | 2 +- tests/virstoragetest.c | 4 +- 7 files changed, 18 insertions(+), 155 deletions(-) -- 2.7.3

qcow-create was a crippled qemu-img impl that shipped with xen. I think supporting this was only relevant for really old distros that didn't have a proper qemu package, like early RHEL5. I think it's fair to drop support --- src/storage/storage_backend.c | 68 ---------------------------------------- src/storage/storage_backend_fs.c | 2 +- 2 files changed, 1 insertion(+), 69 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index a8fff14..7cc23d5 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -154,7 +154,6 @@ static virStorageFileBackendPtr fileBackends[] = { enum { TOOL_QEMU_IMG, TOOL_KVM_IMG, - TOOL_QCOW_CREATE, }; #define READ_BLOCK_SIZE_DEFAULT (1024 * 1024) @@ -1263,69 +1262,6 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, return ret; } -/* - * Xen removed the fully-functional qemu-img, and replaced it - * with a partially functional qcow-create. Go figure ??!? - */ -static int -virStorageBackendCreateQcowCreate(virConnectPtr conn ATTRIBUTE_UNUSED, - virStoragePoolObjPtr pool, - virStorageVolDefPtr vol, - virStorageVolDefPtr inputvol, - unsigned int flags) -{ - int ret; - char *size; - virCommandPtr cmd; - - virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, -1); - - if (flags & VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("metadata preallocation is not supported with " - "qcow-create")); - return -1; - } - - if (inputvol) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot copy from volume with qcow-create")); - return -1; - } - - if (vol->target.format != VIR_STORAGE_FILE_QCOW2) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unsupported storage vol type %d"), - vol->target.format); - return -1; - } - if (vol->target.backingStore != NULL) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("copy-on-write image not supported with " - "qcow-create")); - return -1; - } - if (vol->target.encryption != NULL) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("encrypted volumes not supported with " - "qcow-create")); - return -1; - } - - /* Size in MB - yes different units to qemu-img :-( */ - if (virAsprintf(&size, "%llu", - VIR_DIV_UP(vol->target.capacity, (1024 * 1024))) < 0) - return -1; - - cmd = virCommandNewArgList("qcow-create", size, vol->target.path, NULL); - - ret = virStorageBackendCreateExecCommand(pool, vol, cmd); - virCommandFree(cmd); - VIR_FREE(size); - - return ret; -} - virStorageBackendBuildVolFrom virStorageBackendFSImageToolTypeToFunc(int tool_type) { @@ -1333,8 +1269,6 @@ virStorageBackendFSImageToolTypeToFunc(int tool_type) case TOOL_KVM_IMG: case TOOL_QEMU_IMG: return virStorageBackendCreateQemuImg; - case TOOL_QCOW_CREATE: - return virStorageBackendCreateQcowCreate; default: virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown file create tool type '%d'."), @@ -1354,8 +1288,6 @@ virStorageBackendFindFSImageTool(char **tool) tool_type = TOOL_KVM_IMG; } else if ((tmp = virFindFileInPath("qemu-img")) != NULL) { tool_type = TOOL_QEMU_IMG; - } else if ((tmp = virFindFileInPath("qcow-create")) != NULL) { - tool_type = TOOL_QCOW_CREATE; } if (tool) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index b114b76..f55f5e2 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1209,7 +1209,7 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn, /** * Allocate a new file as a volume. This is either done directly - * for raw/sparse files, or by calling qemu-img/qcow-create for + * for raw/sparse files, or by calling qemu-img for * special kinds of files */ static int -- 2.7.3

On Fri, Apr 15, 2016 at 05:21:26PM -0400, Cole Robinson wrote:
qcow-create was a crippled qemu-img impl that shipped with xen. I
Actually qcow2-create was a complete fresh impl of a qcow *v1* image creation tool, that has no code in common with qemu-img !
think supporting this was only relevant for really old distros that didn't have a proper qemu package, like early RHEL5. I think it's fair to drop support
Yep, any modern deployment of Xen will just use qemu-img these days
--- src/storage/storage_backend.c | 68 ---------------------------------------- src/storage/storage_backend_fs.c | 2 +- 2 files changed, 1 insertion(+), 69 deletions(-)
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

This an ubuntu/debian packaging convention. At one point it may have been an actually different binary, but at least as of ubuntu precise (the oldest supported ubuntu distro, released april 2012) kvm-img is just a symlink to qemu-img for back compat. I think it's safe to drop support for it --- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 4 +--- src/storage/storage_backend.c | 15 +++------------ src/storage/storage_backend_fs.c | 8 ++------ src/util/virfile.c | 2 +- tests/virstoragetest.c | 4 +--- 6 files changed, 9 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ed1e0e5..73187ce 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3072,7 +3072,7 @@ qemuFindQemuImgBinary(virQEMUDriverPtr driver) { if (!driver->qemuImgBinary) virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("unable to find kvm-img or qemu-img")); + "%s", _("unable to find qemu-img")); return driver->qemuImgBinary; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 95612cf..bf05ace 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -718,9 +718,7 @@ qemuStateInitialize(bool privileged, goto error; } - qemu_driver->qemuImgBinary = virFindFileInPath("kvm-img"); - if (!qemu_driver->qemuImgBinary) - qemu_driver->qemuImgBinary = virFindFileInPath("qemu-img"); + qemu_driver->qemuImgBinary = virFindFileInPath("qemu-img"); if (!(qemu_driver->lockManager = virLockManagerPluginNew(cfg->lockManagerName ? diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 7cc23d5..e4b9b39 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -153,7 +153,6 @@ static virStorageFileBackendPtr fileBackends[] = { enum { TOOL_QEMU_IMG, - TOOL_KVM_IMG, }; #define READ_BLOCK_SIZE_DEFAULT (1024 * 1024) @@ -1234,14 +1233,10 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, 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"); - + create_tool = virFindFileInPath("qemu-img"); if (!create_tool) { virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("unable to find kvm-img or qemu-img")); + "%s", _("unable to find qemu-img")); return -1; } @@ -1266,7 +1261,6 @@ virStorageBackendBuildVolFrom virStorageBackendFSImageToolTypeToFunc(int tool_type) { switch (tool_type) { - case TOOL_KVM_IMG: case TOOL_QEMU_IMG: return virStorageBackendCreateQemuImg; default: @@ -1284,11 +1278,8 @@ virStorageBackendFindFSImageTool(char **tool) int tool_type = -1; char *tmp = NULL; - if ((tmp = virFindFileInPath("kvm-img")) != NULL) { - tool_type = TOOL_KVM_IMG; - } else if ((tmp = virFindFileInPath("qemu-img")) != NULL) { + if ((tmp = virFindFileInPath("qemu-img")) != NULL) tool_type = TOOL_QEMU_IMG; - } if (tool) *tool = tmp; diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index f55f5e2..47d0f54 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1343,14 +1343,10 @@ virStorageBackendFilesystemResizeQemuImg(const char *path, char *img_tool; virCommandPtr cmd = NULL; - /* KVM is usually ahead of qemu on features, so try that first */ - img_tool = virFindFileInPath("kvm-img"); - if (!img_tool) - img_tool = virFindFileInPath("qemu-img"); - + img_tool = virFindFileInPath("qemu-img"); if (!img_tool) { virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("unable to find kvm-img or qemu-img")); + "%s", _("unable to find qemu-img")); return -1; } diff --git a/src/util/virfile.c b/src/util/virfile.c index f0412c6..730c08d 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -1580,7 +1580,7 @@ virFileIsLink(const char *linkpath) /* * Finds a requested executable file in the PATH env. e.g.: - * "kvm-img" will return "/usr/bin/kvm-img" + * "qemu-img" will return "/usr/bin/qemu-img" * * You must free the result */ diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 38ce09e..698f472 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -137,9 +137,7 @@ testPrepImages(void) char *buf = NULL; bool compat = false; - qemuimg = virFindFileInPath("kvm-img"); - if (!qemuimg) - qemuimg = virFindFileInPath("qemu-img"); + qemuimg = virFindFileInPath("qemu-img"); if (!qemuimg) goto skip; -- 2.7.3

On Fri, Apr 15, 2016 at 05:21:27PM -0400, Cole Robinson wrote:
This an ubuntu/debian packaging convention. At one point it may have been an actually different binary, but at least as of ubuntu precise (the oldest supported ubuntu distro, released april 2012) kvm-img is just a symlink to qemu-img for back compat.
I think it's safe to drop support for it
Yep, from the days when ubuntu had a separate qemu-kvm package, gone now that kvm is part of main qemu package. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Remove all the plumbing needed for the different qcow-create/kvm-img non-raw file creation. We can drop the error messages because CreateQemuImg will thrown an error for us but with slightly less fidelity (unable to find qemu-img), which I think is acceptable given the unlikeliness of that error in practice. --- src/storage/storage_backend.c | 50 ++-------------------------------------- src/storage/storage_backend.h | 9 +++++--- src/storage/storage_backend_fs.c | 11 +-------- 3 files changed, 9 insertions(+), 61 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index e4b9b39..a71b838 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -151,10 +151,6 @@ static virStorageFileBackendPtr fileBackends[] = { }; -enum { - TOOL_QEMU_IMG, -}; - #define READ_BLOCK_SIZE_DEFAULT (1024 * 1024) #define WRITE_BLOCK_SIZE_DEFAULT (4 * 1024) @@ -1219,7 +1215,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn, return cmd; } -static int +int virStorageBackendCreateQemuImg(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, @@ -1258,43 +1254,9 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, } virStorageBackendBuildVolFrom -virStorageBackendFSImageToolTypeToFunc(int tool_type) -{ - switch (tool_type) { - case TOOL_QEMU_IMG: - return virStorageBackendCreateQemuImg; - default: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unknown file create tool type '%d'."), - tool_type); - } - - return NULL; -} - -int -virStorageBackendFindFSImageTool(char **tool) -{ - int tool_type = -1; - char *tmp = NULL; - - if ((tmp = virFindFileInPath("qemu-img")) != NULL) - tool_type = TOOL_QEMU_IMG; - - if (tool) - *tool = tmp; - else - VIR_FREE(tmp); - - return tool_type; -} - -virStorageBackendBuildVolFrom virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol, virStorageVolDefPtr inputvol) { - int tool_type; - if (!inputvol) return NULL; @@ -1305,15 +1267,7 @@ virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol, vol->target.format != VIR_STORAGE_FILE_RAW) || (inputvol->type == VIR_STORAGE_VOL_FILE && inputvol->target.format != VIR_STORAGE_FILE_RAW)) { - - if ((tool_type = virStorageBackendFindFSImageTool(NULL)) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("creation of non-raw file images is " - "not supported without qemu-img.")); - return NULL; - } - - return virStorageBackendFSImageToolTypeToFunc(tool_type); + return virStorageBackendCreateQemuImg; } if (vol->type == VIR_STORAGE_VOL_PLOOP) diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index a1e39c5..5bc622c 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -109,6 +109,12 @@ int virStorageBackendCreateRaw(virConnectPtr conn, virStorageVolDefPtr inputvol, unsigned int flags); +int virStorageBackendCreateQemuImg(virConnectPtr conn, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + virStorageVolDefPtr inputvol, + unsigned int flags); + int virStorageBackendCreatePloop(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, @@ -126,9 +132,6 @@ bool virStorageBackendIsPloopDir(char *path); virStorageBackendBuildVolFrom virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol, virStorageVolDefPtr inputvol); -int virStorageBackendFindFSImageTool(char **tool); -virStorageBackendBuildVolFrom -virStorageBackendFSImageToolTypeToFunc(int tool_type); int virStorageBackendFindGlusterPoolSources(const char *host, int pooltype, diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 47d0f54..02a129e 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1170,7 +1170,6 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn, unsigned int flags) { virStorageBackendBuildVolFrom create_func; - int tool_type; if (inputvol) { if (vol->target.encryption != NULL) { @@ -1190,16 +1189,8 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn, create_func = createFileDir; } else if (vol->target.format == VIR_STORAGE_FILE_PLOOP) { create_func = virStorageBackendCreatePloop; - } else if ((tool_type = virStorageBackendFindFSImageTool(NULL)) != -1) { - create_func = virStorageBackendFSImageToolTypeToFunc(tool_type); - - if (!create_func) - return -1; } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("creation of non-raw images " - "is not supported without qemu-img")); - return -1; + create_func = virStorageBackendCreateQemuImg; } if (create_func(conn, pool, vol, inputvol, flags) < 0) -- 2.7.3

On 04/15/2016 05:21 PM, Cole Robinson wrote:
Remove all the plumbing needed for the different qcow-create/kvm-img non-raw file creation.
We can drop the error messages because CreateQemuImg will thrown an error for us but with slightly less fidelity (unable to find qemu-img), which I think is acceptable given the unlikeliness of that error in practice. --- src/storage/storage_backend.c | 50 ++-------------------------------------- src/storage/storage_backend.h | 9 +++++--- src/storage/storage_backend_fs.c | 11 +-------- 3 files changed, 9 insertions(+), 61 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index e4b9b39..a71b838 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -151,10 +151,6 @@ static virStorageFileBackendPtr fileBackends[] = { };
-enum { - TOOL_QEMU_IMG, -}; - #define READ_BLOCK_SIZE_DEFAULT (1024 * 1024) #define WRITE_BLOCK_SIZE_DEFAULT (4 * 1024)
@@ -1219,7 +1215,7 @@ virStorageBackendCreateQemuImgCmdFromVol(virConnectPtr conn, return cmd; }
-static int +int virStorageBackendCreateQemuImg(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, @@ -1258,43 +1254,9 @@ virStorageBackendCreateQemuImg(virConnectPtr conn,
Suggestion-only... In this function, there's the following: create_tool = virFindFileInPath("qemu-img"); if (!create_tool) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("unable to find qemu-img")); return -1; } Which is a perfectly reasonable error message; however, perhaps the one from virStorageBackendGetBuildVolFromFunction that gets removed could be more descriptive, that is: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("creation of non-raw file images is " "not supported without qemu-img.")); John
}
virStorageBackendBuildVolFrom -virStorageBackendFSImageToolTypeToFunc(int tool_type) -{ - switch (tool_type) { - case TOOL_QEMU_IMG: - return virStorageBackendCreateQemuImg; - default: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unknown file create tool type '%d'."), - tool_type); - } - - return NULL; -} - -int -virStorageBackendFindFSImageTool(char **tool) -{ - int tool_type = -1; - char *tmp = NULL; - - if ((tmp = virFindFileInPath("qemu-img")) != NULL) - tool_type = TOOL_QEMU_IMG; - - if (tool) - *tool = tmp; - else - VIR_FREE(tmp); - - return tool_type; -} - -virStorageBackendBuildVolFrom virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol, virStorageVolDefPtr inputvol) { - int tool_type; - if (!inputvol) return NULL;
@@ -1305,15 +1267,7 @@ virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol, vol->target.format != VIR_STORAGE_FILE_RAW) || (inputvol->type == VIR_STORAGE_VOL_FILE && inputvol->target.format != VIR_STORAGE_FILE_RAW)) { - - if ((tool_type = virStorageBackendFindFSImageTool(NULL)) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("creation of non-raw file images is " - "not supported without qemu-img.")); - return NULL; - } - - return virStorageBackendFSImageToolTypeToFunc(tool_type); + return virStorageBackendCreateQemuImg; }
if (vol->type == VIR_STORAGE_VOL_PLOOP) diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index a1e39c5..5bc622c 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -109,6 +109,12 @@ int virStorageBackendCreateRaw(virConnectPtr conn, virStorageVolDefPtr inputvol, unsigned int flags);
+int virStorageBackendCreateQemuImg(virConnectPtr conn, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + virStorageVolDefPtr inputvol, + unsigned int flags); + int virStorageBackendCreatePloop(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, @@ -126,9 +132,6 @@ bool virStorageBackendIsPloopDir(char *path); virStorageBackendBuildVolFrom virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol, virStorageVolDefPtr inputvol); -int virStorageBackendFindFSImageTool(char **tool); -virStorageBackendBuildVolFrom -virStorageBackendFSImageToolTypeToFunc(int tool_type);
int virStorageBackendFindGlusterPoolSources(const char *host, int pooltype, diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 47d0f54..02a129e 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1170,7 +1170,6 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn, unsigned int flags) { virStorageBackendBuildVolFrom create_func; - int tool_type;
if (inputvol) { if (vol->target.encryption != NULL) { @@ -1190,16 +1189,8 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn, create_func = createFileDir; } else if (vol->target.format == VIR_STORAGE_FILE_PLOOP) { create_func = virStorageBackendCreatePloop; - } else if ((tool_type = virStorageBackendFindFSImageTool(NULL)) != -1) { - create_func = virStorageBackendFSImageToolTypeToFunc(tool_type); - - if (!create_func) - return -1; } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("creation of non-raw images " - "is not supported without qemu-img")); - return -1; + create_func = virStorageBackendCreateQemuImg; }
if (create_func(conn, pool, vol, inputvol, flags) < 0)

On 04/15/2016 05:21 PM, Cole Robinson wrote:
I don't think we need to try and support qcow-create or kvm-img binaries anymore; everywhere we care about should have a /usr/bin/qemu-img. See patches for more details
Cole Robinson (3): storage: remove support for /usr/bin/qcow-create storage: remove support for /usr/bin/kvm-img storage: drop the plumbing needed for kvm-img/qcow-create
src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 4 +- src/storage/storage_backend.c | 131 ++------------------------------------- src/storage/storage_backend.h | 9 ++- src/storage/storage_backend_fs.c | 21 ++----- src/util/virfile.c | 2 +- tests/virstoragetest.c | 4 +- 7 files changed, 18 insertions(+), 155 deletions(-)
ACK series in general (see my note in 3/3)... I always wondered about the history of this code as I was trudging through it for buildVol. Your call on how long you want to wait to see if anyone comes back with the "no we cannot remove this because" type response (of course there's always git revert! John

On 04/20/2016 08:10 AM, John Ferlan wrote:
On 04/15/2016 05:21 PM, Cole Robinson wrote:
I don't think we need to try and support qcow-create or kvm-img binaries anymore; everywhere we care about should have a /usr/bin/qemu-img. See patches for more details
Cole Robinson (3): storage: remove support for /usr/bin/qcow-create storage: remove support for /usr/bin/kvm-img storage: drop the plumbing needed for kvm-img/qcow-create
src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 4 +- src/storage/storage_backend.c | 131 ++------------------------------------- src/storage/storage_backend.h | 9 ++- src/storage/storage_backend_fs.c | 21 ++----- src/util/virfile.c | 2 +- tests/virstoragetest.c | 4 +- 7 files changed, 18 insertions(+), 155 deletions(-)
ACK series in general (see my note in 3/3)... I always wondered about the history of this code as I was trudging through it for buildVol.
Your call on how long you want to wait to see if anyone comes back with the "no we cannot remove this because" type response (of course there's always git revert!
Dan probably has the most context on the history here, CCd for his opinion (see patch #1 and #2 for what I could drum up on qcow-create and kvm-img relevancy) - Cole

On Wed, Apr 20, 2016 at 08:40:16AM -0400, Cole Robinson wrote:
On 04/20/2016 08:10 AM, John Ferlan wrote:
On 04/15/2016 05:21 PM, Cole Robinson wrote:
I don't think we need to try and support qcow-create or kvm-img binaries anymore; everywhere we care about should have a /usr/bin/qemu-img. See patches for more details
Cole Robinson (3): storage: remove support for /usr/bin/qcow-create storage: remove support for /usr/bin/kvm-img storage: drop the plumbing needed for kvm-img/qcow-create
src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 4 +- src/storage/storage_backend.c | 131 ++------------------------------------- src/storage/storage_backend.h | 9 ++- src/storage/storage_backend_fs.c | 21 ++----- src/util/virfile.c | 2 +- tests/virstoragetest.c | 4 +- 7 files changed, 18 insertions(+), 155 deletions(-)
ACK series in general (see my note in 3/3)... I always wondered about the history of this code as I was trudging through it for buildVol.
Your call on how long you want to wait to see if anyone comes back with the "no we cannot remove this because" type response (of course there's always git revert!
Dan probably has the most context on the history here, CCd for his opinion (see patch #1 and #2 for what I could drum up on qcow-create and kvm-img relevancy)
ACK, its fine. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 04/20/2016 08:48 AM, Daniel P. Berrange wrote:
On Wed, Apr 20, 2016 at 08:40:16AM -0400, Cole Robinson wrote:
On 04/20/2016 08:10 AM, John Ferlan wrote:
On 04/15/2016 05:21 PM, Cole Robinson wrote:
I don't think we need to try and support qcow-create or kvm-img binaries anymore; everywhere we care about should have a /usr/bin/qemu-img. See patches for more details
Cole Robinson (3): storage: remove support for /usr/bin/qcow-create storage: remove support for /usr/bin/kvm-img storage: drop the plumbing needed for kvm-img/qcow-create
src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 4 +- src/storage/storage_backend.c | 131 ++------------------------------------- src/storage/storage_backend.h | 9 ++- src/storage/storage_backend_fs.c | 21 ++----- src/util/virfile.c | 2 +- tests/virstoragetest.c | 4 +- 7 files changed, 18 insertions(+), 155 deletions(-)
ACK series in general (see my note in 3/3)... I always wondered about the history of this code as I was trudging through it for buildVol.
Your call on how long you want to wait to see if anyone comes back with the "no we cannot remove this because" type response (of course there's always git revert!
Dan probably has the most context on the history here, CCd for his opinion (see patch #1 and #2 for what I could drum up on qcow-create and kvm-img relevancy)
ACK, its fine.
Thanks, pushed with John's patch #3 suggestion - Cole
participants (3)
-
Cole Robinson
-
Daniel P. Berrange
-
John Ferlan