[libvirt] [PATCH 3/3] storage: drop the plumbing needed for kvm-img/qcow-create

[John Ferlan]

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