[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 32/41] qemu-file: add writable socket QEMUFile
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 32/41] qemu-file: add writable socket QEMUFile |
Date: |
Thu, 21 Feb 2013 07:09:27 -0500 (EST) |
> > +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;
> > }
> >
> >
>
>
- [Qemu-devel] [PATCH 33/41] qemu-file: simplify and export qemu_ftell, (continued)
- [Qemu-devel] [PATCH 33/41] qemu-file: simplify and export qemu_ftell, Paolo Bonzini, 2013/02/15
- [Qemu-devel] [PATCH 15/41] block-migration: remove variables that are never read, Paolo Bonzini, 2013/02/15
- [Qemu-devel] [PATCH 06/41] qemu-file: pass errno from qemu_fflush via f->last_error, Paolo Bonzini, 2013/02/15
- [Qemu-devel] [PATCH 32/41] qemu-file: add writable socket QEMUFile, Paolo Bonzini, 2013/02/15
- [Qemu-devel] [PATCH 10/41] migration: use qemu_file_set_error, Paolo Bonzini, 2013/02/15
- [Qemu-devel] [PATCH 12/41] migration: do not nest flushing of device data, Paolo Bonzini, 2013/02/15
- [Qemu-devel] [PATCH 14/41] migration: cleanup migration (including thread) in the iothread, Paolo Bonzini, 2013/02/15
- [Qemu-devel] [PATCH 11/41] migration: simplify error handling, Paolo Bonzini, 2013/02/15