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@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
