Re: [libvirt] [PATCH] esx: Fix floppy.fileName handling in the vmx file parser

On Thu, Oct 10, 2013 at 2:21 PM, Eric Blake <eblake(a)redhat.com&gt; wrote: > On 10/09/2013 07:39 PM, Geoff Hickey wrote: >> The vmx file parsing code was reporting errors when parsing floppy.fileName >> entries if the filename didn't end in .flp. There is no such restriction in >> ESX; even using the GUI to configure floppy filenames you can specify any >> arbitrary file with any extension. >> >> Fix by changing the vmx parsing code so that it uses the floppy.fileType >> value to determine whether floppy.fileName refers to a block device or a >> regular file. >> --- >> src/vmx/vmx.c | 26 +++++--------------------- >> 1 file changed, 5 insertions(+), 21 deletions(-) >> >> diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c >> index 38b7cc0..ffe7e7a 100644 >> --- a/src/vmx/vmx.c >> +++ b/src/vmx/vmx.c >> @@ -2250,27 +2250,18 @@ virVMXParseDisk(virVMXContext *ctx, >> virDomainXMLOptionPtr xmlopt, virConfPtr con >> goto cleanup; >> } >> } else if (device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) { >> - if (virFileHasSuffix(fileName, ".flp")) { >> - if (fileType != NULL) { >> - if (STRCASENEQ(fileType, "file")) { >> - virReportError(VIR_ERR_INTERNAL_ERROR, >> - _("Expecting VMX entry '%s' to be >> 'file' but " > > Your mailer corrupted your patch. Would you mind resending it via 'git > send-email', or at a bare minimum as an attachment rather than inline, > so that maintainers can just 'git am' it? > > Applying: esx: Fix floppy.fileName handling in the vmx file parser > fatal: corrupt patch at line 10 > Repository lacks necessary blobs to fall back on 3-way merge. > Cannot fall back to three-way merge. > Patch failed at 0001 esx: Fix floppy.fileName handling in the vmx file > parser > > See also http://libvirt.org/hacking.html for hints. > >> + if (fileType != NULL && STRCASEEQ(fileType, "device")) { > > It's simpler to just use a pointer variable in boolean context instead > of explicit comparison against NULL (this isn't Java). As in: > > if (fileType && STRCASEEQ(fileType, "device")) { > > At any rate, your change looks reasonable to me; I wouldn't mind > applying it if you can resend to touch up those issues. > > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org > > > -- > libvir-list mailing list > libvir-list(a)redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list FWIW, I've been fixing a number of these poor assumptions lately. There's no limitation that there must be a fileName specified either and the way the patch is formulated it will treat it as a fatal error that there is a floppy drive defined with no file name for the floppy image. -- Doug Goldstein