[libvirt] [PATCH v6 0/7] storage:dir: ploop volumes support

This series of patches introduces the support of ploop volumes in storage pools (dir, fs, etc). Ploop volume is a disk loopback block device consists of root.hds (it is the image file) and DiskDescriptor.xml: https://openvz.org/Ploop/format. Due to this fact it can't be treated as file or any other volume type, moreover, in order to successfully manipulate with such volume, ploop tools should be installed. All callbacks except the wipeVol are supported. First patch introduces ploop volume type. This is a directory with two files inside. The name of the directory is the name of the volume. Patch 2 deals with creating an empty volume and cloning the existing one. Clone is done via simle cp operation. If any of this operation fails - directory will be deleted. Patch 3 deletes recursively ploop directory. Patch 4 uses ploop tool to resize volume. Patch 6 adapts all refreshing functions to work with ploop. To get information the directory is checked. The volume is treated as the ploops one only if it contains ploop files. Every time the pool is refreshed DiskDescriptor.xml is restored. This is necessary, because the content of the volume may have changed. Upload and download (patch 7) can't be done if the volume contains snapshots. v6: - There is no DiskDescriptor.xml restoring during the refresh pool operation. This operation is moved to storage driver. - fixed issue with regenerating xml for ploop volume with the snapshots. v5: - added ploop volume type - there is no change in opening volume functions. Now reopening takes place is the volume is ploops one. - restore DiskDescriptor.xml every refresh pool - all information, except format is taken from header - forbidden upload and download ops for volume with snapshots - there is no separate function for deleting the volume - fixed identation and leaks v4: - fixed identation issues. - in case of .uploadVol, DiskDescriptor.xml is restored. - added check of ploops'accessibility v3: - no VIR_STORAGE_VOL_PLOOP type any more - adapted all patches according to previous change - fixed comments v2: - fixed memory leak - chenged the return value of all helper functions to 0/-1. Now check for success is smth like that: vir****Ploop() < 0 - fixed some identation issues.

Ploop image consists of directory with two files: ploop image itself, called root.hds and DiskDescriptor.xml that contains information about ploop device: https://openvz.org/Ploop/format. Such volume are difficult to manipulate in terms of existing volume types because they are neither a single files nor a directory. This patch introduces new volume type - ploop. This volume type is used by ploop volume's exclusively. Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- include/libvirt/libvirt-storage.h | 1 + src/conf/storage_conf.c | 3 ++- src/storage/storage_backend_fs.c | 3 +++ tools/virsh-volume.c | 3 ++- 4 files changed, 8 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt-storage.h b/include/libvirt/libvirt-storage.h index 1a868cc..57a26c4 100644 --- a/include/libvirt/libvirt-storage.h +++ b/include/libvirt/libvirt-storage.h @@ -122,6 +122,7 @@ typedef enum { VIR_STORAGE_VOL_NETWORK = 3, /* Network volumes like RBD (RADOS Block Device) */ VIR_STORAGE_VOL_NETDIR = 4, /* Network accessible directory that can * contain other network volumes */ + VIR_STORAGE_VOL_PLOOP = 5, /* Ploop based volumes*/ # ifdef VIR_ENUM_SENTINELS VIR_STORAGE_VOL_LAST diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 497c65f..daf8f99 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -52,7 +52,8 @@ VIR_LOG_INIT("conf.storage_conf"); VIR_ENUM_IMPL(virStorageVol, VIR_STORAGE_VOL_LAST, - "file", "block", "dir", "network", "netdir") + "file", "block", "dir", "network", + "netdir", "ploop") VIR_ENUM_IMPL(virStoragePool, VIR_STORAGE_POOL_LAST, diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 692c9ff..d54dbfa 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1085,6 +1085,8 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn ATTRIBUTE_UNUSED, 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; @@ -1259,6 +1261,7 @@ virStorageBackendFileSystemVolDelete(virConnectPtr conn ATTRIBUTE_UNUSED, } } break; + case VIR_STORAGE_VOL_PLOOP: case VIR_STORAGE_VOL_BLOCK: case VIR_STORAGE_VOL_NETWORK: case VIR_STORAGE_VOL_NETDIR: diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index cfb8cfc..36dd0ed 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -965,7 +965,8 @@ VIR_ENUM_IMPL(virshStorageVol, N_("block"), N_("dir"), N_("network"), - N_("netdir")) + N_("netdir"), + N_("ploop")) static const char * virshVolumeTypeToString(int type) -- 1.8.3.1

