| Message ID | 1271829445-5328-5-git-send-email-tamura.yoshiaki@lab.ntt.co.jp |
|---|---|
| State | New |
| Headers | show |
On Wed, Apr 21, 2010 at 6:57 AM, Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp> wrote: > @@ -454,6 +458,25 @@ void qemu_fflush(QEMUFile *f) > } > } > > +void *qemu_realloc_buffer(QEMUFile *f, int size) > +{ > + f->buf_max_size = size; > + > + f->buf = qemu_realloc(f->buf, f->buf_max_size); > + if (f->buf == NULL) { > + fprintf(stderr, "qemu file buffer realloc failed\n"); > + exit(1); > + } > + > + return f->buf; > +} > + qemu_realloc() will abort() if there was not enough memory to realloc. Just like qemu_malloc(), you don't need to check for NULL. Stefan
2010/4/21 Stefan Hajnoczi <stefanha@gmail.com>: > On Wed, Apr 21, 2010 at 6:57 AM, Yoshiaki Tamura > <tamura.yoshiaki@lab.ntt.co.jp> wrote: >> @@ -454,6 +458,25 @@ void qemu_fflush(QEMUFile *f) >> } >> } >> >> +void *qemu_realloc_buffer(QEMUFile *f, int size) >> +{ >> + f->buf_max_size = size; >> + >> + f->buf = qemu_realloc(f->buf, f->buf_max_size); >> + if (f->buf == NULL) { >> + fprintf(stderr, "qemu file buffer realloc failed\n"); >> + exit(1); >> + } >> + >> + return f->buf; >> +} >> + > > qemu_realloc() will abort() if there was not enough memory to realloc. > Just like qemu_malloc(), you don't need to check for NULL. Thanks for your comment. I'll remove it. If there is no objection, I would like to take out this patch from the series, and post it by itself. Yoshi > > Stefan > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On 04/21/2010 08:57 AM, Yoshiaki Tamura wrote: > Currently buf size is fixed at 32KB. It would be useful if it could > be flexible. > > Why is this needed? The real buffering is in the kernel anyways; this is only used to reduce the number of write() syscalls.
Avi Kivity wrote: > On 04/21/2010 08:57 AM, Yoshiaki Tamura wrote: >> Currently buf size is fixed at 32KB. It would be useful if it could >> be flexible. >> > > Why is this needed? The real buffering is in the kernel anyways; this is > only used to reduce the number of write() syscalls. This was introduced to buffer the transfered guests image transaction ally on the receiver side. The sender doesn't use it. In case of intermediate state, we just discard this buffer.
On 04/23/2010 12:59 PM, Yoshiaki Tamura wrote: > Avi Kivity wrote: >> On 04/21/2010 08:57 AM, Yoshiaki Tamura wrote: >>> Currently buf size is fixed at 32KB. It would be useful if it could >>> be flexible. >>> >> >> Why is this needed? The real buffering is in the kernel anyways; this is >> only used to reduce the number of write() syscalls. > > This was introduced to buffer the transfered guests image transaction > ally on the receiver side. The sender doesn't use it. > In case of intermediate state, we just discard this buffer. How large can it grow? What's wrong with applying it (perhaps partially) to the guest state? The next state transfer will overwrite it completely, no?
On 04/23/2010 04:53 AM, Avi Kivity wrote: > On 04/21/2010 08:57 AM, Yoshiaki Tamura wrote: >> Currently buf size is fixed at 32KB. It would be useful if it could >> be flexible. >> > > Why is this needed? The real buffering is in the kernel anyways; this > is only used to reduce the number of write() syscalls. With vmstate, we really shouldn't need to do this magic anymore as an optimization. Regards, Anthony Liguori
Avi Kivity wrote: > On 04/23/2010 12:59 PM, Yoshiaki Tamura wrote: >> Avi Kivity wrote: >>> On 04/21/2010 08:57 AM, Yoshiaki Tamura wrote: >>>> Currently buf size is fixed at 32KB. It would be useful if it could >>>> be flexible. >>>> >>> >>> Why is this needed? The real buffering is in the kernel anyways; this is >>> only used to reduce the number of write() syscalls. >> >> This was introduced to buffer the transfered guests image transaction >> ally on the receiver side. The sender doesn't use it. >> In case of intermediate state, we just discard this buffer. > > How large can it grow? It really depends on what workload is running on the guest, but it should be as large as the guest ram size in the worst case. > What's wrong with applying it (perhaps partially) to the guest state? > The next state transfer will overwrite it completely, no? AFAIK, the answer is no. qemu_loadvm_state() calls load handlers of each device emulator, and they will update its state directly, which means even if the transaction was not complete, it's impossible to recover the previous state if we don't make a buffer. I guess your concern is about consuming large size of ram, and I think having an option for writing the transaction to a temporal disk image should be effective.
diff --git a/hw/hw.h b/hw/hw.h index 05131a0..fc9ed29 100644 --- a/hw/hw.h +++ b/hw/hw.h @@ -61,6 +61,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_realloc_buffer(QEMUFile *f, int size); +void qemu_clear_buffer(QEMUFile *f); static inline void qemu_put_ubyte(QEMUFile *f, unsigned int v) { diff --git a/savevm.c b/savevm.c index 2fd3de6..490ab70 100644 --- a/savevm.c +++ b/savevm.c @@ -174,7 +174,8 @@ struct QEMUFile { when reading */ int buf_index; int buf_size; /* 0 when writing */ - uint8_t buf[IO_BUF_SIZE]; + int buf_max_size; + uint8_t *buf; int has_error; }; @@ -424,6 +425,9 @@ QEMUFile *qemu_fopen_ops(void *opaque, QEMUFilePutBufferFunc *put_buffer, f->get_rate_limit = get_rate_limit; f->is_write = 0; + f->buf_max_size = IO_BUF_SIZE; + f->buf = qemu_mallocz(sizeof(uint8_t) * f->buf_max_size); + return f; } @@ -454,6 +458,25 @@ void qemu_fflush(QEMUFile *f) } } +void *qemu_realloc_buffer(QEMUFile *f, int size) +{ + f->buf_max_size = size; + + f->buf = qemu_realloc(f->buf, f->buf_max_size); + if (f->buf == NULL) { + fprintf(stderr, "qemu file buffer realloc failed\n"); + exit(1); + } + + return f->buf; +} + +void qemu_clear_buffer(QEMUFile *f) +{ + f->buf_size = f->buf_index = f->buf_offset = 0; + memset(f->buf, 0, f->buf_max_size); +} + static void qemu_fill_buffer(QEMUFile *f) { int len; @@ -479,6 +502,7 @@ int qemu_fclose(QEMUFile *f) qemu_fflush(f); if (f->close) ret = f->close(f->opaque); + qemu_free(f->buf); qemu_free(f); return ret; }
Currently buf size is fixed at 32KB. It would be useful if it could be flexible. Signed-off-by: Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp> --- hw/hw.h | 2 ++ savevm.c | 26 +++++++++++++++++++++++++- 2 files changed, 27 insertions(+), 1 deletions(-)