On 09.02.2016 16:52, Olga Krishtal wrote: inconvinient indentation, should at least be: if ((err = virDirCreate(vol->target.path, (vol->target.perms->mode == (mode_t) -1 ? VIR_STORAGE_DEFAULT_VOL_PERM_MODE: vol->target.perms->mode), .... that is extra space to indent every nesting. Or you can calc this parameter outside as in other places. looks like not too useful to be a distinct function. arguments indentation better set this to -1, it's common idiom consider next indentation: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("unable to find ploop, please install ploop tools")); and return -1! looks likes this check will never trigger, could be removed consider this indentation virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("encrypted ploop volumes are not supported with " "ploop init")); indentation indentation "%s/root.hds" spaces in "1024 * 1024" line length line length qcow2 code does not set this, why do we need this? line length we need to cleanup filesystem too. 1. 'cp' does not cleanup by itself 2. if input volume is NULL we can create directory and then fail to init ploop and we leave directory on disk on exit.

On 09.02.2016 16:52, Olga Krishtal wrote: break at this level is not easy to follow. Can we introduce a function which will have another switch inside for types of volume types?

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(a)virtuozzo.com&gt; --- src/storage/storage_backend.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 34c9d40..c455908 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -2025,12 +2025,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->target.format == VIR_STORAGE_FILE_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 @@ -2042,10 +2053,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/%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

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 | 28 ++++++++++++++++++++++++++++ src/storage/storage_backend.h | 5 +++++ src/storage/storage_backend_fs.c | 2 ++ 3 files changed, 35 insertions(+) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index c455908..fb4d1b9 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -877,6 +877,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 2290096..c2d148d 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(conn, pool, vol, capacity); } else { if (pre_allocate) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", -- 1.8.3.1

Returns error in case of vol-wipe cmd for a ploop volume Signed-off-by: Olga Krishtal <okrishtal(a)virtuozzo.com&gt; --- 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 fb4d1b9..21dd96d 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -2217,6 +2217,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(a)virtuozzo.com&gt; --- src/storage/storage_backend.c | 90 ++++++++++++++++++++++++++--------- src/storage/storage_backend.h | 2 +- src/storage/storage_backend_fs.c | 6 ++- src/storage/storage_backend_logical.c | 2 +- src/util/virstoragefile.c | 8 +++- 5 files changed, 81 insertions(+), 27 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 21dd96d..0c31f32 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1584,6 +1584,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 @@ -1592,29 +1611,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)) { @@ -1633,6 +1658,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 @@ -1729,6 +1766,7 @@ virStorageBackendVolOpen(const char *path, struct stat *sb, return -1; } + VIR_FREE(ploop_path); return fd; } @@ -1756,8 +1794,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; @@ -1772,7 +1812,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; } @@ -1780,18 +1820,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; @@ -1811,6 +1856,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 c2d148d..c340c69 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..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