[libvirt] [PATCH 6/6] storage:dir: adapts .refreshVol .refreshPool for ploop volumes
Nikolay Shirokovskiy
nshirokovskiy at virtuozzo.com
Wed Feb 10 14:38:52 UTC 2016
On 09.02.2016 16:52, 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 at virtuozzo.com>
> ---
> 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;
> }
I don't think this function is a good place to detect volume format. It is basically
just a wrapper for virFileOpenAs and its function get an fd for a volume.
Adding "root.hds" here would be enough.
virStorageFileGetMetadataInternal?? looks like place that could detect ploops by
directory structure.
>
> @@ -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 },
this is not true, we have not "ploop" magic.
>
> /* All formats with a backing store probe below here */
> [VIR_STORAGE_FILE_COW] = {
>
More information about the libvir-list
mailing list