On 02/15/2013 07:47 PM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> include/migration/qemu-file.h | 2 +-
> migration-tcp.c | 2 +-
> migration-unix.c | 2 +-
> savevm.c | 33 +++++++++++++++++++++++++++++++--
> util/osdep.c | 6 +++++-
> 5 files changed, 39 insertions(+), 6 deletions(-)
>
> diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
> index 987e719..25e8461 100644
> --- a/include/migration/qemu-file.h
> +++ b/include/migration/qemu-file.h
> @@ -76,7 +76,7 @@ typedef struct 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);
> +QEMUFile *qemu_fopen_socket(int fd, const char *mode);
> QEMUFile *qemu_popen_cmd(const char *command, const char *mode);
> int qemu_get_fd(QEMUFile *f);
> int qemu_fclose(QEMUFile *f);
> diff --git a/migration-tcp.c b/migration-tcp.c
> index e78a296..7d975b5 100644
> --- a/migration-tcp.c
> +++ b/migration-tcp.c
> @@ -95,7 +95,7 @@ static void tcp_accept_incoming_migration(void *opaque)
> goto out;
> }
>
> - f = qemu_fopen_socket(c);
> + f = qemu_fopen_socket(c, "rb");
> if (f == NULL) {
> fprintf(stderr, "could not qemu_fopen socket\n");
> goto out;
> diff --git a/migration-unix.c b/migration-unix.c
> index 218835a..4693b43 100644
> --- a/migration-unix.c
> +++ b/migration-unix.c
> @@ -95,7 +95,7 @@ static void unix_accept_incoming_migration(void *opaque)
> goto out;
> }
>
> - f = qemu_fopen_socket(c);
> + f = qemu_fopen_socket(c, "rb");
> if (f == NULL) {
> fprintf(stderr, "could not qemu_fopen socket\n");
> goto out;
> diff --git a/savevm.c b/savevm.c
> index f593acd..b6cf415 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -198,6 +198,18 @@ static int socket_get_buffer(void *opaque, uint8_t *buf,
> int64_t pos, int size)
> return len;
> }
>
> +static int socket_put_buffer(void *opaque, const uint8_t *buf, int64_t pos,
> int size)
> +{
> + QEMUFileSocket *s = opaque;
> + ssize_t len;
> +
> + len = qemu_send_full(s->fd, buf, size, 0);
> + if (len == -1) {
> + len = -socket_error();
> + }
We won't always detect partial buffer write here
Why not check len < size? this way you won't need to change qemu_send_full
> + return len;
> +}
> +
> static int socket_close(void *opaque)
> {
> QEMUFileSocket *s = opaque;
> @@ -369,12 +381,29 @@ static const QEMUFileOps socket_read_ops = {
> .close = socket_close
> };
>
> -QEMUFile *qemu_fopen_socket(int fd)
> +static const QEMUFileOps socket_write_ops = {
> + .get_fd = socket_get_fd,
> + .put_buffer = socket_put_buffer,
> + .close = socket_close
> +};
> +
> +QEMUFile *qemu_fopen_socket(int fd, const char *mode)
> {
> QEMUFileSocket *s = g_malloc0(sizeof(QEMUFileSocket));
>
> + if (mode == NULL ||
> + (mode[0] != 'r' && mode[0] != 'w') ||
> + mode[1] != 'b' || mode[2] != 0) {
> + fprintf(stderr, "qemu_fopen: Argument validity check failed\n");
> + return NULL;
> + }
> +
> s->fd = fd;
> - s->file = qemu_fopen_ops(s, &socket_read_ops);
> + if (mode[0] == 'w') {
> + s->file = qemu_fopen_ops(s, &socket_write_ops);
> + } else {
> + s->file = qemu_fopen_ops(s, &socket_read_ops);
> + }
> return s->file;
> }
>
> diff --git a/util/osdep.c b/util/osdep.c
> index 5b51a03..be168e5 100644
> --- a/util/osdep.c
> +++ b/util/osdep.c
> @@ -243,8 +243,12 @@ ssize_t qemu_write_full(int fd, const void *buf, size_t
> count)
> while (count) {
> ret = write(fd, buf, count);
> if (ret < 0) {
> - if (errno == EINTR)
> + if (errno == EINTR) {
> continue;
> + }
> + if (total == 0) {
> + total = ret;
> + }
I don't like it, first it only detect the first write failure (if the error
happens when total !=0
the function won't return -1).
Secondly this behavior is different from the function documentation.
Regards,
Orit
> break;
> }
>
>
