[libvirt] [PATCH v2 1/5] storage: util: Add boolean differentiating between gluster lookup type

[Andrea Bolognani]

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