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@redhat.com>

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 *
+static int
-esxParseVMXFileName(const char *fileName, void *opaque)
+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,
+        *out = g_strdup_printf("[%s] %s", datastoreName, directoryAndFileName);
-                                 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;
 }
 
 

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 *
+int
-vmwareCopyVMXFileName(const char *datastorePath, void *opaque G_GNUC_UNUSED)
+vmwareParseVMXFileName(const char *datastorePath,
+                       void *opaque G_GNUC_UNUSED,
+                       char **out)
 {
-    char *path;
+    *out = g_strdup(datastorePath);
 
-    path = g_strdup(datastorePath);
+    return *out ? 0 : -1;
-    return path;
+}
+
+char *
+vmwareFormatVMXFileName(const char *datastorePath,
+                        void *opaque G_GNUC_UNUSED)
+{
+    return g_strdup(datastorePath);
 }

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);

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;

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,
+        if (ctx->parseFileName(fileName,
-                                                            ctx->opaque);
+                              ctx->opaque,
-
+                              &(*def)->source->data.file.path) < 0)
-        if ((*def)->source->data.file.path == NULL)
             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,
+        if (ctx->parseFileName(fileName,
-                                                            ctx->opaque);
+                              ctx->opaque,
-
+                              &(*def)->source->data.file.path) < 0)
-        if ((*def)->source->data.file.path == NULL)
             goto cleanup;
     } else {
         virReportError(VIR_ERR_INTERNAL_ERROR,

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);

tests/vmx2xmltest.c

@@ -127,15 +127,18 @@ testCompareHelper(const void *data)
     return ret;
 }
 
-static char *
+static int
-testParseVMXFileName(const char *fileName, void *opaque G_GNUC_UNUSED)
+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