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

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(a)redhat.com&gt; >--- > 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 >