From: Orit Wasserman <owasserm@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org, quintela@redhat.com
Subject: Re: [Qemu-devel] [PATCH 02/12] migration: consolidate QEMUFile methods in a single QEMUFileOps struct
Date: Sun, 28 Oct 2012 13:24:55 +0200 [thread overview]
Message-ID: <508D1607.6080909@redhat.com> (raw)
In-Reply-To: <1350555758-29988-3-git-send-email-pbonzini@redhat.com>
On 10/18/2012 12:22 PM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> buffered_file.c | 13 ++++---
> qemu-file.h | 16 +++++----
> savevm.c | 108 +++++++++++++++++++++++++++++++-------------------------
> 3 file modificati, 79 inserzioni(+), 58 rimozioni(-)
>
> diff --git a/buffered_file.c b/buffered_file.c
> index ed92df1..a5c0b12 100644
> --- a/buffered_file.c
> +++ b/buffered_file.c
> @@ -234,6 +234,14 @@ static void buffered_rate_tick(void *opaque)
> buffered_put_buffer(s, NULL, 0, 0);
> }
>
> +static const QEMUFileOps buffered_file_ops = {
> + .put_buffer = buffered_put_buffer,
> + .close = buffered_close,
> + .rate_limit = buffered_rate_limit,
> + .get_rate_limit = buffered_get_rate_limit,
> + .set_rate_limit = buffered_set_rate_limit,
> +};
> +
> QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state)
> {
> QEMUFileBuffered *s;
> @@ -243,10 +251,7 @@ QEMUFile *qemu_fopen_ops_buffered(MigrationState *migration_state)
> s->migration_state = migration_state;
> s->xfer_limit = migration_state->bandwidth_limit / 10;
>
> - s->file = qemu_fopen_ops(s, buffered_put_buffer, NULL,
> - buffered_close, buffered_rate_limit,
> - buffered_set_rate_limit,
> - buffered_get_rate_limit);
> + s->file = qemu_fopen_ops(s, &buffered_file_ops);
>
> s->timer = qemu_new_timer_ms(rt_clock, buffered_rate_tick, s);
>
> diff --git a/qemu-file.h b/qemu-file.h
> index 9c8985b..c89e8e0 100644
> --- a/qemu-file.h
> +++ b/qemu-file.h
> @@ -59,12 +59,16 @@ typedef int (QEMUFileRateLimit)(void *opaque);
> typedef int64_t (QEMUFileSetRateLimit)(void *opaque, int64_t new_rate);
> typedef int64_t (QEMUFileGetRateLimit)(void *opaque);
>
> -QEMUFile *qemu_fopen_ops(void *opaque, QEMUFilePutBufferFunc *put_buffer,
> - QEMUFileGetBufferFunc *get_buffer,
> - QEMUFileCloseFunc *close,
> - QEMUFileRateLimit *rate_limit,
> - QEMUFileSetRateLimit *set_rate_limit,
> - QEMUFileGetRateLimit *get_rate_limit);
> +typedef struct QEMUFileOps {
> + QEMUFilePutBufferFunc *put_buffer;
> + QEMUFileGetBufferFunc *get_buffer;
> + QEMUFileCloseFunc *close;
> + QEMUFileRateLimit *rate_limit;
> + QEMUFileSetRateLimit *set_rate_limit;
> + QEMUFileGetRateLimit *get_rate_limit;
> +} QEMUFileOps;
> +
> +QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops);
> QEMUFile *qemu_fopen(const char *filename, const char *mode);
> QEMUFile *qemu_fdopen(int fd, const char *mode);
> QEMUFile *qemu_fopen_socket(int fd);
> diff --git a/savevm.c b/savevm.c
> index 7068390..c3ce00a 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -162,12 +162,7 @@ void qemu_announce_self(void)
> #define IO_BUF_SIZE 32768
>
> struct QEMUFile {
> - QEMUFilePutBufferFunc *put_buffer;
> - QEMUFileGetBufferFunc *get_buffer;
> - QEMUFileCloseFunc *close;
> - QEMUFileRateLimit *rate_limit;
> - QEMUFileSetRateLimit *set_rate_limit;
> - QEMUFileGetRateLimit *get_rate_limit;
> + const QEMUFileOps *ops;
> void *opaque;
> int is_write;
>
> @@ -256,6 +251,16 @@ static int stdio_fclose(void *opaque)
> return ret;
> }
>
> +static const QEMUFileOps stdio_pipe_read_ops = {
> + .get_buffer = stdio_get_buffer,
> + .close = stdio_pclose
> +};
> +
> +static const QEMUFileOps stdio_pipe_write_ops = {
> + .put_buffer = stdio_put_buffer,
> + .close = stdio_pclose
> +};
> +
> QEMUFile *qemu_popen(FILE *stdio_file, const char *mode)
> {
> QEMUFileStdio *s;
> @@ -270,11 +275,9 @@ QEMUFile *qemu_popen(FILE *stdio_file, const char *mode)
> s->stdio_file = stdio_file;
>
> if(mode[0] == 'r') {
> - s->file = qemu_fopen_ops(s, NULL, stdio_get_buffer, stdio_pclose,
> - NULL, NULL, NULL);
> + s->file = qemu_fopen_ops(s, &stdio_pipe_read_ops);
> } else {
> - s->file = qemu_fopen_ops(s, stdio_put_buffer, NULL, stdio_pclose,
> - NULL, NULL, NULL);
> + s->file = qemu_fopen_ops(s, &stdio_pipe_write_ops);
> }
> return s->file;
> }
> @@ -302,6 +305,16 @@ int qemu_stdio_fd(QEMUFile *f)
> return fd;
> }
>
> +static const QEMUFileOps stdio_file_read_ops = {
> + .get_buffer = stdio_get_buffer,
> + .close = stdio_fclose
> +};
> +
> +static const QEMUFileOps stdio_file_write_ops = {
> + .put_buffer = stdio_put_buffer,
> + .close = stdio_fclose
> +};
> +
> QEMUFile *qemu_fdopen(int fd, const char *mode)
> {
> QEMUFileStdio *s;
> @@ -319,11 +332,9 @@ QEMUFile *qemu_fdopen(int fd, const char *mode)
> goto fail;
>
> if(mode[0] == 'r') {
> - s->file = qemu_fopen_ops(s, NULL, stdio_get_buffer, stdio_fclose,
> - NULL, NULL, NULL);
> + s->file = qemu_fopen_ops(s, &stdio_file_read_ops);
> } else {
> - s->file = qemu_fopen_ops(s, stdio_put_buffer, NULL, stdio_fclose,
> - NULL, NULL, NULL);
> + s->file = qemu_fopen_ops(s, &stdio_file_write_ops);
> }
> return s->file;
>
> @@ -332,13 +343,17 @@ fail:
> return NULL;
> }
>
> +static const QEMUFileOps socket_read_ops = {
> + .get_buffer = socket_get_buffer,
> + .close = socket_close
> +};
> +
> QEMUFile *qemu_fopen_socket(int fd)
> {
> QEMUFileSocket *s = g_malloc0(sizeof(QEMUFileSocket));
>
> s->fd = fd;
> - s->file = qemu_fopen_ops(s, NULL, socket_get_buffer, socket_close,
> - NULL, NULL, NULL);
> + s->file = qemu_fopen_ops(s, &socket_read_ops);
> return s->file;
> }
>
> @@ -360,11 +375,9 @@ QEMUFile *qemu_fopen(const char *filename, const char *mode)
> goto fail;
>
> if(mode[0] == 'w') {
> - s->file = qemu_fopen_ops(s, stdio_put_buffer, NULL, stdio_fclose,
> - NULL, NULL, NULL);
> + s->file = qemu_fopen_ops(s, &stdio_file_write_ops);
> } else {
> - s->file = qemu_fopen_ops(s, NULL, stdio_get_buffer, stdio_fclose,
> - NULL, NULL, NULL);
> + s->file = qemu_fopen_ops(s, &stdio_file_read_ops);
> }
> return s->file;
> fail:
> @@ -389,32 +402,31 @@ static int bdrv_fclose(void *opaque)
> return bdrv_flush(opaque);
> }
>
> +static const QEMUFileOps bdrv_read_ops = {
> + .get_buffer = block_get_buffer,
> + .close = bdrv_fclose
> +};
> +
> +static const QEMUFileOps bdrv_write_ops = {
> + .put_buffer = block_put_buffer,
> + .close = bdrv_fclose
> +};
> +
> static QEMUFile *qemu_fopen_bdrv(BlockDriverState *bs, int is_writable)
> {
> if (is_writable)
> - return qemu_fopen_ops(bs, block_put_buffer, NULL, bdrv_fclose,
> - NULL, NULL, NULL);
> - return qemu_fopen_ops(bs, NULL, block_get_buffer, bdrv_fclose, NULL, NULL, NULL);
> + return qemu_fopen_ops(bs, &bdrv_write_ops);
> + return qemu_fopen_ops(bs, &bdrv_read_ops);
> }
>
> -QEMUFile *qemu_fopen_ops(void *opaque, QEMUFilePutBufferFunc *put_buffer,
> - QEMUFileGetBufferFunc *get_buffer,
> - QEMUFileCloseFunc *close,
> - QEMUFileRateLimit *rate_limit,
> - QEMUFileSetRateLimit *set_rate_limit,
> - QEMUFileGetRateLimit *get_rate_limit)
> +QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops)
> {
> QEMUFile *f;
>
> f = g_malloc0(sizeof(QEMUFile));
>
> f->opaque = opaque;
> - f->put_buffer = put_buffer;
> - f->get_buffer = get_buffer;
> - f->close = close;
> - f->rate_limit = rate_limit;
> - f->set_rate_limit = set_rate_limit;
> - f->get_rate_limit = get_rate_limit;
> + f->ops = ops;
> f->is_write = 0;
>
> return f;
> @@ -437,11 +449,11 @@ static int qemu_fflush(QEMUFile *f)
> {
> int ret = 0;
>
> - if (!f->put_buffer)
> + if (!f->ops->put_buffer)
> return 0;
>
> if (f->is_write && f->buf_index > 0) {
> - ret = f->put_buffer(f->opaque, f->buf, f->buf_offset, f->buf_index);
> + ret = f->ops->put_buffer(f->opaque, f->buf, f->buf_offset, f->buf_index);
> if (ret >= 0) {
> f->buf_offset += f->buf_index;
> }
> @@ -455,7 +467,7 @@ static void qemu_fill_buffer(QEMUFile *f)
> int len;
> int pending;
>
> - if (!f->get_buffer)
> + if (!f->ops->get_buffer)
> return;
>
> if (f->is_write)
> @@ -468,7 +480,7 @@ static void qemu_fill_buffer(QEMUFile *f)
> f->buf_index = 0;
> f->buf_size = pending;
>
> - len = f->get_buffer(f->opaque, f->buf + pending, f->buf_offset,
> + len = f->ops->get_buffer(f->opaque, f->buf + pending, f->buf_offset,
> IO_BUF_SIZE - pending);
> if (len > 0) {
> f->buf_size += len;
> @@ -492,8 +504,8 @@ int qemu_fclose(QEMUFile *f)
> int ret;
> ret = qemu_fflush(f);
>
> - if (f->close) {
> - int ret2 = f->close(f->opaque);
> + if (f->ops->close) {
> + int ret2 = f->ops->close(f->opaque);
> if (ret >= 0) {
> ret = ret2;
> }
> @@ -510,7 +522,7 @@ int qemu_fclose(QEMUFile *f)
>
> int qemu_file_put_notify(QEMUFile *f)
> {
> - return f->put_buffer(f->opaque, NULL, 0, 0);
> + return f->ops->put_buffer(f->opaque, NULL, 0, 0);
> }
>
> void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
> @@ -657,16 +669,16 @@ static int64_t qemu_ftell(QEMUFile *f)
>
> int qemu_file_rate_limit(QEMUFile *f)
> {
> - if (f->rate_limit)
> - return f->rate_limit(f->opaque);
> + if (f->ops->rate_limit)
> + return f->ops->rate_limit(f->opaque);
>
> return 0;
> }
>
> int64_t qemu_file_get_rate_limit(QEMUFile *f)
> {
> - if (f->get_rate_limit)
> - return f->get_rate_limit(f->opaque);
> + if (f->ops->get_rate_limit)
> + return f->ops->get_rate_limit(f->opaque);
>
> return 0;
> }
> @@ -675,8 +687,8 @@ int64_t qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate)
> {
> /* any failed or completed migration keeps its state to allow probing of
> * migration data, but has no associated file anymore */
> - if (f && f->set_rate_limit)
> - return f->set_rate_limit(f->opaque, new_rate);
> + if (f && f->ops->set_rate_limit)
> + return f->ops->set_rate_limit(f->opaque, new_rate);
>
> return 0;
> }
>
Reviewed-by: Orit Wasserman <owasserm@redhat.com>
next prev parent reply other threads:[~2012-10-28 9:25 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-18 10:22 [Qemu-devel] [PATCH 0/9] Incoming migration coroutine Paolo Bonzini
2012-10-18 10:22 ` [Qemu-devel] [PATCH 01/12] migration: unify stdio-based QEMUFile operations Paolo Bonzini
2012-10-28 11:20 ` Orit Wasserman
2012-10-30 12:19 ` Juan Quintela
2012-10-18 10:22 ` [Qemu-devel] [PATCH 02/12] migration: consolidate QEMUFile methods in a single QEMUFileOps struct Paolo Bonzini
2012-10-28 11:24 ` Orit Wasserman [this message]
2012-10-30 12:26 ` Juan Quintela
2012-10-18 10:22 ` [Qemu-devel] [PATCH 03/12] migration: add qemu_get_fd Paolo Bonzini
2012-10-28 11:27 ` Orit Wasserman
2012-10-30 12:27 ` Juan Quintela
2012-10-18 10:22 ` [Qemu-devel] [PATCH 04/12] migration: replace qemu_stdio_fd with qemu_get_fd Paolo Bonzini
2012-10-28 9:29 ` Orit Wasserman
2012-10-30 12:28 ` Juan Quintela
2012-10-18 10:22 ` [Qemu-devel] [PATCH 05/12] migration: clean up server sockets and handlers before invoking process_incoming_migration Paolo Bonzini
2012-10-28 9:35 ` Orit Wasserman
2012-10-30 12:29 ` Juan Quintela
2012-10-18 10:22 ` [Qemu-devel] [PATCH 06/12] migration: use migrate_fd_close in migrate_fd_cleanup Paolo Bonzini
2012-10-28 9:47 ` Orit Wasserman
2012-10-28 15:05 ` Paolo Bonzini
2012-10-18 10:22 ` [Qemu-devel] [PATCH 07/12] migration: use closesocket, not close Paolo Bonzini
2012-10-28 9:49 ` Orit Wasserman
2012-10-30 12:32 ` Juan Quintela
2012-10-18 10:22 ` [Qemu-devel] [PATCH 08/12] migration: xxx_close will only be called once Paolo Bonzini
2012-10-28 9:53 ` Orit Wasserman
2012-10-30 12:35 ` Juan Quintela
2012-10-18 10:22 ` [Qemu-devel] [PATCH 09/12] migration: close socket QEMUFile from socket_close Paolo Bonzini
2012-10-28 9:55 ` Orit Wasserman
2012-10-30 12:40 ` Juan Quintela
2012-10-18 10:22 ` [Qemu-devel] [PATCH 10/12] migration: move qemu_fclose to process_incoming_migration Paolo Bonzini
2012-10-28 9:56 ` Orit Wasserman
2012-10-18 10:22 ` [Qemu-devel] [PATCH 11/12] migration: handle EAGAIN while reading QEMUFile Paolo Bonzini
2012-10-28 10:01 ` Orit Wasserman
2012-10-18 10:22 ` [Qemu-devel] [PATCH 12/12] migration: move process_incoming_migration to a coroutine Paolo Bonzini
2012-10-28 10:07 ` Orit Wasserman
-- strict thread matches above, loose matches on Subject: below --
2012-11-02 17:50 [Qemu-devel] [PULL 00/12] Incoming migration coroutine Paolo Bonzini
2012-11-02 17:50 ` [Qemu-devel] [PATCH 02/12] migration: consolidate QEMUFile methods in a single QEMUFileOps struct Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=508D1607.6080909@redhat.com \
--to=owasserm@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=quintela@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).