[libvirt] [RFC 8/7] rfc: prepare to merge virStorageFileMetadata

--- It's late for me, but I wanted to throw this out there to make sure I'm on the right track. My premise here is that the current virStorageFileMetadata is awkward to work with, because you have to ask the parent for details about the child, instead of directly storing those details in the child. So this patch proposes a change to the metadata struct to make it possible to track details in the right struct. Then I would have a series of patches to make metadata generation populate the duplicate information as documented in the comments here, followed by teaching all clients of the populated data to grab the information from the right struct instead of the parent. At which point, I'd be ready for RFC 9/7... src/util/virstoragefile.c | 4 ++++ src/util/virstoragefile.h | 35 ++++++++++++++++++++++++++++++----- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index e6a985d..5d5adb4 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1204,6 +1204,10 @@ virStorageFileFreeMetadata(virStorageFileMetadata *meta) if (!meta) return; + VIR_FREE(meta->path); + VIR_FREE(meta->canonName); + 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 56105d0..da90374 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -108,17 +108,42 @@ 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 this file as spelled by the user (top level) or + * metadata of the parent (if this is a backing store). */ + char *path; + /* Canonical name of this file, used to detect loops in the + * backing store chain. */ + char *canonName; + /* Directory to start from if backingStoreRaw is a relative file + * name */ + char *relDir; + /* Name of the backing store recorded in metadata of the parent */ + char *backingStoreRaw; + + /* Backing chain. backingMeta->origName should match + * backingStoreRaw; but the fields are duplicated in the common + * case to make it possible to detect missing backing files, as + * follows: if backingStoreRaw is NULL, this should be NULL. If + * this is NULL and backingStoreRaw is non-NULL, there was an + * error following the chain (such as missing file). Otherwise, + * information about the backing store 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->canon */ + 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