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