Re: [Qemu-devel] [PATCH v7 10/42] Return path: Open a return path on QEMUFile for sockets

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Juan Quintela (quintela@redhat.com) wrote:
> "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > Postcopy needs a method to send messages from the destination back to
> > the source, this is the 'return path'.
> >
> > Wire it up for 'socket' QEMUFile's.
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >  include/migration/qemu-file.h |  7 +++++
> >  migration/qemu-file-unix.c    | 69 +++++++++++++++++++++++++++++++++++++------
> >  migration/qemu-file.c         | 12 ++++++++
> >  3 files changed, 79 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
> > index d43c835..7721c42 100644
> > --- a/include/migration/qemu-file.h
> > +++ b/include/migration/qemu-file.h
> > @@ -85,6 +85,11 @@ typedef size_t (QEMURamSaveFunc)(QEMUFile *f, void *opaque,
> 
> Hi
> 
> > +/*
> > + * Give a QEMUFile* off the same socket but data in the opposite
> > + * direction.
> > + */
> > +static QEMUFile *socket_dup_return_path(void *opaque)
> 
> We call it dup
> 
> > +{
> > +    QEMUFileSocket *forward = opaque;
> > +    QEMUFileSocket *reverse;
> > +
> > +    if (qemu_file_get_error(forward->file)) {
> > +        /* If the forward file is in error, don't try and open a return */
> > +        return NULL;
> > +    }
> > +
> > +    reverse = g_malloc0(sizeof(QEMUFileSocket));
> > +    reverse->fd = forward->fd;
> 
> But we don't dup it :p

Oh yeh, we used to :-)  I've replaced _dup with _get

> For the cest, I am ok with the patch.
> 
> Reviewed-by: Juan Quintela <quintela@redhat.com>

Thanks.

Dave

> 
> 
> > +    /* I don't think there's a better way to tell which direction 'this' is */
> > +    if (forward->file->ops->get_buffer != NULL) {
> > +        /* being called from the read side, so we need to be able to write */
> > +        return qemu_fopen_ops(reverse, &socket_return_write_ops);
> > +    } else {
> > +        return qemu_fopen_ops(reverse, &socket_return_read_ops);
> > +    }
> > +}
> > +
> >  static ssize_t unix_writev_buffer(void *opaque, struct iovec *iov, int iovcnt,
> >                                    int64_t pos)
> >  {
> > @@ -204,18 +254,19 @@ QEMUFile *qemu_fdopen(int fd, const char *mode)
> >  }
> >  
> >  static const QEMUFileOps socket_read_ops = {
> > -    .get_fd     = socket_get_fd,
> > -    .get_buffer = socket_get_buffer,
> > -    .close      = socket_close,
> > -    .shut_down  = socket_shutdown
> > -
> > +    .get_fd          = socket_get_fd,
> > +    .get_buffer      = socket_get_buffer,
> > +    .close           = socket_close,
> > +    .shut_down       = socket_shutdown,
> > +    .get_return_path = socket_dup_return_path
> >  };
> >  
> >  static const QEMUFileOps socket_write_ops = {
> > -    .get_fd        = socket_get_fd,
> > -    .writev_buffer = socket_writev_buffer,
> > -    .close         = socket_close,
> > -    .shut_down     = socket_shutdown
> > +    .get_fd          = socket_get_fd,
> > +    .writev_buffer   = socket_writev_buffer,
> > +    .close           = socket_close,
> > +    .shut_down       = socket_shutdown,
> > +    .get_return_path = socket_dup_return_path
> >  };
> >  
> >  QEMUFile *qemu_fopen_socket(int fd, const char *mode)
> > diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> > index c746129..7d9d983 100644
> > --- a/migration/qemu-file.c
> > +++ b/migration/qemu-file.c
> > @@ -43,6 +43,18 @@ int qemu_file_shutdown(QEMUFile *f)
> >      return f->ops->shut_down(f->opaque, true, true);
> >  }
> >  
> > +/*
> > + * Result: QEMUFile* for a 'return path' for comms in the opposite direction
> > + *         NULL if not available
> > + */
> > +QEMUFile *qemu_file_get_return_path(QEMUFile *f)
> > +{
> > +    if (!f->ops->get_return_path) {
> > +        return NULL;
> > +    }
> > +    return f->ops->get_return_path(f->opaque);
> > +}
> > +
> >  bool qemu_file_mode_is_not_valid(const char *mode)
> >  {
> >      if (mode == NULL ||
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK