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

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

2014-06-11 Thread Peter Krempa

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)]
 0x00410088 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.

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)

 
 Roman Bogorodskiy
 

Peter



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2014-06-11 Thread Roman Bogorodskiy

  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)]
  0x00410088 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


pgpGhdCYunwEZ.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2014-06-09 Thread Peter Krempa

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)]
 0x00410088 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.

Peter




signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2014-06-09 Thread 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)]
  0x00410088 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


pgpFC6AqSWjaK.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2014-06-07 Thread Roman Bogorodskiy

  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)]
0x00410088 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;
 +

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

2014-05-30 Thread Peter Krempa

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;
+
+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 @@

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

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

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

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

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

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

6 matches

Site Navigation

Mail list logo

Footer information