10:19 a.m.
This is perfectly valid in VMWare and the VM just boots with an empty drive. We
used to just skip the whole drive before, but since we changed how we parse
empty cdrom drives this now results in an error and the user not being able to
even dump the XML. Instead of erroring out, just keep the drive empty.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1903953
v2:
- Do not report and reset an error, but handle it more nicely.
v1:
- https://www.redhat.com/archives/libvir-list/2020-December/msg00840.html
Martin Kletzander (5):
esx: Unindent unnecessary conditional branch
tests: Use g_autofree in testParseVMXFileName
vmx: Make virVMXParseFileName return an integer
vmx: Allow missing cdrom image file in virVMXParseFileName
vmx: Treat missing cdrom-image as empty drive
src/esx/esx_driver.c | 160 ++++++++++--------
src/vmware/vmware_conf.c | 10 +-
src/vmx/vmx.c | 25 +--
src/vmx/vmx.h | 5 +-
.../vmx2xmldata/vmx2xml-cdrom-ide-missing.vmx | 6 +
tests/vmx2xmltest.c | 36 ++--
6 files changed, 136 insertions(+), 106 deletions(-)
create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-missing.vmx
--
2.29.2
10:19 a.m.
New subject: [libvirt PATCH v2 1/5] esx: Unindent unnecessary conditional branch
The positive branch can just return and the huge negative part does not need to
be indented an extra level. Best viewed with `-w`.
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/esx/esx_driver.c | 144 +++++++++++++++++++++----------------------
1 file changed, 72 insertions(+), 72 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index a17bf58a5124..51c26e29c65e 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -143,100 +143,100 @@ esxParseVMXFileName(const char *fileName, void *opaque)
if (!strchr(fileName, '/') && !strchr(fileName, '\\')) {
/* Plain file name, use same directory as for the .vmx file */
- result = g_strdup_printf("%s/%s",
data->datastorePathWithoutFileName,
- fileName);
- } else {
- if (esxVI_String_AppendValueToList(&propertyNameList,
- "summary.name") < 0 ||
- esxVI_LookupDatastoreList(data->ctx, propertyNameList,
- &datastoreList) < 0) {
- return NULL;
- }
+ return g_strdup_printf("%s/%s", data->datastorePathWithoutFileName,
+ fileName);
+ }
- /* Search for datastore by mount path */
- for (datastore = datastoreList; datastore;
- datastore = datastore->_next) {
- esxVI_DatastoreHostMount_Free(&hostMount);
- datastoreName = NULL;
-
- if (esxVI_LookupDatastoreHostMount(data->ctx, datastore->obj,
- &hostMount,
- esxVI_Occurrence_RequiredItem) < 0 ||
- esxVI_GetStringValue(datastore, "summary.name",
&datastoreName,
- esxVI_Occurrence_RequiredItem) < 0) {
- goto cleanup;
- }
+ if (esxVI_String_AppendValueToList(&propertyNameList,
+ "summary.name") < 0 ||
+ esxVI_LookupDatastoreList(data->ctx, propertyNameList,
+ &datastoreList) < 0) {
+ return NULL;
+ }
- tmp = (char *)STRSKIP(fileName, hostMount->mountInfo->path);
+ /* Search for datastore by mount path */
+ for (datastore = datastoreList; datastore;
+ datastore = datastore->_next) {
+ esxVI_DatastoreHostMount_Free(&hostMount);
+ datastoreName = NULL;
- if (!tmp)
- continue;
+ if (esxVI_LookupDatastoreHostMount(data->ctx, datastore->obj,
+ &hostMount,
+ esxVI_Occurrence_RequiredItem) < 0 ||
+ esxVI_GetStringValue(datastore, "summary.name",
&datastoreName,
+ esxVI_Occurrence_RequiredItem) < 0) {
+ goto cleanup;
+ }
- /* Found a match. Strip leading separators */
- while (*tmp == '/' || *tmp == '\\')
- ++tmp;
+ tmp = (char *)STRSKIP(fileName, hostMount->mountInfo->path);
- strippedFileName = g_strdup(tmp);
+ if (!tmp)
+ continue;
- tmp = strippedFileName;
+ /* Found a match. Strip leading separators */
+ while (*tmp == '/' || *tmp == '\\')
+ ++tmp;
- /* Convert \ to / */
- while (*tmp != '\0') {
- if (*tmp == '\\')
- *tmp = '/';
+ strippedFileName = g_strdup(tmp);
- ++tmp;
- }
+ tmp = strippedFileName;
- result = g_strdup_printf("[%s] %s", datastoreName,
strippedFileName);
+ /* Convert \ to / */
+ while (*tmp != '\0') {
+ if (*tmp == '\\')
+ *tmp = '/';
- break;
+ ++tmp;
}
- /* Fallback to direct datastore name match */
- if (!result && STRPREFIX(fileName, "/vmfs/volumes/")) {
- copyOfFileName = g_strdup(fileName);
-
- /* Expected format: '/vmfs/volumes/<datastore>/<path>'
*/
- if (!(tmp = STRSKIP(copyOfFileName, "/vmfs/volumes/")) ||
- !(datastoreName = strtok_r(tmp, "/", &saveptr)) ||
- !(directoryAndFileName = strtok_r(NULL, "", &saveptr))) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("File name '%s' doesn't have expected
format "
-
"'/vmfs/volumes/<datastore>/<path>'"), fileName);
- goto cleanup;
- }
-
- esxVI_ObjectContent_Free(&datastoreList);
+ result = g_strdup_printf("[%s] %s", datastoreName, strippedFileName);
- if (esxVI_LookupDatastoreByName(data->ctx, datastoreName,
- NULL, &datastoreList,
- esxVI_Occurrence_OptionalItem) < 0) {
- goto cleanup;
- }
+ break;
+ }
- if (!datastoreList) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("File name '%s' refers to non-existing
datastore '%s'"),
- fileName, datastoreName);
- goto cleanup;
- }
+ /* Fallback to direct datastore name match */
+ if (!result && STRPREFIX(fileName, "/vmfs/volumes/")) {
+ copyOfFileName = g_strdup(fileName);
- result = g_strdup_printf("[%s] %s", datastoreName,
- directoryAndFileName);
+ /* Expected format: '/vmfs/volumes/<datastore>/<path>' */
+ if (!(tmp = STRSKIP(copyOfFileName, "/vmfs/volumes/")) ||
+ !(datastoreName = strtok_r(tmp, "/", &saveptr)) ||
+ !(directoryAndFileName = strtok_r(NULL, "", &saveptr))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("File name '%s' doesn't have expected
format "
+
"'/vmfs/volumes/<datastore>/<path>'"), fileName);
+ goto cleanup;
}
- /* If it's an absolute path outside of a datastore just use it as is */
- if (!result && *fileName == '/') {
- /* FIXME: need to deal with Windows paths here too */
- result = g_strdup(fileName);
+ esxVI_ObjectContent_Free(&datastoreList);
+
+ if (esxVI_LookupDatastoreByName(data->ctx, datastoreName,
+ NULL, &datastoreList,
+ esxVI_Occurrence_OptionalItem) < 0) {
+ goto cleanup;
}
- if (!result) {
+ if (!datastoreList) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Could not handle file name '%s'"),
fileName);
+ _("File name '%s' refers to non-existing
datastore '%s'"),
+ fileName, datastoreName);
goto cleanup;
}
+
+ result = 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 == '/') {
+ /* FIXME: need to deal with Windows paths here too */
+ result = g_strdup(fileName);
+ }
+
+ if (!result) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not handle file name '%s'"),
fileName);
+ goto cleanup;
}
cleanup:
--
2.29.2
10:19 a.m.
New subject: [libvirt PATCH v2 2/5] tests: Use g_autofree in testParseVMXFileName
There's only one variable to clean-up, others are just tokens inside that
variable, but it is nicer anyway. Positive returns have not been converted
because the function will change soon and it would not make much sense.
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
tests/vmx2xmltest.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c
index 376116bb750a..8cc227bbedfc 100644
--- a/tests/vmx2xmltest.c
+++ b/tests/vmx2xmltest.c
@@ -130,7 +130,7 @@ testCompareHelper(const void *data)
static char *
testParseVMXFileName(const char *fileName, void *opaque G_GNUC_UNUSED)
{
- char *copyOfFileName = NULL;
+ g_autofree char *copyOfFileName = NULL;
char *tmp = NULL;
char *saveptr = NULL;
char *datastoreName = NULL;
@@ -145,7 +145,7 @@ 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)
{
- goto cleanup;
+ return NULL;
}
src = g_strdup_printf("[%s] %s", datastoreName, directoryAndFileName);
@@ -154,15 +154,12 @@ testParseVMXFileName(const char *fileName, void *opaque
G_GNUC_UNUSED)
src = g_strdup(fileName);
} else if (strchr(fileName, '/') != NULL) {
/* Found relative path, this is not supported */
- src = NULL;
+ return NULL;
} else {
/* Found single file name referencing a file inside a datastore */
src = g_strdup_printf("[datastore] directory/%s", fileName);
}
- cleanup:
- VIR_FREE(copyOfFileName);
-
return src;
}
--
2.29.2
10:19 a.m.
New subject: [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>
---
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
--
2.29.2
5:32 p.m.
New subject: [libvirt PATCH v2 3/5] vmx: Make virVMXParseFileName return an integer
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(a)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
3:15 a.m.
New subject: [libvirt PATCH v2 3/5] vmx: Make virVMXParseFileName return an integer
On 1/5/21 12:32 AM, Daniel Henrique Barboza wrote:
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(a)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/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.
Indeed. I think we will need to change virVMXFormatFileName() too at the
same time, because of vmwareCopyVMXFileName() which is passed as
formatFileName callback.
BTW: I've found out that we don't automatically enable vmware driver. I
had to enable it explicitly:
meson -Dsystem=true -Ddriver_vmware=enabled _build
I'm looking into that.
Michal
5:36 a.m.
New subject: [libvirt PATCH v2 3/5] vmx: Make virVMXParseFileName return an integer
On Tue, Jan 05, 2021 at 10:15:47AM +0100, Michal Privoznik wrote:
On 1/5/21 12:32 AM, Daniel Henrique Barboza wrote:
>
> 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(a)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/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.
Indeed. I think we will need to change virVMXFormatFileName() too at the
same time, because of vmwareCopyVMXFileName() which is passed as
formatFileName callback.
Not only that, but vmware_driver uses the smae function for parsing and
formatting. They are, however, just g_strdup()s, so I can split them no
problem. Thanks for noticing.
Anyway, just looking at the commits they should be done a bit differently, I had
some cock-ups there, I guess. I'll send a v3.
BTW: I've found out that we don't automatically enable vmware
driver. I
had to enable it explicitly:
meson -Dsystem=true -Ddriver_vmware=enabled _build
I'm looking into that.
We're not building it downstream even, I think, I managed to fix it for ESX
where the issue was happening actually. The vmware driver just controls vmware
player/workstation locally. Are there actually people using that?
Michal
10:19 a.m.
New subject: [libvirt PATCH v2 4/5] vmx: Allow missing cdrom image file in virVMXParseFileName
This will be used later.
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/esx/esx_driver.c | 13 +++++++++----
src/vmware/vmware_conf.c | 2 +-
src/vmx/vmx.c | 12 +++++++-----
src/vmx/vmx.h | 5 ++++-
tests/vmx2xmltest.c | 13 ++++++++++++-
5 files changed, 33 insertions(+), 12 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 86d5396147a3..0271f81a5655 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -128,7 +128,8 @@ esxFreePrivate(esxPrivate **priv)
static int
esxParseVMXFileName(const char *fileName,
void *opaque,
- char **out)
+ char **out,
+ bool allow_missing)
{
esxVMX_Data *data = opaque;
esxVI_String *propertyNameList = NULL;
@@ -222,9 +223,13 @@ esxParseVMXFileName(const char *fileName,
}
if (!datastoreList) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("File name '%s' refers to non-existing
datastore '%s'"),
- fileName, datastoreName);
+ if (allow_missing) {
+ ret = 0;
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("File name '%s' refers to non-existing
datastore '%s'"),
+ fileName, datastoreName);
+ }
goto cleanup;
}
diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c
index c90cb10faf7c..4b0832f50b3c 100644
--- a/src/vmware/vmware_conf.c
+++ b/src/vmware/vmware_conf.c
@@ -509,7 +509,7 @@ vmwareExtractPid(const char * vmxPath)
int
vmwareCopyVMXFileName(const char *datastorePath, void *opaque G_GNUC_UNUSED,
- char **out)
+ char **out, bool allow_missing G_GNUC_UNUSED)
{
*out = g_strdup(datastorePath);
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index b2b2244415a1..4d098a5fa4d6 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 (ctx->parseFileName(fileName, ctx->opaque, &tmp) < 0)
+ if (ctx->parseFileName(fileName, ctx->opaque, &tmp, false) < 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 (ctx->parseFileName(fileName, ctx->opaque, &tmp) < 0)
+ if (ctx->parseFileName(fileName, ctx->opaque, &tmp, true) < 0)
goto cleanup;
virDomainDiskSetSource(*def, tmp);
VIR_FREE(tmp);
@@ -2515,7 +2515,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt,
virConfPtr con
virDomainDiskSetType(*def, VIR_STORAGE_TYPE_FILE);
if (fileName &&
- ctx->parseFileName(fileName, ctx->opaque, &tmp) < 0)
+ ctx->parseFileName(fileName, ctx->opaque, &tmp, false) < 0)
goto cleanup;
virDomainDiskSetSource(*def, tmp);
VIR_FREE(tmp);
@@ -2977,7 +2977,8 @@ virVMXParseSerial(virVMXContext *ctx, virConfPtr conf, int port,
(*def)->source->type = VIR_DOMAIN_CHR_TYPE_FILE;
if (ctx->parseFileName(fileName,
ctx->opaque,
- &(*def)->source->data.file.path) < 0)
+ &(*def)->source->data.file.path,
+ false) < 0)
goto cleanup;
} else if (STRCASEEQ(fileType, "pipe")) {
/*
@@ -3142,7 +3143,8 @@ virVMXParseParallel(virVMXContext *ctx, virConfPtr conf, int port,
(*def)->source->type = VIR_DOMAIN_CHR_TYPE_FILE;
if (ctx->parseFileName(fileName,
ctx->opaque,
- &(*def)->source->data.file.path) < 0)
+ &(*def)->source->data.file.path,
+ false) < 0)
goto cleanup;
} else {
virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/vmx/vmx.h b/src/vmx/vmx.h
index e5420c970a4b..550c1264f3b8 100644
--- a/src/vmx/vmx.h
+++ b/src/vmx/vmx.h
@@ -36,7 +36,10 @@ virDomainXMLOptionPtr virVMXDomainXMLConfInit(virCapsPtr caps);
* Context
*/
-typedef int (*virVMXParseFileName)(const char *fileName, void *opaque, char **src);
+typedef int (*virVMXParseFileName)(const char *fileName,
+ void *opaque,
+ char **src,
+ bool allow_missing);
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 412b201f0242..116d729a0147 100644
--- a/tests/vmx2xmltest.c
+++ b/tests/vmx2xmltest.c
@@ -130,7 +130,8 @@ testCompareHelper(const void *data)
static int
testParseVMXFileName(const char *fileName,
void *opaque G_GNUC_UNUSED,
- char **src)
+ char **src,
+ bool allow_missing)
{
g_autofree char *copyOfFileName = NULL;
char *tmp = NULL;
@@ -149,6 +150,16 @@ testParseVMXFileName(const char *fileName,
return -1;
}
+ if (STREQ(datastoreName, "missing") ||
+ STREQ(directoryAndFileName, "missing")) {
+ if (allow_missing)
+ return 0;
+
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "Missing file name '%s'", fileName);
+ return -1;
+ }
+
*src = g_strdup_printf("[%s] %s", datastoreName,
directoryAndFileName);
} else if (STRPREFIX(fileName, "/")) {
/* Found absolute path referencing a file outside a datastore */
--
2.29.2
10:19 a.m.
New subject: [libvirt PATCH v2 5/5] vmx: Treat missing cdrom-image as empty drive
This is perfectly valid in VMWare and the VM just boots with an empty drive. We
used to just skip the whole drive before, but since we changed how we parse
empty cdrom drives this results in an error. Make it behave more closer to
VMWare.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1903953
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/vmx/vmx.c | 2 +-
tests/vmx2xmldata/vmx2xml-cdrom-ide-missing.vmx | 6 ++++++
tests/vmx2xmltest.c | 1 +
3 files changed, 8 insertions(+), 1 deletion(-)
create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-missing.vmx
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 4d098a5fa4d6..2c631e32e7df 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -2447,10 +2447,10 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt,
virConfPtr con
goto cleanup;
}
- virDomainDiskSetType(*def, VIR_STORAGE_TYPE_FILE);
if (ctx->parseFileName(fileName, ctx->opaque, &tmp, true) < 0)
goto cleanup;
virDomainDiskSetSource(*def, tmp);
+ virDomainDiskSetType(*def, VIR_STORAGE_TYPE_FILE);
VIR_FREE(tmp);
} else if (deviceType && STRCASEEQ(deviceType, "atapi-cdrom"))
{
virDomainDiskSetType(*def, VIR_STORAGE_TYPE_BLOCK);
diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-missing.vmx
b/tests/vmx2xmldata/vmx2xml-cdrom-ide-missing.vmx
new file mode 100644
index 000000000000..bef1ebbba272
--- /dev/null
+++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-missing.vmx
@@ -0,0 +1,6 @@
+config.version = "8"
+virtualHW.version = "4"
+ide0:0.present = "true"
+ide0:0.deviceType = "cdrom-image"
+ide0:0.fileName = "/vmfs/volumes/missing/dir/file.iso"
+displayName = "test"
diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c
index 116d729a0147..624ee14ece12 100644
--- a/tests/vmx2xmltest.c
+++ b/tests/vmx2xmltest.c
@@ -232,6 +232,7 @@ mymain(void)
DO_TEST("cdrom-ide-raw-device", "cdrom-ide-raw-device");
DO_TEST("cdrom-ide-raw-auto-detect",
"cdrom-ide-raw-auto-detect");
DO_TEST("cdrom-ide-raw-auto-detect",
"cdrom-ide-raw-auto-detect");
+ DO_TEST("cdrom-ide-missing", "cdrom-ide-empty");
DO_TEST("floppy-file", "floppy-file");
DO_TEST("floppy-device", "floppy-device");
--
2.29.2
1417
days inactive
1432
days old
8 comments
3 participants
participants (3)
-
Daniel Henrique Barboza -
Martin Kletzander -
Michal Privoznik