Re: [Libguestfs] [PATCH] v2v: -i vmx: Refuse to load a disk image by accident.

On Wed, Oct 11, 2017 at 05:17:14PM +0200, Pino Toscano wrote: > On Wednesday, 11 October 2017 15:25:26 CEST Richard W.M. Jones wrote: > > If you accidentally point -i vmx at a disk image, it will try to load > > the whole thing into memory and crash. Catch this narrow case and > > print an error. > > > > However don't fail if ‘file’ is not installed or if we don't know what > > the file is. > > --- > > v2v/parse_vmx.ml | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/v2v/parse_vmx.ml b/v2v/parse_vmx.ml > > index 65d5a0edd..f6c34e2cf 100644 > > --- a/v2v/parse_vmx.ml > > +++ b/v2v/parse_vmx.ml > > @@ -268,6 +268,18 @@ let remove_vmx_escapes str = > > > > (* Parsing. *) > > let rec parse_file vmx_filename = > > + (* One person pointed -i vmx at the VMDK file, resulting in an out > > + * of memory error. Avoid this narrow case. > > + *) > > + let () = > > + let cmd = sprintf "file -b %s ||:" (quote vmx_filename) in > > + let out = external_command cmd in > > + match out with > > + | line :: _ when String.find line "disk image" >= 0 -> > > + error (f_"-i vmx: %s: this may be a disk image. You must specify the .vmx file only on the command line.") > > + vmx_filename > > + | _ -> () in > > + > > (* Read the whole file as a list of lines. *) > > let str = read_whole_file vmx_filename in > > if verbose () then eprintf "VMX file:\n%s\n" str; > > Hm I'm not sure about this, since we don't do this for other parameters > (e.g. the XML for -i libvirtxml). Let's drop this for now, since it's user error. What's probably needed is a more generic mechanism that we can use where the tools call read_whole_file.