[libvirt] [PATCH 2/4] storage: disk: Fix segfault creating volume without target path

[Daniel P. Berrange]

On Fri, Jul 10, 2009 at 03:32:21PM -0400, Cole Robinson wrote:
> Remove unneeded target path duplication, which could carelessly dereference
> NULL. Make it clear where 'key' is actually filled in.
> ---
>  src/storage_backend_disk.c |   33 +++++++++++++++++++--------------
>  1 files changed, 19 insertions(+), 14 deletions(-)
> 
> diff --git a/src/storage_backend_disk.c b/src/storage_backend_disk.c
> index 1eb3eac..e50825b 100644
> --- a/src/storage_backend_disk.c
> +++ b/src/storage_backend_disk.c
> @@ -226,10 +226,18 @@ virStorageBackendDiskMakeVol(virConnectPtr conn,
>      if (STREQ(groups[2], "metadata") ||
>          STREQ(groups[2], "data")) {
>          virStorageVolDefPtr vol = data;
> -        /* We're searching for a specific vol only, so ignore others */
> -        if (vol &&
> -            STRNEQ(vol->name, groups[0]))
> -            return 0;
> +
> +        if (vol) {
> +            /* We're searching for a specific vol only */
> +            if (vol->key) {
> +                if (STRNEQ(vol->key, groups[0]))
> +                    return 0;
> +            } else if (virStorageVolDefFindByKey(pool, groups[0]) != NULL) {
> +                /* If no key, the volume must be newly created. If groups[0]
> +                 * isn't already a volume, assume it's the path we want */
> +                return 0;
> +            }
> +        }
>  
>          return virStorageBackendDiskMakeDataVol(conn, pool, groups, vol);
>      } else if (STREQ(groups[2], "free")) {
> @@ -553,15 +561,9 @@ virStorageBackendDiskCreateVol(virConnectPtr conn,
>          return -1;
>      }
>  
> -    if (vol->key == NULL) {
> -        /* XXX base off a unique key of the underlying disk */
> -        if ((vol->key = strdup(vol->target.path)) == NULL) {
> -            virReportOOMError(conn);
> -            return -1;
> -        }
> -    }
> -
> -    if(virStorageBackendDiskPartBoundries(conn, pool, &startOffset, &endOffset, vol->allocation) != 0) {
> +    if (virStorageBackendDiskPartBoundries(conn, pool, &startOffset,
> +                                           &endOffset,
> +                                           vol->allocation) != 0) {
>         return -1;
>      }
>  
> @@ -580,7 +582,10 @@ virStorageBackendDiskCreateVol(virConnectPtr conn,
>      VIR_FREE(pool->def->source.devices[0].freeExtents);
>      pool->def->source.devices[0].nfreeExtent = 0;
>  
> -    /* Fetch actual extent info */
> +    /* Specifying a target path is meaningless */
> +    VIR_FREE(vol->target.path);
> +
> +    /* Fetch actual extent info, generate key */
>      if (virStorageBackendDiskReadPartitions(conn, pool, vol) < 0)
>          return -1;
>  

ACK

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|