[libvirt] [PATCH] esx: Rework datastore path parsing and handling

[Eric Blake]

On 08/26/2010 04:37 PM, Matthias Bolte wrote:
> Instead of splitting the path part of a datastore path into
> directory and file name, keep this in one piece. An example:
>
>    "[datastore] directory/file"
>
> was split into this before:
>
>    datastoreName = "datastore"
>    directoryName = "directory"
>    fileName = "file"
>
> Now it's split into this:
>
>    datastoreName = "datastore"
>    directoryName = "directory"
>    directoryAndFileName = "directory/file"

Seems reasonable.

> This simplifies code using esxUtil_ParseDatastorePath, because
> directoryAndFileName is used more often than fileName. Also the
> old approach expected the datastore path to reference a actual

s/ a / an /

> @@ -52,8 +52,7 @@ typedef struct _esxVMX_Data esxVMX_Data;
>
>   struct _esxVMX_Data {
>       esxVI_Context *ctx;
> -    const char *datastoreName;
> -    const char *directoryName;
> +    char *datastorePathWithoutFileName;

I guess losing the const here is okay.

> @@ -2612,8 +2608,9 @@ esxDomainDumpXML(virDomainPtr domain, int flags)
>       esxVI_ObjectContent_Free(&virtualMachine);
>       VIR_FREE(datastoreName);
>       VIR_FREE(directoryName);
> -    VIR_FREE(fileName);
> +    VIR_FREE(directoryAndFileName);
>       VIR_FREE(url);
> +    VIR_FREE(data.datastorePathWithoutFileName);
>       VIR_FREE(vmx);
>       virDomainDefFree(def);
>
> @@ -2640,8 +2637,7 @@ esxDomainXMLFromNative(virConnectPtr conn, const char *nativeFormat,
>       }
>
>       data.ctx = priv->primary;
> -    data.datastoreName = "?";
> -    data.directoryName = "?";
> +    data.datastorePathWithoutFileName = (char *)"[?] ?";

Are you missing a strdup() here?  I'm worried that the 
VIR_FREE(data.datastorePathWithoutFileName) in esxDomainDumpXML will now 
try to free static storage.

> diff --git a/src/esx/esx_storage_driver.c b/src/esx/esx_storage_driver.c
> index af8876a..d2d8f22 100644
> --- a/src/esx/esx_storage_driver.c
> +++ b/src/esx/esx_storage_driver.c
> @@ -611,10 +611,8 @@ esxStoragePoolListStorageVolumes(virStoragePoolPtr pool, char **const names,
>       esxVI_HostDatastoreBrowserSearchResults *searchResultsList = NULL;
>       esxVI_HostDatastoreBrowserSearchResults *searchResults = NULL;
>       esxVI_FileInfo *fileInfo = NULL;
> -    char *datastoreName = NULL;
> -    char *directoryName = NULL;
> -    char *fileName = NULL;
> -    char *prefix = NULL;
> +    char *directoryAndFileName = NULL;
> +    int length;

s/int/size_t/

> +++ b/src/esx/esx_vi.c
> @@ -2946,7 +2946,9 @@ esxVI_LookupFileInfoByDatastorePath(esxVI_Context *ctx,
>       int result = -1;
>       char *datastoreName = NULL;
>       char *directoryName = NULL;
> +    char *directoryAndFileName = NULL;
>       char *fileName = NULL;
> +    int length;

s/int/size_t/

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org