Re: [libvirt] [PATCH 3/3] storage: Create common file/dir volume backend helpers

[Olga Krishtal]

On 21/01/17 20:23, John Ferlan wrote:

Move all the volume functions to storage_util to create local/common helpers
using the same naming syntax as the existing upload, download, and wipe
virStorageBackend*Local API's.

In the process of doing so, found more API's that can now become local
to storage_util. In order to distinguish between local/external - I
changed the names of the now local only ones from "virStorageBackend..."
to just "storageBackend..."

Signed-off-by: John Ferlan 
---
  src/storage/storage_backend_fs.c | 372 ++--
  src/storage/storage_util.c   | 511 ---
  src/storage/storage_util.h   |  44 ++--
  3 files changed, 465 insertions(+), 462 deletions(-)

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 6f331d6..9710648 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -625,342 +625,6 @@ virStorageBackendFileSystemBuild(virConnectPtr conn 
ATTRIBUTE_UNUSED,
  }
  
  
-/**

- * Set up a volume definition to be added to a pool's volume list, but
- * don't do any file creation or allocation. By separating the two processes,
- * we allow allocation progress reporting (by polling the volume's 'info'
- * function), and can drop the parent pool lock during the (slow) allocation.
- */
-static int
-virStorageBackendFileSystemVolCreate(virConnectPtr conn ATTRIBUTE_UNUSED,
- virStoragePoolObjPtr pool,
- virStorageVolDefPtr vol)
-{
-
-if (vol->target.format == VIR_STORAGE_FILE_DIR)
-vol->type = VIR_STORAGE_VOL_DIR;
-else if (vol->target.format == VIR_STORAGE_FILE_PLOOP)
-vol->type = VIR_STORAGE_VOL_PLOOP;
-else
-vol->type = VIR_STORAGE_VOL_FILE;
-
-/* Volumes within a directory pools are not recursive; do not
- * allow escape to ../ or a subdir */
-if (strchr(vol->name, '/')) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   _("volume name '%s' cannot contain '/'"), vol->name);
-return -1;
-}
-
-VIR_FREE(vol->target.path);
-if (virAsprintf(>target.path, "%s/%s",
-pool->def->target.path,
-vol->name) == -1)
-return -1;
-
-if (virFileExists(vol->target.path)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   _("volume target path '%s' already exists"),
-   vol->target.path);
-return -1;
-}
-
-VIR_FREE(vol->key);
-return VIR_STRDUP(vol->key, vol->target.path);
-}
-
-static int createFileDir(virConnectPtr conn ATTRIBUTE_UNUSED,
- virStoragePoolObjPtr pool,
- virStorageVolDefPtr vol,
- virStorageVolDefPtr inputvol,
- unsigned int flags)
-{
-int err;
-
-virCheckFlags(0, -1);
-
-if (inputvol) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s",
-   _("cannot copy from volume to a directory volume"));
-return -1;
-}
-
-if (vol->target.backingStore) {
-virReportError(VIR_ERR_NO_SUPPORT, "%s",
-   _("backing storage not supported for directories 
volumes"));
-return -1;
-}
-
-
-if ((err = 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,
-(pool->def->type == VIR_STORAGE_POOL_NETFS
- ? VIR_DIR_CREATE_AS_UID : 0))) < 0) {
-return -1;
-}
-
-return 0;
-}
-
-static int
-_virStorageBackendFileSystemVolBuild(virConnectPtr conn,
- virStoragePoolObjPtr pool,
- virStorageVolDefPtr vol,
- virStorageVolDefPtr inputvol,
- unsigned int flags)
-{
-virStorageBackendBuildVolFrom create_func;
-
-if (inputvol) {
-if (vol->target.encryption != NULL) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   "%s", _("storage pool does not support "
-   "building encrypted volumes from "
-   "other volumes"));
-return -1;
-}
-create_func = virStorageBackendGetBuildVolFromFunction(vol,
-   inputvol);
-if (!create_func)
-return -1;
-} else if (vol->target.format == VIR_STORAGE_FILE_RAW &&
-   vol->target.encryption == NULL) {
-create_func = 

[libvirt] [PATCH 3/3] storage: Create common file/dir volume backend helpers

[John Ferlan]

Move all the volume functions to storage_util to create local/common helpers
using the same naming syntax as the existing upload, download, and wipe
virStorageBackend*Local API's.

In the process of doing so, found more API's that can now become local
to storage_util. In order to distinguish between local/external - I
changed the names of the now local only ones from "virStorageBackend..."
to just "storageBackend..."

Signed-off-by: John Ferlan 
---
 src/storage/storage_backend_fs.c | 372 ++--
 src/storage/storage_util.c   | 511 ---
 src/storage/storage_util.h   |  44 ++--
 3 files changed, 465 insertions(+), 462 deletions(-)

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 6f331d6..9710648 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -625,342 +625,6 @@ virStorageBackendFileSystemBuild(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 }
 
 
-/**
- * Set up a volume definition to be added to a pool's volume list, but
- * don't do any file creation or allocation. By separating the two processes,
- * we allow allocation progress reporting (by polling the volume's 'info'
- * function), and can drop the parent pool lock during the (slow) allocation.
- */
-static int
-virStorageBackendFileSystemVolCreate(virConnectPtr conn ATTRIBUTE_UNUSED,
- virStoragePoolObjPtr pool,
- virStorageVolDefPtr vol)
-{
-
-if (vol->target.format == VIR_STORAGE_FILE_DIR)
-vol->type = VIR_STORAGE_VOL_DIR;
-else if (vol->target.format == VIR_STORAGE_FILE_PLOOP)
-vol->type = VIR_STORAGE_VOL_PLOOP;
-else
-vol->type = VIR_STORAGE_VOL_FILE;
-
-/* Volumes within a directory pools are not recursive; do not
- * allow escape to ../ or a subdir */
-if (strchr(vol->name, '/')) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   _("volume name '%s' cannot contain '/'"), vol->name);
-return -1;
-}
-
-VIR_FREE(vol->target.path);
-if (virAsprintf(>target.path, "%s/%s",
-pool->def->target.path,
-vol->name) == -1)
-return -1;
-
-if (virFileExists(vol->target.path)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   _("volume target path '%s' already exists"),
-   vol->target.path);
-return -1;
-}
-
-VIR_FREE(vol->key);
-return VIR_STRDUP(vol->key, vol->target.path);
-}
-
-static int createFileDir(virConnectPtr conn ATTRIBUTE_UNUSED,
- virStoragePoolObjPtr pool,
- virStorageVolDefPtr vol,
- virStorageVolDefPtr inputvol,
- unsigned int flags)
-{
-int err;
-
-virCheckFlags(0, -1);
-
-if (inputvol) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s",
-   _("cannot copy from volume to a directory volume"));
-return -1;
-}
-
-if (vol->target.backingStore) {
-virReportError(VIR_ERR_NO_SUPPORT, "%s",
-   _("backing storage not supported for directories 
volumes"));
-return -1;
-}
-
-
-if ((err = 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,
-(pool->def->type == VIR_STORAGE_POOL_NETFS
- ? VIR_DIR_CREATE_AS_UID : 0))) < 0) {
-return -1;
-}
-
-return 0;
-}
-
-static int
-_virStorageBackendFileSystemVolBuild(virConnectPtr conn,
- virStoragePoolObjPtr pool,
- virStorageVolDefPtr vol,
- virStorageVolDefPtr inputvol,
- unsigned int flags)
-{
-virStorageBackendBuildVolFrom create_func;
-
-if (inputvol) {
-if (vol->target.encryption != NULL) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   "%s", _("storage pool does not support "
-   "building encrypted volumes from "
-   "other volumes"));
-return -1;
-}
-create_func = virStorageBackendGetBuildVolFromFunction(vol,
-   inputvol);
-if (!create_func)
-return -1;
-} else if (vol->target.format == VIR_STORAGE_FILE_RAW &&
-   vol->target.encryption == NULL) {
-create_func = virStorageBackendCreateRaw;
-} else if (vol->target.format ==