From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:52359) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T9Cy0-00024T-5W for qemu-devel@nongnu.org; Wed, 05 Sep 2012 06:36:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T9Cxz-0004Py-6A for qemu-devel@nongnu.org; Wed, 05 Sep 2012 06:36:16 -0400 Message-ID: <50472B0E.4080205@redhat.com> Date: Wed, 05 Sep 2012 12:35:58 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1346833574-26193-1-git-send-email-riegamaths@gmail.com> <20120905094003.GD4786@stefanha-thinkpad.localdomain> In-Reply-To: <20120905094003.GD4786@stefanha-thinkpad.localdomain> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [Qemu-trivial] [PATCH] block: output more error messages if failed to create temporary snapshot List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: qemu-trivial , qemu-devel , riegamaths@gmail.com Am 05.09.2012 11:40, schrieb Stefan Hajnoczi: > On Wed, Sep 05, 2012 at 04:26:14PM +0800, riegamaths@gmail.com wrote: >> From: Dunrong Huang >> >> If we failed to create temporary snapshot, the error message did not m= atch >> with the error, for example: >> >> $ TMPDIR=3D/tmp/bad_path qemu-system-x86_64 -enable-kvm debian.qcow2 -= snapshot >> qemu-system-x86_64: -enable-kvm: could not open disk image /home/maths= linux/Images/debian.qcow2: No such file or directory >> >> Indeed, the file which cant be created is /tmp/bad_path/vl.xxxxxx, no= t >> debian.qcow2. so the error message makes users feel confused. >> >> Signed-off-by: Dunrong Huang >> --- >> block.c | 2 ++ >> 1 =E4=B8=AA=E6=96=87=E4=BB=B6=E8=A2=AB=E4=BF=AE=E6=94=B9=EF=BC=8C=E6=8F= =92=E5=85=A5 2 =E8=A1=8C(+) >> >> 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 =3D mkstemp(filename); >> if (fd < 0 || close(fd)) { >> + fprintf(stderr, "Could not create temporary snapshot in %s di= rectory: " >> + "%s\n", tmpdir, strerror(errno)); >=20 > The error message is fine for fd < 0 but not for close(0) !=3D 0. Also= , > close(2) is allowed to change errno (even on success) so this is not > portable and could clobber the errno value. I don't think this error message is fine in get_tmp_filename(). This function happens to be used for temporary snapshots, but this is not part of its interface. Today it's also used in vvfat and there the message would only be confusing. Other use cases may be introduced in the future. If you want to introduce an error message, do it in the caller. Kevin