diff mbox

[v1,12/22] migration: convert exec socket protocol to use QIOChannel

Message ID 1452599056-27357-13-git-send-email-berrange@redhat.com
State New
Headers show

Commit Message

Daniel P. Berrangé Jan. 12, 2016, 11:44 a.m. UTC
Convert the exec socket migration protocol driver to use
QIOChannel and QEMUFileChannel, instead of the stdio
popen APIs. It can be unconditionally built because the
QIOChannelCommand class can report suitable error messages
on platforms which can't fork processes.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 migration/Makefile.objs |  3 +--
 migration/exec.c        | 48 ++++++++++++++++++++++++++++++------------------
 migration/migration.c   |  4 ----
 3 files changed, 31 insertions(+), 24 deletions(-)

Comments

Dr. David Alan Gilbert Feb. 2, 2016, 6:53 p.m. UTC | #1
* Daniel P. Berrange (berrange@redhat.com) wrote:
> Convert the exec socket migration protocol driver to use
> QIOChannel and QEMUFileChannel, instead of the stdio
> popen APIs. It can be unconditionally built because the
> QIOChannelCommand class can report suitable error messages
> on platforms which can't fork processes.
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  migration/Makefile.objs |  3 +--
>  migration/exec.c        | 48 ++++++++++++++++++++++++++++++------------------
>  migration/migration.c   |  4 ----
>  3 files changed, 31 insertions(+), 24 deletions(-)
> 
> diff --git a/migration/Makefile.objs b/migration/Makefile.objs
> index 64f95cd..3c90c44 100644
> --- a/migration/Makefile.objs
> +++ b/migration/Makefile.objs
> @@ -1,11 +1,10 @@
> -common-obj-y += migration.o tcp.o unix.o fd.o
> +common-obj-y += migration.o tcp.o unix.o fd.o exec.o
>  common-obj-y += vmstate.o
>  common-obj-y += qemu-file.o qemu-file-buf.o qemu-file-unix.o qemu-file-stdio.o
>  common-obj-y += qemu-file-channel.o
>  common-obj-y += xbzrle.o postcopy-ram.o
>  
>  common-obj-$(CONFIG_RDMA) += rdma.o
> -common-obj-$(CONFIG_POSIX) += exec.o
>  
>  common-obj-y += block.o
>  
> diff --git a/migration/exec.c b/migration/exec.c
> index 8406d2b..6159aba 100644
> --- a/migration/exec.c
> +++ b/migration/exec.c
> @@ -15,14 +15,8 @@
>   * GNU GPL, version 2 or (at your option) any later version.
>   */
>  
> -#include "qemu-common.h"
> -#include "qemu/sockets.h"
> -#include "qemu/main-loop.h"
>  #include "migration/migration.h"
> -#include "migration/qemu-file.h"
> -#include "block/block.h"
> -#include <sys/types.h>
> -#include <sys/wait.h>
> +#include "io/channel-command.h"
>  
>  //#define DEBUG_MIGRATION_EXEC
>  
> @@ -36,34 +30,52 @@
>  
>  void exec_start_outgoing_migration(MigrationState *s, const char *command, Error **errp)
>  {
> -    s->file = qemu_popen_cmd(command, "w");
> -    if (s->file == NULL) {
> -        error_setg_errno(errp, errno, "failed to popen the migration target");
> +    QIOChannel *ioc;
> +    const char *argv[] = { "/bin/sh", "-c", command, NULL };
> +
> +    DPRINTF("Attempting to start an outgoing migration\n");

No new DPRINTF's - trace_ please.

> +    ioc = QIO_CHANNEL(qio_channel_command_new_spawn(argv,
> +                                                    O_WRONLY,
> +                                                    errp));
> +    if (!ioc) {
>          return;
>      }
>  
> +    s->file = qemu_fopen_channel_output(ioc);
> +    object_unref(OBJECT(ioc));
> +
>      migrate_fd_connect(s);
>  }
>  
> -static void exec_accept_incoming_migration(void *opaque)
> +static gboolean exec_accept_incoming_migration(QIOChannel *ioc,
> +                                               GIOCondition condition,
> +                                               gpointer opaque)
>  {
>      QEMUFile *f = opaque;
> -
> -    qemu_set_fd_handler(qemu_get_fd(f), NULL, NULL, NULL);
>      process_incoming_migration(f);
> +    return FALSE;

As previous patch, comment the magical FALSE.

Other than those two minor ones;

(If we're lucky this might fix the rcu race that I can trigger with exec: migration).

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Dave

>  }
>  
>  void exec_start_incoming_migration(const char *command, Error **errp)
>  {
>      QEMUFile *f;
> +    QIOChannel *ioc;
> +    const char *argv[] = { "/bin/sh", "-c", command, NULL };
>  
>      DPRINTF("Attempting to start an incoming migration\n");
> -    f = qemu_popen_cmd(command, "r");
> -    if(f == NULL) {
> -        error_setg_errno(errp, errno, "failed to popen the migration source");
> +    ioc = QIO_CHANNEL(qio_channel_command_new_spawn(argv,
> +                                                    O_RDONLY,
> +                                                    errp));
> +    if (!ioc) {
>          return;
>      }
>  
> -    qemu_set_fd_handler(qemu_get_fd(f), exec_accept_incoming_migration, NULL,
> -                        f);
> +    f = qemu_fopen_channel_input(ioc);
> +    object_unref(OBJECT(ioc));
> +
> +    qio_channel_add_watch(ioc,
> +                          G_IO_IN,
> +                          exec_accept_incoming_migration,
> +                          f,
> +                          NULL);
>  }
> diff --git a/migration/migration.c b/migration/migration.c
> index 211879e..2d2079d 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -309,10 +309,8 @@ void qemu_start_incoming_migration(const char *uri, Error **errp)
>      } else if (strstart(uri, "rdma:", &p)) {
>          rdma_start_incoming_migration(p, errp);
>  #endif
> -#if !defined(WIN32)
>      } else if (strstart(uri, "exec:", &p)) {
>          exec_start_incoming_migration(p, errp);
> -#endif
>      } else if (strstart(uri, "unix:", &p)) {
>          unix_start_incoming_migration(p, errp);
>      } else if (strstart(uri, "fd:", &p)) {
> @@ -1014,10 +1012,8 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>      } else if (strstart(uri, "rdma:", &p)) {
>          rdma_start_outgoing_migration(s, p, &local_err);
>  #endif
> -#if !defined(WIN32)
>      } else if (strstart(uri, "exec:", &p)) {
>          exec_start_outgoing_migration(s, p, &local_err);
> -#endif
>      } else if (strstart(uri, "unix:", &p)) {
>          unix_start_outgoing_migration(s, p, &local_err);
>      } else if (strstart(uri, "fd:", &p)) {
> -- 
> 2.5.0
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff mbox

Patch

diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index 64f95cd..3c90c44 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -1,11 +1,10 @@ 
-common-obj-y += migration.o tcp.o unix.o fd.o
+common-obj-y += migration.o tcp.o unix.o fd.o exec.o
 common-obj-y += vmstate.o
 common-obj-y += qemu-file.o qemu-file-buf.o qemu-file-unix.o qemu-file-stdio.o
 common-obj-y += qemu-file-channel.o
 common-obj-y += xbzrle.o postcopy-ram.o
 
 common-obj-$(CONFIG_RDMA) += rdma.o
-common-obj-$(CONFIG_POSIX) += exec.o
 
 common-obj-y += block.o
 
diff --git a/migration/exec.c b/migration/exec.c
index 8406d2b..6159aba 100644
--- a/migration/exec.c
+++ b/migration/exec.c
@@ -15,14 +15,8 @@ 
  * GNU GPL, version 2 or (at your option) any later version.
  */
 
-#include "qemu-common.h"
-#include "qemu/sockets.h"
-#include "qemu/main-loop.h"
 #include "migration/migration.h"
-#include "migration/qemu-file.h"
-#include "block/block.h"
-#include <sys/types.h>
-#include <sys/wait.h>
+#include "io/channel-command.h"
 
 //#define DEBUG_MIGRATION_EXEC
 
@@ -36,34 +30,52 @@ 
 
 void exec_start_outgoing_migration(MigrationState *s, const char *command, Error **errp)
 {
-    s->file = qemu_popen_cmd(command, "w");
-    if (s->file == NULL) {
-        error_setg_errno(errp, errno, "failed to popen the migration target");
+    QIOChannel *ioc;
+    const char *argv[] = { "/bin/sh", "-c", command, NULL };
+
+    DPRINTF("Attempting to start an outgoing migration\n");
+    ioc = QIO_CHANNEL(qio_channel_command_new_spawn(argv,
+                                                    O_WRONLY,
+                                                    errp));
+    if (!ioc) {
         return;
     }
 
+    s->file = qemu_fopen_channel_output(ioc);
+    object_unref(OBJECT(ioc));
+
     migrate_fd_connect(s);
 }
 
-static void exec_accept_incoming_migration(void *opaque)
+static gboolean exec_accept_incoming_migration(QIOChannel *ioc,
+                                               GIOCondition condition,
+                                               gpointer opaque)
 {
     QEMUFile *f = opaque;
-
-    qemu_set_fd_handler(qemu_get_fd(f), NULL, NULL, NULL);
     process_incoming_migration(f);
+    return FALSE;
 }
 
 void exec_start_incoming_migration(const char *command, Error **errp)
 {
     QEMUFile *f;
+    QIOChannel *ioc;
+    const char *argv[] = { "/bin/sh", "-c", command, NULL };
 
     DPRINTF("Attempting to start an incoming migration\n");
-    f = qemu_popen_cmd(command, "r");
-    if(f == NULL) {
-        error_setg_errno(errp, errno, "failed to popen the migration source");
+    ioc = QIO_CHANNEL(qio_channel_command_new_spawn(argv,
+                                                    O_RDONLY,
+                                                    errp));
+    if (!ioc) {
         return;
     }
 
-    qemu_set_fd_handler(qemu_get_fd(f), exec_accept_incoming_migration, NULL,
-                        f);
+    f = qemu_fopen_channel_input(ioc);
+    object_unref(OBJECT(ioc));
+
+    qio_channel_add_watch(ioc,
+                          G_IO_IN,
+                          exec_accept_incoming_migration,
+                          f,
+                          NULL);
 }
diff --git a/migration/migration.c b/migration/migration.c
index 211879e..2d2079d 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -309,10 +309,8 @@  void qemu_start_incoming_migration(const char *uri, Error **errp)
     } else if (strstart(uri, "rdma:", &p)) {
         rdma_start_incoming_migration(p, errp);
 #endif
-#if !defined(WIN32)
     } else if (strstart(uri, "exec:", &p)) {
         exec_start_incoming_migration(p, errp);
-#endif
     } else if (strstart(uri, "unix:", &p)) {
         unix_start_incoming_migration(p, errp);
     } else if (strstart(uri, "fd:", &p)) {
@@ -1014,10 +1012,8 @@  void qmp_migrate(const char *uri, bool has_blk, bool blk,
     } else if (strstart(uri, "rdma:", &p)) {
         rdma_start_outgoing_migration(s, p, &local_err);
 #endif
-#if !defined(WIN32)
     } else if (strstart(uri, "exec:", &p)) {
         exec_start_outgoing_migration(s, p, &local_err);
-#endif
     } else if (strstart(uri, "unix:", &p)) {
         unix_start_outgoing_migration(s, p, &local_err);
     } else if (strstart(uri, "fd:", &p)) {