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

[Roman Bogorodskiy]

  Peter Krempa wrote:

> On 06/07/14 20:35, Roman Bogorodskiy wrote:
> >   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
> 
> >> @@ -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) 
> > 
> > 
> 
> Hmm, so FreeBSD doesn't currently compile the storage driver? We should
> probably look into enabling it. All the stuff that was done by the code
> was compiling just fine on FreeBSD previously so enabling just the local
> filesystem storage backend should work well. I'll have a look what that
> would include.

virstorage.c test calls testStorageFileGetMetadata() which calls
virStorageFileGetMetadata().

Inside of that, the call sequence is:

virStorageFileGetMetadata -> virStorageFileGetMetadataRecurse

virStorageFileGetMetadataRecurse then checks
!virStorageFileSupportsBackingChainTraversal() and returns 0.

virStorageFileSupportsBackingChainTraversal tries to initialise backend
using virStorageFileBackendForTypeInternal().

virStorageFileBackendForTypeInternal loops through fileBackends which
looks this way:

static virStorageFileBackendPtr fileBackends[] = {
#if WITH_STORAGE_FS
    &virStorageFileBackendFile,
    &virStorageFileBackendBlock,
#endif
#if WITH_STORAGE_GLUSTER
    &virStorageFileBackendGluster,
#endif
    NULL 
};

FreeBSD doesn't support neither WITH_STORAGE_GLUSTER nor WITH_STORAGE_FS. So
we end up having chain->backingStore == NULL.

PS I may be wrong here as it's my first experience with the storage code
and I haven't yet spend much time on it.

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/20140609/2091577d/attachment-0001.sig>