Code review comment for lp://staging/~soren/nova/virt-test-improvements

> Nice work here Soren.
>
> Some notes:
>
>
> Received this error when running the tests:
> http://paste.openstack.org/show/2357/

Which OS are you running this on? Can you give me the outout of "uname -a" and see if its df(1) command can somehow be pursuaded to provide its output in bytes?

>> + raise Exception("Please extend fake libvirt module to support this "
>> + "auth method")
> i18n.

I added i18n here, but generally, I don't really think doing so in general for strings that only exist in the tests is super useful. Kind of like how we don't i18n enable our code comments.

> > 1420 + try:
> > 1421 + dst_fd, dst_path = tempfile.mkstemp()
> > 1422 + os.close(dst_fd)
> > 1423 +
> > 1424 + src_fd, src_path = tempfile.mkstemp()
> > 1425 + with os.fdopen(src_fd) as fp:
> > 1426 + fp.write('canary')
> > 1427 +
> > 1428 + libvirt_utils.copy_image(src_path, dst_path)
> > 1429 + with open(dst_path, 'r') as fp:
> > 1430 + self.assertEquals(fp.read(), 'canary')
> > 1431 + finally:
> > 1432 + os.unlink(src_path)
> > 1433 + os.unlink(dst_path)
> Not likely to happen, but if an exception was is raised after the first
> `mkstemp()` but before the second `mkstemp()`, the `finally` will attempt to
> unlink `src_path` which hasn't been defined yet.

Fixed.
> > 1990 +# Copyright 2010 United States Government as represented by the
> > 1991 +# Administrator of the National Aeronautics and Space
> Administration.
> > 1992 +# All Rights Reserved.
> > 1993 +# Copyright (c) 2010 Citrix Systems, Inc.
> > 1994 +# Copyright (c) 2011 Piston Cloud Computing, Inc
> > 1995 +# Copyright (c) 2011 OpenStack LLC
>
> Given that this is a new file, should we just start it out with:

Parts of its contents are based on an existing file, so the copyright carries over. This was intentional.

> > 2105 + if umask:
> > 2106 + saved_umask = os.umask(umask)
> > 2107 +
> > 2108 + with open(path, 'w') as f:
> > 2109 + f.write(contents)
> > 2110 +
> > 2111 + if umask:
> > 2112 + os.umask(saved_umask)
> Should this be inside a try/finally?

Good call. Fixed.

Thanks!