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

In-Reply-To: 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.

In order to support ploop in storage pools we need separate volume type: VIR_STORAGE_VOL_PLOOP Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- include/libvirt/libvirt-storage.h | 2 +- src/conf/storage_conf.c | 2 +- src/storage/storage_backend_fs.c | 3 +++ tools/virsh-volume.c | 3 ++- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/include/libvirt/libvirt-storage.h b/include/libvirt/libvirt-storage.h index 1a868cc..b1e6ba5 100644 --- a/include/libvirt/libvirt-storage.h +++ b/include/libvirt/libvirt-storage.h @@ -122,7 +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 # endif diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 3657dfd..01b52a4 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -52,7 +52,7 @@ 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..dd0d80e 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; @@ -1243,6 +1245,7 @@ virStorageBackendFileSystemVolDelete(virConnectPtr conn ATTRIBUTE_UNUSED, switch ((virStorageVolType) vol->type) { case VIR_STORAGE_VOL_FILE: case VIR_STORAGE_VOL_DIR: + case VIR_STORAGE_VOL_PLOOP: if (virFileRemove(vol->target.path, vol->target.perms->uid, vol->target.perms->gid) < 0) { /* Silently ignore failures where the vol has already gone away */ diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 35f0cbd..1f4b6b6 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -956,7 +956,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

08.02.2016 16:04, Olga Krishtal пишет:
In order to support ploop in storage pools we need separate volume type: VIR_STORAGE_VOL_PLOOP
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- include/libvirt/libvirt-storage.h | 2 +- src/conf/storage_conf.c | 2 +- src/storage/storage_backend_fs.c | 3 +++ tools/virsh-volume.c | 3 ++- 4 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/include/libvirt/libvirt-storage.h b/include/libvirt/libvirt-storage.h index 1a868cc..b1e6ba5 100644 --- a/include/libvirt/libvirt-storage.h +++ b/include/libvirt/libvirt-storage.h @@ -122,7 +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 # endif diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 3657dfd..01b52a4 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -52,7 +52,7 @@ 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..dd0d80e 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;
@@ -1243,6 +1245,7 @@ virStorageBackendFileSystemVolDelete(virConnectPtr conn ATTRIBUTE_UNUSED, switch ((virStorageVolType) vol->type) { case VIR_STORAGE_VOL_FILE: case VIR_STORAGE_VOL_DIR: + case VIR_STORAGE_VOL_PLOOP:
since you delete this chunk in the third patch, I don't think it's necessary here
if (virFileRemove(vol->target.path, vol->target.perms->uid, vol->target.perms->gid) < 0) { /* Silently ignore failures where the vol has already gone away */ diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 35f0cbd..1f4b6b6 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -956,7 +956,8 @@ VIR_ENUM_IMPL(virshStorageVol, N_("block"), N_("dir"), N_("network"), - N_("netdir")) + N_("netdir"), + N_("ploop"))
static const char * virshVolumeTypeToString(int type)

These callbacks let us to create ploop volumes in directory 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 the same as input volume. Ploop volume consists of ploop image file and a corresponding DiskDescriptor.xml file. Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/storage/storage_backend.c | 90 ++++++++++++++++++++++++++++++++++++++++ src/storage/storage_backend.h | 8 ++++ src/storage/storage_backend_fs.c | 2 + 3 files changed, 100 insertions(+) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index c07b642..6bd4618 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -773,6 +773,94 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool, return ret; } +/* Set up directory for ploop volume. If function fails, + * volume won't be created. + */ + +static int +virVolCreateDirForPloop(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, + virStorageVolDefPtr vol) +{ + int err; + 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, + 0)) < 0) { + return -1; + } + return 0; +} + +int virStorageBackendCreatePloop(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + virStorageVolDefPtr inputvol, + unsigned int flags) +{ + int ret = 0; + char *size = NULL; + char *path = NULL; + virCommandPtr cmd = NULL; + + virCheckFlags(0, -1); + if (vol->target.format != VIR_STORAGE_FILE_PLOOP) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported storage vol type %d"), + vol->target.format); + return -1; + } + if (inputvol && inputvol->target.format != VIR_STORAGE_FILE_PLOOP) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported input storage vol type %d"), + inputvol->target.format); + return -1; + } + + if (vol->target.encryption != NULL) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("encrypted ploop volumes are not supported with")); + return -1; + } + + if (vol->target.backingStore != NULL) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("copy-on-write ploop is not yet supported")); + return -1; + } + + if (!inputvol) { + if (virVolCreateDirForPloop(pool, vol) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("error creating directory for ploop " + "ploop init")); + return -1; + } + if (virAsprintf(&path, "%s/%s", vol->target.path, "root.hds") < 0) + return -1; + + if (virAsprintf(&size, "%lluM", VIR_DIV_UP(vol->target.capacity, (1024*1024))) < 0) { + ret = -1; + goto cleanup; + } + + cmd = virCommandNewArgList("ploop", "init", "-s", size, "-t", "ext4", path, NULL); + } 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(size); + VIR_FREE(path); + return ret; +} + enum { QEMU_IMG_BACKING_FORMAT_NONE = 0, QEMU_IMG_BACKING_FORMAT_FLAG, @@ -1293,6 +1381,8 @@ virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol, if (vol->type == VIR_STORAGE_VOL_BLOCK) return virStorageBackendCreateBlockFrom; + if (vol->type == VIR_STORAGE_VOL_PLOOP) + return virStorageBackendCreatePloop; else return virStorageBackendCreateRaw; } 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 dd0d80e..56f6ef7 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

