Re: [libvirt] [PATCH 4/5] conf: track more fields in backing chain metadata
On 04/04/2014 03:31 AM, Peter Krempa wrote:
> 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;
Hmm, this field seems pretty redundant to me, IIUC it's the same data as
in "path".
No, it's not.
Given the chain:
base <- top
my goal is to have:
{ .path = "top",
.canonName = "/path/to/top",
.relDir = ".",
.backingStoreRaw = "base",
.backingMeta = {
.path = "base",
.canonName = "/path/to/base",
.relDir = ".",
.backingStoreRaw = NULL,
.backingMeta = NULL,
}
}
OTOH, the "path" field should contain the canonical path as the target
is to convert it to the new storage file struct. In that case the
"canonName" will be redundant and backingStoreRaw needs to be kept to
track the original name.
Rather, "path" should be (but is not yet) the name as specified by the
user or the metadata of the parent file, unmodified (for both local
files and for network elements like gluster://server/vol/img); while
canonName should be the possibly munged name (munged to canonical
absolute name for local files, unmunged for network elements). Loop
detection is done by registering canonName in the hash table while
following the backing chain.
In any case ... one of them seems to be duplicate.
No, path is about the current element, while backingStoreRaw is about
the child element.
My other goal in this refactor is to make detection of missing backing
chains much saner. Right now, for the chain 'missing <- foo', we have
this for 'foo':
{ .backingStoreRaw = "missing",
.backingStore = NULL,
.backingStoreIsFile = false,
.backingMeta = NULL,
}
while for the chain 'gluster://.../base <- foo', we have
{ .backingStoreRaw = NULL,
.backingStore = "gluster://.../base",
.backingStoreIsFile = false,
.backingMeta = NULL,
}
where code that doesn't care about whether foo exists (such as storage
volume dumpxml) vs. code that DOES care (sVirt labeling before starting
a qemu domain) has to pay attention to multiple fields in the parent to
decide whether to raise an error; furthermore, since there is never any
chain emitted for a non-file backing, we can't support any network file
of anything other than raw. After the conversion, I envision:
{ .path = "foo",
.type = VIR_STORAGE_TYPE_FILE,
.format = VIR_STORAGE_FILE_QCOW2,
.backingStoreRaw = "missing",
.backingMeta = NULL,
}
or maybe:
{ .path = "foo",
.type = VIR_STORAGE_TYPE_FILE,
.format = VIR_STORAGE_FILE_QCOW2,
.backingStoreRaw = "missing",
.backingMeta = {
.path = "missing",
.type = VIR_STORAGE_TYPE_NONE,
.format = VIR_STORAGE_FILE_NONE,
}
}
as the indication of a missing backing file, and:
{ .path = "foo",
.type = VIR_STORAGE_TYPE_FILE,
.format = VIR_STORAGE_FILE_QCOW2,
.backingStoreRaw = "gluster://.../base",
.backingMeta = {
.path = "gluster://.../base",
.type = VIR_STORAGE_TYPE_NETWORK,
.format = VIR_STORAGE_FILE_RAW,
.backingStoreRaw = NULL,
.backingMeta = NULL,
}
}
as indication of a chain involving a network backing file, and where we
have a much easier task of identifying a network type resource that can
be something other than raw, so that we can actually have a chain of
multiple network devices if we know how to probe that network file (the
way we do for gluster).
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 604 bytes)