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

[Martin Kletzander]

On Mon, Dec 21, 2020 at 01:09:13PM +0100, Ján Tomko wrote:
>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 at redhat.com>
>>---
>> 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.
>

Yeah, I did not like that either, but esx, you know...

Since it's you, asking co nicely, I'll give it another shot.

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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20201221/a4cd6498/attachment-0001.sig>