Re: [libvirt PATCH v2 3/5] vmx: Make virVMXParseFileName return an integer

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(a)redhat.com&gt; --- 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);

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