Peter Krempa wrote:
> On 06/09/14 16:07, Roman Bogorodskiy wrote:
> > 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.
>
> Yes, the problem is that VIR_STORAGE_FS isn't enabled on freebsd.
>
> I don't know whether it's worth analyzing the code to find out if it can
> be enabled on freebsd or not. (the needed portions certainly can be
> enabled, but some of the other portions might be problematic.
Providing a fully functional virStorageBackendFileSystem backend will
certainly require some effort, at least getmntent_r() part and mouting
part needs to be re-implemnented for BSD.
I'll probably take a look later.
>
> At any rate, I'm working on a refactor of the test that will allow to
> skip the test (and remove a few ugly pieces of code)
Thanks!
Roman Bogorodskiy
Attachment:
pgpv5cji0s2wI.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list