[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 03/10] storage: gluster: Introduce dummy functions for creating a volume



On 01/09/2014 09:15 AM, Peter Krempa wrote:
> The temporary pool code will need to initialize dummy gluster volumes
> which needs the createVol function of the storage backend. This patch
> implements it only for that purpose. When an user will get an error

s/When an user/A user/

> message that it is not implemented on an attempt to create a volume in a
> gluster pool.
> ---
>  src/storage/storage_backend_gluster.c | 68 +++++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
> 

> +
> +    VIR_FREE(vol->key);
> +    if (virAsprintf(&vol->key, "/%s%s%s", state->volname, state->dir,
> +                    vol->name) < 0)

This doesn't match the docs, which say gluster volume keys lack a
leading slash; see also virStorageBackendGlusterRefreshVol (where it
took me a couple tries to get it right).

> +        goto cleanup;
> +
> +    tmp = state->uri->path;
> +    state->uri->path = vol->key;
> +    VIR_FREE(vol->target.path);
> +    if (!(vol->target.path = virURIFormat(state->uri))) {
> +        state->uri->path = tmp;
> +        goto cleanup;
> +    }
> +    state->uri->path = tmp;

In fact, since both this function and virStorageBackendGlusterRefreshVol
are doing the same thing, it might be nice to factor it into a helper
function, so that if we change our minds yet again about the leading
slash in gluster key names, we only have to change one spot in code.

> +
> +    if (internal) {
> +        if (glfs_stat(state->vol, vol->name, &st) == 0 &&
> +            S_ISDIR(st.st_mode)) {
> +            vol->type = VIR_STORAGE_VOL_NETDIR;
> +            vol->target.format = VIR_STORAGE_FILE_DIR;
> +        }
> +    } else {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("gluster pool backend doesn't "
> +                         "yet support volume creation"));
> +        goto cleanup;

It might be nice to hoist the error reporting earlier in the function,
instead of first doing malloc's only to throw them away.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]