On 05/22/2014 07:47 AM, Peter Krempa wrote: > Different protocols have different means to uniquely identify a storage > file. This patch implements a storage driver API to retrieve a unique > string describing a volume. The current implementation works for local > storage only and returns the canonical path of the volume. > > To add caching support the local filesystem driver now has a private > structure holding the cached string, which is created only when it's > initially accessed. > > This patch provides the implementation for local files only for start. > --- > src/storage/storage_backend.h | 3 +++ > src/storage/storage_backend_fs.c | 49 > ++++++++++++++++++++++++++++++++++++++++ > src/storage/storage_driver.c | 30 ++++++++++++++++++++++++ > src/storage/storage_driver.h | 1 + > 4 files changed, 83 insertions(+) >
>
> +typedef struct _virStorageFileBackendFsPriv virStorageFileBackendFsPriv;
> +typedef virStorageFileBackendFsPriv *virStorageFileBackendFsPrivPtr;
> +
> +struct _virStorageFileBackendFsPriv {
> + char *uid; /* unique file identifier (canonical path) */
The name 'uid' is misleading (too commonly used to mean user-id, which
it is not). Can you change it to something like 'fid' (short for
file-id) or 'canon'?
> +};
> +
> +
> static void
> virStorageFileBackendFileDeinit(virStorageSourcePtr src)
> {
> @@ -1346,16 +1354,27 @@ virStorageFileBackendFileDeinit(virStorageSourcePtr
> src)
> virStorageTypeToString(virStorageSourceGetActualType(src)),
> src->path);
>
> + virStorageFileBackendFsPrivPtr priv = src->drv->priv;
> +
> + VIR_FREE(priv->uid);
As evidence, if I see this line in isolation, I think "why are you
freeing an integer" - I had to look at the header to learn "oh, it's not
a usual uid, but a string".
> +/*
> + * virStorageFileGetUniqueIdentifier: Get a unique string describing the
> volume
> + *
> + * @src: file structure pointing to the file
> + *
> + * Returns a string uniquely describing a single volume (canonical path).
> + * The string shall not be freed. Returns NULL on error and sets a libvirt
> + * error code */
Is it also worth documenting that the string is invalidated when the
input parameter 'src' is freed?
The idea makes sense, but I'd like to see a v3.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
