a41c76
From f600a3df720e1d68c621e739acbed0554f5f5fb0 Mon Sep 17 00:00:00 2001
a41c76
Message-Id: <f600a3df720e1d68c621e739acbed0554f5f5fb0@dist-git>
a41c76
From: Pino Toscano <ptoscano@redhat.com>
a41c76
Date: Fri, 20 Mar 2020 13:25:39 +0100
a41c76
Subject: [PATCH] vmx: make 'fileName' optional for CD-ROMs
a41c76
MIME-Version: 1.0
a41c76
Content-Type: text/plain; charset=UTF-8
a41c76
Content-Transfer-Encoding: 8bit
a41c76
a41c76
It seems like CD-ROMs may have no 'fileName' property specified in case
a41c76
there is nothing configured as attachment for the drive. Hence, make
a41c76
sure that virVMXParseDisk() do not consider it mandatory anymore,
a41c76
considering it an empty block cdrom device. Sadly virVMXParseDisk() is
a41c76
used also to parse disk and floppies, so make sure that a NULL fileName
a41c76
is handled in cdrom- and floppy-related paths.
a41c76
a41c76
https://bugzilla.redhat.com/show_bug.cgi?id=1808610
a41c76
a41c76
Signed-off-by: Pino Toscano <ptoscano@redhat.com>
a41c76
Reviewed-by: Ján Tomko <jtomko@redhat.com>
a41c76
Tested-by: Richard W.M. Jones <rjones@redhat.com>
a41c76
(cherry picked from commit c5ee737bc5c0fc61451e45ff41526270bdf0113c)
a41c76
Signed-off-by: Pino Toscano <ptoscano@redhat.com>
a41c76
Message-Id: <20200320122539.141785-3-ptoscano@redhat.com>
a41c76
Reviewed-by: Ján Tomko <jtomko@redhat.com>
a41c76
---
a41c76
 src/vmx/vmx.c                                 | 25 ++++++++++---------
a41c76
 .../vmx2xmldata/vmx2xml-cdrom-ide-empty-2.vmx |  4 +++
a41c76
 .../vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml | 23 +++++++++++++++++
a41c76
 tests/vmx2xmltest.c                           |  1 +
a41c76
 4 files changed, 41 insertions(+), 12 deletions(-)
a41c76
 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.vmx
a41c76
 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml
a41c76
a41c76
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
a41c76
index fc4fa1c22b..d0bcb9c2de 100644
a41c76
--- a/src/vmx/vmx.c
a41c76
+++ b/src/vmx/vmx.c
a41c76
@@ -2206,7 +2206,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
a41c76
         goto cleanup;
a41c76
 
a41c76
     /* vmx:fileName -> def:src, def:type */
a41c76
-    if (virVMXGetConfigString(conf, fileName_name, &fileName, false) < 0)
a41c76
+    if (virVMXGetConfigString(conf, fileName_name, &fileName, true) < 0)
a41c76
         goto cleanup;
a41c76
 
a41c76
     /* vmx:writeThrough -> def:cachemode */
a41c76
@@ -2217,7 +2217,8 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
a41c76
 
a41c76
     /* Setup virDomainDiskDef */
