Message ID | 1360950433-17106-33-git-send-email-pbonzini@redhat.com |
---|---|
State | New |
Headers | show |
On 02/15/2013 07:47 PM, Paolo Bonzini wrote: > Signed-off-by: Paolo Bonzini <pbonzini@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; > } > >
> > +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 Ok. Paolo > > + 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; > > } > > > > > >
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(); + } + 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; + } break; }
Signed-off-by: Paolo Bonzini <pbonzini@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(-)