On 10/13/2012 06:00 PM, Eric Blake wrote:
> In order to temporarily label files read/write during a commit
> operation, we need to crawl the backing chain and find the absolute
> file name that needs labeling in the first place, as well as the
> name of the file that owns the backing file.
>
> * src/util/storage_file.c (virStorageFileChainLookup): New
> function.
> * src/util/storage_file.h: Declare it.
> * src/libvirt_private.syms (storage_file.h): Export it.
> ---
> src/libvirt_private.syms | 1 +
> src/util/storage_file.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++
> src/util/storage_file.h | 7 ++++++
> 3 files changed, 71 insertions(+)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index fe31bbe..e40d630 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1125,6 +1125,7 @@ virStorageGenerateQcowPassphrase;
>
>
> # storage_file.h
> +virStorageFileChainLookup;
> virStorageFileFormatTypeFromString;
> virStorageFileFormatTypeToString;
> virStorageFileFreeMetadata;
> diff --git a/src/util/storage_file.c b/src/util/storage_file.c
> index 6f299c0..218891e 100644
> --- a/src/util/storage_file.c
> +++ b/src/util/storage_file.c
> @@ -1260,3 +1260,66 @@ const char *virStorageFileGetSCSIKey(const char *path)
> return NULL;
> }
> #endif
> +
> +/* Given a CHAIN that starts at the named file START, return the
> + * canonical name for the backing file NAME within that chain, or pass
> + * NULL to find the base of the chain. If *META is not NULL, set it
> + * to the point in the chain that describes NAME (or to NULL if the
> + * backing element is not a file). If *PARENT is not NULL, set it to
> + * the canonical name of the parent (or to NULL if NAME matches
> + * START). The results point within CHAIN, and must not be
> + * independently freed. */
> +const char *
> +virStorageFileChainLookup(virStorageFileMetadataPtr chain, const char *start,
> + const char *name, virStorageFileMetadataPtr *meta,
> + const char **parent)
> +{
> + virStorageFileMetadataPtr owner;
> + const char *tmp;
> +
> + if (!parent)
> + parent = &tmp;
> +
> + *parent = NULL;
> + if (name ? STREQ(start, name) || virFileLinkPointsTo(start, name) :
> + !chain->backingStore) {
Hmm. First time I've ever seen ?: used inside an if statement (since for
me the entire purpose of ?: is to eliminate the need for an if). Maybe
this would be more quickly parseable as (the admittedly longer):
if ((name & (STREQ(start, name) || virFileLinkPointsTo(start,
name))) ||
(!name && !chain->backingStore)) {
Not mandatory though.
> + if (meta)
> + *meta = chain;
> + return start;
Don't you need to make sure start is an absolute path?
> + }
> +
> + owner = chain;
> + *parent = start;
> + while (owner) {
> + if (!owner->backingStore)
> + goto error;
> + if (!name) {
> + if (!owner->backingMeta ||
> + !owner->backingMeta->backingStore)
> + break;
> + } else if (STREQ_NULLABLE(name, owner->backingStoreRaw) ||
> + STREQ(name, owner->backingStore)) {
> + break;
> + } else if (owner->backingStoreIsFile) {
> + char *abs = absolutePathFromBaseFile(*parent, name);
> + if (abs && STREQ(abs, owner->backingStore)) {
> + VIR_FREE(abs);
> + break;
> + }
> + VIR_FREE(abs);
> + }
> + *parent = owner->backingStore;
> + owner = owner->backingMeta;
> + }
> + if (!owner)
> + goto error;
> + if (meta)
> + *meta = owner->backingMeta;
> + return owner->backingStore;
and I'm recalling from the previous patch that owner->backingStore is
already stored in its canonical form, right?
> +
> +error:
> + *parent = NULL;
> + if (meta)
> + *meta = NULL;
> + return NULL;
> +}
> diff --git a/src/util/storage_file.h b/src/util/storage_file.h
> index 685fcb8..9b00dba 100644
> --- a/src/util/storage_file.h
> +++ b/src/util/storage_file.h
> @@ -78,6 +78,13 @@ virStorageFileMetadataPtr virStorageFileGetMetadataFromFD(const char *path,
> int fd,
> int format);
>
> +const char *virStorageFileChainLookup(virStorageFileMetadataPtr chain,
> + const char *start,
> + const char *name,
> + virStorageFileMetadataPtr *meta,
> + const char **parent)
> + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
> +
> void virStorageFileFreeMetadata(virStorageFileMetadataPtr meta);
>
> int virStorageFileResize(const char *path, unsigned long long capacity);
ACK as-is if start doesn't need to be canonicalized (i.e. if I didn't
understand :-), otherwise ACK with that change.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list