a41c76
     if (device == VIR_DOMAIN_DISK_DEVICE_DISK) {
a41c76
-        if (virStringHasCaseSuffix(fileName, ".iso") ||
a41c76
+        if (fileName == NULL ||
a41c76
+            virStringHasCaseSuffix(fileName, ".iso") ||
a41c76
             STREQ(fileName, "emptyBackingString") ||
a41c76
             (deviceType &&
a41c76
              (STRCASEEQ(deviceType, "atapi-cdrom") ||
a41c76
@@ -2276,7 +2277,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
a41c76
             goto cleanup;
a41c76
         }
a41c76
     } else if (device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
a41c76
-        if (virStringHasCaseSuffix(fileName, ".vmdk")) {
a41c76
+        if (fileName && virStringHasCaseSuffix(fileName, ".vmdk")) {
a41c76
             /*
a41c76
              * This function was called in order to parse a CDROM device, but
a41c76
              * .vmdk files are for harddisk devices only. Just ignore it,
a41c76
@@ -2284,7 +2285,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
a41c76
              * handle it.
a41c76
              */
a41c76
             goto ignore;
a41c76
-        } else if (virStringHasCaseSuffix(fileName, ".iso")) {
a41c76
+        } else if (fileName && virStringHasCaseSuffix(fileName, ".iso")) {
a41c76
             char *tmp;
a41c76
 
a41c76
             if (deviceType && STRCASENEQ(deviceType, "cdrom-image")) {
a41c76
@@ -2305,7 +2306,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
a41c76
         } else if (deviceType && STRCASEEQ(deviceType, "atapi-cdrom")) {
a41c76
             virDomainDiskSetType(*def, VIR_STORAGE_TYPE_BLOCK);
a41c76
 
a41c76
-            if (STRCASEEQ(fileName, "auto detect")) {
a41c76
+            if (fileName && STRCASEEQ(fileName, "auto detect")) {
a41c76
                 ignore_value(virDomainDiskSetSource(*def, NULL));
a41c76
                 (*def)->startupPolicy = VIR_DOMAIN_STARTUP_POLICY_OPTIONAL;
a41c76
             } else if (virDomainDiskSetSource(*def, fileName) < 0) {
a41c76
@@ -2316,7 +2317,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
a41c76
             (*def)->device = VIR_DOMAIN_DISK_DEVICE_LUN;
a41c76
             virDomainDiskSetType(*def, VIR_STORAGE_TYPE_BLOCK);
a41c76
 
a41c76
-            if (STRCASEEQ(fileName, "auto detect")) {
a41c76
+            if (fileName && STRCASEEQ(fileName, "auto detect")) {
a41c76
                 ignore_value(virDomainDiskSetSource(*def, NULL));
a41c76
                 (*def)->startupPolicy = VIR_DOMAIN_STARTUP_POLICY_OPTIONAL;
a41c76
             } else if (virDomainDiskSetSource(*def, fileName) < 0) {
a41c76
@@ -2324,7 +2325,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
a41c76
             }
a41c76
         } else if (busType == VIR_DOMAIN_DISK_BUS_SCSI &&
a41c76
                    deviceType && STRCASEEQ(deviceType, "scsi-passthru")) {
a41c76
-            if (STRPREFIX(fileName, "/vmfs/devices/cdrom/")) {
a41c76
+            if (fileName && STRPREFIX(fileName, "/vmfs/devices/cdrom/")) {
a41c76
                 /* SCSI-passthru CD-ROMs actually are device='lun' */
a41c76
                 (*def)->device = VIR_DOMAIN_DISK_DEVICE_LUN;
a41c76
                 virDomainDiskSetType(*def, VIR_STORAGE_TYPE_BLOCK);
a41c76
@@ -2340,7 +2341,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
a41c76
                  */
a41c76
                 goto ignore;
a41c76
             }
a41c76
-        } else if (STREQ(fileName, "emptyBackingString")) {
a41c76
+        } else if (fileName && STREQ(fileName, "emptyBackingString")) {
a41c76
             if (deviceType && STRCASENEQ(deviceType, "cdrom-image")) {
a41c76
                 virReportError(VIR_ERR_INTERNAL_ERROR,
a41c76
                                _("Expecting VMX entry '%s' to be 'cdrom-image' "
a41c76
@@ -2354,7 +2355,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
a41c76
             virReportError(VIR_ERR_INTERNAL_ERROR,
a41c76
                            _("Invalid or not yet handled value '%s' "
a41c76
                              "for VMX entry '%s' for device type '%s'"),
a41c76
-                           fileName, fileName_name,
a41c76
+                           NULLSTR(fileName), fileName_name,
a41c76
                            deviceType ? deviceType : "unknown");
a41c76
             goto cleanup;
a41c76
         }
a41c76
@@ -2364,10 +2365,10 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
a41c76
             if (virDomainDiskSetSource(*def, fileName) < 0)
a41c76
                 goto cleanup;
a41c76
         } else if (fileType != NULL && STRCASEEQ(fileType, "file")) {
a41c76
-            char *tmp;
a41c76
+            char *tmp = NULL;
a41c76
 
a41c76
             virDomainDiskSetType(*def, VIR_STORAGE_TYPE_FILE);
a41c76
-            if (!(tmp = ctx->parseFileName(fileName, ctx->opaque)))
a41c76
+            if (fileName && !(tmp = ctx->parseFileName(fileName, ctx->opaque)))
a41c76
                 goto cleanup;
a41c76
             if (virDomainDiskSetSource(*def, tmp) < 0) {
a41c76
                 VIR_FREE(tmp);
a41c76
@@ -2378,7 +2379,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
a41c76
             virReportError(VIR_ERR_INTERNAL_ERROR,
a41c76
                            _("Invalid or not yet handled value '%s' "
a41c76
                              "for VMX entry '%s' for device type '%s'"),
a41c76
-                           fileName, fileName_name,
a41c76
+                           NULLSTR(fileName), fileName_name,
a41c76
                            deviceType ? deviceType : "unknown");
a41c76
             goto cleanup;
a41c76
         }
a41c76
diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.vmx b/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.vmx
a41c76
new file mode 100644
a41c76
index 0000000000..36286cb20f
a41c76
--- /dev/null
a41c76
+++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.vmx
a41c76
@@ -0,0 +1,4 @@
a41c76
+config.version = "8"
a41c76
+virtualHW.version = "4"
a41c76
+ide0:0.present = "true"
a41c76
+ide0:0.deviceType = "atapi-cdrom"
a41c76
diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml b/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml
a41c76
new file mode 100644
a41c76
index 0000000000..af4a5ff9f6
a41c76
--- /dev/null
a41c76
+++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml
a41c76
@@ -0,0 +1,23 @@
a41c76
+<domain type='vmware'>
a41c76
+  <uuid>00000000-0000-0000-0000-000000000000</uuid>
a41c76
+  <memory unit='KiB'>32768</memory>
a41c76
+  <currentMemory unit='KiB'>32768</currentMemory>
a41c76
+  <vcpu placement='static'>1</vcpu>
a41c76
+  <os>
a41c76
+    <type arch='i686'>hvm</type>
a41c76
+  </os>
a41c76
+  <clock offset='utc'/>
a41c76
+  <on_poweroff>destroy</on_poweroff>
a41c76
+  <on_reboot>restart</on_reboot>
a41c76
+  <on_crash>destroy</on_crash>
a41c76
+  <devices>
a41c76
+    <disk type='block' device='cdrom'>
a41c76
+      <target dev='hda' bus='ide'/>
a41c76
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
a41c76
+    </disk>
a41c76
+    <controller type='ide' index='0'/>
a41c76
+    <video>
a41c76
+      <model type='vmvga' vram='4096' primary='yes'/>
a41c76
+    </video>
a41c76
+  </devices>
a41c76
+</domain>
a41c76
diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c
a41c76
index 8d7b8ba2a4..1966aed6fe 100644
a41c76
--- a/tests/vmx2xmltest.c
a41c76
+++ b/tests/vmx2xmltest.c
a41c76
@@ -218,6 +218,7 @@ mymain(void)
a41c76
     DO_TEST("cdrom-scsi-passthru", "cdrom-scsi-passthru");
a41c76
     DO_TEST("cdrom-ide-file", "cdrom-ide-file");
a41c76
     DO_TEST("cdrom-ide-empty", "cdrom-ide-empty");
a41c76
+    DO_TEST("cdrom-ide-empty-2", "cdrom-ide-empty-2");
a41c76
     DO_TEST("cdrom-ide-device", "cdrom-ide-device");
a41c76
     DO_TEST("cdrom-ide-raw-device", "cdrom-ide-raw-device");
a41c76
     DO_TEST("cdrom-ide-raw-auto-detect", "cdrom-ide-raw-auto-detect");
a41c76
-- 
a41c76
2.25.1
a41c76