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