diff mbox

[v7,10/42] Return path: Open a return path on QEMUFile for sockets

Message ID 1434450415-11339-11-git-send-email-dgilbert@redhat.com
State New
Headers show

Commit Message

Dr. David Alan Gilbert June 16, 2015, 10:26 a.m. UTC 
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(-)

Comments

Juan Quintela June 17, 2015, 12:23 p.m. UTC | #1 
"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

For the cest, I am ok with the patch.

Reviewed-by: Juan Quintela <quintela@redhat.com>


> +    /* 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 June 17, 2015, 5:07 p.m. UTC | #2 
* 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
Amit Shah July 13, 2015, 10:12 a.m. UTC | #3 
On (Tue) 16 Jun 2015 [11:26:23], Dr. David Alan Gilbert (git) 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>

Reviewed-by: Amit Shah <amit.shah@redhat.com>

Thanks, this looks better than the dup way of doing it.

		Amit
diff mbox

Patch

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,
                                uint64_t *bytes_sent);
 
 /*
+ * Return a QEMUFile for comms in the opposite direction
+ */
+typedef QEMUFile *(QEMURetPathFunc)(void *opaque);
+
+/*
  * Stop any read or write (depending on flags) on the underlying
  * transport on the QEMUFile.
  * Existing blocking reads/writes must be woken
@@ -102,6 +107,7 @@  typedef struct QEMUFileOps {
     QEMURamHookFunc *after_ram_iterate;
     QEMURamHookFunc *hook_ram_load;
     QEMURamSaveFunc *save_page;
+    QEMURetPathFunc *get_return_path;
     QEMUFileShutdownFunc *shut_down;
 } QEMUFileOps;
 
@@ -192,6 +198,7 @@  int64_t qemu_file_get_rate_limit(QEMUFile *f);
 int qemu_file_get_error(QEMUFile *f);
 void qemu_file_set_error(QEMUFile *f, int ret);
 int qemu_file_shutdown(QEMUFile *f);
+QEMUFile *qemu_file_get_return_path(QEMUFile *f);
 void qemu_fflush(QEMUFile *f);
 void qemu_file_change_blocking(QEMUFile *f, bool block);
 
diff --git a/migration/qemu-file-unix.c b/migration/qemu-file-unix.c
index bfbc086..561621e 100644
--- a/migration/qemu-file-unix.c
+++ b/migration/qemu-file-unix.c
@@ -96,6 +96,56 @@  static int socket_shutdown(void *opaque, bool rd, bool wr)
     }
 }
 
+static int socket_return_close(void *opaque)
+{
+    QEMUFileSocket *s = opaque;
+    /*
+     * Note: We don't close the socket, that should be done by the forward
+     * path.
+     */
+    g_free(s);
+    return 0;
+}
+
+static const QEMUFileOps socket_return_read_ops = {
+    .get_fd          = socket_get_fd,
+    .get_buffer      = socket_get_buffer,
+    .close           = socket_return_close,
+    .shut_down       = socket_shutdown,
+};
+
+static const QEMUFileOps socket_return_write_ops = {
+    .get_fd          = socket_get_fd,
+    .writev_buffer   = socket_writev_buffer,
+    .close           = socket_return_close,
+    .shut_down       = socket_shutdown,
+};
+
+/*
+ * Give a QEMUFile* off the same socket but data in the opposite
+ * direction.
+ */
+static QEMUFile *socket_dup_return_path(void *opaque)
+{
+    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;
+    /* 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 ||