From c1286d50e2598972815394bfb6ae49c532ec48a8 Mon Sep 17 00:00:00 2001 From: Martin Kletzander Date: Mon, 21 Dec 2020 14:48:49 +0100 Subject: [PATCH] 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 --- src/esx/esx_driver.c | 31 ++++++++++++++++++------------- src/vmware/vmware_conf.c | 20 ++++++++++++++------ src/vmware/vmware_conf.h | 9 ++++++++- src/vmware/vmware_driver.c | 6 +++--- src/vmx/vmx.c | 25 ++++++++++++------------- src/vmx/vmx.h | 2 +- tests/vmx2xmltest.c | 21 ++++++++++++--------- 7 files changed, 68 insertions(+), 46 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 51c26e29c6..86d5396147 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//' */ @@ -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 e44673247f..4f7dc3001d 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -139,7 +139,7 @@ vmwareLoadDomains(struct vmware_driver *driver) char *saveptr = NULL; virCommandPtr cmd; - ctx.parseFileName = vmwareCopyVMXFileName; + ctx.parseFileName = vmwareParseVMXFileName; ctx.formatFileName = NULL; ctx.autodetectSCSIControllerModel = NULL; ctx.datacenterPath = NULL; @@ -507,11 +507,19 @@ vmwareExtractPid(const char * vmxPath) return pid_value; } -char * -vmwareCopyVMXFileName(const char *datastorePath, void *opaque G_GNUC_UNUSED) +int +vmwareParseVMXFileName(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; +} + +char * +vmwareFormatVMXFileName(const char *datastorePath, + void *opaque G_GNUC_UNUSED) +{ + return g_strdup(datastorePath); } diff --git a/src/vmware/vmware_conf.h b/src/vmware/vmware_conf.h index 5e0ef3744f..6f86983f51 100644 --- a/src/vmware/vmware_conf.h +++ b/src/vmware/vmware_conf.h @@ -83,4 +83,11 @@ int vmwareMakePath(char *srcDir, char *srcName, char *srcExt, int vmwareExtractPid(const char * vmxPath); -char *vmwareCopyVMXFileName(const char *datastorePath, void *opaque); +int +vmwareParseVMXFileName(const char *datastorePath, + void *opaque, + char **out); + +char * +vmwareFormatVMXFileName(const char *datastorePath, + void *opaque); diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index 087664a341..481c59ef3c 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -409,7 +409,7 @@ vmwareDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int fla parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; ctx.parseFileName = NULL; - ctx.formatFileName = vmwareCopyVMXFileName; + ctx.formatFileName = vmwareFormatVMXFileName; ctx.autodetectSCSIControllerModel = NULL; ctx.datacenterPath = NULL; @@ -662,7 +662,7 @@ vmwareDomainCreateXML(virConnectPtr conn, const char *xml, parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; ctx.parseFileName = NULL; - ctx.formatFileName = vmwareCopyVMXFileName; + ctx.formatFileName = vmwareFormatVMXFileName; ctx.autodetectSCSIControllerModel = NULL; ctx.datacenterPath = NULL; @@ -950,7 +950,7 @@ vmwareConnectDomainXMLFromNative(virConnectPtr conn, const char *nativeFormat, return NULL; } - ctx.parseFileName = vmwareCopyVMXFileName; + ctx.parseFileName = vmwareParseVMXFileName; ctx.formatFileName = NULL; ctx.autodetectSCSIControllerModel = NULL; ctx.datacenterPath = NULL; diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index b86dbe9ca2..97591842f7 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2388,7 +2388,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con */ goto ignore; } else if (virStringHasCaseSuffix(fileName, ".vmdk")) { - char *tmp; + char *tmp = NULL; if (deviceType != NULL) { if (busType == VIR_DOMAIN_DISK_BUS_SCSI && @@ -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); @@ -2438,7 +2438,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con */ goto ignore; } else if (fileName && virStringHasCaseSuffix(fileName, ".iso")) { - char *tmp; + char *tmp = NULL; if (deviceType && STRCASENEQ(deviceType, "cdrom-image")) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -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 df5d39157b..e5420c970a 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 8cc227bbed..bb7c498d1b 100644 --- a/tests/vmx2xmltest.c +++ b/tests/vmx2xmltest.c @@ -127,15 +127,18 @@ 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; + + *src = NULL; if (STRPREFIX(fileName, "/vmfs/volumes/")) { /* Found absolute path referencing a file inside a datastore */ @@ -145,22 +148,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