11:35 p.m.
The current use of virStorageFileMetadata is awkward; to learn
some of the information about a child node, you have to read
fields in the parent node. This does not lend itself well to
modifying backing chains (whether inserting a new node in the
chain, or consolidating existing nodes); better would be to
learn about a child node directly in that node. This patch
sets up some new fields which contain redundant information,
although not necessarily in the final desired state for the
new fields (see the next patch for actual tests of what is there
now). Then later patches will do any refactoring necessary to
get the fields to their desired states, and update clients to
get the information from the new fields, so we can finally
delete the fields that are tracking information about the wrong
node.
More concretely, compare these three example backing chains:
good <- one
missing <- two
gluster://server/vol/img <- three
Pre-patch, querying the chains gives:
{ .backingStore = "/path/to/good",
.backingStoreRaw = "good",
.backingStoreIsFile = true,
.backingStoreFormat = VIR_STORAGE_FILE_RAW,
.backingMeta = {
.backingStore = NULL,
.backingStoreRaw = NULL,
.backingStoreIsFile = false,
.backingMeta = NULL,
}
}
{ .backingStore = NULL,
.backingStoreRaw = "missing",
.backingStoreIsFile = false,
.backingStoreFormat = VIR_STORAGE_FILE_NONE,
.backingMeta = NULL,
}
{ .backingStore = "gluster://server/vol/img",
.backingStoreRaw = NULL,
.backingStoreIsFile = false,
.backingStoreFormat = VIR_STORAGE_FILE_RAW,
.backingMeta = NULL,
}
Deciding whether to ignore a missing backing file (as in virsh
vol-dumpxml) or report an error (as in security manager sVirt
labeling) requires reading multiple fields. Plus, the format
is hard-coded to treat all network protocols as end-of-the-chain,
as if they were raw. By the end of this patch series, the goal
is to instead represent these three situations as:
{ .path = "one",
.canonPath = "/path/to/one",
.type = VIR_STORAGE_TYPE_FILE,
.format = VIR_STORAGE_FILE_QCOW2,
.backingStoreRaw = "good",
.backingMeta = {
.path = "good",
.canonPath = "/path/to/good",
.type = VIR_STORAGE_TYPE_FILE,
.format = VIR_STORAGE_FILE_RAW,
.backingStoreRaw = NULL,
.backingMeta = NULL,
}
}
{ .path = "two",
.canonPath = "/path/to/two",
.type = VIR_STORAGE_TYPE_FILE,
.format = VIR_STORAGE_FILE_QCOW2,
.backingStoreRaw = "missing",
.backingMeta = NULL,
}
{ .path = "three",
.canonPath = "/path/to/three",
.type = VIR_STORAGE_TYPE_FILE,
.format = VIR_STORAGE_FILE_QCOW2,
.backingStoreRaw = "gluster://server/vol/img",
.backingMeta = {
.path = "gluster://server/vol/img",
.canonPath = "gluster://server/vol/img",
.type = VIR_STORAGE_TYPE_NETWORK,
.format = VIR_STORAGE_FILE_RAW,
.backingStoreRaw = NULL,
.backingMeta = NULL,
}
}
or, for the second file, maybe also allowing:
{ .path = "two",
.canonPath = "/path/to/two",
.type = VIR_STORAGE_TYPE_FILE,
.format = VIR_STORAGE_FILE_QCOW2,
.backingStoreRaw = "missing",
.backingMeta = {
.path = "missing",
.canonPath = NULL,
.type = VIR_STORAGE_TYPE_NONE,
.format = VIR_STORAGE_FILE_NONE,
.backingStoreRaw = NULL,
.backingMeta = NULL,
}
}
* src/util/virstoragefile.h (_virStorageFileMetadata): Add
path, canonPath, relDir, type, and format fields. Reorder
existing fields, and add lots of comments.
* src/util/virstoragefile.c (virStorageFileFreeMetadata): Clean
new fields.
(virStorageFileGetMetadataInternal)
(virStorageFileGetMetadataFromFDInternal): Start populating new
fields.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/util/virstoragefile.c | 26 +++++++++++++++++++++++++-
src/util/virstoragefile.h | 36 +++++++++++++++++++++++++++++++-----
2 files changed, 56 insertions(+), 6 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index e516acf..0008cd7 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -789,6 +789,13 @@ virStorageFileGetMetadataInternal(const char *path,
VIR_DEBUG("path=%s, canonPath=%s, dir=%s, buf=%p, len=%zu, format=%d",
path, canonPath, directory, buf, len, format);
+ if (VIR_STRDUP(meta->path, path) < 0)
+ goto cleanup;
+ if (VIR_STRDUP(meta->canonPath, canonPath) < 0)
+ goto cleanup;
+ if (VIR_STRDUP(meta->relDir, directory) < 0)
+ goto cleanup;
+
if (format == VIR_STORAGE_FILE_AUTO)
format = virStorageFileProbeFormatFromBuf(path, buf, len);
@@ -798,6 +805,7 @@ virStorageFileGetMetadataInternal(const char *path,
format);
goto cleanup;
}
+ meta->format = format;
/* XXX we should consider moving virStorageBackendUpdateVolInfo
* code into this method, for non-magic files
@@ -1020,8 +1028,14 @@ virStorageFileGetMetadataFromFDInternal(const char *path,
return -1;
}
- /* No header to probe for directories, but also no backing file */
if (S_ISDIR(sb.st_mode)) {
+ /* No header to probe for directories, but also no backing
+ * file; therefore, no inclusion loop is possible, and we
+ * don't need canonName or relDir. */
+ if (VIR_STRDUP(meta->path, path) < 0)
+ goto cleanup;
+ meta->type = VIR_STORAGE_TYPE_DIR;
+ meta->format = VIR_STORAGE_FILE_DIR;
ret = 0;
goto cleanup;
}
@@ -1039,6 +1053,12 @@ virStorageFileGetMetadataFromFDInternal(const char *path,
ret = virStorageFileGetMetadataInternal(path, canonPath, directory,
buf, len, format, meta);
+ if (ret == 0) {
+ if (S_ISREG(sb.st_mode))
+ meta->type = VIR_STORAGE_TYPE_FILE;
+ else if (S_ISBLK(sb.st_mode))
+ meta->type = VIR_STORAGE_TYPE_BLOCK;
+ }
cleanup:
VIR_FREE(buf);
return ret;
@@ -1266,6 +1286,10 @@ virStorageFileFreeMetadata(virStorageFileMetadata *meta)
if (!meta)
return;
+ VIR_FREE(meta->path);
+ VIR_FREE(meta->canonPath);
+ VIR_FREE(meta->relDir);
+
virStorageFileFreeMetadata(meta->backingMeta);
VIR_FREE(meta->backingStore);
VIR_FREE(meta->backingStoreRaw);
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 83ec2bd..984d1e3 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -112,17 +112,43 @@ struct _virStorageTimestamps {
typedef struct _virStorageFileMetadata virStorageFileMetadata;
typedef virStorageFileMetadata *virStorageFileMetadataPtr;
struct _virStorageFileMetadata {
- char *backingStore; /* Canonical name (absolute file, or protocol) */
- char *backingStoreRaw; /* If file, original name, possibly relative */
- char *directory; /* The directory containing basename of backingStoreRaw */
- int backingStoreFormat; /* enum virStorageFileFormat */
- bool backingStoreIsFile;
+ /* Name of the current file as spelled by the user (top level) or
+ * metadata of the overlay (if this is a backing store). */
+ char *path;
+ /* Canonical name of the current file, used to detect loops in the
+ * backing store chain. */
+ char *canonPath;
+ /* Directory to start from if backingStoreRaw is a relative file
+ * name. */
+ char *relDir;
+ /* Name of the child backing store recorded in metadata of the
+ * current file. */
+ char *backingStoreRaw;
+
+ /* Backing chain. In the common case, the child's
+ * backingMeta->path will be a duplicate of this file's
+ * backingStoreRaw; this setup makes it possible to detect missing
+ * backing files: if backingStoreRaw is NULL, this field should be
+ * NULL. If this field is NULL and backingStoreRaw is non-NULL,
+ * there was an error following the chain (such as a missing
+ * file). Otherwise, information about the child is here. */
virStorageFileMetadataPtr backingMeta;
+ /* Details about the current image */
+ int type; /* enum virStorageType */
+ int format; /* enum virStorageFileFormat */
virStorageEncryptionPtr encryption;
unsigned long long capacity;
virBitmapPtr features; /* bits described by enum virStorageFileFeature */
char *compat;
+
+ /* Fields I'm trying to delete, because it is confusing to have to
+ * query the parent metadata for details about the backing
+ * store. */
+ char *backingStore; /* Canonical name (absolute file, or protocol). Should be same as
backingMeta->canonPath */
+ char *directory; /* The directory containing basename of backingStoreRaw. Should be
same as backingMeta->relDir */
+ int backingStoreFormat; /* enum virStorageFileFormat. Should be same as
backingMeta->format */
+ bool backingStoreIsFile; /* Should be same as backingMeta->type <
VIR_STORAGE_TYPE_NETWORK */
};
--
1.9.0