From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:54162) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tdlxg-0007jj-KO for qemu-devel@nongnu.org; Wed, 28 Nov 2012 13:02:21 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Tdlxa-0006EG-Ko for qemu-devel@nongnu.org; Wed, 28 Nov 2012 13:02:16 -0500 Received: from mail-ia0-f173.google.com ([209.85.210.173]:54479) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tdlxa-0006EA-G0 for qemu-devel@nongnu.org; Wed, 28 Nov 2012 13:02:10 -0500 Received: by mail-ia0-f173.google.com with SMTP id w21so8472066iac.4 for ; Wed, 28 Nov 2012 10:02:09 -0800 (PST) Sender: fluxion Date: Wed, 28 Nov 2012 11:54:45 -0600 From: mdroth Message-ID: <20121128175445.GE8690@vm> References: <1354021324-31561-1-git-send-email-lcapitulino@redhat.com> <1354021324-31561-4-git-send-email-lcapitulino@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1354021324-31561-4-git-send-email-lcapitulino@redhat.com> Subject: Re: [Qemu-devel] [PATCH 03/10] qemu-ga: qmp_guest_file_*: improve error reporting List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: qemu-devel@nongnu.org On Tue, Nov 27, 2012 at 11:01:57AM -0200, Luiz Capitulino wrote: > Use error_setg_errno() when possible with an improved error description. > > Signed-off-by: Luiz Capitulino > --- > qga/commands-posix.c | 22 +++++++++++++--------- > 1 file changed, 13 insertions(+), 9 deletions(-) > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > index c284083..92fc550 100644 > --- a/qga/commands-posix.c > +++ b/qga/commands-posix.c > @@ -138,7 +138,8 @@ int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode, E > slog("guest-file-open called, filepath: %s, mode: %s", path, mode); > fh = fopen(path, mode); > if (!fh) { > - error_set(err, QERR_OPEN_FILE_FAILED, path); > + error_setg_errno(err, errno, "failed to open file '%s' (mode: '%s')", > + path, mode); > return -1; > } > > @@ -149,7 +150,8 @@ int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode, E > ret = fcntl(fd, F_GETFL); > ret = fcntl(fd, F_SETFL, ret | O_NONBLOCK); > if (ret == -1) { > - error_set(err, QERR_QGA_COMMAND_FAILED, "fcntl() failed"); > + error_setg_errno(err, errno, "failed to make file '%s' non-blocking", > + path); > fclose(fh); > return -1; > } > @@ -171,7 +173,7 @@ void qmp_guest_file_close(int64_t handle, Error **err) > > ret = fclose(gfh->fh); > if (ret == EOF) { > - error_set(err, QERR_QGA_COMMAND_FAILED, "fclose() failed"); > + error_setg_errno(err, errno, "failed to close handle"); > return; > } > > @@ -195,7 +197,8 @@ struct GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count, > if (!has_count) { > count = QGA_READ_COUNT_DEFAULT; > } else if (count < 0) { > - error_set(err, QERR_INVALID_PARAMETER, "count"); > + error_setg(err, "value '%" PRId64 "' is invalid for argument count", > + count); > return NULL; > } > > @@ -203,8 +206,8 @@ struct GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count, > buf = g_malloc0(count+1); > read_count = fread(buf, 1, count, fh); > if (ferror(fh)) { > + error_setg_errno(err, errno, "failed to read file"); > slog("guest-file-read failed, handle: %ld", handle); > - error_set(err, QERR_QGA_COMMAND_FAILED, "fread() failed"); > } else { I'm not sure about relying on errno for FILE/f*() functions. C99 doesn't appear to require setting it for implementations (unfortunately), and glibc doesn't document it for fwrite/fread (although it does for most others). I'm guessing it's okay for glibc, but there be cases where we print random error messages. We can do this portably by setting errno = 0 before the fread()/f*()s, and using error_setg() if it's still 0 after we detect an error, but that's pretty nasty. Unless we can confirm there's aren't any implementations out there we care about where errno isn't set, maybe we should just stick to error_setg() for the f* functions? Hate to throw away error messages, but incorrect/random errors would be worse.