vmx: make 'fileName' optional for CD-ROMs

It seems like CD-ROMs may have no 'fileName' property specified in case
there is nothing configured as attachment for the drive. Hence, make
sure that virVMXParseDisk() do not consider it mandatory anymore,
considering it an empty block cdrom device. Sadly virVMXParseDisk() is
used also to parse disk and floppies, so make sure that a NULL fileName
is handled in cdrom- and floppy-related paths.

https://bugzilla.redhat.com/show_bug.cgi?id=1808610

Signed-off-by: Pino Toscano <ptoscano@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Tested-by: Richard W.M. Jones <rjones@redhat.com>

diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index bfc9bc7404bcab39448f33102202494df941e9f7..6c6ef7acf3405009cd1bcbc095c119cc833ce5da 100644 (file)
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -2207,7 +2207,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
         goto cleanup;
 
     /* vmx:fileName -> def:src, def:type */
-    if (virVMXGetConfigString(conf, fileName_name, &fileName, false) < 0)
+    if (virVMXGetConfigString(conf, fileName_name, &fileName, true) < 0)
         goto cleanup;
 
     /* vmx:writeThrough -> def:cachemode */
@@ -2218,7 +2218,8 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
 
     /* Setup virDomainDiskDef */
     if (device == VIR_DOMAIN_DISK_DEVICE_DISK) {
-        if (virStringHasCaseSuffix(fileName, ".iso") ||
+        if (fileName == NULL ||
+            virStringHasCaseSuffix(fileName, ".iso") ||
             STREQ(fileName, "emptyBackingString") ||
             (deviceType &&
              (STRCASEEQ(deviceType, "atapi-cdrom") ||
@@ -2277,7 +2278,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
             goto cleanup;
         }
     } else if (device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
-        if (virStringHasCaseSuffix(fileName, ".vmdk")) {
+        if (fileName && virStringHasCaseSuffix(fileName, ".vmdk")) {
             /*
              * This function was called in order to parse a CDROM device, but
              * .vmdk files are for harddisk devices only. Just ignore it,
@@ -2285,7 +2286,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
              * handle it.
              */
             goto ignore;
-        } else if (virStringHasCaseSuffix(fileName, ".iso")) {
+        } else if (fileName && virStringHasCaseSuffix(fileName, ".iso")) {
             char *tmp;
 
             if (deviceType && STRCASENEQ(deviceType, "cdrom-image")) {
@@ -2306,7 +2307,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
         } else if (deviceType && STRCASEEQ(deviceType, "atapi-cdrom")) {
             virDomainDiskSetType(*def, VIR_STORAGE_TYPE_BLOCK);
 
-            if (STRCASEEQ(fileName, "auto detect")) {
+            if (fileName && STRCASEEQ(fileName, "auto detect")) {
                 ignore_value(virDomainDiskSetSource(*def, NULL));
                 (*def)->startupPolicy = VIR_DOMAIN_STARTUP_POLICY_OPTIONAL;
             } else if (virDomainDiskSetSource(*def, fileName) < 0) {
@@ -2317,7 +2318,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
             (*def)->device = VIR_DOMAIN_DISK_DEVICE_LUN;
             virDomainDiskSetType(*def, VIR_STORAGE_TYPE_BLOCK);
 
-            if (STRCASEEQ(fileName, "auto detect")) {
+            if (fileName && STRCASEEQ(fileName, "auto detect")) {
                 ignore_value(virDomainDiskSetSource(*def, NULL));
                 (*def)->startupPolicy = VIR_DOMAIN_STARTUP_POLICY_OPTIONAL;
             } else if (virDomainDiskSetSource(*def, fileName) < 0) {
@@ -2325,7 +2326,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
             }
         } else if (busType == VIR_DOMAIN_DISK_BUS_SCSI &&
                    deviceType && STRCASEEQ(deviceType, "scsi-passthru")) {
-            if (STRPREFIX(fileName, "/vmfs/devices/cdrom/")) {
+            if (fileName && STRPREFIX(fileName, "/vmfs/devices/cdrom/")) {
                 /* SCSI-passthru CD-ROMs actually are device='lun' */
                 (*def)->device = VIR_DOMAIN_DISK_DEVICE_LUN;
                 virDomainDiskSetType(*def, VIR_STORAGE_TYPE_BLOCK);
@@ -2341,7 +2342,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
                  */
                 goto ignore;
             }
-        } else if (STREQ(fileName, "emptyBackingString")) {
+        } else if (fileName && STREQ(fileName, "emptyBackingString")) {
             if (deviceType && STRCASENEQ(deviceType, "cdrom-image")) {
                 virReportError(VIR_ERR_INTERNAL_ERROR,
                                _("Expecting VMX entry '%s' to be 'cdrom-image' "
@@ -2355,7 +2356,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("Invalid or not yet handled value '%s' "
                              "for VMX entry '%s' for device type '%s'"),
-                           fileName, fileName_name,
+                           NULLSTR(fileName), fileName_name,
                            deviceType ? deviceType : "unknown");
             goto cleanup;
         }
@@ -2365,10 +2366,10 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
             if (virDomainDiskSetSource(*def, fileName) < 0)
                 goto cleanup;
         } else if (fileType != NULL && STRCASEEQ(fileType, "file")) {
-            char *tmp;
+            char *tmp = NULL;
 
             virDomainDiskSetType(*def, VIR_STORAGE_TYPE_FILE);
-            if (!(tmp = ctx->parseFileName(fileName, ctx->opaque)))
+            if (fileName && !(tmp = ctx->parseFileName(fileName, ctx->opaque)))
                 goto cleanup;
             if (virDomainDiskSetSource(*def, tmp) < 0) {
                 VIR_FREE(tmp);
@@ -2379,7 +2380,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("Invalid or not yet handled value '%s' "
                              "for VMX entry '%s' for device type '%s'"),
-                           fileName, fileName_name,
+                           NULLSTR(fileName), fileName_name,
                            deviceType ? deviceType : "unknown");
             goto cleanup;
         }

diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.vmx b/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.vmx
new file mode 100644 (file)
index 0000000..36286cb
--- /dev/null
+++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.vmx
@@ -0,0 +1,4 @@
+config.version = "8"
+virtualHW.version = "4"
+ide0:0.present = "true"
+ide0:0.deviceType = "atapi-cdrom"

diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml b/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml
new file mode 100644 (file)
index 0000000..af4a5ff
--- /dev/null
+++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml
@@ -0,0 +1,23 @@
+<domain type='vmware'>
+  <uuid>00000000-0000-0000-0000-000000000000</uuid>
+  <memory unit='KiB'>32768</memory>
+  <currentMemory unit='KiB'>32768</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='i686'>hvm</type>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <disk type='block' device='cdrom'>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <controller type='ide' index='0'/>
+    <video>
+      <model type='vmvga' vram='4096' primary='yes'/>
+    </video>
+  </devices>
+</domain>

diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c
index 8d7b8ba2a4ea6d0cbc6d7f5f2e8dcc65a8a76b84..1966aed6fe4be9b7dff4d264ae9afbab9fac9a0e 100644 (file)
--- a/tests/vmx2xmltest.c
+++ b/tests/vmx2xmltest.c
@@ -218,6 +218,7 @@ mymain(void)
     DO_TEST("cdrom-scsi-passthru", "cdrom-scsi-passthru");
     DO_TEST("cdrom-ide-file", "cdrom-ide-file");
     DO_TEST("cdrom-ide-empty", "cdrom-ide-empty");
+    DO_TEST("cdrom-ide-empty-2", "cdrom-ide-empty-2");
     DO_TEST("cdrom-ide-device", "cdrom-ide-device");
     DO_TEST("cdrom-ide-raw-device", "cdrom-ide-raw-device");
     DO_TEST("cdrom-ide-raw-auto-detect", "cdrom-ide-raw-auto-detect");