The native gluster pool source list data differs from the data used for
attaching gluster volumes as netfs pools. Currently the only difference
was the format. Since native pools don't use it and later there will be
more difference add a boolean to swithc between the types instead.
---
 src/storage/storage_backend_fs.c      |  5 ++---
 src/storage/storage_backend_gluster.c |  3 +--
 src/storage/storage_util.c            | 10 +++++++---
 src/storage/storage_util.h            |  2 +-
 4 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 1fc127a8c..a1b45e149 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -185,9 +185,8 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSE

     retNFS = virStorageBackendFileSystemNetFindNFSPoolSources(&state);

-    retGluster = virStorageBackendFindGlusterPoolSources(state.host,
-                                                         VIR_STORAGE_POOL_NETFS_GLUSTERFS,
-                                                         &state.list, false);
+    retGluster = virStorageBackendFindGlusterPoolSources(state.host, &state.list,
+                                                         true, false);

     if (retGluster < 0)
         goto cleanup;
diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c
index 52c9ee372..cde7f9edb 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -513,8 +513,7 @@ virStorageBackendGlusterFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED,
     }

     if ((rc = virStorageBackendFindGlusterPoolSources(source->hosts[0].name,
-                                                      0, /* currently ignored */
-                                                      &list, true)) < 0)
+                                                      &list, false, true)) < 0)
         goto cleanup;

     if (rc == 0) {
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 7687eb89a..cad706199 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -2839,18 +2839,21 @@ virStorageBackendDeleteLocal(virConnectPtr conn ATTRIBUTE_UNUSED,
 /**
  * virStorageBackendFindGlusterPoolSources:
  * @host: host to detect volumes on
- * @pooltype: src->format is set to this value
  * @list: list of storage pool sources to be filled
+ * @netfs: lookup will be used with netfs pools
  * @report: report error if the 'gluster' cli tool is missing
  *
  * Looks up gluster volumes on @host and fills them to @list.
  *
+ * If @netfs is specified the data is tweaked so that it can be used with netfs
+ * type pools. Otherwise the data is for use with native gluster pools.
+ *
  * Returns number of volumes on the host on success, or -1 on error.
  */
 int
 virStorageBackendFindGlusterPoolSources(const char *host,
-                                        int pooltype,
                                         virStoragePoolSourceListPtr list,
+                                        bool netfs,
                                         bool report)
 {
     char *glusterpath = NULL;
@@ -2918,7 +2921,8 @@ virStorageBackendFindGlusterPoolSources(const char *host,
         if (VIR_STRDUP(src->hosts[0].name, host) < 0)
             goto cleanup;

-        src->format = pooltype;
+        if (netfs)
+            src->format = VIR_STORAGE_POOL_NETFS_GLUSTERFS;
     }

     ret = nnodes;
diff --git a/src/storage/storage_util.h b/src/storage/storage_util.h
index fa3b6522c..741baa78d 100644
--- a/src/storage/storage_util.h
+++ b/src/storage/storage_util.h
@@ -94,8 +94,8 @@ int virStorageBackendRefreshLocal(virConnectPtr conn,
                                   virStoragePoolObjPtr pool);

 int virStorageBackendFindGlusterPoolSources(const char *host,
-                                            int pooltype,
                                             virStoragePoolSourceListPtr list,
+                                            bool netfs,
                                             bool report);

 bool virStorageBackendDeviceIsEmpty(const char *devpath,
-- 
2.12.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Thu, 2017-03-30 at 17:12 +0200, Peter Krempa wrote:
> The native gluster pool source list data differs from the data used for
> attaching gluster volumes as
netfs pools. Currently the only difference
> was the format. Since native pools don't use it and later there will be
> more difference add a boolean to swithc
between the types instead.

s/difference/differences/
s/swithc/switch/

[...]
> @@ -2839,18 +2839,21 @@ virStorageBackendDeleteLocal(virConnectPtr conn ATTRIBUTE_UNUSED,
>  /**
>   * virStorageBackendFindGlusterPoolSources:
>   * @host: host to detect volumes on
> - * @pooltype: src->format is set to this value
>   * @list: list of storage pool sources to be filled
> + * @netfs: lookup will be used with netfs pools
>   * @report: report error if the 'gluster' cli tool is missing
>   *
>   * Looks up gluster volumes on @host and fills them to @list.
>   *
> + * If @netfs is specified the data is tweaked so that it can be used with netfs
> + * type pools. Otherwise the data is for use with native gluster pools.
> + *
>   * Returns number of volumes on the host on success, or -1 on error.
>   */
>  int
>  virStorageBackendFindGlusterPoolSources(const char *host,
> -                                        int pooltype,
>                                          virStoragePoolSourceListPtr list,
> +                                        bool netfs,

I suggest using virStoragePoolType instead of bool here, eg.
callers will pass either VIR_STORAGE_POOL_GLUSTER for native
gluster pools or VIR_STORAGE_POOL_NETFS for netfs pools.
Passing any other value in the enumeration would of course
result in an error.

This would make the calling sites less opaque.

[...]
> @@ -2918,7 +2921,8 @@ virStorageBackendFindGlusterPoolSources(const char *host,
>          if (VIR_STRDUP(src->hosts[0].name, host) < 0)
>              goto cleanup;
> 
> -        src->format = pooltype;
> +        if (netfs)
> +            src->format = VIR_STORAGE_POOL_NETFS_GLUSTERFS;

In patch 5/5 you're going to move this chunk of code earlier
in the function while changing it: I suggest you move it in
this patch instead, so there's only a single code motion
instead of two.


ACK with the above suggestions addressed, or if you manage
to convince me that they're best left unaddressed :)

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list