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)]
0x0000000000410088 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;
> }
>
> /* check whether we need to go deeper */
> - if (!src->backingStoreRaw)
> - return 0;
> + if (!src->backingStoreRaw) {
> + ret = 0;
> + goto cleanup;
> + }
>
> if (VIR_ALLOC(backingStore) < 0)
> - return -1;
> + goto cleanup;
>
> if (VIR_STRDUP(backingStore->relPath, src->backingStoreRaw) < 0)
> goto cleanup;
> @@ -3201,7 +3238,6 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
> backingStore->format = backingFormat;
>
> if (virStorageFileGetMetadataRecurse(backingStore,
> - backingStore->path,
> uid, gid, allow_probe,
> cycle) < 0) {
> /* if we fail somewhere midway, just accept and return a
> @@ -3215,6 +3251,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
> ret = 0;
>
> cleanup:
> + virStorageFileDeinit(src);
> virStorageSourceFree(backingStore);
> return ret;
> }
> @@ -3253,12 +3290,6 @@ virStorageFileGetMetadata(virStorageSourcePtr src,
> return -1;
>
> if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) {
> - if (!(canonPath = canonicalize_file_name(src->path))) {
> - virReportSystemError(errno, _("unable to resolve '%s'"),
> - src->path);
> - goto cleanup;
> - }
> -
> if (!src->relPath &&
> VIR_STRDUP(src->relPath, src->path) < 0)
> goto cleanup;
> @@ -3277,7 +3308,7 @@ virStorageFileGetMetadata(virStorageSourcePtr src,
> if (src->format <= VIR_STORAGE_FILE_NONE)
> src->format = allow_probe ? VIR_STORAGE_FILE_AUTO : VIR_STORAGE_FILE_RAW;
>
> - ret = virStorageFileGetMetadataRecurse(src, canonPath, uid, gid,
> + ret = virStorageFileGetMetadataRecurse(src, uid, gid,
> allow_probe, cycle);
>
> cleanup:
> --
> 1.9.3
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
Roman Bogorodskiy
Attachment:
pgpCDoWwxBYpL.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list