On Mon, Apr 11, 2016 at 07:16:19PM +0300, Olga Krishtal wrote:
Ploop image consists of directory with two files: ploop image itself, called root.hds and DiskDescriptor.xml that contains information about ploop device: https://openvz.org/Ploop/format. Such volume are difficult to manipulate in terms of existing volume types because they are neither a single files nor a directory. This patch introduces new volume type - ploop. This volume type is used by ploop volume's exclusively.
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- include/libvirt/libvirt-storage.h | 1 + src/conf/storage_conf.c | 3 ++- src/storage/storage_backend_fs.c | 3 +++ tools/virsh-volume.c | 3 ++- 4 files changed, 8 insertions(+), 2 deletions(-)
ACK Jan

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@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; + } + + 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); + 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) + return virStorageBackendCreatePloop; if (vol->type == VIR_STORAGE_VOL_BLOCK) return virStorageBackendCreateBlockFrom; else diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 20e6079..852d6ed 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -108,6 +108,14 @@ int virStorageBackendCreateRaw(virConnectPtr conn, virStorageVolDefPtr vol, virStorageVolDefPtr inputvol, unsigned int flags); + +int virStorageBackendCreatePloop(virConnectPtr conn, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + virStorageVolDefPtr inputvol, + unsigned int flags); + + virStorageBackendBuildVolFrom virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol, virStorageVolDefPtr inputvol); diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index d54dbfa..8517b26 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1179,6 +1179,8 @@ _virStorageBackendFileSystemVolBuild(virConnectPtr conn, create_func = virStorageBackendCreateRaw; } else if (vol->target.format == VIR_STORAGE_FILE_DIR) { 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); -- 1.8.3.1

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@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;

Recursively deletes whole directory of a ploop volume. To delete ploop image it has to be unmounted. Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/storage/storage_backend_fs.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 8517b26..77c94c9 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1264,6 +1264,9 @@ virStorageBackendFileSystemVolDelete(virConnectPtr conn ATTRIBUTE_UNUSED, } break; case VIR_STORAGE_VOL_PLOOP: + if (virFileDeleteTree(vol->target.path) < 0) + return -1; + break; case VIR_STORAGE_VOL_BLOCK: case VIR_STORAGE_VOL_NETWORK: case VIR_STORAGE_VOL_NETDIR: -- 1.8.3.1

On Mon, Apr 11, 2016 at 07:16:21PM +0300, Olga Krishtal wrote:
Recursively deletes whole directory of a ploop volume. To delete ploop image it has to be unmounted.
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/storage/storage_backend_fs.c | 3 +++ 1 file changed, 3 insertions(+)
ACK
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 8517b26..77c94c9 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1264,6 +1264,9 @@ virStorageBackendFileSystemVolDelete(virConnectPtr conn ATTRIBUTE_UNUSED, } break; case VIR_STORAGE_VOL_PLOOP: + if (virFileDeleteTree(vol->target.path) < 0) + return -1;
The indentation is off here. Jan
+ break; case VIR_STORAGE_VOL_BLOCK: case VIR_STORAGE_VOL_NETWORK: case VIR_STORAGE_VOL_NETDIR: -- 1.8.3.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Mon, Apr 11, 2016 at 07:16:21PM +0300, Olga Krishtal wrote:
Recursively deletes whole directory of a ploop volume. To delete ploop image it has to be unmounted.
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/storage/storage_backend_fs.c | 3 +++ 1 file changed, 3 insertions(+)
ACK Jan

