From mboxrd@z Thu Jan  1 00:00:00 1970
From: Anthony Liguori <anthony@codemonkey.ws>
Subject: Re: [PATCH] qemu: qemu_fopen_fd: differentiate between reader and
 writer user
Date: Wed, 15 Oct 2008 23:14:25 -0500
Message-ID: <48F6BFA1.9070608@codemonkey.ws>
References: <1223829030-14962-1-git-send-email-uril@qumranet.com> <48F22BF1.3000608@redhat.com> <48F23D4D.2050709@codemonkey.ws> <48F23F42.10405@redhat.com> <48F277A0.8040407@codemonkey.ws> <48F2BA83.7000101@codemonkey.ws> <48F69AAB.4010404@il.qumranet.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: Avi Kivity <avi@redhat.com>, Uri Lublin <uril@qumranet.com>,
	kvm@vger.kernel.org
To: Uri Lublin <uri@il.qumranet.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from yx-out-2324.google.com ([74.125.44.30]:22671 "EHLO
	yx-out-2324.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1750885AbYJPEO3 (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 16 Oct 2008 00:14:29 -0400
Received: by yx-out-2324.google.com with SMTP id 8so690033yxm.1
        for <kvm@vger.kernel.org>; Wed, 15 Oct 2008 21:14:28 -0700 (PDT)
In-Reply-To: <48F69AAB.4010404@il.qumranet.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Uri Lublin wrote:
> Anthony Liguori wrote:

I have already cut your text, but I don't understand the comment about 
not being "full duplex".  Is there a reason why migration needs to be 
bidirectional?  I don't think there's a fundamental reason it needs to 
be and I think there are some advantages to it being unidirectional.

> >@@ -6278,12 +6253,13 @@ typedef struct QEMUFileStdio
> >     FILE *outfile;
> > } QEMUFileStdio;
> >
> >-static void file_put_buffer(void *opaque, const uint8_t *buf,
> >+static int file_put_buffer(void *opaque, const uint8_t *buf,
> >                             int64_t pos, int size)
> > {
> >     QEMUFileStdio *s = opaque;
> >     fseek(s->outfile, pos, SEEK_SET);
> >     fwrite(buf, 1, size, s->outfile);
> >+    return size;
>
> Better return the size that was actually written

At this level in the API, you have to write all of the data.  The reason 
I introduced a return value at all was so that you could return an 
error.  This is necessary to detect that a migration or save/restore has 
failed.  I agree the QEMUFile API isn't ideal ATM.  The whole buffered 
thing is a big hack.  I'm open to refactoring suggestions/patches.

> > void qemu_fflush(QEMUFile *f)
> > {
> >     if (!f->put_buffer)
> >         return;
> >-    if (f->buf_index > 0) {
> >-        f->put_buffer(f->opaque, f->buf, f->buf_offset, f->buf_index);
> >-        f->buf_offset += f->buf_index;
> >+    if (f->is_write && f->buf_index > 0) {
> >+        int len;
> >+
> >+    len = f->put_buffer(f->opaque, f->buf, f->buf_offset, 
> f->buf_index);
> >+    if (len > 0)
> >+        f->buf_offset += f->buf_index;
> >+    else
> >+        f->has_error = 1;
>
> Untabify.

I don't see a tab in subversion so perhaps I cleaned that up before 
committing :-)

Thanks for the review!

Regards,

Anthony Liguori

>
> >         f->buf_index = 0;
> >     }
> > }
>