Re: [libvirt] [PATCH] esx: Rework datastore path parsing and handling
2010/9/1 Eric Blake <eblake(a)redhat.com>:
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 /
I'll fix that before pushing.
> @@ -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.
It should have never been const in the first place.
> @@ -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.
This is in esxDomainXMLFromNative, data.datastorePathWithoutFileName
doesn't get freed here, so not strdup'ing is fine here.
In esxDomainDumpXML data.datastorePathWithoutFileName is allocated via
virAsprintf and therefore it need to be freed in esxDomainDumpXML.
So nothing to change here.
> 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/
I'll fix those two before pushing.
Matthias