Re: [Qemu-trivial] [PATCH] block: output more error messages if failed to create temporary snapshot

[Stefan Hajnoczi]

On Wed, Sep 05, 2012 at 04:26:14PM +0800, address@hidden wrote:
> From: Dunrong Huang <address@hidden>
> 
> If we failed to create temporary snapshot, the error message did not match
> with the error, for example:
> 
> $ TMPDIR=/tmp/bad_path qemu-system-x86_64 -enable-kvm debian.qcow2 -snapshot
> qemu-system-x86_64: -enable-kvm: could not open disk image 
> /home/mathslinux/Images/debian.qcow2: No such file or directory
> 
> Indeed, the file which cant be created is /tmp/bad_path/vl.xxxxxx,  not
> debian.qcow2. so the error message makes users feel confused.
> 
> Signed-off-by: Dunrong Huang <address@hidden>
> ---
>  block.c | 2 ++
>  1 个文件被修改，插入 2 行(+)
> 
> diff --git a/block.c b/block.c
> index 470bdcc..c9e5140 100644
> --- a/block.c
> +++ b/block.c
> @@ -434,6 +434,8 @@ int get_tmp_filename(char *filename, int size)
>      }
>      fd = mkstemp(filename);
>      if (fd < 0 || close(fd)) {
> +        fprintf(stderr, "Could not create temporary snapshot in %s 
> directory: "
> +                "%s\n", tmpdir, strerror(errno));

The error message is fine for fd < 0 but not for close(0) != 0.  Also,
close(2) is allowed to change errno (even on success) so this is not
portable and could clobber the errno value.

Please split into:

if (fd < 0) {
    ...
}
if (close(fd) != 0) {
    ...
}

Stefan