On 17/02/16 14:39, Olga Krishtal wrote: Pls, disregard this patch.

On 17.02.2016 14:40, Olga Krishtal wrote: you need to change indent back to function level from ternary operator indentation you don't remove created dir on this return path use virCommandAddArgFormat instead of series of virAsprintf if target directory exists you will not get what you want. I think best would be to create dir before branching.

On 17.02.2016 14:40, Olga Krishtal wrote: Let's get rid of function to be consistent with inplace virFileRemove below.

On 17.02.2016 14:40, Olga Krishtal wrote: 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.

On 18/02/16 16:57, Ján Tomko wrote: 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.

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(a)virtuozzo.com&gt; --- 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 18/02/16 11:15, Nikolay Shirokovskiy wrote: Strange? This name is widely used in code. Ok Ok

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(a)virtuozzo.com&gt; --- 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 Wed, Feb 17, 2016 at 02:40:05PM +0300, Olga Krishtal wrote: 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. 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? This should not be needed if the format is detected outside virStorageBackendVolOpen. Jan

On 18/02/16 17:04, Maxim Nestratov wrote: Ok 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; } 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.