[libvirt] [PATCH 2/7] storage:dir: .buildVol and .buildVolFrom callbacks for ploop

[Ján Tomko]

On Mon, Apr 11, 2016 at 07:16:20PM +0300, Olga Krishtal wrote:
> These callbacks let us to create ploop volumes in dir, fs and etc. pools.
> If a ploop volume was created via buildVol callback, then this volume
> is an empty ploop device with DiskDescriptor.xml.
> If the volume was created via .buildFrom - then its content is similar to
> input volume content.
> 
> Signed-off-by: Olga Krishtal <okrishtal at virtuozzo.com>
> ---
>  src/storage/storage_backend.c    | 78 ++++++++++++++++++++++++++++++++++++++++
>  src/storage/storage_backend.h    |  8 +++++
>  src/storage/storage_backend_fs.c |  2 ++
>  3 files changed, 88 insertions(+)
> 
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index 231eccf..d109980 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -773,6 +773,81 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool,
>      return ret;
>  }
>  
> +/* Create ploop directory with ploop image and DiskDescriptor.xml
> + * if function fails to create image file the directory will be deleted.*/
> +int
> +virStorageBackendCreatePloop(virConnectPtr conn ATTRIBUTE_UNUSED,
> +                             virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
> +                             virStorageVolDefPtr vol,
> +                             virStorageVolDefPtr inputvol,
> +                             unsigned int flags)
> +{
> +    int ret = -1;
> +    virCommandPtr cmd = NULL;
> +    char *create_tool = NULL;
> +
> +    virCheckFlags(0, -1);
> +
> +    if (inputvol && inputvol->target.format != VIR_STORAGE_FILE_PLOOP) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("unsupported input storage vol type %d"),
> +                       inputvol->target.format);
> +        goto cleanup;
> +    }
> +
> +    if (vol->target.encryption != NULL) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("encrypted ploop volumes are not supported with "
> +                         "ploop init"));
> +        goto cleanup;
> +    }
> +
> +    if (vol->target.backingStore != NULL) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("copy-on-write ploop volumes are not yet supported"));
> +        goto cleanup;
> +    }
> +
> +    create_tool = virFindFileInPath("ploop");
> +    if (!create_tool && !inputvol) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       "%s", _("unable to find ploop, please install "
> +                               "ploop tools"));
> +        return ret;
> +    }
> +

All the errors above can just 'return -1;', there is nothing to clean
up.

> +    if (!inputvol) {
> +        if ((virDirCreate(vol->target.path,
> +                          (vol->target.perms->mode == (mode_t) -1 ?
> +                           VIR_STORAGE_DEFAULT_VOL_PERM_MODE:
> +                           vol->target.perms->mode),
> +                          vol->target.perms->uid,
> +                          vol->target.perms->gid,
> +                          0)) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("error creating directory for ploop volume"));
> +            goto cleanup;
> +        }
> +        cmd = virCommandNewArgList(create_tool, "init", "-s", NULL);
> +        virCommandAddArgFormat(cmd, "%lluM", VIR_DIV_UP(vol->target.capacity,
> +                                                        (1024 * 1024)));
> +        virCommandAddArgList(cmd, "-t", "ext4", NULL);
> +        virCommandAddArgFormat(cmd, "%s/root.hds", vol->target.path);
> +
> +    } else {
> +        vol->target.capacity = inputvol->target.capacity;
> +        cmd = virCommandNewArgList("cp", "-r", inputvol->target.path,
> +                                   vol->target.path, NULL);
> +    }
> +    ret = virCommandRun(cmd, NULL);
> + cleanup:
> +    virCommandFree(cmd);
> +    VIR_FREE(create_tool);
> +    if (ret < 0)
> +        virFileDeleteTree(vol->target.path);

virDirCreate cleans up after itself if it fails - we don't need to
delete it in this case.

> +    return ret;
> +}
> +
>  enum {
>      QEMU_IMG_BACKING_FORMAT_NONE = 0,
>      QEMU_IMG_BACKING_FORMAT_FLAG,
> @@ -1291,6 +1366,9 @@ virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol,
>          return virStorageBackendFSImageToolTypeToFunc(tool_type);
>      }
>  
> +    if (vol->type == VIR_STORAGE_VOL_PLOOP &&

> +        inputvol->type == VIR_STORAGE_VOL_PLOOP)

After removing the inputvol->type check, libvirt will output the nicer
error message in virStorageBackendCreatePloop if it has a different
type.

> +        return virStorageBackendCreatePloop;
>      if (vol->type == VIR_STORAGE_VOL_BLOCK)
>          return virStorageBackendCreateBlockFrom;
>      else

I have squashed the following changes in:

Jan

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index d109980..6a035a2 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -785,6 +785,7 @@ virStorageBackendCreatePloop(virConnectPtr conn ATTRIBUTE_UNUSED,
     int ret = -1;
     virCommandPtr cmd = NULL;
     char *create_tool = NULL;
+    bool created = false;
 
     virCheckFlags(0, -1);
 
@@ -792,20 +793,20 @@ virStorageBackendCreatePloop(virConnectPtr conn ATTRIBUTE_UNUSED,
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("unsupported input storage vol type %d"),
                        inputvol->target.format);
-        goto cleanup;
+        return -1;
     }
 
     if (vol->target.encryption != NULL) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                        _("encrypted ploop volumes are not supported with "
                          "ploop init"));
-        goto cleanup;
+        return -1;
     }
 
     if (vol->target.backingStore != NULL) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                        _("copy-on-write ploop volumes are not yet supported"));
-        goto cleanup;
+        return -1;
     }
 
     create_tool = virFindFileInPath("ploop");
@@ -813,7 +814,7 @@ virStorageBackendCreatePloop(virConnectPtr conn ATTRIBUTE_UNUSED,
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        "%s", _("unable to find ploop, please install "
                                "ploop tools"));
-        return ret;
+        return -1;
     }
 
     if (!inputvol) {
@@ -839,11 +840,12 @@ virStorageBackendCreatePloop(virConnectPtr conn ATTRIBUTE_UNUSED,
         cmd = virCommandNewArgList("cp", "-r", inputvol->target.path,
                                    vol->target.path, NULL);
     }
+    created = true;
     ret = virCommandRun(cmd, NULL);
  cleanup:
     virCommandFree(cmd);
     VIR_FREE(create_tool);
-    if (ret < 0)
+    if (ret < 0 && created)
         virFileDeleteTree(vol->target.path);
     return ret;
 }
@@ -1366,8 +1368,7 @@ virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol,
         return virStorageBackendFSImageToolTypeToFunc(tool_type);
     }
 
-    if (vol->type == VIR_STORAGE_VOL_PLOOP &&
-        inputvol->type == VIR_STORAGE_VOL_PLOOP)
+    if (vol->type == VIR_STORAGE_VOL_PLOOP)
         return virStorageBackendCreatePloop;
     if (vol->type == VIR_STORAGE_VOL_BLOCK)
         return virStorageBackendCreateBlockFrom;