1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH v3 0/5] Fix gluster pool lookup issues
  4. View patch

[libvirt] [PATCH v3 5/5] storage: gluster: Use volume name as "<name>" field in the XML

Peter Krempa posted 5 patches 8 years, 10 months ago
[libvirt] [PATCH v3 5/5] storage: gluster: Use volume name as "<name>" field in the XML
Posted by Peter Krempa 8 years, 10 months ago 
For native gluster pools the <dir> field denotes a directory inside the
pool. For the actual pool name the <name> field has to be used.
---
 src/storage/storage_util.c                               | 16 ++++++++++++++--
 tests/virstorageutildata/gluster-parse-basic-native.xml  |  3 ++-
 .../virstorageutildata/gluster-parse-multivol-native.xml |  9 ++++++---
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 1e44a2da4..7cc125a38 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -2846,6 +2846,7 @@ virStorageUtilGlusterExtractPoolSources(const char *host,
     xmlXPathContextPtr ctxt = NULL;
     xmlNodePtr *nodes = NULL;
     virStoragePoolSource *src = NULL;
+    char *volname;
     size_t i;
     int nnodes;
     int ret = -1;
@@ -2862,14 +2863,25 @@ virStorageUtilGlusterExtractPoolSources(const char *host,
         if (!(src = virStoragePoolSourceListNewSource(list)))
             goto cleanup;

-        if (!(src->dir = virXPathString("string(./name)", ctxt))) {
+        if (!(volname = virXPathString("string(./name)", ctxt))) {
             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                            _("failed to extract gluster volume name"));
             goto cleanup;
         }

-        if (pooltype == VIR_STORAGE_POOL_NETFS)
+        if (pooltype == VIR_STORAGE_POOL_NETFS) {
             src->format = VIR_STORAGE_POOL_NETFS_GLUSTERFS;
+            src->dir = volname;
+        } else if (pooltype == VIR_STORAGE_POOL_GLUSTER) {
+            src->name = volname;
+
+            if (VIR_STRDUP(src->dir, "/") < 0)
+                goto cleanup;
+        } else {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("unsupported gluster lookup"));
+            goto cleanup;
+        }

         if (VIR_ALLOC_N(src->hosts, 1) < 0)
             goto cleanup;
diff --git a/tests/virstorageutildata/gluster-parse-basic-native.xml b/tests/virstorageutildata/gluster-parse-basic-native.xml
index fbde06f3b..895d0f3fd 100644
--- a/tests/virstorageutildata/gluster-parse-basic-native.xml
+++ b/tests/virstorageutildata/gluster-parse-basic-native.xml
@@ -1,6 +1,7 @@
 <sources>
   <source>
     <host name='testhost'/>
-    <dir path='vol0'/>
+    <dir path='/'/>
+    <name>vol0</name>
   </source>
 </sources>
diff --git a/tests/virstorageutildata/gluster-parse-multivol-native.xml b/tests/virstorageutildata/gluster-parse-multivol-native.xml
index d2d8fefc6..c758ac5aa 100644
--- a/tests/virstorageutildata/gluster-parse-multivol-native.xml
+++ b/tests/virstorageutildata/gluster-parse-multivol-native.xml
@@ -1,14 +1,17 @@
 <sources>
   <source>
     <host name='testhost'/>
-    <dir path='aaa'/>
+    <dir path='/'/>
+    <name>aaa</name>
   </source>
   <source>
     <host name='testhost'/>
-    <dir path='test'/>
+    <dir path='/'/>
+    <name>test</name>
   </source>
   <source>
     <host name='testhost'/>
-    <dir path='test1'/>
+    <dir path='/'/>
+    <name>test1</name>
   </source>
 </sources>
-- 
2.12.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 5/5] storage: gluster: Use volume name as "<name>" field in the XML
Posted by John Ferlan 8 years, 10 months ago 

