Peter Krempa wrote:
> Use the virStorageFileGetUniqueIdentifier() function to get a unique
> identifier regardless of the target storage type instead of relying on
> canonicalize_path().
>
> A new function that checks whether we support a given image is
> introduced to avoid errors for unimplemented backends.
> ---
>
> Notes:
> Version 3:
> - fixed typo
> - ACKed by Eric
>
> src/storage/storage_driver.c | 77
> +++-
> 1 file changed, 54 insertions(+), 23 deletions(-)
>
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index f92a553..5c4188f 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -2788,6 +2788,30 @@ virStorageFileIsInitialized(virStorageSourcePtr src)
> return !!src->drv;
> }
>
> +
> +static bool
> +virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr src)
> +{
> +int actualType = virStorageSourceGetActualType(src);
> +virStorageFileBackendPtr backend;
> +
> +if (!src)
> +return false;
> +
> +if (src->drv) {
> +backend = src->drv->backend;
> +} else {
> +if (!(backend = virStorageFileBackendForTypeInternal(actualType,
> + src->protocol,
> + false)))
> +return false;
> +}
> +
> +return backend->storageFileGetUniqueIdentifier &&
> + backend->storageFileReadHeader &&
> + backend->storageFileAccess;
> +}
> +
> void
> virStorageFileDeinit(virStorageSourcePtr src)
> {
> @@ -3104,7 +3128,6 @@ virFindBackingFile(const char *start, const char *path,
> /* Recursive workhorse for virStorageFileGetMetadata. */
> static int
> virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
> - const char *canonPath,
> uid_t uid, gid_t gid,
> bool allow_probe,
> virHashTablePtr cycle)
> @@ -3112,49 +3135,63 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr
> src,
> int fd;
> int ret = -1;
> struct stat st;
> +const char *uniqueName;
> virStorageSourcePtr backingStore = NULL;
> int backingFormat;
>
> -VIR_DEBUG("path=%s canonPath=%s dir=%s format=%d uid=%d gid=%d probe=%d",
> - src->path, canonPath, NULLSTR(src->relDir), src->format,
> +VIR_DEBUG("path=%s dir=%s format=%d uid=%d gid=%d probe=%d",
> + src->path, NULLSTR(src->relDir), src->format,
>(int)uid, (int)gid, allow_probe);
>
> -if (virHashLookup(cycle, canonPath)) {
> -virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("backing store for %s is self-referential"),
> - src->path);
> +/* exit if we can't load information about the current image */
> +if (!virStorageFileSupportsBackingChainTraversal(src))
> +return 0;
After this change I get virstrogetest failing on FreeBSD, which doesn't
support any of the file storage backends currently:
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 806c06400 (LWP 100061/virstoragetest)]
0x00410088 in mymain () at virstoragetest.c:937
937 TEST_LOOKUP(3, "qcow2", chain->backingStore->path,
chain->backingStore,
Current language: auto; currently minimal
(gdb) p chain
$1 = 0x806c7b100
(gdb) p chain->backingStore
$2 = 0x0
(gdb)
> +
> +if (virStorageFileInitAs(src, uid, gid) < 0)
> return -1;
> +
> +if (!(uniqueName = virStorageFileGetUniqueIdentifier(src)))
> +goto cleanup;
> +
> +if (virHashLookup(cycle, uniqueName)) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("backing store for %s (%s) is self-referential"),
> + src->path, uniqueName);
> +goto cleanup;
> }
>
> -if (virHashAddEntry(cycle, canonPath, (void *)1) < 0)
> -return -1;
> +if (virHashAddEntry(cycle, uniqueName, (void *)1) < 0)
> +goto cleanup;
>
> if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) {
> if ((fd = virFileOpenAs(src->path, O_RDONLY, 0, uid, gid, 0)) < 0) {
> virReportSystemError(-fd, _("Failed to open file '%s'"),
> src->path);
> -return -1;
> +goto cleanup;
> }
>
> if (virStorageFileGetMetadataFromFDInternal(src, fd,
> &backingFormat) < 0) {
> VIR_FORCE_CLOSE(fd);
> -return -1;
> +goto cleanup;
> }
>
> if (VIR_CLOSE(fd) < 0)
> VIR_WARN("could not close file %s", src->path);
> } else {
> /* TODO: currently we call this only for local storage */
> -return 0;
> +ret = 0;
> +goto cleanup
