From f600a3df720e1d68c621e739acbed0554f5f5fb0 Mon Sep 17 00:00:00 2001

Message-Id: <f600a3df720e1d68c621e739acbed0554f5f5fb0@dist-git>

From: Pino Toscano <ptoscano@redhat.com>

Date: Fri, 20 Mar 2020 13:25:39 +0100

Subject: [PATCH] vmx: make 'fileName' optional for CD-ROMs

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

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>

(cherry picked from commit c5ee737bc5c0fc61451e45ff41526270bdf0113c)

Signed-off-by: Pino Toscano <ptoscano@redhat.com>

Message-Id: <20200320122539.141785-3-ptoscano@redhat.com>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

---

 src/vmx/vmx.c                                 | 25 ++++++++++---------

 .../vmx2xmldata/vmx2xml-cdrom-ide-empty-2.vmx |  4 +++

 .../vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml | 23 +++++++++++++++++

 tests/vmx2xmltest.c                           |  1 +

 4 files changed, 41 insertions(+), 12 deletions(-)

 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.vmx

 create mode 100644 tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml

diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c

index fc4fa1c22b..d0bcb9c2de 100644

--- a/src/vmx/vmx.c

+++ b/src/vmx/vmx.c

@@ -2206,7 +2206,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 */

@@ -2217,7 +2217,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") ||

@@ -2276,7 +2277,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,

@@ -2284,7 +2285,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")) {

@@ -2305,7 +2306,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) {

@@ -2316,7 +2317,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) {

@@ -2324,7 +2325,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);

@@ -2340,7 +2341,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' "

@@ -2354,7 +2355,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;

         }

@@ -2364,10 +2365,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);

@@ -2378,7 +2379,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

index 0000000000..36286cb20f

--- /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

index 0000000000..af4a5ff9f6

--- /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 8d7b8ba2a4..1966aed6fe 100644

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

-- 

2.25.1