On 04/04/2017 08:20 AM, Peter Krempa wrote:
> For native gluster pools the <dir> field denotes a directory inside the
> pool. For the actual pool name the <name> field has to be used.
> ---
>  src/storage/storage_util.c                               | 16 ++++++++++++++--
>  tests/virstorageutildata/gluster-parse-basic-native.xml  |  3 ++-
>  .../virstorageutildata/gluster-parse-multivol-native.xml |  9 ++++++---
>  3 files changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
> index 1e44a2da4..7cc125a38 100644
> --- a/src/storage/storage_util.c
> +++ b/src/storage/storage_util.c
> @@ -2846,6 +2846,7 @@ virStorageUtilGlusterExtractPoolSources(const char *host,
>      xmlXPathContextPtr ctxt = NULL;
>      xmlNodePtr *nodes = NULL;
>      virStoragePoolSource *src = NULL;
> +    char *volname;
>      size_t i;
>      int nnodes;
>      int ret = -1;
> @@ -2862,14 +2863,25 @@ virStorageUtilGlusterExtractPoolSources(const char *host,
>          if (!(src = virStoragePoolSourceListNewSource(list)))
>              goto cleanup;
> 
> -        if (!(src->dir = virXPathString("string(./name)", ctxt))) {
> +        if (!(volname = virXPathString("string(./name)", ctxt))) {
>              virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                             _("failed to extract gluster volume name"));
>              goto cleanup;
>          }
> 
> -        if (pooltype == VIR_STORAGE_POOL_NETFS)
> +        if (pooltype == VIR_STORAGE_POOL_NETFS) {
>              src->format = VIR_STORAGE_POOL_NETFS_GLUSTERFS;
> +            src->dir = volname;
> +        } else if (pooltype == VIR_STORAGE_POOL_GLUSTER) {
> +            src->name = volname;
> +
> +            if (VIR_STRDUP(src->dir, "/") < 0)
> +                goto cleanup;
> +        } else {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("unsupported gluster lookup"));
> +            goto cleanup;

Coverity gleefully tells me volname is leaked this morning...

John 	

> +        }
> 
>          if (VIR_ALLOC_N(src->hosts, 1) < 0)
>              goto cleanup;
[...]

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 5/5] storage: gluster: Use volume name as "<name>" field in the XML
Posted by Peter Krempa 8 years, 10 months ago 
On Wed, Apr 05, 2017 at 07:03:39 -0400, John Ferlan wrote:
> 
> 
> On 04/04/2017 08:20 AM, Peter Krempa wrote:
> > For native gluster pools the <dir> field denotes a directory inside the
> > pool. For the actual pool name the <name> field has to be used.
> > ---
> >  src/storage/storage_util.c                               | 16 ++++++++++++++--
> >  tests/virstorageutildata/gluster-parse-basic-native.xml  |  3 ++-
> >  .../virstorageutildata/gluster-parse-multivol-native.xml |  9 ++++++---
> >  3 files changed, 22 insertions(+), 6 deletions(-)
> > 
> > diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
> > index 1e44a2da4..7cc125a38 100644
> > --- a/src/storage/storage_util.c
> > +++ b/src/storage/storage_util.c
> > @@ -2846,6 +2846,7 @@ virStorageUtilGlusterExtractPoolSources(const char *host,
> >      xmlXPathContextPtr ctxt = NULL;
> >      xmlNodePtr *nodes = NULL;
> >      virStoragePoolSource *src = NULL;
> > +    char *volname;
> >      size_t i;
> >      int nnodes;
> >      int ret = -1;
> > @@ -2862,14 +2863,25 @@ virStorageUtilGlusterExtractPoolSources(const char *host,
> >          if (!(src = virStoragePoolSourceListNewSource(list)))
> >              goto cleanup;
> > 
> > -        if (!(src->dir = virXPathString("string(./name)", ctxt))) {
> > +        if (!(volname = virXPathString("string(./name)", ctxt))) {
> >              virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> >                             _("failed to extract gluster volume name"));
> >              goto cleanup;
> >          }
> > 
> > -        if (pooltype == VIR_STORAGE_POOL_NETFS)
> > +        if (pooltype == VIR_STORAGE_POOL_NETFS) {
> >              src->format = VIR_STORAGE_POOL_NETFS_GLUSTERFS;
> > +            src->dir = volname;
> > +        } else if (pooltype == VIR_STORAGE_POOL_GLUSTER) {
> > +            src->name = volname;
> > +
> > +            if (VIR_STRDUP(src->dir, "/") < 0)
> > +                goto cleanup;
> > +        } else {
> > +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > +                           _("unsupported gluster lookup"));
> > +            goto cleanup;
> 
> Coverity gleefully tells me volname is leaked this morning...

Sigh. I blame Andrea for his idea of passing pooltype, which resulted
into this. It won't ever happen since the else section is dead code,
but I'll add the free in this case since it's an actual bug.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.