From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1ME0L0-0002gx-Ee for qemu-devel@nongnu.org; Tue, 09 Jun 2009 08:21:58 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1ME0Kv-0002ft-5I for qemu-devel@nongnu.org; Tue, 09 Jun 2009 08:21:57 -0400 Received: from [199.232.76.173] (port=54546 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1ME0Ku-0002fd-Vv for qemu-devel@nongnu.org; Tue, 09 Jun 2009 08:21:53 -0400 Received: from mx2.redhat.com ([66.187.237.31]:34342) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1ME0Ku-0000sc-IW for qemu-devel@nongnu.org; Tue, 09 Jun 2009 08:21:52 -0400 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n59CLpWn026899 for ; Tue, 9 Jun 2009 08:21:51 -0400 Date: Tue, 9 Jun 2009 15:20:57 +0300 From: "Michael S. Tsirkin" Subject: Re: [Qemu-devel] [PATCH] exec-migration: handle EINTR in popen_get_buffer() Message-ID: <20090609122057.GF3476@redhat.com> References: <1244478441-26288-1-git-send-email-uril@redhat.com> <20090608175452.GA4995@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20090608175452.GA4995@redhat.com> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Uri Lublin Cc: qemu-devel@nongnu.org On Mon, Jun 08, 2009 at 08:54:52PM +0300, Michael S. Tsirkin wrote: > On Mon, Jun 08, 2009 at 07:27:21PM +0300, Uri Lublin wrote: > > Sometimes, upon interrupt, fread returns with no data, and > > the (incoming exec) migration fails. > > > > Fix by retrying on such a case. > > > > Signed-off-by: Uri Lublin > > --- > > savevm.c | 9 ++++++++- > > 1 files changed, 8 insertions(+), 1 deletions(-) > > > > diff --git a/savevm.c b/savevm.c > > index 248aea3..df2486d 100644 > > --- a/savevm.c > > +++ b/savevm.c > > @@ -215,7 +215,14 @@ static int popen_put_buffer(void *opaque, const uint8_t *buf, int64_t pos, int s > > static int popen_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size) > > { > > QEMUFilePopen *s = opaque; > > - return fread(buf, 1, size, s->popen_file); > > + FILE *fp = s->popen_file; > > + int bytes; > > + > > + do { > > + clearerr(fp); > > Would it make sense to only clearerr on EINTR - if we intend to retry? > > > + bytes = fread(buf, 1, size, fp); > > + } while ((bytes == 0) && ferror(fp) && (errno == EINTR)); > > > This does nothing about partial reads (bytes != 0) > I think it's intentional because the user actually retries > partial reads. Right? > > > + return bytes; > > } > > > > static int popen_close(void *opaque) > > Looking at qemu_fill_buffer, it seems that it is enough to set > bytes to -EAGAIN. User will then retry. Correct? Looking at the code some more, at least qemu_get_byte really seems to expect a reliable read underneath, so it won't work. Generally the fact that qemu_get_byte returns 0 on error seems broken to me maybe we could fix that and then have a single loop retrying reads. But that's for another patch. Acked-by: Michael S. Tsirkin -- MST