[libvirt] [PATCH] storage: Cleanup logical volume creation code

[John Ferlan]

On 03/06/2013 08:04 AM, Jiri Denemark wrote:
> This patch plugs two memory leaks, removes some useless and confusing
> constructs and renames renames "cleanup" label as "error" since it is
> only used for error path rather then being common for both success and
> error paths.
> ---
>  src/storage/storage_backend_logical.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
> index bb709df..4757fe7 100644
> --- a/src/storage/storage_backend_logical.c
> +++ b/src/storage/storage_backend_logical.c
> @@ -700,7 +700,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
>                                    virStoragePoolObjPtr pool,
>                                    virStorageVolDefPtr vol)
>  {
> -    int fdret, fd = -1;
> +    int fd = -1;
>      virCommandPtr cmd = NULL;
>      virErrorPtr err;
>  
> @@ -741,11 +741,12 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
>          virCommandAddArg(cmd, pool->def->source.name);
>  
>      if (virCommandRun(cmd, NULL) < 0)
> -        goto cleanup;
> +        goto error;
>  
> -    if ((fdret = virStorageBackendVolOpen(vol->target.path)) < 0)
> -        goto cleanup;
> -    fd = fdret;
> +    virCommandFree(cmd);

You'll need a 'cmd = NULL' here so that we don't end up in error: making
the same call (virCommandFree() does call VIR_FREE(cmd), but that only
sets cmd = NULL in that context, when we return here it's not NULL anymore).

> +
> +    if ((fd = virStorageBackendVolOpen(vol->target.path)) < 0)
> +        goto error;
>  
>      /* We can only chown/grp if root */
>      if (getuid() == 0) {
> @@ -753,40 +754,40 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
>              virReportSystemError(errno,
>                                   _("cannot set file owner '%s'"),
>                                   vol->target.path);
> -            goto cleanup;
> +            goto error;
>          }
>      }
>      if (fchmod(fd, vol->target.perms.mode) < 0) {
>          virReportSystemError(errno,
>                               _("cannot set file mode '%s'"),
>                               vol->target.path);
> -        goto cleanup;
> +        goto error;
>      }
>  
>      if (VIR_CLOSE(fd) < 0) {
>          virReportSystemError(errno,
>                               _("cannot close file '%s'"),
>                               vol->target.path);
> -        goto cleanup;
> +        goto error;
>      }
> -    fd = -1;
>  
>      /* Fill in data about this new vol */
>      if (virStorageBackendLogicalFindLVs(pool, vol) < 0) {
>          virReportSystemError(errno,
>                               _("cannot find newly created volume '%s'"),
>                               vol->target.path);
> -        goto cleanup;
> +        goto error;
>      }
>  
>      return 0;
>  
> - cleanup:
> + error:
>      err = virSaveLastError();
>      VIR_FORCE_CLOSE(fd);
>      virStorageBackendLogicalDeleteVol(conn, pool, vol, 0);
>      virCommandFree(cmd);
>      virSetError(err);
> +    virFreeError(err);
>      return -1;
>  }
>  
>