Changes the size of given ploop volume via ploop resize tool. Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/storage/storage_backend.c | 25 +++++++++++++++++++++++++ src/storage/storage_backend.h | 2 ++ src/storage/storage_backend_fs.c | 2 ++ 3 files changed, 29 insertions(+) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index d109980..fddec3c 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -848,6 +848,31 @@ virStorageBackendCreatePloop(virConnectPtr conn ATTRIBUTE_UNUSED, return ret; } +int +virStoragePloopResize(virStorageVolDefPtr vol, + unsigned long long capacity) +{ + int ret = -1; + virCommandPtr cmd = NULL; + char *resize_tool = NULL; + + resize_tool = virFindFileInPath("ploop"); + if (!resize_tool) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("unable to find ploop, please install ploop tools")); + return -1; + } + cmd = virCommandNewArgList(resize_tool, "resize", "-s", NULL); + virCommandAddArgFormat(cmd, "%lluM", VIR_DIV_UP(capacity, (1024 * 1024))); + + virCommandAddArgFormat(cmd, "%s/DiskDescriptor.xml", vol->target.path); + + ret = virCommandRun(cmd, NULL); + virCommandFree(cmd); + VIR_FREE(resize_tool); + return ret; +} + enum { QEMU_IMG_BACKING_FORMAT_NONE = 0, QEMU_IMG_BACKING_FORMAT_FLAG, diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 852d6ed..73f2bfa 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -115,6 +115,8 @@ int virStorageBackendCreatePloop(virConnectPtr conn, virStorageVolDefPtr inputvol, unsigned int flags); +int virStoragePloopResize(virStorageVolDefPtr vol, + unsigned long long capacity); virStorageBackendBuildVolFrom virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol, diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 77c94c9..eae2b2e 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1379,6 +1379,8 @@ virStorageBackendFileSystemVolResize(virConnectPtr conn ATTRIBUTE_UNUSED, if (vol->target.format == VIR_STORAGE_FILE_RAW) { return virStorageFileResize(vol->target.path, capacity, vol->target.allocation, pre_allocate); + } else if (vol->target.format == VIR_STORAGE_FILE_PLOOP) { + return virStoragePloopResize(vol, capacity); } else { if (pre_allocate) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", -- 1.8.3.1

On Mon, Apr 11, 2016 at 07:16:22PM +0300, Olga Krishtal wrote:
Changes the size of given ploop volume via ploop resize tool.
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/storage/storage_backend.c | 25 +++++++++++++++++++++++++ src/storage/storage_backend.h | 2 ++ src/storage/storage_backend_fs.c | 2 ++ 3 files changed, 29 insertions(+)
ACK Jan

Returns error in case of vol-wipe cmd for a ploop volume Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/storage/storage_backend.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index fddec3c..8b936ae 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -2162,6 +2162,12 @@ virStorageBackendVolWipeLocal(virConnectPtr conn ATTRIBUTE_UNUSED, VIR_DEBUG("Wiping volume with path '%s' and algorithm %u", vol->target.path, algorithm); + if (vol->target.format == VIR_STORAGE_FILE_PLOOP) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("wiping for ploop volumes is not supported")); + goto cleanup; + } + fd = open(vol->target.path, O_RDWR); if (fd == -1) { virReportSystemError(errno, -- 1.8.3.1

On Mon, Apr 11, 2016 at 07:16:23PM +0300, Olga Krishtal wrote:
Returns error in case of vol-wipe cmd for a ploop volume
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/storage/storage_backend.c | 6 ++++++ 1 file changed, 6 insertions(+)
ACK Jan

Refreshes meta-information such as allocation, capacity, format, etc. Ploop volumes differ from other volume types. Path to volume is the path to directory with image file root.hds and DiskDescriptor.xml. https://openvz.org/Ploop/format Due to this fact, operations of opening the volume have to be done once again. get the information. To decide whether the given volume is ploops one, it is necessary to check the presence of root.hds and DiskDescriptor.xml files in volumes' directory. Only in this case the volume can be manipulated as the ploops one. Such strategy helps us to resolve problems that might occure, when we upload some other volume type from ploop source. Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/storage/storage_backend.c | 62 ++++++++++++++++++++++++++++++++++++++-- src/storage/storage_backend.h | 5 ++++ src/storage/storage_backend_fs.c | 20 ++++++++++--- src/util/virstoragefile.c | 7 +++-- 4 files changed, 85 insertions(+), 9 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 8b936ae..e159334 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1702,6 +1702,56 @@ virStorageBackendVolOpen(const char *path, struct stat *sb, return fd; } +/* virStorageIsPloop function checks whether given directory is ploop volume's + * directory. + */ +bool +virStorageBackendIsPloopDir(char *path) +{ + bool ret = false; + char *root = NULL; + char *desc = NULL; + if (virAsprintf(&root, "%s/root.hds", path) < 0) + return ret; + if (!virFileExists(root)) + goto cleanup; + if (virAsprintf(&desc, "%s/DiskDescriptor.xml", path) < 0) + goto cleanup; + if (!virFileExists(desc)) + goto cleanup; + + ret = true; + cleanup: + VIR_FREE(root); + VIR_FREE(desc); + return ret; +} + +/* In case of ploop volumes, path to volume is the path to the ploop + * directory. To get information about allocation, header information + * and etc. we need to perform virStorageBackendVolOpen and + * virStorageBackendUpdateVolTargetFd once again. + */ +int +virStorageBackendRedoPloopUpdate(virStorageSourcePtr target, struct stat *sb, + int *fd, unsigned int flags) +{ + char *path = NULL; + int ret = -1; + + if (virAsprintf(&path, "%s/root.hds", target->path) < 0) + return -1; + VIR_FORCE_CLOSE(*fd); + if ((*fd = virStorageBackendVolOpen(path, sb, flags)) < 0) + goto cleanup; + ret = virStorageBackendUpdateVolTargetInfoFD(target, *fd, sb); + + cleanup: + + VIR_FREE(path); + return ret; +} + /* * virStorageBackendUpdateVolTargetInfo * @target: target definition ptr of volume to update @@ -1709,7 +1759,6 @@ virStorageBackendVolOpen(const char *path, struct stat *sb, * @openflags: various VolOpenCheckMode flags to handle errors on open * @readflags: VolReadErrorMode flags to handle read error after open * is successful, but read is not. - * * Returns 0 for success, -1 on a legitimate error condition, and -2 * if the openflags used VIR_STORAGE_VOL_OPEN_NOERROR and some sort of * open error occurred. It is up to the caller to handle. A -2 may also @@ -1737,8 +1786,15 @@ virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target, if (target->type == VIR_STORAGE_VOL_FILE && target->format != VIR_STORAGE_FILE_NONE) { if (S_ISDIR(sb.st_mode)) { - ret = 0; - goto cleanup; + if (virStorageBackendIsPloopDir(target->path)) { + if ((ret = virStorageBackendRedoPloopUpdate(target, &sb, &fd, + openflags)) < 0) + goto cleanup; + target->format = VIR_STORAGE_FILE_PLOOP; + } else { + ret = 0; + goto cleanup; + } } if (lseek(fd, 0, SEEK_SET) == (off_t)-1) { diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 73f2bfa..a1e39c5 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -118,6 +118,11 @@ int virStorageBackendCreatePloop(virConnectPtr conn, int virStoragePloopResize(virStorageVolDefPtr vol, unsigned long long capacity); +int virStorageBackendRedoPloopUpdate(virStorageSourcePtr target, + struct stat *sb, int *fd, + unsigned int flags); +bool virStorageBackendIsPloopDir(char *path); + virStorageBackendBuildVolFrom virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol, virStorageVolDefPtr inputvol); diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index eae2b2e..5e57366 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -69,6 +69,7 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, int fd = -1; int ret = -1; int rc; + int format = VIR_STORAGE_FILE_AUTO; virStorageSourcePtr meta = NULL; struct stat sb; @@ -84,14 +85,22 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, goto cleanup; if (S_ISDIR(sb.st_mode)) { - target->format = VIR_STORAGE_FILE_DIR; - ret = 0; - goto cleanup; + if (virStorageBackendIsPloopDir(target->path)) { + if (virStorageBackendRedoPloopUpdate(target, &sb, &fd, + VIR_STORAGE_VOL_FS_PROBE_FLAGS) + < 0) + goto cleanup; + format = VIR_STORAGE_FILE_PLOOP; + } else { + target->format = VIR_STORAGE_FILE_DIR; + ret = 0; + goto cleanup; + } } if (!(meta = virStorageFileGetMetadataFromFD(target->path, fd, - VIR_STORAGE_FILE_AUTO, + format, &backingStoreFormat))) goto cleanup; @@ -949,6 +958,9 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED, if (vol->target.format == VIR_STORAGE_FILE_DIR) vol->type = VIR_STORAGE_VOL_DIR; + if (vol->target.format == VIR_STORAGE_FILE_PLOOP) + vol->type = VIR_STORAGE_VOL_PLOOP; + if (vol->target.backingStore) { ignore_value(virStorageBackendUpdateVolTargetInfo(vol->target.backingStore, false, diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 49b1745..05ac254 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -184,6 +184,8 @@ qedGetBackingStore(char **, int *, const char *, size_t); #define QED_F_BACKING_FILE 0x01 #define QED_F_BACKING_FORMAT_NO_PROBE 0x04 +#define PLOOP_IMAGE_SIZE_OFFSET 36 +#define PLOOP_SIZE_MULTIPLIER 512 static struct FileTypeInfo const fileTypeInfo[] = { [VIR_STORAGE_FILE_NONE] = { 0, NULL, NULL, LV_LITTLE_ENDIAN, @@ -236,8 +238,9 @@ static struct FileTypeInfo const fileTypeInfo[] = { -1, {0}, 0, 0, 0, 0, NULL, NULL }, [VIR_STORAGE_FILE_VHD] = { 0, NULL, NULL, LV_LITTLE_ENDIAN, -1, {0}, 0, 0, 0, 0, NULL, NULL }, - [VIR_STORAGE_FILE_PLOOP] = { 0, NULL, NULL, LV_LITTLE_ENDIAN, - -1, {0}, 0, 0, 0, 0, NULL, NULL }, + [VIR_STORAGE_FILE_PLOOP] = { 0, "WithouFreSpacExt", NULL, LV_LITTLE_ENDIAN, + -2, {0}, PLOOP_IMAGE_SIZE_OFFSET, 0, + PLOOP_SIZE_MULTIPLIER, -1, NULL, NULL }, /* All formats with a backing store probe below here */ [VIR_STORAGE_FILE_COW] = { -- 1.8.3.1

On Mon, Apr 11, 2016 at 07:16:24PM +0300, Olga Krishtal wrote:
Refreshes meta-information such as allocation, capacity, format, etc. Ploop volumes differ from other volume types. Path to volume is the path to directory with image file root.hds and DiskDescriptor.xml. https://openvz.org/Ploop/format Due to this fact, operations of opening the volume have to be done once again. get the information.
To decide whether the given volume is ploops one, it is necessary to check the presence of root.hds and DiskDescriptor.xml files in volumes' directory. Only in this case the volume can be manipulated as the ploops one. Such strategy helps us to resolve problems that might occure, when we upload some other volume type from ploop source.
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/storage/storage_backend.c | 62 ++++++++++++++++++++++++++++++++++++++-- src/storage/storage_backend.h | 5 ++++ src/storage/storage_backend_fs.c | 20 ++++++++++--- src/util/virstoragefile.c | 7 +++-- 4 files changed, 85 insertions(+), 9 deletions(-)
@@ -84,14 +85,22 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, goto cleanup;
if (S_ISDIR(sb.st_mode)) { - target->format = VIR_STORAGE_FILE_DIR; - ret = 0; - goto cleanup; + if (virStorageBackendIsPloopDir(target->path)) { + if (virStorageBackendRedoPloopUpdate(target, &sb, &fd, + VIR_STORAGE_VOL_FS_PROBE_FLAGS) + < 0) + goto cleanup; + format = VIR_STORAGE_FILE_PLOOP; + } else { + target->format = VIR_STORAGE_FILE_DIR; + ret = 0; + goto cleanup; + } }
if (!(meta = virStorageFileGetMetadataFromFD(target->path, fd, - VIR_STORAGE_FILE_AUTO, + format,
This change does not seem necessary. RedoPloopUpdate should point the fd at the root.hds file which should have the right "WithouFreSpacExt" magic and fill out the format. ACK with VIR_STORAGE_FILE_AUTO left in. Jan

In case of ploop volume, target path of the volume is the path to the directory that contains image file named root.hds and DiskDescriptor.xml. While using uploadVol and downloadVol callbacks we need to open root.hds itself. Upload or download operations with ploop volume are only allowed when images do not have snapshots. Otherwise operation fails. Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/storage/storage_backend.c | 99 +++++++++++++++++++++++++++++++++++++++++-- src/storage/storage_driver.c | 51 +++++++++++++++++++++- 2 files changed, 146 insertions(+), 4 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index e159334..372f5b1 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -2070,6 +2070,47 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool, return stablepath; } +/* + * Check whether the ploop image has snapshots. + * return: -1 - failed to check + * 0 - no snapshots + * 1 - at least one snapshot + */ +static int +virStorageBackendPloopHasSnapshots(char *path) +{ + virCommandPtr cmd = NULL; + char *output = NULL; + char *snap_tool = NULL; + int ret = -1; + + snap_tool = virFindFileInPath("ploop"); + if (!snap_tool) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("can't upload volume," + " please install ploop tool ")); + return ret; + } + + cmd = virCommandNewArgList(snap_tool, "snapshot-list", NULL); + virCommandAddArgFormat(cmd, "%s/DiskDescriptor.xml", path); + virCommandSetOutputBuffer(cmd, &output); + + if ((ret = virCommandRun(cmd, NULL)) < 0) + goto cleanup; + + if (!strstr(output, "root.hds.")) { + ret = 1; + goto cleanup; + } + ret = 0; + + cleanup: + VIR_FREE(output); + virCommandFree(cmd); + return ret; +} + int virStorageBackendVolUploadLocal(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, @@ -2079,12 +2120,41 @@ virStorageBackendVolUploadLocal(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long long len, unsigned int flags) { + char *path = NULL; + char *target_path = vol->target.path; + int ret = -1; + int has_snap = 0; + virCheckFlags(0, -1); + /* if volume has target format VIR_STORAGE_FILE_PLOOP + * we need to restore DiskDescriptor.xml, according to + * new contents of volume. This operation will be perfomed + * when volUpload is fully finished. */ + if (vol->target.format == VIR_STORAGE_FILE_PLOOP) { + /* Fail if the volume contains snapshots or we failed to check it.*/ + has_snap = virStorageBackendPloopHasSnapshots(vol->target.path); + if (has_snap < 0) { + goto cleanup; + } else if (!has_snap) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("can't upload volume, all existing snapshots" + " will be lost")); + goto cleanup; + } + + if (virAsprintf(&path, "%s/root.hds", vol->target.path) < 0) + return -1; + target_path = path; + } /* Not using O_CREAT because the file is required to already exist at * this point */ - return virFDStreamOpenBlockDevice(stream, vol->target.path, - offset, len, O_WRONLY); + ret = virFDStreamOpenBlockDevice(stream, target_path, + offset, len, O_WRONLY); + + cleanup: + VIR_FREE(path); + return ret; } int @@ -2096,10 +2166,33 @@ virStorageBackendVolDownloadLocal(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long long len, unsigned int flags) { + char *path = NULL; + char *target_path = vol->target.path; + int ret = -1; + int has_snap = 0; + virCheckFlags(0, -1); + if (vol->target.format == VIR_STORAGE_FILE_PLOOP) { + has_snap = virStorageBackendPloopHasSnapshots(vol->target.path); + if (has_snap < 0) { + goto cleanup; + } else if (!has_snap) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("can't upload volume, all existing snapshots" + " will be lost")); + goto cleanup; + } + if (virAsprintf(&path, "%s/root.hds", vol->target.path) < 0) + goto cleanup; + target_path = path; + } - return virFDStreamOpenBlockDevice(stream, vol->target.path, + ret = virFDStreamOpenBlockDevice(stream, target_path, offset, len, O_RDONLY); + + cleanup: + VIR_FREE(path); + return ret; } diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 1d96618..883b4be 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -63,6 +63,7 @@ typedef struct _virStorageVolStreamInfo virStorageVolStreamInfo; typedef virStorageVolStreamInfo *virStorageVolStreamInfoPtr; struct _virStorageVolStreamInfo { char *pool_name; + char *vol_path; }; static void storageDriverLock(void) @@ -2204,6 +2205,48 @@ virStorageVolPoolRefreshDataFree(void *opaque) VIR_FREE(cbdata); } +static int +virStorageBackendPloopRestoreDesc(char *path) +{ + int ret = -1; + virCommandPtr cmd = NULL; + char *refresh_tool = NULL; + char *desc = NULL; + + if (virAsprintf(&desc, "%s/DiskDescriptor.xml", path) < 0) + return ret; + + if (virFileRemove(desc, 0, 0) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("refresh ploop failed:" + " unuble to delete DiskDescriptor.xml")); + goto cleanup; + } + + refresh_tool = virFindFileInPath("ploop"); + if (!refresh_tool) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("unable to find ploop, please install ploop tools")); + goto cleanup; + } + + cmd = virCommandNewArgList(refresh_tool, "restore-descriptor", + path, NULL); + virCommandAddArgFormat(cmd, "%s/root.hds", path); + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + + ret = 0; + + cleanup: + VIR_FREE(refresh_tool); + virCommandFree(cmd); + VIR_FREE(desc); + return ret; +} + + + /** * Thread to handle the pool refresh * @@ -2219,6 +2262,10 @@ virStorageVolPoolRefreshThread(void *opaque) virStorageBackendPtr backend; storageDriverLock(); + if (cbdata->vol_path) { + if (virStorageBackendPloopRestoreDesc(cbdata->vol_path) < 0) + goto cleanup; + } if (!(pool = virStoragePoolObjFindByName(&driver->pools, cbdata->pool_name))) goto cleanup; @@ -2296,7 +2343,6 @@ storageVolUpload(virStorageVolPtr obj, vol->name); goto cleanup; } - if (!backend->uploadVol) { virReportError(VIR_ERR_NO_SUPPORT, "%s", _("storage pool doesn't support volume upload")); @@ -2313,6 +2359,9 @@ storageVolUpload(virStorageVolPtr obj, if (VIR_ALLOC(cbdata) < 0 || VIR_STRDUP(cbdata->pool_name, pool->def->name) < 0) goto cleanup; + if (vol->target.type == VIR_STORAGE_VOL_PLOOP && + VIR_STRDUP(cbdata->vol_path, vol->target.path) < 0) + goto cleanup; } if ((ret = backend->uploadVol(obj->conn, pool, vol, stream, -- 1.8.3.1

On Mon, Apr 11, 2016 at 07:16:25PM +0300, Olga Krishtal wrote:
In case of ploop volume, target path of the volume is the path to the directory that contains image file named root.hds and DiskDescriptor.xml. While using uploadVol and downloadVol callbacks we need to open root.hds itself. Upload or download operations with ploop volume are only allowed when images do not have snapshots. Otherwise operation fails.
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/storage/storage_backend.c | 99 +++++++++++++++++++++++++++++++++++++++++-- src/storage/storage_driver.c | 51 +++++++++++++++++++++- 2 files changed, 146 insertions(+), 4 deletions(-)
ACK
+static int +virStorageBackendPloopHasSnapshots(char *path) +{ + virCommandPtr cmd = NULL; + char *output = NULL; + char *snap_tool = NULL; + int ret = -1; + + snap_tool = virFindFileInPath("ploop"); + if (!snap_tool) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("can't upload volume," + " please install ploop tool "));
This error message has a trailing space so I replaced it with the one used by other calls: - "%s", _("can't upload volume," - " please install ploop tool ")); + "%s", _("unable to find ploop, please install " + "ploop tools"));
+ return ret; + } + + cmd = virCommandNewArgList(snap_tool, "snapshot-list", NULL); + virCommandAddArgFormat(cmd, "%s/DiskDescriptor.xml", path); + virCommandSetOutputBuffer(cmd, &output); + + if ((ret = virCommandRun(cmd, NULL)) < 0) + goto cleanup; + + if (!strstr(output, "root.hds.")) { + ret = 1;
@@ -2296,7 +2343,6 @@ storageVolUpload(virStorageVolPtr obj, vol->name); goto cleanup; } -
Unrelated whitespace change.
if (!backend->uploadVol) { virReportError(VIR_ERR_NO_SUPPORT, "%s", _("storage pool doesn't support volume upload")); @@ -2313,6 +2359,9 @@ storageVolUpload(virStorageVolPtr obj, if (VIR_ALLOC(cbdata) < 0 || VIR_STRDUP(cbdata->pool_name, pool->def->name) < 0) goto cleanup; + if (vol->target.type == VIR_STORAGE_VOL_PLOOP && + VIR_STRDUP(cbdata->vol_path, vol->target.path) < 0) + goto cleanup;
The indentation is off here. Jan

On Mon, Apr 11, 2016 at 07:16:18PM +0300, Olga Krishtal wrote:
This series of patches introduces the support of ploop volumes in storage pools (dir, fs, etc). Ploop volume is a disk loopback block device consists of root.hds (it is the image file) and DiskDescriptor.xml: https://openvz.org/Ploop/format. Due to this fact it can't be treated as file or any other volume type, moreover, in order to successfully manipulate with such volume, ploop tools should be installed.
All callbacks except the wipeVol are supported. First patch introduces ploop volume type. This is a directory with two files inside. The name of the directory is the name of the volume. Patch 2 deals with creating an empty volume and cloning the existing one. Clone is done via simle cp operation. If any of this operation fails - directory will be deleted. Patch 3 deletes recursively ploop directory. Patch 4 uses ploop tool to resize volume. Patch 6 adapts all refreshing functions to work with ploop. To get information the directory is checked. The volume is treated as the ploops one only if it contains ploop files. Every time the pool is refreshed DiskDescriptor.xml is restored. This is necessary, because the content of the volume may have changed. Upload and download (patch 7) can't be done if the volume contains snapshots.
I have squashed in the changes mentioned in the individual patch reviews and pushed the series. Jan
participants (2)
-
Ján Tomko
-
Olga Krishtal