From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:54863) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UIftt-0004d2-C6 for qemu-devel@nongnu.org; Thu, 21 Mar 2013 09:51:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UIftr-0002Bo-HZ for qemu-devel@nongnu.org; Thu, 21 Mar 2013 09:51:25 -0400 Received: from mx1.redhat.com ([209.132.183.28]:31554) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UIftr-0002BV-8i for qemu-devel@nongnu.org; Thu, 21 Mar 2013 09:51:23 -0400 Message-ID: <514B10A2.2030502@redhat.com> Date: Thu, 21 Mar 2013 15:52:34 +0200 From: Orit Wasserman MIME-Version: 1.0 References: <1363872230-15081-1-git-send-email-owasserm@redhat.com> <1363872230-15081-6-git-send-email-owasserm@redhat.com> <514B0EDB.10808@redhat.com> In-Reply-To: <514B0EDB.10808@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 5/8] Use writev ops if available List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: mst@redhat.com, chegu_vinod@hp.com, qemu-devel@nongnu.org, quintela@redhat.com On 03/21/2013 03:44 PM, Paolo Bonzini wrote: > Il 21/03/2013 14:23, Orit Wasserman ha scritto: >> Update qemu_fflush and stdio_close to use writev ops if they are available >> >> Signed-off-by: Orit Wasserman >> --- >> savevm.c | 30 ++++++++++++++++++++---------- >> 1 file changed, 20 insertions(+), 10 deletions(-) >> >> diff --git a/savevm.c b/savevm.c >> index ab81dd3..fde59d3 100644 >> --- a/savevm.c >> +++ b/savevm.c >> @@ -292,7 +292,7 @@ static int stdio_fclose(void *opaque) >> QEMUFileStdio *s = opaque; >> int ret = 0; >> >> - if (s->file->ops->put_buffer) { >> + if (s->file->ops->put_buffer || s->file->ops->writev_buffer) { >> int fd = fileno(s->stdio_file); >> struct stat st; >> >> @@ -515,24 +515,34 @@ static void qemu_file_set_error(QEMUFile *f, int ret) >> } >> } >> >> -/** Flushes QEMUFile buffer >> +/** >> + * Flushes QEMUFile buffer >> * >> + * If there is writev_buffer QEMUFileOps it uses it otherwise uses >> + * put_buffer ops. >> */ >> static void qemu_fflush(QEMUFile *f) >> { >> int ret = 0; >> >> - if (!f->ops->put_buffer) { >> + if (f->ops->writev_buffer) { >> + if (f->is_write && f->iovcnt > 0) { >> + ret = f->ops->writev_buffer(f->opaque, f->iov, f->iovcnt); > > This needs to update f->pos. > >> + } >> + } else if (f->ops->put_buffer) { >> + if (f->is_write && f->buf_index > 0) { >> + ret = f->ops->put_buffer(f->opaque, f->buf, f->pos, f->buf_index); >> + } > > I think this has to go through all the iovecs (i.e. never look at > f->buf_index, only f->iovcnt). Otherwise RAM migration does not work, > because the page data is not copied to f->buf. But that's pretty much > it, the series looks good. You are right I will fix it. > > However, I'd still prefer to have qemu_put_buffer_no_copy coalesce > adjacent iovecs. Otherwise you might end up with only 3-400 bytes in > each sendmsg when serializing device data. Yes this will be helpful for the device state. Orit > > Paolo > >> + } else { >> return; >> } >> - if (f->is_write && f->buf_index > 0) { >> - ret = f->ops->put_buffer(f->opaque, f->buf, f->pos, f->buf_index); >> - if (ret >= 0) { >> - f->pos += f->buf_index; >> - } >> - f->buf_index = 0; >> - f->iovcnt = 0; >> + >> + if (ret >= 0) { >> + f->pos += f->buf_index; >> } >> + f->buf_index = 0; >> + f->iovcnt = 0; >> + >> if (ret < 0) { >> qemu_file_set_error(f, ret); >> } >> >