08.02.2016 16:04, Olga Krishtal пишет:
These callbacks let us to create ploop volumes in directory 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 the same as input volume.
Ploop volume consists of ploop image file and a corresponding DiskDescriptor.xml file.
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/storage/storage_backend.c | 90 ++++++++++++++++++++++++++++++++++++++++ src/storage/storage_backend.h | 8 ++++ src/storage/storage_backend_fs.c | 2 + 3 files changed, 100 insertions(+)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index c07b642..6bd4618 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -773,6 +773,94 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool, return ret; }
+/* Set up directory for ploop volume. If function fails, + * volume won't be created. + */ + +static int +virVolCreateDirForPloop(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, + virStorageVolDefPtr vol) +{ + int err; + 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, + 0)) < 0) { + return -1; + } + return 0; +} + +int virStorageBackendCreatePloop(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + virStorageVolDefPtr inputvol, + unsigned int flags) +{ + int ret = 0; + char *size = NULL; + char *path = NULL; + virCommandPtr cmd = NULL; + + virCheckFlags(0, -1); + if (vol->target.format != VIR_STORAGE_FILE_PLOOP) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported storage vol type %d"), + vol->target.format); + return -1; + } + if (inputvol && inputvol->target.format != VIR_STORAGE_FILE_PLOOP) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported input storage vol type %d"), + inputvol->target.format); + return -1; + } + + if (vol->target.encryption != NULL) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("encrypted ploop volumes are not supported with"));
looks like an uncompleted phrase
+ return -1; + } + + if (vol->target.backingStore != NULL) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("copy-on-write ploop is not yet supported"));
s/ploop is not/ploop volumes are not
+ return -1; + } + + if (!inputvol) { + if (virVolCreateDirForPloop(pool, vol) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("error creating directory for ploop " + "ploop init"));
s/ploop ploop init/ploop volume
+ return -1; + } + if (virAsprintf(&path, "%s/%s", vol->target.path, "root.hds") < 0) + return -1; + + if (virAsprintf(&size, "%lluM", VIR_DIV_UP(vol->target.capacity, (1024*1024))) < 0) { + ret = -1; + goto cleanup; + } + + cmd = virCommandNewArgList("ploop", "init", "-s", size, "-t", "ext4", path, NULL); + } 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(size); + VIR_FREE(path); + return ret; +} + enum { QEMU_IMG_BACKING_FORMAT_NONE = 0, QEMU_IMG_BACKING_FORMAT_FLAG, @@ -1293,6 +1381,8 @@ virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol,
if (vol->type == VIR_STORAGE_VOL_BLOCK) return virStorageBackendCreateBlockFrom; + if (vol->type == VIR_STORAGE_VOL_PLOOP) + return virStorageBackendCreatePloop; else return virStorageBackendCreateRaw; } 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 dd0d80e..56f6ef7 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);

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.c | 6 ++++++ src/storage/storage_backend.h | 2 ++ src/storage/storage_backend_fs.c | 5 ++++- 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 6bd4618..be57630 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -861,6 +861,12 @@ int virStorageBackendCreatePloop(virConnectPtr conn ATTRIBUTE_UNUSED, return ret; } +int virStorageBackendDeletePloop(virConnectPtr conn ATTRIBUTE_UNUSED, + virStorageVolDefPtr vol) +{ + return virFileDeleteTree(vol->target.path); +} + 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..7d354c5 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 virStorageBackendDeletePloop(virConnectPtr conn, + virStorageVolDefPtr vol); virStorageBackendBuildVolFrom virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol, diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 56f6ef7..0df1880 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1247,7 +1247,6 @@ virStorageBackendFileSystemVolDelete(virConnectPtr conn ATTRIBUTE_UNUSED, switch ((virStorageVolType) vol->type) { case VIR_STORAGE_VOL_FILE: case VIR_STORAGE_VOL_DIR: - case VIR_STORAGE_VOL_PLOOP: if (virFileRemove(vol->target.path, vol->target.perms->uid, vol->target.perms->gid) < 0) { /* Silently ignore failures where the vol has already gone away */ @@ -1264,6 +1263,10 @@ virStorageBackendFileSystemVolDelete(virConnectPtr conn ATTRIBUTE_UNUSED, } } break; + case VIR_STORAGE_VOL_PLOOP: + if (virStorageBackendDeletePloop(conn, vol) < 0) + return -1; + break; case VIR_STORAGE_VOL_BLOCK: case VIR_STORAGE_VOL_NETWORK: case VIR_STORAGE_VOL_NETDIR: -- 1.8.3.1

08.02.2016 16:04, Olga Krishtal пишет:
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.c | 6 ++++++ src/storage/storage_backend.h | 2 ++ src/storage/storage_backend_fs.c | 5 ++++- 3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 6bd4618..be57630 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -861,6 +861,12 @@ int virStorageBackendCreatePloop(virConnectPtr conn ATTRIBUTE_UNUSED, return ret; }
+int virStorageBackendDeletePloop(virConnectPtr conn ATTRIBUTE_UNUSED, + virStorageVolDefPtr vol) +{ + return virFileDeleteTree(vol->target.path); +} + 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..7d354c5 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 virStorageBackendDeletePloop(virConnectPtr conn, + virStorageVolDefPtr vol);
virStorageBackendBuildVolFrom virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol, diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 56f6ef7..0df1880 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1247,7 +1247,6 @@ virStorageBackendFileSystemVolDelete(virConnectPtr conn ATTRIBUTE_UNUSED, switch ((virStorageVolType) vol->type) { case VIR_STORAGE_VOL_FILE: case VIR_STORAGE_VOL_DIR: - case VIR_STORAGE_VOL_PLOOP:
This chunk is unnecessary if you remove related one from patch #1
if (virFileRemove(vol->target.path, vol->target.perms->uid, vol->target.perms->gid) < 0) { /* Silently ignore failures where the vol has already gone away */ @@ -1264,6 +1263,10 @@ virStorageBackendFileSystemVolDelete(virConnectPtr conn ATTRIBUTE_UNUSED, } } break; + case VIR_STORAGE_VOL_PLOOP: + if (virStorageBackendDeletePloop(conn, vol) < 0) + return -1; + break; case VIR_STORAGE_VOL_BLOCK: case VIR_STORAGE_VOL_NETWORK: case VIR_STORAGE_VOL_NETDIR:

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. To accomplish this goal we must change path from path/to/ploop directory to path/to/ploop/root.hds Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/storage/storage_backend.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index be57630..f67953a 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -2016,12 +2016,23 @@ virStorageBackendVolUploadLocal(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long long len, unsigned int flags) { + char *path = NULL; + char *target_path = vol->target.path; + int ret; + virCheckFlags(0, -1); + if (vol->type == VIR_STORAGE_VOL_PLOOP) { + if (virAsprintf(&path, "%s/%s", vol->target.path, "root.hds") < 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, + ret = virFDStreamOpenBlockDevice(stream, target_path, offset, len, O_WRONLY); + VIR_FREE(path); + return ret; } int @@ -2033,10 +2044,20 @@ virStorageBackendVolDownloadLocal(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long long len, unsigned int flags) { + char *path = NULL; + char *target_path = vol->target.path; + int ret; virCheckFlags(0, -1); + if (vol->type == VIR_STORAGE_VOL_PLOOP) { + if (virAsprintf(&path, "%s/%s", vol->target.path, "root.hds") < 0) + return -1; + target_path = path; + } - return virFDStreamOpenBlockDevice(stream, vol->target.path, + ret = virFDStreamOpenBlockDevice(stream, target_path, offset, len, O_RDONLY); + VIR_FREE(path); + return ret; } -- 1.8.3.1

08.02.2016 16:04, Olga Krishtal пишет:
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. To accomplish this goal we must change path from path/to/ploop directory to path/to/ploop/root.hds ACK

To change the size of ploop image file we use ploop resize cmd that takes 2 args: new size and path/to/DiskDescriptor.xml Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/storage/storage_backend.c | 28 ++++++++++++++++++++++++++++ src/storage/storage_backend.h | 5 +++++ src/storage/storage_backend_fs.c | 3 +++ 3 files changed, 36 insertions(+) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index f67953a..38ea601 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -867,6 +867,34 @@ int virStorageBackendDeletePloop(virConnectPtr conn ATTRIBUTE_UNUSED, return virFileDeleteTree(vol->target.path); } +int virStoragePloopResize(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, + virStorageVolDefPtr vol, + unsigned long long capacity) +{ + int ret; + char *path = NULL; + char *size = NULL; + virCommandPtr cmd = NULL; + + if (virAsprintf(&path, "%s/%s", vol->target.path, "DiskDescriptor.xml") < 0) + return -1; + + if (virAsprintf(&size, "%lluM", VIR_DIV_UP(capacity, 512)) < 0) { + ret = -1; + goto cleanup; + } + cmd = virCommandNewArgList("ploop", "resize", "-s", size, path, NULL); + + ret = virCommandRun(cmd, NULL); + + cleanup: + virCommandFree(cmd); + VIR_FREE(path); + VIR_FREE(size); + 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 7d354c5..1de8dfe 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -118,6 +118,11 @@ int virStorageBackendCreatePloop(virConnectPtr conn, int virStorageBackendDeletePloop(virConnectPtr conn, virStorageVolDefPtr vol); +int virStoragePloopResize(virConnectPtr conn, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + unsigned long long capacity); + virStorageBackendBuildVolFrom virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol, virStorageVolDefPtr inputvol); diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 0df1880..2ec828b 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1379,6 +1379,9 @@ virStorageBackendFileSystemVolResize(virConnectPtr conn ATTRIBUTE_UNUSED, if (vol->target.format == VIR_STORAGE_FILE_RAW) { return virStorageFileResize(vol->target.path, capacity, vol->target.allocation, pre_allocate); + } + if (vol->target.format == VIR_STORAGE_FILE_PLOOP) { + return virStoragePloopResize(conn, pool, vol, capacity); } else { if (pre_allocate) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", -- 1.8.3.1

08.02.2016 16:04, Olga Krishtal пишет:
To change the size of ploop image file we use ploop resize cmd that takes 2 args: new size and path/to/DiskDescriptor.xml
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/storage/storage_backend.c | 28 ++++++++++++++++++++++++++++ src/storage/storage_backend.h | 5 +++++ src/storage/storage_backend_fs.c | 3 +++ 3 files changed, 36 insertions(+)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index f67953a..38ea601 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -867,6 +867,34 @@ int virStorageBackendDeletePloop(virConnectPtr conn ATTRIBUTE_UNUSED, return virFileDeleteTree(vol->target.path); }
+int virStoragePloopResize(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, + virStorageVolDefPtr vol, + unsigned long long capacity) +{ + int ret; + char *path = NULL; + char *size = NULL; + virCommandPtr cmd = NULL; + + if (virAsprintf(&path, "%s/%s", vol->target.path, "DiskDescriptor.xml") < 0) + return -1; + + if (virAsprintf(&size, "%lluM", VIR_DIV_UP(capacity, 512)) < 0) { + ret = -1; + goto cleanup; + } + cmd = virCommandNewArgList("ploop", "resize", "-s", size, path, NULL); + + ret = virCommandRun(cmd, NULL); + + cleanup: + virCommandFree(cmd); + VIR_FREE(path); + VIR_FREE(size); + 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 7d354c5..1de8dfe 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -118,6 +118,11 @@ int virStorageBackendCreatePloop(virConnectPtr conn, int virStorageBackendDeletePloop(virConnectPtr conn, virStorageVolDefPtr vol);
+int virStoragePloopResize(virConnectPtr conn, + virStoragePoolObjPtr pool, + virStorageVolDefPtr vol, + unsigned long long capacity); + virStorageBackendBuildVolFrom virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol, virStorageVolDefPtr inputvol); diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 0df1880..2ec828b 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1379,6 +1379,9 @@ virStorageBackendFileSystemVolResize(virConnectPtr conn ATTRIBUTE_UNUSED, if (vol->target.format == VIR_STORAGE_FILE_RAW) { return virStorageFileResize(vol->target.path, capacity, vol->target.allocation, pre_allocate); + } + if (vol->target.format == VIR_STORAGE_FILE_PLOOP) {
I would prefer seeing else if statement here, to make it clear that this is another case of the previous vol->target.format check
+ return virStoragePloopResize(conn, pool, vol, capacity); } else { if (pre_allocate) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",

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 38ea601..582bb5c 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -2207,6 +2207,12 @@ virStorageBackendVolWipeLocal(virConnectPtr conn ATTRIBUTE_UNUSED, VIR_DEBUG("Wiping volume with path '%s' and algorithm %u", vol->target.path, algorithm); + if (vol->type == VIR_STORAGE_VOL_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

To update information about ploop volumes inside the a single pool we need to be sure that it is the ploop directory and not some other directory. Ploop volume directory obligatory contains root.hds - image file and disk descriptor - DiskDescriptor.xml. If path to a volume is a path to some directory, we check the existance of this files. The capacity of a ploop volume is obtained via offset in the header file: https://openvz.org/Ploop/format Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/storage/storage_backend.c | 90 ++++++++++++++++++++++++++--------- src/storage/storage_backend.h | 2 +- src/storage/storage_backend_fs.c | 8 +++- src/storage/storage_backend_logical.c | 2 +- src/util/virstoragefile.c | 8 +++- 5 files changed, 83 insertions(+), 27 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 582bb5c..528e8e0 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1575,6 +1575,25 @@ virStorageBackendDetectBlockVolFormatFD(virStorageSourcePtr target, return 0; } +static bool virStorageBackendIsPloopDir(char *path) +{ + char *file = NULL; + bool ret = false; + + if (virAsprintf(&file, "%s/%s", path, "root.hds") < 0) + return ret; + if (!virFileExists(file)) + goto cleanup; + VIR_FREE(file); + if (virAsprintf(&file, "%s/%s", path, "DiskDescriptor.xml") < 0) + goto cleanup; + if (!virFileExists(file)) + goto cleanup; + ret = true; + cleanup: + VIR_FREE(file); + return ret; +} /* * Allows caller to silently ignore files with improper mode @@ -1583,29 +1602,35 @@ virStorageBackendDetectBlockVolFormatFD(virStorageSourcePtr target, * return -2 if file mode is unexpected or the volume is a dangling * symbolic link. */ + +#define FAILED_STAT(path, ret) { \ + if (errno == ENOENT) { \ + if (noerror) { \ + VIR_WARN("ignoring missing file '%s'", path);\ + ret = -2; \ + } \ + virReportError(VIR_ERR_NO_STORAGE_VOL, \ + _("no storage vol with matching path '%s'"), path); \ + ret = -1; \ + } \ + virReportSystemError(errno, \ + _("cannot stat file '%s'"), path);\ + ret = -1;\ +} + int -virStorageBackendVolOpen(const char *path, struct stat *sb, +virStorageBackendVolOpen(virStorageSourcePtr target, struct stat *sb, unsigned int flags) { - int fd, mode = 0; - char *base = last_component(path); + int fd, mode = 0, ret = 0; + char *base = last_component(target->path); bool noerror = (flags & VIR_STORAGE_VOL_OPEN_NOERROR); + char *path = target->path; + char *ploop_path = NULL; if (lstat(path, sb) < 0) { - if (errno == ENOENT) { - if (noerror) { - VIR_WARN("ignoring missing file '%s'", path); - return -2; - } - virReportError(VIR_ERR_NO_STORAGE_VOL, - _("no storage vol with matching path '%s'"), - path); - return -1; - } - virReportSystemError(errno, - _("cannot stat file '%s'"), - path); - return -1; + FAILED_STAT(path, ret); + return ret; } if (S_ISFIFO(sb->st_mode)) { @@ -1624,6 +1649,18 @@ virStorageBackendVolOpen(const char *path, struct stat *sb, virReportError(VIR_ERR_INTERNAL_ERROR, _("Volume path '%s' is a socket"), path); return -1; + } else if (S_ISDIR(sb->st_mode)) { + if (virStorageBackendIsPloopDir(path)) { + if (virAsprintf(&ploop_path, "%s/%s", target->path, "root.hds") < 0) + return -1; + path = ploop_path; + target->format = VIR_STORAGE_FILE_PLOOP; + if (lstat(path, sb) < 0) { + FAILED_STAT(path, ret); + VIR_FREE(ploop_path); + return ret; + } + } } /* O_NONBLOCK should only matter during open() for fifos and @@ -1720,6 +1757,7 @@ virStorageBackendVolOpen(const char *path, struct stat *sb, return -1; } + VIR_FREE(ploop_path); return fd; } @@ -1747,8 +1785,10 @@ virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target, virStorageSourcePtr meta = NULL; char *buf = NULL; ssize_t len = VIR_STORAGE_MAX_HEADER; + char *path = NULL; + char *target_path = target->path; - if ((ret = virStorageBackendVolOpen(target->path, &sb, openflags)) < 0) + if ((ret = virStorageBackendVolOpen(target, &sb, openflags)) < -1) goto cleanup; fd = ret; @@ -1763,7 +1803,7 @@ virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target, } if (lseek(fd, 0, SEEK_SET) == (off_t)-1) { - virReportSystemError(errno, _("cannot seek to start of '%s'"), target->path); + virReportSystemError(errno, _("cannot seek to start of '%s'"), target_path); ret = -1; goto cleanup; } @@ -1771,18 +1811,23 @@ virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target, if ((len = virFileReadHeaderFD(fd, len, &buf)) < 0) { if (readflags & VIR_STORAGE_VOL_READ_NOERROR) { VIR_WARN("ignoring failed header read for '%s'", - target->path); + target_path); ret = -2; } else { virReportSystemError(errno, _("cannot read header '%s'"), - target->path); + target_path); ret = -1; } goto cleanup; } - if (!(meta = virStorageFileGetMetadataFromBuf(target->path, buf, len, target->format, + if (target->format == VIR_STORAGE_FILE_PLOOP) { + if (virAsprintf(&path, "%s/%s", target->path, "root.hds") < 0) + return -1; + target_path = path; + } + if (!(meta = virStorageFileGetMetadataFromBuf(target_path, buf, len, target->format, NULL))) { ret = -1; goto cleanup; @@ -1802,6 +1847,7 @@ virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target, virStorageSourceFree(meta); VIR_FORCE_CLOSE(fd); VIR_FREE(buf); + VIR_FREE(path); return ret; } diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 1de8dfe..7dca559 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -206,7 +206,7 @@ enum { # define VIR_STORAGE_VOL_OPEN_DEFAULT (VIR_STORAGE_VOL_OPEN_REG |\ VIR_STORAGE_VOL_OPEN_BLOCK) -int virStorageBackendVolOpen(const char *path, struct stat *sb, +int virStorageBackendVolOpen(virStorageSourcePtr target, struct stat *sb, unsigned int flags) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 2ec828b..84e4c4a 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -75,7 +75,7 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, if (encryption) *encryption = NULL; - if ((rc = virStorageBackendVolOpen(target->path, &sb, + if ((rc = virStorageBackendVolOpen(target, &sb, VIR_STORAGE_VOL_FS_PROBE_FLAGS)) < 0) return rc; /* Take care to propagate rc, it is not always -1 */ fd = rc; @@ -83,6 +83,10 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, if (virStorageBackendUpdateVolTargetInfoFD(target, fd, &sb) < 0) goto cleanup; + if (target->format == VIR_STORAGE_FILE_PLOOP) { + ret = 0; + goto cleanup; + } if (S_ISDIR(sb.st_mode)) { target->format = VIR_STORAGE_FILE_DIR; ret = 0; @@ -948,6 +952,8 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED, /* directory based volume */ 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, diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index ba26223..aaace5b 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -955,7 +955,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, virCommandFree(cmd); cmd = NULL; - if ((fd = virStorageBackendVolOpen(vol->target.path, &sb, + if ((fd = virStorageBackendVolOpen(&vol->target, &sb, VIR_STORAGE_VOL_OPEN_DEFAULT)) < 0) goto error; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 101070f..d92da30 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -184,6 +184,10 @@ qedGetBackingStore(char **, int *, const char *, size_t); #define QED_F_BACKING_FILE 0x01 #define QED_F_BACKING_FORMAT_NO_PROBE 0x04 +/* Location of ploop image size in the header file * + * https://openvz.org/Ploop/format */ +#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 +240,8 @@ 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, "ploop", NULL, LV_LITTLE_ENDIAN, + -1, {0}, PLOOP_IMAGE_SIZE_OFFSET, 0, PLOOP_SIZE_MULTIPLIER, 0, NULL, NULL }, /* All formats with a backing store probe below here */ [VIR_STORAGE_FILE_COW] = { -- 1.8.3.1

08.02.2016 16:04, Olga Krishtal пишет:
To update information about ploop volumes inside the a single pool we need to be sure that it is the ploop directory and not some other directory. Ploop volume directory obligatory contains root.hds - image file and disk descriptor - DiskDescriptor.xml. If path to a volume is a path to some directory, we check the existance of this files.
The capacity of a ploop volume is obtained via offset in the header file: https://openvz.org/Ploop/format
ACK, except minor comment inline
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/storage/storage_backend.c | 90 ++++++++++++++++++++++++++--------- src/storage/storage_backend.h | 2 +- src/storage/storage_backend_fs.c | 8 +++- src/storage/storage_backend_logical.c | 2 +- src/util/virstoragefile.c | 8 +++- 5 files changed, 83 insertions(+), 27 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 582bb5c..528e8e0 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1575,6 +1575,25 @@ virStorageBackendDetectBlockVolFormatFD(virStorageSourcePtr target, return 0; }
+static bool virStorageBackendIsPloopDir(char *path) +{ + char *file = NULL; + bool ret = false; + + if (virAsprintf(&file, "%s/%s", path, "root.hds") < 0) + return ret; + if (!virFileExists(file)) + goto cleanup; + VIR_FREE(file); + if (virAsprintf(&file, "%s/%s", path, "DiskDescriptor.xml") < 0) + goto cleanup; + if (!virFileExists(file)) + goto cleanup; + ret = true; + cleanup: + VIR_FREE(file); + return ret; +}
/* * Allows caller to silently ignore files with improper mode @@ -1583,29 +1602,35 @@ virStorageBackendDetectBlockVolFormatFD(virStorageSourcePtr target, * return -2 if file mode is unexpected or the volume is a dangling * symbolic link. */ + +#define FAILED_STAT(path, ret) { \ + if (errno == ENOENT) { \ + if (noerror) { \ + VIR_WARN("ignoring missing file '%s'", path);\ + ret = -2; \ + } \ + virReportError(VIR_ERR_NO_STORAGE_VOL, \ + _("no storage vol with matching path '%s'"), path); \ + ret = -1; \ + } \ + virReportSystemError(errno, \ + _("cannot stat file '%s'"), path);\ + ret = -1;\ +} + int -virStorageBackendVolOpen(const char *path, struct stat *sb, +virStorageBackendVolOpen(virStorageSourcePtr target, struct stat *sb, unsigned int flags) { - int fd, mode = 0; - char *base = last_component(path); + int fd, mode = 0, ret = 0; + char *base = last_component(target->path); bool noerror = (flags & VIR_STORAGE_VOL_OPEN_NOERROR); + char *path = target->path; + char *ploop_path = NULL;
if (lstat(path, sb) < 0) { - if (errno == ENOENT) { - if (noerror) { - VIR_WARN("ignoring missing file '%s'", path); - return -2; - } - virReportError(VIR_ERR_NO_STORAGE_VOL, - _("no storage vol with matching path '%s'"), - path); - return -1; - } - virReportSystemError(errno, - _("cannot stat file '%s'"), - path); - return -1; + FAILED_STAT(path, ret); + return ret; }
if (S_ISFIFO(sb->st_mode)) { @@ -1624,6 +1649,18 @@ virStorageBackendVolOpen(const char *path, struct stat *sb, virReportError(VIR_ERR_INTERNAL_ERROR, _("Volume path '%s' is a socket"), path); return -1; + } else if (S_ISDIR(sb->st_mode)) { + if (virStorageBackendIsPloopDir(path)) { + if (virAsprintf(&ploop_path, "%s/%s", target->path, "root.hds") < 0) + return -1; + path = ploop_path; + target->format = VIR_STORAGE_FILE_PLOOP; + if (lstat(path, sb) < 0) { + FAILED_STAT(path, ret); + VIR_FREE(ploop_path); + return ret; + } + } }
/* O_NONBLOCK should only matter during open() for fifos and @@ -1720,6 +1757,7 @@ virStorageBackendVolOpen(const char *path, struct stat *sb, return -1; }
+ VIR_FREE(ploop_path); return fd; }
@@ -1747,8 +1785,10 @@ virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target, virStorageSourcePtr meta = NULL; char *buf = NULL; ssize_t len = VIR_STORAGE_MAX_HEADER; + char *path = NULL; + char *target_path = target->path;
- if ((ret = virStorageBackendVolOpen(target->path, &sb, openflags)) < 0) + if ((ret = virStorageBackendVolOpen(target, &sb, openflags)) < -1) goto cleanup; fd = ret;
@@ -1763,7 +1803,7 @@ virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target, }
if (lseek(fd, 0, SEEK_SET) == (off_t)-1) { - virReportSystemError(errno, _("cannot seek to start of '%s'"), target->path); + virReportSystemError(errno, _("cannot seek to start of '%s'"), target_path); ret = -1; goto cleanup; } @@ -1771,18 +1811,23 @@ virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target, if ((len = virFileReadHeaderFD(fd, len, &buf)) < 0) { if (readflags & VIR_STORAGE_VOL_READ_NOERROR) { VIR_WARN("ignoring failed header read for '%s'", - target->path); + target_path); ret = -2; } else { virReportSystemError(errno, _("cannot read header '%s'"), - target->path); + target_path);
I'm not sure these changes are necessary
ret = -1; } goto cleanup; }
- if (!(meta = virStorageFileGetMetadataFromBuf(target->path, buf, len, target->format, + if (target->format == VIR_STORAGE_FILE_PLOOP) { + if (virAsprintf(&path, "%s/%s", target->path, "root.hds") < 0) + return -1; + target_path = path; + } + if (!(meta = virStorageFileGetMetadataFromBuf(target_path, buf, len, target->format, NULL))) { ret = -1; goto cleanup; @@ -1802,6 +1847,7 @@ virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target, virStorageSourceFree(meta); VIR_FORCE_CLOSE(fd); VIR_FREE(buf); + VIR_FREE(path); return ret; }
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 1de8dfe..7dca559 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -206,7 +206,7 @@ enum { # define VIR_STORAGE_VOL_OPEN_DEFAULT (VIR_STORAGE_VOL_OPEN_REG |\ VIR_STORAGE_VOL_OPEN_BLOCK)
-int virStorageBackendVolOpen(const char *path, struct stat *sb, +int virStorageBackendVolOpen(virStorageSourcePtr target, struct stat *sb, unsigned int flags) ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 2ec828b..84e4c4a 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -75,7 +75,7 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, if (encryption) *encryption = NULL;
- if ((rc = virStorageBackendVolOpen(target->path, &sb, + if ((rc = virStorageBackendVolOpen(target, &sb, VIR_STORAGE_VOL_FS_PROBE_FLAGS)) < 0) return rc; /* Take care to propagate rc, it is not always -1 */ fd = rc; @@ -83,6 +83,10 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, if (virStorageBackendUpdateVolTargetInfoFD(target, fd, &sb) < 0) goto cleanup;
+ if (target->format == VIR_STORAGE_FILE_PLOOP) { + ret = 0; + goto cleanup; + } if (S_ISDIR(sb.st_mode)) { target->format = VIR_STORAGE_FILE_DIR; ret = 0; @@ -948,6 +952,8 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED, /* directory based volume */ 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, diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index ba26223..aaace5b 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -955,7 +955,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, virCommandFree(cmd); cmd = NULL;
- if ((fd = virStorageBackendVolOpen(vol->target.path, &sb, + if ((fd = virStorageBackendVolOpen(&vol->target, &sb, VIR_STORAGE_VOL_OPEN_DEFAULT)) < 0) goto error;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 101070f..d92da30 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -184,6 +184,10 @@ qedGetBackingStore(char **, int *, const char *, size_t); #define QED_F_BACKING_FILE 0x01 #define QED_F_BACKING_FORMAT_NO_PROBE 0x04
+/* Location of ploop image size in the header file * + * https://openvz.org/Ploop/format */ +#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 +240,8 @@ 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, "ploop", NULL, LV_LITTLE_ENDIAN, + -1, {0}, PLOOP_IMAGE_SIZE_OFFSET, 0, PLOOP_SIZE_MULTIPLIER, 0, NULL, NULL },
/* All formats with a backing store probe below here */ [VIR_STORAGE_FILE_COW] = {

08.02.2016 16:04, Olga Krishtal пишет:
In-Reply-To:
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.
Overall looks good, except minor issues in some patches. Also, I wonder if we should add some check to ensure ploop binary existence? This would make user experience much better. For instance, instead of unconditionally returning virStorageBackendCreatePloop in case of ploop volumes in virStorageBackendGetBuildVolFromFunction we could check for ploop external tool and report an error message that it wasn't detected and it is required to be installed for PLOOP storage pools. Maxim Nestratov

08.02.2016 20:45, Maxim Nestratov пишет:
08.02.2016 16:04, Olga Krishtal пишет:
In-Reply-To:
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.
Overall looks good, except minor issues in some patches. Also, I wonder if we should add some check to ensure ploop binary existence? This would make user experience much better. For instance, instead of unconditionally returning virStorageBackendCreatePloop in case of ploop volumes in virStorageBackendGetBuildVolFromFunction we could check for ploop external tool and report an error message that it wasn't detected and it is required to be installed for PLOOP storage pools.
Maxim Nestratov
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Hmm. I thought over the idea of introducing a new storage pool type PLOOP and I'm not sure that it is good. Actually it makes existing storage pools like Directory unusable for ploop format without any strong reason. Thus, I would tend to ask you to change the approach of introducing a new pool storage type to using existing ones.
participants (2)
-
Maxim Nestratov
-
Olga Krishtal