[libvirt] [PATCHv3 07/36] storage: Switch metadata crawler to use storage driver to get unique path

Roman Bogorodskiy bogorodskiy at gmail.com
Sat Jun 7 18:35:15 UTC 2014


  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 at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

Roman Bogorodskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140607/02e012e8/attachment-0001.sig>


More information about the libvir-list mailing list