[et-mgmt-tools] [PATCH] virt-convert: check for qemu-img failure
Cole Robinson
crobinso at redhat.com
Thu Jul 3 15:59:44 UTC 2008
John Levon wrote:
> I didn't go the "qemu-info" route (yet), as it only produces
> human-readable output: I didn't see a way to get it to print just the
> file format.
>
One potential option would be to try 'qemu-info -f vmx imagename', throw
away the output and just check the return value. I don't know if this
works for all formats but in my quick check the above command on a raw
file failed. It would prevent us from autodetecting the format, but it's
at least an option.
> regards
> john
>
>
> Cleanup on failure
>
> If we can't convert the disks or export the file, perform some cleanup.
>
> Signed-off-by: John Levon <john.levon at sun.com>
>
> diff --git a/virt-convert b/virt-convert
> --- a/virt-convert
> +++ b/virt-convert
> @@ -31,6 +31,7 @@ import virtconv.vmconfig as vmconfig
> import virtconv.vmconfig as vmconfig
>
> def parse_args():
> + """Parse and verify command line."""
> opts = OptionParser()
> opts.set_usage("%prog [options] inputdir|input.vmx "
> "[outputdir|output.xml]")
> @@ -93,6 +94,36 @@ def parse_args():
>
> return options
>
> +def rmrf(path):
> + """Remove a directory and all its contents."""
> +
> + assert path is not None
> +
> + for dirpath, _, files in os.walk(path):
> + for filename in files:
> + os.remove(os.path.join(dirpath, filename))
> + for dirpath, subdirs, _ in os.walk(path, topdown=False):
> + for dirname in subdirs:
> + os.rmdir(os.path.join(dirpath, dirname))
> + os.rmdir(path)
> +
> +def cleanup(msg, options, created_dir):
> + """
> + After failure, clean up anything we created. Take a conservative
> + approach: only if we created the output directory do we delete
> + anything.
> + """
> + logging.error(msg)
> +
> + if created_dir:
> + try:
> + rmrf(options.output_dir)
> + except OSError, e:
> + logging.error("Couldn't clean up output directory \"%s\": %s" %
> + (options.output_dir, e.strerror))
> +
> + sys.exit(1)
> +
Hmm, this scares the hell out of me, even with the created_dir
check. If someone in the future ever messed up this code we could
accidently start deleting someones homedir.
I think a whitelist approach would be better: have convert_disks clean
up its own files, and keep all other file/directory creation in the
main() function (like it currently is) so it can handle the rest
of the cleanup on a per file basis. At the end, if the output_dir
is empty and we created it, we can delete it. Maybe it's overly
paranoid, but better to be safe than sorry.
- Cole
More information about the et-mgmt-tools
mailing list