Re: [libvirt PATCH] vmx: Don't error out on missing filename for cdrom

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On a Monday in 2020, Martin Kletzander wrote:
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

Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
---
src/vmx/vmx.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index b86dbe9ca267..40e4ef962992 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -2447,10 +2447,18 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con
                goto cleanup;
            }

+            tmp = ctx->parseFileName(fileName, ctx->opaque);
            virDomainDiskSetType(*def, VIR_STORAGE_TYPE_FILE);
-            if (!(tmp = ctx->parseFileName(fileName, ctx->opaque)))
-                goto cleanup;
-            virDomainDiskSetSource(*def, tmp);
+            /* It is easily possible to have a cdrom with non-existing filename
+             * as the image and vmware just provides an empty cdrom.
+             *
+             * See: https://bugzilla.redhat.com/1903953
+             */
+            if (tmp) {
+                virDomainDiskSetSource(*def, tmp);
+            } else {
+                virResetLastError();

Using virResetLastError elsewhere than beginning of a libvirt API is
suspicious. If it's not an error, we should not have logged it in the
first place.

Jano

+            }
            VIR_FREE(tmp);
        } else if (deviceType && STRCASEEQ(deviceType, "atapi-cdrom")) {
            virDomainDiskSetType(*def, VIR_STORAGE_TYPE_BLOCK);
--
2.29.2

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux