From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yoshiaki Tamura Subject: Re: [RFC PATCH 07/20] Introduce qemu_put_vector() and qemu_put_vector_prepare() to use put_vector() in QEMUFile. Date: Fri, 23 Apr 2010 13:02:20 +0900 Message-ID: <4BD11BCC.2050908@lab.ntt.co.jp> References: <1271829445-5328-1-git-send-email-tamura.yoshiaki@lab.ntt.co.jp> <1271829445-5328-8-git-send-email-tamura.yoshiaki@lab.ntt.co.jp> <4BD0A3B0.8040609@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, qemu-devel@nongnu.org, avi@redhat.com, Anthony Liguori , mtosatti@redhat.com, ohmura.kei@lab.ntt.co.jp, yoshikawa.takuya@oss.ntt.co.jp To: Anthony Liguori Return-path: Received: from tama500.ecl.ntt.co.jp ([129.60.39.148]:58359 "EHLO tama500.ecl.ntt.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750722Ab0DWECq (ORCPT ); Fri, 23 Apr 2010 00:02:46 -0400 In-Reply-To: <4BD0A3B0.8040609@linux.vnet.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: Anthony Liguori wrote: > On 04/21/2010 12:57 AM, Yoshiaki Tamura wrote: >> For fool proof purpose, qemu_put_vector_parepare should be called >> before qemu_put_vector. Then, if qemu_put_* functions except this is >> called after qemu_put_vector_prepare, program will abort(). >> >> Signed-off-by: Yoshiaki Tamura > > I don't get it. What's this protecting against? This was introduced to prevent mixing the order of normal write and vector write, and flush QEMUFile buffer before handling vectors. While qemu_put_buffer copies data to QEMUFile buffer, qemu_put_vector() will bypass that buffer. It's just fool proof purpose for what we encountered at beginning, and if the user of qemu_put_vector() is careful enough, we can remove qemu_put_vectore_prepare(). While writing this message, I started to think that just calling qemu_fflush() in qemu_put_vector() would be enough... > > Regards, > > Anthony Liguori > >> --- >> hw/hw.h | 2 ++ >> savevm.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 55 insertions(+), 0 deletions(-) >> >> diff --git a/hw/hw.h b/hw/hw.h >> index 921cf90..10e6dda 100644 >> --- a/hw/hw.h >> +++ b/hw/hw.h >> @@ -77,6 +77,8 @@ void qemu_fflush(QEMUFile *f); >> int qemu_fclose(QEMUFile *f); >> void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size); >> void qemu_put_byte(QEMUFile *f, int v); >> +void qemu_put_vector(QEMUFile *f, QEMUIOVector *qiov); >> +void qemu_put_vector_prepare(QEMUFile *f); >> void *qemu_realloc_buffer(QEMUFile *f, int size); >> void qemu_clear_buffer(QEMUFile *f); >> >> diff --git a/savevm.c b/savevm.c >> index 944e788..22d928c 100644 >> --- a/savevm.c >> +++ b/savevm.c >> @@ -180,6 +180,7 @@ struct QEMUFile { >> uint8_t *buf; >> >> int has_error; >> + int prepares_vector; >> }; >> >> typedef struct QEMUFileStdio >> @@ -557,6 +558,58 @@ void qemu_put_byte(QEMUFile *f, int v) >> qemu_fflush(f); >> } >> >> +void qemu_put_vector(QEMUFile *f, QEMUIOVector *v) >> +{ >> + struct iovec *iov; >> + int cnt; >> + size_t bufsize; >> + uint8_t *buf; >> + >> + if (qemu_file_get_rate_limit(f) != 0) { >> + fprintf(stderr, >> + "Attempted to write vector while bandwidth limit is not zero.\n"); >> + abort(); >> + } >> + >> + /* checks prepares vector. >> + * For fool proof purpose, qemu_put_vector_parepare should be called >> + * before qemu_put_vector. Then, if qemu_put_* functions except this >> + * is called after qemu_put_vector_prepare, program will abort(). >> + */ >> + if (!f->prepares_vector) { >> + fprintf(stderr, >> + "You should prepare with qemu_put_vector_prepare.\n"); >> + abort(); >> + } else if (f->prepares_vector&& f->buf_index != 0) { >> + fprintf(stderr, "Wrote data after qemu_put_vector_prepare.\n"); >> + abort(); >> + } >> + f->prepares_vector = 0; >> + >> + if (f->put_vector) { >> + qemu_iovec_to_vector(v,&iov,&cnt); >> + f->put_vector(f->opaque, iov, 0, cnt); >> + } else { >> + qemu_iovec_to_size(v,&bufsize); >> + buf = qemu_malloc(bufsize + 1 /* for '\0' */); >> + qemu_iovec_to_buffer(v, buf); >> + qemu_put_buffer(f, buf, bufsize); >> + qemu_free(buf); >> + } >> + >> +} >> + >> +void qemu_put_vector_prepare(QEMUFile *f) >> +{ >> + if (f->prepares_vector) { >> + /* prepare vector */ >> + fprintf(stderr, "Attempted to prepare vector twice\n"); >> + abort(); >> + } >> + f->prepares_vector = 1; >> + qemu_fflush(f); >> +} >> + >> int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size1) >> { >> int size, l; > > > >