[libvirt PATCH v2 3/5] vmx: Make virVMXParseFileName return an integer
Daniel Henrique Barboza
danielhb413 at gmail.com
Mon Jan 4 23:32:35 UTC 2021
On 12/21/20 1:19 PM, Martin Kletzander wrote:
> And return the actual extracted value in a parameter. This way we can later
> return success even without any extracted value.
>
> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> ---
> src/esx/esx_driver.c | 31 ++++++++++++++++++-------------
> src/vmware/vmware_conf.c | 10 +++++-----
> src/vmx/vmx.c | 21 ++++++++++-----------
> src/vmx/vmx.h | 2 +-
> tests/vmx2xmltest.c | 19 ++++++++++---------
> 5 files changed, 44 insertions(+), 39 deletions(-)
>
> diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
> index 51c26e29c65e..86d5396147a3 100644
> --- a/src/esx/esx_driver.c
> +++ b/src/esx/esx_driver.c
> @@ -125,10 +125,11 @@ esxFreePrivate(esxPrivate **priv)
> * exception and need special handling. Parse the datastore name and use it
> * to lookup the datastore by name to verify that it exists.
> */
> -static char *
> -esxParseVMXFileName(const char *fileName, void *opaque)
> +static int
> +esxParseVMXFileName(const char *fileName,
> + void *opaque,
> + char **out)
> {
> - char *result = NULL;
> esxVMX_Data *data = opaque;
> esxVI_String *propertyNameList = NULL;
> esxVI_ObjectContent *datastoreList = NULL;
> @@ -140,18 +141,22 @@ esxParseVMXFileName(const char *fileName, void *opaque)
> char *strippedFileName = NULL;
> char *copyOfFileName = NULL;
> char *directoryAndFileName;
> + int ret = -1;
> +
> + *out = NULL;
>
> if (!strchr(fileName, '/') && !strchr(fileName, '\\')) {
> /* Plain file name, use same directory as for the .vmx file */
> - return g_strdup_printf("%s/%s", data->datastorePathWithoutFileName,
> + *out = g_strdup_printf("%s/%s", data->datastorePathWithoutFileName,
> fileName);
> + return 0;
> }
>
> if (esxVI_String_AppendValueToList(&propertyNameList,
> "summary.name") < 0 ||
> esxVI_LookupDatastoreList(data->ctx, propertyNameList,
> &datastoreList) < 0) {
> - return NULL;
> + return -1;
> }
>
> /* Search for datastore by mount path */
> @@ -189,13 +194,13 @@ esxParseVMXFileName(const char *fileName, void *opaque)
> ++tmp;
> }
>
> - result = g_strdup_printf("[%s] %s", datastoreName, strippedFileName);
> + *out = g_strdup_printf("[%s] %s", datastoreName, strippedFileName);
>
> break;
> }
>
> /* Fallback to direct datastore name match */
> - if (!result && STRPREFIX(fileName, "/vmfs/volumes/")) {
> + if (!*out && STRPREFIX(fileName, "/vmfs/volumes/")) {
> copyOfFileName = g_strdup(fileName);
>
> /* Expected format: '/vmfs/volumes/<datastore>/<path>' */
> @@ -223,22 +228,22 @@ esxParseVMXFileName(const char *fileName, void *opaque)
> goto cleanup;
> }
>
> - result = g_strdup_printf("[%s] %s", datastoreName,
> - directoryAndFileName);
> + *out = g_strdup_printf("[%s] %s", datastoreName, directoryAndFileName);
> }
>
> /* If it's an absolute path outside of a datastore just use it as is */
> - if (!result && *fileName == '/') {
> + if (!*out && *fileName == '/') {
> /* FIXME: need to deal with Windows paths here too */
> - result = g_strdup(fileName);
> + *out = g_strdup(fileName);
> }
>
> - if (!result) {
> + if (!*out) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Could not handle file name '%s'"), fileName);
> goto cleanup;
> }
>
> + ret = 0;
> cleanup:
> esxVI_String_Free(&propertyNameList);
> esxVI_ObjectContent_Free(&datastoreList);
> @@ -246,7 +251,7 @@ esxParseVMXFileName(const char *fileName, void *opaque)
> VIR_FREE(strippedFileName);
> VIR_FREE(copyOfFileName);
>
> - return result;
> + return ret;
> }
>
>
> diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c
> index e44673247ff1..c90cb10faf7c 100644
> --- a/src/vmware/vmware_conf.c
> +++ b/src/vmware/vmware_conf.c
> @@ -507,11 +507,11 @@ vmwareExtractPid(const char * vmxPath)
> return pid_value;
> }
>
> -char *
> -vmwareCopyVMXFileName(const char *datastorePath, void *opaque G_GNUC_UNUSED)
> +int
> +vmwareCopyVMXFileName(const char *datastorePath, void *opaque G_GNUC_UNUSED,
> + char **out)
> {
> - char *path;
> + *out = g_strdup(datastorePath);
>
> - path = g_strdup(datastorePath);
> - return path;
> + return *out ? 0 : -1;
> }
> diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
> index b86dbe9ca267..b2b2244415a1 100644
> --- a/src/vmx/vmx.c
> +++ b/src/vmx/vmx.c
> @@ -2411,7 +2411,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
> }
>
> virDomainDiskSetType(*def, VIR_STORAGE_TYPE_FILE);
> - if (!(tmp = ctx->parseFileName(fileName, ctx->opaque)))
> + if (ctx->parseFileName(fileName, ctx->opaque, &tmp) < 0)
> goto cleanup;
> virDomainDiskSetSource(*def, tmp);
> VIR_FREE(tmp);
> @@ -2448,7 +2448,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
> }
>
> virDomainDiskSetType(*def, VIR_STORAGE_TYPE_FILE);
> - if (!(tmp = ctx->parseFileName(fileName, ctx->opaque)))
> + if (ctx->parseFileName(fileName, ctx->opaque, &tmp) < 0)
> goto cleanup;
> virDomainDiskSetSource(*def, tmp);
> VIR_FREE(tmp);
> @@ -2514,7 +2514,8 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
> char *tmp = NULL;
>
> virDomainDiskSetType(*def, VIR_STORAGE_TYPE_FILE);
> - if (fileName && !(tmp = ctx->parseFileName(fileName, ctx->opaque)))
> + if (fileName &&
> + ctx->parseFileName(fileName, ctx->opaque, &tmp) < 0)
> goto cleanup;
> virDomainDiskSetSource(*def, tmp);
> VIR_FREE(tmp);
> @@ -2974,10 +2975,9 @@ virVMXParseSerial(virVMXContext *ctx, virConfPtr conf, int port,
> } else if (STRCASEEQ(fileType, "file")) {
> (*def)->target.port = port;
> (*def)->source->type = VIR_DOMAIN_CHR_TYPE_FILE;
> - (*def)->source->data.file.path = ctx->parseFileName(fileName,
> - ctx->opaque);
> -
> - if ((*def)->source->data.file.path == NULL)
> + if (ctx->parseFileName(fileName,
> + ctx->opaque,
> + &(*def)->source->data.file.path) < 0)
> goto cleanup;
> } else if (STRCASEEQ(fileType, "pipe")) {
> /*
> @@ -3140,10 +3140,9 @@ virVMXParseParallel(virVMXContext *ctx, virConfPtr conf, int port,
> } else if (STRCASEEQ(fileType, "file")) {
> (*def)->target.port = port;
> (*def)->source->type = VIR_DOMAIN_CHR_TYPE_FILE;
> - (*def)->source->data.file.path = ctx->parseFileName(fileName,
> - ctx->opaque);
> -
> - if ((*def)->source->data.file.path == NULL)
> + if (ctx->parseFileName(fileName,
> + ctx->opaque,
> + &(*def)->source->data.file.path) < 0)
> goto cleanup;
> } else {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> diff --git a/src/vmx/vmx.h b/src/vmx/vmx.h
> index df5d39157b99..e5420c970a4b 100644
> --- a/src/vmx/vmx.h
> +++ b/src/vmx/vmx.h
> @@ -36,7 +36,7 @@ virDomainXMLOptionPtr virVMXDomainXMLConfInit(virCapsPtr caps);
> * Context
> */
>
> -typedef char * (*virVMXParseFileName)(const char *fileName, void *opaque);
> +typedef int (*virVMXParseFileName)(const char *fileName, void *opaque, char **src);
This change is making the build break in my env:
../src/vmware/vmware_conf.c: In function ‘vmwareLoadDomains’:
../src/vmware/vmware_conf.c:142:23: error: assignment to ‘virVMXParseFileName’ {aka ‘int (*)(const char *, void *, char **)’} from incompatible pointer type ‘char * (*)(const char *, void *)’ [-Werror=incompatible-pointer-types]
142 | ctx.parseFileName = vmwareCopyVMXFileName;
| ^
../src/vmware/vmware_conf.c: At top level:
../src/vmware/vmware_conf.c:511:1: error: conflicting types for ‘vmwareCopyVMXFileName’
511 | vmwareCopyVMXFileName(const char *datastorePath, void *opaque G_GNUC_UNUSED,
| ^~~~~~~~~~~~~~~~~~~~~
In file included from ../src/vmware/vmware_conf.c:32:
../src/vmware/vmware_conf.h:86:7: note: previous declaration of ‘vmwareCopyVMXFileName’ was here
86 | char *vmwareCopyVMXFileName(const char *datastorePath, void *opaque);
| ^~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
(...)
../src/vmware/vmware_driver.c: In function ‘vmwareConnectDomainXMLFromNative’:
../src/vmware/vmware_driver.c:953:23: error: assignment to ‘virVMXParseFileName’ {aka ‘int (*)(const char *, void *, char **)’} from incompatible pointer type ‘char * (*)(const char *, void *)’ [-Werror=incompatible-pointer-types]
953 | ctx.parseFileName = vmwareCopyVMXFileName;
| ^
cc1: all warnings being treated as errors
I believe there are a handful of virVMXParseFileName impl instances that needs
to be changed as well.
Thanks,
DHB
> typedef char * (*virVMXFormatFileName)(const char *src, void *opaque);
> typedef int (*virVMXAutodetectSCSIControllerModel)(virDomainDiskDefPtr def,
> int *model, void *opaque);
> diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c
> index 8cc227bbedfc..412b201f0242 100644
> --- a/tests/vmx2xmltest.c
> +++ b/tests/vmx2xmltest.c
> @@ -127,15 +127,16 @@ testCompareHelper(const void *data)
> return ret;
> }
>
> -static char *
> -testParseVMXFileName(const char *fileName, void *opaque G_GNUC_UNUSED)
> +static int
> +testParseVMXFileName(const char *fileName,
> + void *opaque G_GNUC_UNUSED,
> + char **src)
> {
> g_autofree char *copyOfFileName = NULL;
> char *tmp = NULL;
> char *saveptr = NULL;
> char *datastoreName = NULL;
> char *directoryAndFileName = NULL;
> - char *src = NULL;
>
> if (STRPREFIX(fileName, "/vmfs/volumes/")) {
> /* Found absolute path referencing a file inside a datastore */
> @@ -145,22 +146,22 @@ testParseVMXFileName(const char *fileName, void *opaque G_GNUC_UNUSED)
> if ((tmp = STRSKIP(copyOfFileName, "/vmfs/volumes/")) == NULL ||
> (datastoreName = strtok_r(tmp, "/", &saveptr)) == NULL ||
> (directoryAndFileName = strtok_r(NULL, "", &saveptr)) == NULL) {
> - return NULL;
> + return -1;
> }
>
> - src = g_strdup_printf("[%s] %s", datastoreName, directoryAndFileName);
> + *src = g_strdup_printf("[%s] %s", datastoreName, directoryAndFileName);
> } else if (STRPREFIX(fileName, "/")) {
> /* Found absolute path referencing a file outside a datastore */
> - src = g_strdup(fileName);
> + *src = g_strdup(fileName);
> } else if (strchr(fileName, '/') != NULL) {
> /* Found relative path, this is not supported */
> - return NULL;
> + return -1;
> } else {
> /* Found single file name referencing a file inside a datastore */
> - src = g_strdup_printf("[datastore] directory/%s", fileName);
> + *src = g_strdup_printf("[datastore] directory/%s", fileName);
> }
>
> - return src;
> + return 0;
> }
>
> static int
>
More information about the libvir-list
mailing list