[libvirt] [PATCH rfc v4 0/6] storage:dir: ploop volumes support
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.
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/storage/storage_backend.c | 3 +-- src/storage/storage_backend.h | 3 +-- src/storage/storage_backend_fs.c | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index d97ad22..dc95036 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -870,8 +870,7 @@ int virStorageBackendCreatePloop(virConnectPtr conn ATTRIBUTE_UNUSED, return ret; } -int virStorageBackendDeletePloop(virConnectPtr conn ATTRIBUTE_UNUSED, - virStorageVolDefPtr vol) +int virStorageBackendDeletePloop(virStorageVolDefPtr vol) { return virFileDeleteTree(vol->target.path); } diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index b99671a..de48012 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -115,8 +115,7 @@ int virStorageBackendCreatePloop(virConnectPtr conn, virStorageVolDefPtr inputvol, unsigned int flags); -int virStorageBackendDeletePloop(virConnectPtr conn, - virStorageVolDefPtr vol); +int virStorageBackendDeletePloop(virStorageVolDefPtr vol); int virStoragePloopResize(virStorageVolDefPtr vol, unsigned long long capacity); diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 9d934cc..f978b6e 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1250,7 +1250,7 @@ virStorageBackendFileSystemVolDelete(virConnectPtr conn ATTRIBUTE_UNUSED, case VIR_STORAGE_VOL_FILE: case VIR_STORAGE_VOL_DIR: if (vol->target.format == VIR_STORAGE_FILE_PLOOP) { - if (virStorageBackendDeletePloop(conn, vol) < 0) + if (virStorageBackendDeletePloop(vol) < 0) return -1; break; } -- 1.8.3.1
On 17/02/16 14:39, Olga Krishtal wrote:
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/storage/storage_backend.c | 3 +-- src/storage/storage_backend.h | 3 +-- src/storage/storage_backend_fs.c | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index d97ad22..dc95036 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -870,8 +870,7 @@ int virStorageBackendCreatePloop(virConnectPtr conn ATTRIBUTE_UNUSED, return ret; }
-int virStorageBackendDeletePloop(virConnectPtr conn ATTRIBUTE_UNUSED, - virStorageVolDefPtr vol) +int virStorageBackendDeletePloop(virStorageVolDefPtr vol) { return virFileDeleteTree(vol->target.path); } diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index b99671a..de48012 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -115,8 +115,7 @@ int virStorageBackendCreatePloop(virConnectPtr conn, virStorageVolDefPtr inputvol, unsigned int flags);
-int virStorageBackendDeletePloop(virConnectPtr conn, - virStorageVolDefPtr vol); +int virStorageBackendDeletePloop(virStorageVolDefPtr vol);
int virStoragePloopResize(virStorageVolDefPtr vol, unsigned long long capacity); diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 9d934cc..f978b6e 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1250,7 +1250,7 @@ virStorageBackendFileSystemVolDelete(virConnectPtr conn ATTRIBUTE_UNUSED, case VIR_STORAGE_VOL_FILE: case VIR_STORAGE_VOL_DIR: if (vol->target.format == VIR_STORAGE_FILE_PLOOP) { - if (virStorageBackendDeletePloop(conn, vol) < 0) + if (virStorageBackendDeletePloop(vol) < 0) return -1; break; } Pls, disregard this patch.
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 | 100 ++++++++++++++++++++++++++++++++++++++- src/storage/storage_backend.h | 8 ++++ src/storage/storage_backend_fs.c | 2 + 3 files changed, 109 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index c07b642..d70642e 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -773,6 +773,103 @@ 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 = -1; + char *size = NULL; + char *path = NULL; + virCommandPtr cmd = NULL; + char *create_tool = NULL; + + virCheckFlags(0, -1); + + create_tool = virFindFileInPath("ploop"); + if (!create_tool && !inputvol) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("unable to find ploop, please install " + "ploop tools")); + 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 " + "ploop init")); + return -1; + } + + if (vol->target.backingStore != NULL) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("copy-on-write ploop volumes are not yet supported")); + return -1; + } + + if (!inputvol) { + if (virVolCreateDirForPloop(pool, vol) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("error creating directory for ploop volume")); + return -1; + } + if (virAsprintf(&path, "%s/root.hds", vol->target.path) < 0) + return -1; + + if (virAsprintf(&size, "%lluM", VIR_DIV_UP(vol->target.capacity, + (1024 * 1024))) < 0) { + goto cleanup; + } + + cmd = virCommandNewArgList(create_tool, "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); + if (ret < 0) + virFileDeleteTree(vol->target.path); + + cleanup: + + virCommandFree(cmd); + VIR_FREE(size); + VIR_FREE(path); + VIR_FREE(create_tool); + return ret; +} + enum { QEMU_IMG_BACKING_FORMAT_NONE = 0, QEMU_IMG_BACKING_FORMAT_FLAG, @@ -1280,7 +1377,8 @@ virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol, vol->target.format != VIR_STORAGE_FILE_RAW) || (inputvol->type == VIR_STORAGE_VOL_FILE && inputvol->target.format != VIR_STORAGE_FILE_RAW)) { - + if (vol->target.format == VIR_STORAGE_FILE_PLOOP) + return virStorageBackendCreatePloop; if ((tool_type = virStorageBackendFindFSImageTool(NULL)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("creation of non-raw file images is " 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 692c9ff..80c7e9e 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1177,6 +1177,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 17.02.2016 14:40, Olga Krishtal wrote:
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 | 100 ++++++++++++++++++++++++++++++++++++++- src/storage/storage_backend.h | 8 ++++ src/storage/storage_backend_fs.c | 2 + 3 files changed, 109 insertions(+), 1 deletion(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index c07b642..d70642e 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -773,6 +773,103 @@ 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,
you need to change indent back to function level from ternary operator
+ 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 = -1; + char *size = NULL; + char *path = NULL; + virCommandPtr cmd = NULL; + char *create_tool = NULL; + + virCheckFlags(0, -1); + + create_tool = virFindFileInPath("ploop"); + if (!create_tool && !inputvol) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("unable to find ploop, please install " + "ploop tools")); + 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 " + "ploop init")); + return -1; + } + + if (vol->target.backingStore != NULL) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("copy-on-write ploop volumes are not yet supported")); + return -1; + } + + if (!inputvol) { + if (virVolCreateDirForPloop(pool, vol) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("error creating directory for ploop volume"));
indentation
+ return -1; + } + if (virAsprintf(&path, "%s/root.hds", vol->target.path) < 0) + return -1;
you don't remove created dir on this return path
+ + if (virAsprintf(&size, "%lluM", VIR_DIV_UP(vol->target.capacity, + (1024 * 1024))) < 0) { + goto cleanup; + }
use virCommandAddArgFormat instead of series of virAsprintf
+ + cmd = virCommandNewArgList(create_tool, "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);
if target directory exists you will not get what you want. I think best would be to create dir before branching.
+ } + ret = virCommandRun(cmd, NULL); + if (ret < 0) + virFileDeleteTree(vol->target.path); + + cleanup: + + virCommandFree(cmd); + VIR_FREE(size); + VIR_FREE(path); + VIR_FREE(create_tool); + return ret; +} + enum { QEMU_IMG_BACKING_FORMAT_NONE = 0, QEMU_IMG_BACKING_FORMAT_FLAG, @@ -1280,7 +1377,8 @@ virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol, vol->target.format != VIR_STORAGE_FILE_RAW) || (inputvol->type == VIR_STORAGE_VOL_FILE && inputvol->target.format != VIR_STORAGE_FILE_RAW)) { - + if (vol->target.format == VIR_STORAGE_FILE_PLOOP) + return virStorageBackendCreatePloop; if ((tool_type = virStorageBackendFindFSImageTool(NULL)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("creation of non-raw file images is " 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 692c9ff..80c7e9e 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1177,6 +1177,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 | 5 +++++ src/storage/storage_backend.h | 1 + src/storage/storage_backend_fs.c | 5 +++++ 3 files changed, 11 insertions(+) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index d70642e..9f0e020 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -870,6 +870,11 @@ int virStorageBackendCreatePloop(virConnectPtr conn ATTRIBUTE_UNUSED, return ret; } +int virStorageBackendDeletePloop(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..3529755 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -115,6 +115,7 @@ int virStorageBackendCreatePloop(virConnectPtr conn, virStorageVolDefPtr inputvol, unsigned int flags); +int virStorageBackendDeletePloop(virStorageVolDefPtr vol); virStorageBackendBuildVolFrom virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol, diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 80c7e9e..f494fd2 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1245,6 +1245,11 @@ virStorageBackendFileSystemVolDelete(virConnectPtr conn ATTRIBUTE_UNUSED, switch ((virStorageVolType) vol->type) { case VIR_STORAGE_VOL_FILE: case VIR_STORAGE_VOL_DIR: + if (vol->target.format == VIR_STORAGE_FILE_PLOOP) { + if (virStorageBackendDeletePloop(vol) < 0) + return -1; + break; + } if (virFileRemove(vol->target.path, vol->target.perms->uid, vol->target.perms->gid) < 0) { /* Silently ignore failures where the vol has already gone away */ -- 1.8.3.1
On 17.02.2016 14:40, Olga Krishtal wrote:
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 | 5 +++++ src/storage/storage_backend.h | 1 + src/storage/storage_backend_fs.c | 5 +++++ 3 files changed, 11 insertions(+)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index d70642e..9f0e020 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -870,6 +870,11 @@ int virStorageBackendCreatePloop(virConnectPtr conn ATTRIBUTE_UNUSED, return ret; }
+int virStorageBackendDeletePloop(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..3529755 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -115,6 +115,7 @@ int virStorageBackendCreatePloop(virConnectPtr conn, virStorageVolDefPtr inputvol, unsigned int flags);
+int virStorageBackendDeletePloop(virStorageVolDefPtr vol);
virStorageBackendBuildVolFrom virStorageBackendGetBuildVolFromFunction(virStorageVolDefPtr vol, diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 80c7e9e..f494fd2 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1245,6 +1245,11 @@ virStorageBackendFileSystemVolDelete(virConnectPtr conn ATTRIBUTE_UNUSED, switch ((virStorageVolType) vol->type) { case VIR_STORAGE_VOL_FILE: case VIR_STORAGE_VOL_DIR: + if (vol->target.format == VIR_STORAGE_FILE_PLOOP) { + if (virStorageBackendDeletePloop(vol) < 0) + return -1; + break; + }
Let's get rid of function to be consistent with inplace virFileRemove below.
if (virFileRemove(vol->target.path, vol->target.perms->uid, vol->target.perms->gid) < 0) { /* Silently ignore failures where the vol has already gone away */
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 In case of .uploadVol, we have to additionaly update DiskDescriptor.xml Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/storage/storage_backend.c | 66 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 9f0e020..ac44fdf 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -2023,12 +2023,63 @@ virStorageBackendVolUploadLocal(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long long len, unsigned int flags) { + char *path = NULL; + char *target_path = vol->target.path; + int ret; + virCommandPtr cmd = NULL; + char *create_tool = NULL; + virCheckFlags(0, -1); /* Not using O_CREAT because the file is required to already exist at * this point */ - return virFDStreamOpenBlockDevice(stream, vol->target.path, + if (vol->target.format != VIR_STORAGE_FILE_PLOOP) { + return virFDStreamOpenBlockDevice(stream, target_path, offset, len, O_WRONLY); + } else { + if (virAsprintf(&path, "%s/root.hds", vol->target.path) < 0) + return -1; + target_path = path; + ret = virFDStreamOpenBlockDevice(stream, target_path, + offset, len, O_WRONLY); + if (ret < 0) { + goto cleanup; + } + + create_tool = virFindFileInPath("ploop"); + if (!create_tool) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("unable to find ploop, please install " + "ploop tools")); + ret = -1; + goto cleanup; + } + cmd = virCommandNewArgList(create_tool, "restore-descriptor", + vol->target.path, target_path, NULL); + VIR_FREE(path); + if (virAsprintf(&path, "%s/DiskDescriptor.xml", + vol->target.path) < 0) { + ret = -1; + goto cleanup; + } + if (virFileExists(path)) { + if (virFileRemove(path, 0, 0) < 0) { + ret = -1; + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("unable " + "to delete existing DiskDescriptor.xml")); + goto cleanup; + } + } + if (virCommandRun(cmd, NULL) < 0) { + ret = -1; + goto cleanup; + } + } + + cleanup: + VIR_FREE(cmd); + VIR_FREE(path); + return ret; } int @@ -2040,10 +2091,21 @@ 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->target.format == VIR_STORAGE_FILE_PLOOP) { + if (virAsprintf(&path, "%s/root.hds", vol->target.path) < 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
On 17.02.2016 14:40, 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. To accomplish this goal we must change path from path/to/ploop directory to path/to/ploop/root.hds
In case of .uploadVol, we have to additionaly update DiskDescriptor.xml
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/storage/storage_backend.c | 66 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 2 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 9f0e020..ac44fdf 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -2023,12 +2023,63 @@ virStorageBackendVolUploadLocal(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long long len, unsigned int flags) { + char *path = NULL; + char *target_path = vol->target.path; + int ret; + virCommandPtr cmd = NULL; + char *create_tool = NULL; + virCheckFlags(0, -1);
/* Not using O_CREAT because the file is required to already exist at * this point */ - return virFDStreamOpenBlockDevice(stream, vol->target.path, + if (vol->target.format != VIR_STORAGE_FILE_PLOOP) { + return virFDStreamOpenBlockDevice(stream, target_path, offset, len, O_WRONLY); + } else { + if (virAsprintf(&path, "%s/root.hds", vol->target.path) < 0) + return -1; + target_path = path; + ret = virFDStreamOpenBlockDevice(stream, target_path, + offset, len, O_WRONLY); + if (ret < 0) { + goto cleanup; + } + + create_tool = virFindFileInPath("ploop"); + if (!create_tool) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("unable to find ploop, please install " + "ploop tools")); + ret = -1; + goto cleanup; + } + cmd = virCommandNewArgList(create_tool, "restore-descriptor", + vol->target.path, target_path, NULL); + VIR_FREE(path); + if (virAsprintf(&path, "%s/DiskDescriptor.xml", + vol->target.path) < 0) { + ret = -1; + goto cleanup; + } + if (virFileExists(path)) { + if (virFileRemove(path, 0, 0) < 0) { + ret = -1; + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("unable " + "to delete existing DiskDescriptor.xml")); + goto cleanup; + } + } + if (virCommandRun(cmd, NULL) < 0) { + ret = -1; + goto cleanup; + }
Unfortunately we can't restore descriptor at this moment as wee only create stream here. Later someone will write data into the stream and only after stream is closed we can update our structures.
+ } + + cleanup: + VIR_FREE(cmd); + VIR_FREE(path); + return ret; }
int @@ -2040,10 +2091,21 @@ 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->target.format == VIR_STORAGE_FILE_PLOOP) { + if (virAsprintf(&path, "%s/root.hds", vol->target.path) < 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; }
On Wed, Feb 17, 2016 at 02:40:02PM +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. To accomplish this goal we must change path from path/to/ploop directory to path/to/ploop/root.hds
In case of .uploadVol, we have to additionaly update DiskDescriptor.xml
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/storage/storage_backend.c | 66 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 2 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 9f0e020..ac44fdf 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -2023,12 +2023,63 @@ virStorageBackendVolUploadLocal(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long long len, unsigned int flags) { + char *path = NULL; + char *target_path = vol->target.path; + int ret; + virCommandPtr cmd = NULL; + char *create_tool = NULL; + virCheckFlags(0, -1);
/* Not using O_CREAT because the file is required to already exist at * this point */ - return virFDStreamOpenBlockDevice(stream, vol->target.path, + if (vol->target.format != VIR_STORAGE_FILE_PLOOP) { + return virFDStreamOpenBlockDevice(stream, target_path, offset, len, O_WRONLY); + } else {
Looking at the last patch, it seems a volume could be detected as VIR_STORAGE_FILE_PLOOP if it's a disk image matching the magic, but this code assumes it's a directory with "root.hds" image and the XML.
+ if (virAsprintf(&path, "%s/root.hds", vol->target.path) < 0) + return -1;
I thought the target.path was already pointing to the image, not directory. Jan
On 18/02/16 16:57, Ján Tomko wrote:
On Wed, Feb 17, 2016 at 02:40:02PM +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. To accomplish this goal we must change path from path/to/ploop directory to path/to/ploop/root.hds
In case of .uploadVol, we have to additionaly update DiskDescriptor.xml
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/storage/storage_backend.c | 66 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 2 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 9f0e020..ac44fdf 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -2023,12 +2023,63 @@ virStorageBackendVolUploadLocal(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long long len, unsigned int flags) { + char *path = NULL; + char *target_path = vol->target.path; + int ret; + virCommandPtr cmd = NULL; + char *create_tool = NULL; + virCheckFlags(0, -1);
/* Not using O_CREAT because the file is required to already exist at * this point */ - return virFDStreamOpenBlockDevice(stream, vol->target.path, + if (vol->target.format != VIR_STORAGE_FILE_PLOOP) { + return virFDStreamOpenBlockDevice(stream, target_path, offset, len, O_WRONLY); + } else { Looking at the last patch, it seems a volume could be detected as VIR_STORAGE_FILE_PLOOP if it's a disk image matching the magic, but this code assumes it's a directory with "root.hds" image and the XML.
+ if (virAsprintf(&path, "%s/root.hds", vol->target.path) < 0) + return -1; I thought the target.path was already pointing to the image, not directory.
Jan The thing is that that volume target.path is path to folder with root.hds and DiskDescriptor.xml. And after resize we have to update DiskDescriptor.xml. This can be done only after volume will be fully upload, I mean after stream operations will be finished. Now I am looking for the correct place to do it. I think about refreshPool. But not quite sure yet.
On Thu, Feb 18, 2016 at 06:17:09PM +0300, Olga Krishtal wrote:
On 18/02/16 16:57, Ján Tomko wrote:
On Wed, Feb 17, 2016 at 02:40:02PM +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. To accomplish this goal we must change path from path/to/ploop directory to path/to/ploop/root.hds
In case of .uploadVol, we have to additionaly update DiskDescriptor.xml
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> --- src/storage/storage_backend.c | 66 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 64 insertions(+), 2 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 9f0e020..ac44fdf 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -2023,12 +2023,63 @@ virStorageBackendVolUploadLocal(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long long len, unsigned int flags) { + char *path = NULL; + char *target_path = vol->target.path; + int ret; + virCommandPtr cmd = NULL; + char *create_tool = NULL; + virCheckFlags(0, -1);
/* Not using O_CREAT because the file is required to already exist at * this point */ - return virFDStreamOpenBlockDevice(stream, vol->target.path, + if (vol->target.format != VIR_STORAGE_FILE_PLOOP) { + return virFDStreamOpenBlockDevice(stream, target_path, offset, len, O_WRONLY); + } else { Looking at the last patch, it seems a volume could be detected as VIR_STORAGE_FILE_PLOOP if it's a disk image matching the magic, but this code assumes it's a directory with "root.hds" image and the XML.
+ if (virAsprintf(&path, "%s/root.hds", vol->target.path) < 0) + return -1; I thought the target.path was already pointing to the image, not directory.
Jan The thing is that that volume target.path is path to folder with root.hds and DiskDescriptor.xml.
I don't like VOL_TYPE_FILE pointing to a directory.
And after resize we have to update DiskDescriptor.xml. This can be done only after volume will be fully upload, I mean after stream operations will be finished. Now I am looking for the correct place to do it.
If DiskDescriptor.xml contains the headers, then they should be updated by the same tool that changes the data.
I think about refreshPool. But not quite sure yet.
The purpose of refreshPool is to update libvirtd's knowledge of the files by reading the on-disk metadata, not chaning the on-disk metadata. Jan
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 | 33 +++++++++++++++++++++++++++++++++ src/storage/storage_backend.h | 3 +++ src/storage/storage_backend_fs.c | 2 ++ 3 files changed, 38 insertions(+) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index ac44fdf..18d414c 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -875,6 +875,39 @@ int virStorageBackendDeletePloop(virStorageVolDefPtr vol) return virFileDeleteTree(vol->target.path); } +int virStoragePloopResize(virStorageVolDefPtr vol, + unsigned long long capacity) +{ + int ret = -1; + char *path = NULL; + char *size = NULL; + virCommandPtr cmd = NULL; + char *create_tool = NULL; + + create_tool = virFindFileInPath("ploop"); + if (!create_tool) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("unable to find ploop, please install " + "ploop tools")); + return -1; + } + if (virAsprintf(&path, "%s/DiskDescriptor.xml", vol->target.path) < 0) + return -1; + + if (virAsprintf(&size, "%lluM", VIR_DIV_UP(capacity, (1024 * 1024))) < 0) + goto cleanup; + + cmd = virCommandNewArgList(create_tool, "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 3529755..65e91dc 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -117,6 +117,9 @@ int virStorageBackendCreatePloop(virConnectPtr conn, int virStorageBackendDeletePloop(virStorageVolDefPtr vol); +int virStoragePloopResize(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 f494fd2..abb6e47 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1378,6 +1378,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 17.02.2016 14:40, Olga Krishtal wrote:
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 | 33 +++++++++++++++++++++++++++++++++ src/storage/storage_backend.h | 3 +++ src/storage/storage_backend_fs.c | 2 ++ 3 files changed, 38 insertions(+)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index ac44fdf..18d414c 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -875,6 +875,39 @@ int virStorageBackendDeletePloop(virStorageVolDefPtr vol) return virFileDeleteTree(vol->target.path); }
+int virStoragePloopResize(virStorageVolDefPtr vol, + unsigned long long capacity) +{ + int ret = -1; + char *path = NULL; + char *size = NULL; + virCommandPtr cmd = NULL; + char *create_tool = NULL;
strange name
+ + create_tool = virFindFileInPath("ploop"); + if (!create_tool) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("unable to find ploop, please install " + "ploop tools")); + return -1; + } + if (virAsprintf(&path, "%s/DiskDescriptor.xml", vol->target.path) < 0) + return -1; + + if (virAsprintf(&size, "%lluM", VIR_DIV_UP(capacity, (1024 * 1024))) < 0) + goto cleanup;
use virCommandAddArgFormat
+ + cmd = virCommandNewArgList(create_tool, "resize", "-s", size, path, NULL); + + ret = virCommandRun(cmd, NULL); + + cleanup: + virCommandFree(cmd); + VIR_FREE(path); + VIR_FREE(size); + return ret;
you need to free 'create_tool' as well
+} + 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 3529755..65e91dc 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -117,6 +117,9 @@ int virStorageBackendCreatePloop(virConnectPtr conn,
int virStorageBackendDeletePloop(virStorageVolDefPtr vol);
+int virStoragePloopResize(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 f494fd2..abb6e47 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1378,6 +1378,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",
On 18/02/16 11:15, Nikolay Shirokovskiy wrote:
On 17.02.2016 14:40, Olga Krishtal wrote:
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 | 33 +++++++++++++++++++++++++++++++++ src/storage/storage_backend.h | 3 +++ src/storage/storage_backend_fs.c | 2 ++ 3 files changed, 38 insertions(+)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index ac44fdf..18d414c 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -875,6 +875,39 @@ int virStorageBackendDeletePloop(virStorageVolDefPtr vol) return virFileDeleteTree(vol->target.path); }
+int virStoragePloopResize(virStorageVolDefPtr vol, + unsigned long long capacity) +{ + int ret = -1; + char *path = NULL; + char *size = NULL; + virCommandPtr cmd = NULL; + char *create_tool = NULL; strange name
Strange? This name is widely used in code.
+ + create_tool = virFindFileInPath("ploop"); + if (!create_tool) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("unable to find ploop, please install " + "ploop tools")); + return -1; + } + if (virAsprintf(&path, "%s/DiskDescriptor.xml", vol->target.path) < 0) + return -1; + + if (virAsprintf(&size, "%lluM", VIR_DIV_UP(capacity, (1024 * 1024))) < 0) + goto cleanup; use virCommandAddArgFormat
Ok
+ + cmd = virCommandNewArgList(create_tool, "resize", "-s", size, path, NULL); + + ret = virCommandRun(cmd, NULL); + + cleanup: + virCommandFree(cmd); + VIR_FREE(path); + VIR_FREE(size); + return ret; you need to free 'create_tool' as well Ok +} + 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 3529755..65e91dc 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -117,6 +117,9 @@ int virStorageBackendCreatePloop(virConnectPtr conn,
int virStorageBackendDeletePloop(virStorageVolDefPtr vol);
+int virStoragePloopResize(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 f494fd2..abb6e47 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1378,6 +1378,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",
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 18d414c..60e6bd0 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -2260,6 +2260,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
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 | 92 ++++++++++++++++++++++++++--------- src/storage/storage_backend.h | 2 +- src/storage/storage_backend_fs.c | 6 ++- src/storage/storage_backend_logical.c | 2 +- src/util/virstoragefile.c | 9 +++- 5 files changed, 84 insertions(+), 27 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 60e6bd0..0e3c99c 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1587,6 +1587,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 @@ -1595,29 +1614,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)) { @@ -1636,6 +1661,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 @@ -1732,6 +1769,7 @@ virStorageBackendVolOpen(const char *path, struct stat *sb, return -1; } + VIR_FREE(ploop_path); return fd; } @@ -1759,8 +1797,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; @@ -1775,7 +1815,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; } @@ -1783,18 +1823,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; @@ -1814,6 +1859,7 @@ virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target, virStorageSourceFree(meta); VIR_FORCE_CLOSE(fd); VIR_FREE(buf); + VIR_FREE(path); return ret; } @@ -2076,6 +2122,8 @@ virStorageBackendVolUploadLocal(virConnectPtr conn ATTRIBUTE_UNUSED, ret = virFDStreamOpenBlockDevice(stream, target_path, offset, len, O_WRONLY); if (ret < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("error " + "occured during the upload of ploop volume")); goto cleanup; } diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 65e91dc..de48012 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -203,7 +203,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 abb6e47..f978b6e 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; 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..5184310 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,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, "WithoutFreSpaceExt", 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
On 17.02.2016 14:40, Olga Krishtal wrote:
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 | 92 ++++++++++++++++++++++++++--------- src/storage/storage_backend.h | 2 +- src/storage/storage_backend_fs.c | 6 ++- src/storage/storage_backend_logical.c | 2 +- src/util/virstoragefile.c | 9 +++- 5 files changed, 84 insertions(+), 27 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 60e6bd0..0e3c99c 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1587,6 +1587,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;
let's stick with "%s/root.hds" in all places.
+ 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 @@ -1595,29 +1614,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;\ +}
There is no need to introduce macros here. Function is enought.
+ 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)) { @@ -1636,6 +1661,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 @@ -1732,6 +1769,7 @@ virStorageBackendVolOpen(const char *path, struct stat *sb, return -1; }
+ VIR_FREE(ploop_path);
we don't have cleanup label in this function and a lot of returns above this freeing thus we have memory leaks. I think instead of changing all returns in this function we could leave it as it is and pass good path here from outside. To check is this a ploop or not we can just call virFileExist for hds file (i mean not using stat call as i'm not sure is it heavier than access or not and current code seems to optimize stat calls).
return fd; }
@@ -1759,8 +1797,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;
@@ -1775,7 +1815,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; } @@ -1783,18 +1823,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))) {
do we need path to actual hds here? and all the changes above from target->path to target_path
ret = -1; goto cleanup; @@ -1814,6 +1859,7 @@ virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target, virStorageSourceFree(meta); VIR_FORCE_CLOSE(fd); VIR_FREE(buf); + VIR_FREE(path); return ret; }
@@ -2076,6 +2122,8 @@ virStorageBackendVolUploadLocal(virConnectPtr conn ATTRIBUTE_UNUSED, ret = virFDStreamOpenBlockDevice(stream, target_path, offset, len, O_WRONLY); if (ret < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("error " + "occured during the upload of ploop volume"));
must be in correspondent patch
goto cleanup; }
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 65e91dc..de48012 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -203,7 +203,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 abb6e47..f978b6e 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; + }
looks like if we return here we don't update capacity properly.
if (S_ISDIR(sb.st_mode)) { target->format = VIR_STORAGE_FILE_DIR; ret = 0; 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..5184310 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,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, "WithoutFreSpaceExt", 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] = {
On Wed, Feb 17, 2016 at 02:40:05PM +0300, Olga Krishtal wrote:
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.
With each ploop volume being a directory with a ploop disk image and the XML, I think they deserve a separate pool type. The ploop image (root.hds) could be detected as such by the fs pool, but creating and deleting the directories feels out of place in this backend. Also, this should be documented in docs/storage.html.in, maybe with some examples.
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 | 92 ++++++++++++++++++++++++++--------- src/storage/storage_backend.h | 2 +- src/storage/storage_backend_fs.c | 6 ++- src/storage/storage_backend_logical.c | 2 +- src/util/virstoragefile.c | 9 +++- 5 files changed, 84 insertions(+), 27 deletions(-)
@@ -1636,6 +1661,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;
virStorageBackendVolOpen should be just opening the volume, not probing its format. Also, this is not really a 'ploop file', but a 'ploop dir' - wouldn't the lone disk images matching the magic in virstoragefile.c be indistinguishable form the directories?
+ 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
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 65e91dc..de48012 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -203,7 +203,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)
This should not be needed if the format is detected outside virStorageBackendVolOpen. Jan
18.02.2016 16:46, Ján Tomko пишет:
On Wed, Feb 17, 2016 at 02:40:05PM +0300, Olga Krishtal wrote:
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.
With each ploop volume being a directory with a ploop disk image and the XML, I think they deserve a separate pool type. The ploop image (root.hds) could be detected as such by the fs pool, but creating and deleting the directories feels out of place in this backend.
Actually one of the main intention of implementing ploop disk support in storage pool was ability to make it possible to work with existing storage pools like NFS, DIR, FS. Creating a separate storage pool makes it impossible. Also, our future plans to expand storage pools with new pool type like VZ storage and CEPH FS becomes worthless either.
Also, this should be documented in docs/storage.html.in, maybe with some examples.
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 | 92 ++++++++++++++++++++++++++--------- src/storage/storage_backend.h | 2 +- src/storage/storage_backend_fs.c | 6 ++- src/storage/storage_backend_logical.c | 2 +- src/util/virstoragefile.c | 9 +++- 5 files changed, 84 insertions(+), 27 deletions(-)
@@ -1636,6 +1661,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; virStorageBackendVolOpen should be just opening the volume, not probing its format.
Also, this is not really a 'ploop file', but a 'ploop dir' - wouldn't the lone disk images matching the magic in virstoragefile.c be indistinguishable form the directories?
+ 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 diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 65e91dc..de48012 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -203,7 +203,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) This should not be needed if the format is detected outside virStorageBackendVolOpen.
Jan
On Thu, Feb 18, 2016 at 05:04:16PM +0300, Maxim Nestratov wrote:
18.02.2016 16:46, Ján Tomko пишет:
On Wed, Feb 17, 2016 at 02:40:05PM +0300, Olga Krishtal wrote:
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.
With each ploop volume being a directory with a ploop disk image and the XML, I think they deserve a separate pool type.
On second thought, the pool is still just a directory, it's the volumes that are different, so it does belong in the directory-based pools. That leaves the mixing of the ploop volumes with the ploop disk images, don't we need a new STORAGE_VOL type?
The ploop image (root.hds) could be detected as such by the fs pool, but creating and deleting the directories feels out of place in this backend.
Actually one of the main intention of implementing ploop disk support in storage pool was ability to make it possible to work with existing storage pools like NFS, DIR, FS.
The 'image in a directory' still won't work with older libvirtd.
Creating a separate storage pool makes it impossible. Also, our future plans to expand storage pools with new pool type like VZ storage and CEPH FS becomes worthless either.
Jan
18.02.2016 16:46, Ján Tomko пишет:
On Wed, Feb 17, 2016 at 02:40:05PM +0300, Olga Krishtal wrote:
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.
With each ploop volume being a directory with a ploop disk image and the XML, I think they deserve a separate pool type. On second thought, the pool is still just a directory, it's the volumes
On Thu, Feb 18, 2016 at 05:04:16PM +0300, Maxim Nestratov wrote: that are different, so it does belong in the directory-based pools. The main idea of ploop is to have an image file, use it as a block device, and create and use a file system on that device. It is looks like loop device. However, we do need DiskDescriptor.xml store at the same directory
On 18/02/16 18:17, Ján Tomko wrote: that root.hds (file, that contains ploop).
That leaves the mixing of the ploop volumes with the ploop disk images, don't we need a new STORAGE_VOL type?
At the beginning I also had thoughts about it, but: 1) New STORAGE_VOL type will be exclusively for us. No one else will use it. So, we have volume time only for one format. 2) We can avoid such situation, because our ploop format looks a bit like qcow format. (Of course they have have different internal structure, but still, the only difference in such case - DiskDescriptor.xml, that can not be stored in the same file) 3) And we are able to work with STORAGE_VOL_FILE, because we are a file upon loop device.
The ploop image (root.hds) could be detected as such by the fs pool, but creating and deleting the directories feels out of place in this backend. Actually one of the main intention of implementing ploop disk support in storage pool was ability to make it possible to work with existing storage pools like NFS, DIR, FS. The 'image in a directory' still won't work with older libvirtd.
Creating a separate storage pool makes it impossible. Also, our future plans to expand storage pools with new pool type like VZ storage and CEPH FS becomes worthless either.
Jan
On Thu, Feb 18, 2016 at 07:39:37PM +0300, Olga Krishtal wrote:
18.02.2016 16:46, Ján Tomko пишет:
On Wed, Feb 17, 2016 at 02:40:05PM +0300, Olga Krishtal wrote:
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.
With each ploop volume being a directory with a ploop disk image and the XML, I think they deserve a separate pool type. On second thought, the pool is still just a directory, it's the volumes
On Thu, Feb 18, 2016 at 05:04:16PM +0300, Maxim Nestratov wrote: that are different, so it does belong in the directory-based pools. The main idea of ploop is to have an image file, use it as a block device, and create and use a file system on that device. It is looks like loop device. However, we do need DiskDescriptor.xml store at the same directory
On 18/02/16 18:17, Ján Tomko wrote: that root.hds (file, that contains ploop).
What will happen if the root.hds file will be placed directly in the pool directory, with no DiskDescriptor.xml? It seems this code would detect it as a ploop image.
That leaves the mixing of the ploop volumes with the ploop disk images, don't we need a new STORAGE_VOL type?
At the beginning I also had thoughts about it, but: 1) New STORAGE_VOL type will be exclusively for us. No one else will use it. So, we have volume time only for one format.
Well this is the only format that has its headers stored externally.
2) We can avoid such situation, because our ploop format looks a bit like qcow format. (Of course they have have different internal structure, but still, the only difference in such case - DiskDescriptor.xml, that can not be stored in the same file) 3) And we are able to work with STORAGE_VOL_FILE, because we are a file upon loop device.
Isn't creating this loop device outside of libvirt's control? All it sees is a directory with two files in it. Jan
On 18/02/16 18:17, Ján Tomko wrote: On Thu, Feb 18, 2016 at 05:04:16PM +0300, Maxim Nestratov wrote: 18.02.2016 16:46, Ján Tomko пишет: On Wed, Feb 17, 2016 at 02:40:05PM +0300, Olga Krishtal wrote: 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. With each ploop volume being a directory with a ploop disk image and the XML, I think they deserve a separate pool type. On second thought, the pool is still just a directory, it's the volumes that are different, so it does belong in the directory-based pools. The main idea of ploop is to have an image file, use it as a block device, and create and use a file system on that device. It is looks like loop device. However, we do need DiskDescriptor.xml store at the same directory that root.hds (file, that contains ploop). That leaves the mixing of the ploop volumes with the ploop disk images, don't we need a new STORAGE_VOL type? At the beginning I also had thoughts about it, but: 1) New STORAGE_VOL type will be exclusively for us. No one else will use it. So, we have volume time only for one format. 2) We can avoid such situation, because our ploop format looks a bit like qcow format. (Of course they have have different internal structure, but still, the only difference in such case - DiskDescriptor.xml, that can not be stored in the same file) 3)And we are able to work with STORAGE_VOL_FILE, b The ploop image (root.hds) could be detected as such by the fs pool, but creating and deleting the directories feels out of place in this backend. Actually one of the main intention of implementing ploop disk support in storage pool was ability to make it possible to work with existing storage pools like NFS, DIR, FS. The 'image in a directory' still won't work with older libvirtd. Creating a separate storage pool makes it impossible. Also, our future plans to expand storage pools with new pool type like VZ storage and CEPH FS becomes worthless either. Jan
On 18/02/16 17:04, Maxim Nestratov wrote:
18.02.2016 16:46, Ján Tomko пишет:
On Wed, Feb 17, 2016 at 02:40:05PM +0300, Olga Krishtal wrote:
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.
With each ploop volume being a directory with a ploop disk image and the XML, I think they deserve a separate pool type. The ploop image (root.hds) could be detected as such by the fs pool, but creating and deleting the directories feels out of place in this backend.
Actually one of the main intention of implementing ploop disk support in storage pool was ability to make it possible to work with existing storage pools like NFS, DIR, FS. Creating a separate storage pool makes it impossible. Also, our future plans to expand storage pools with new pool type like VZ storage and CEPH FS becomes worthless either.
Also, this should be documented in docs/storage.html.in, maybe with some examples.
Ok
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 | 92 ++++++++++++++++++++++++++--------- src/storage/storage_backend.h | 2 +- src/storage/storage_backend_fs.c | 6 ++- src/storage/storage_backend_logical.c | 2 +- src/util/virstoragefile.c | 9 +++- 5 files changed, 84 insertions(+), 27 deletions(-)
@@ -1636,6 +1661,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; virStorageBackendVolOpen should be just opening the volume, not probing its format.
How about virStorageBackendProbeTarget function: I won't touch VirStorageBackendVolOpen, but in if (S_ISDIR(sb.st_mode)) { if(this is ploop directory) { do some stuff } target->format = VIR_STORAGE_FILE_DIR; ret = 0; goto cleanup; }
Also, this is not really a 'ploop file', but a 'ploop dir' - wouldn't the lone disk images matching the magic in virstoragefile.c be indistinguishable form the directories?
The difference between ploop volume and dir, that directory volume is not mounted yet. In ploop volume will contain root.hds at least. So, we are able to read its header and get basic information.
+ 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 diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 65e91dc..de48012 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -203,7 +203,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) This should not be needed if the format is detected outside virStorageBackendVolOpen.
Jan
participants (4)
-
Ján Tomko -
Maxim Nestratov -
Nikolay Shirokovskiy -
Olga Krishtal