Re: [Qemu-devel] [PATCH v4 01/15] coroutine: add co_sleep_ns() coroutine sleep function

[Paolo Bonzini <pbonzini@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 848 bytes --]

On 01/12/2012 11:58 AM, Stefan Hajnoczi wrote:
> This is something I'm looking at right now and will probably want to
> discuss with Paolo.
>
> In a coroutine we're probably using a main loop and timers should be
> available there.  In general, the problem we're starting to see is
> that some block layer features are using timers (I/O throttling, QED
> deferred dirty bit clearing, image streaming).  The question is do we
> isolate this functionality so it is unavailable from a qemu-tool world
> when there's no main loop, or do we move everything into a main loop?

Yes, I would move things into a main loop just like qemu-nbd does.  I 
even had some prototype patches for it.  The qemu-img part is broken 
because it uses callbacks that haven't been made asynchronous yet; the 
one for qemu-io should be okay modulo some more testing.

Paolo

[-- Attachment #2: tools-co.patch --]
[-- Type: text/x-patch, Size: 8122 bytes --]

>From 9b67defc5eacb9254a8140a4daa52949f0fe60e2 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Mon, 26 Sep 2011 10:59:46 +0200
Subject: [PATCH 1/2] qemu-io: use main_loop_wait

This will let timers run during aio_read and aio_write commands,
though not during synchronous commands.

Not-tested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cmd.c     |   10 +++++-----
 qemu-io.c |    8 +++++---
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/cmd.c b/cmd.c
index 0806e18..7ffbb71 100644
--- a/cmd.c
+++ b/cmd.c
@@ -25,6 +25,7 @@
 
 #include "cmd.h"
 #include "qemu-aio.h"
+#include "main-loop.h"
 
 #define _(x)	x	/* not gettext support yet */
 
@@ -146,7 +147,7 @@ static void prep_fetchline(void *opaque)
 {
     int *fetchable = opaque;
 
-    qemu_aio_set_fd_handler(STDIN_FILENO, NULL, NULL, NULL, NULL, NULL);
+    qemu_set_fd_handler(STDIN_FILENO, NULL, NULL, NULL);
     *fetchable= 1;
 }
 
@@ -193,12 +194,11 @@ void command_loop(void)
         if (!prompted) {
             printf("%s", get_prompt());
             fflush(stdout);
-            qemu_aio_set_fd_handler(STDIN_FILENO, prep_fetchline, NULL, NULL,
-                                    NULL, &fetchable);
+            qemu_set_fd_handler(STDIN_FILENO, prep_fetchline, NULL, &fetchable);
             prompted = 1;
         }
 
-        qemu_aio_wait();
+        main_loop_wait(false);
 
         if (!fetchable) {
             continue;
@@ -221,7 +221,7 @@ void command_loop(void)
         prompted = 0;
         fetchable = 0;
     }
-    qemu_aio_set_fd_handler(STDIN_FILENO, NULL, NULL, NULL, NULL, NULL);
+    qemu_set_fd_handler(STDIN_FILENO, NULL, NULL, NULL);
 }
 
 /* from libxcmd/input.c */
diff --git a/qemu-io.c b/qemu-io.c
index ffa62fb..6e1bee3 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -15,6 +15,7 @@
 #include <libgen.h>
 
 #include "qemu-common.h"
+#include "main-loop.h"
 #include "block_int.h"
 #include "cmd.h"
 
@@ -249,7 +250,7 @@ static int do_aio_readv(QEMUIOVector *qiov, int64_t offset, int *total)
     bdrv_aio_readv(bs, offset >> 9, qiov, qiov->size >> 9,
                    aio_rw_done, &async_ret);
     while (async_ret == NOT_DONE) {
-        qemu_aio_wait();
+        main_loop_wait(false);
     }
 
     *total = qiov->size;
@@ -263,7 +264,7 @@ static int do_aio_writev(QEMUIOVector *qiov, int64_t offset, int *total)
     bdrv_aio_writev(bs, offset >> 9, qiov, qiov->size >> 9,
                     aio_rw_done, &async_ret);
     while (async_ret == NOT_DONE) {
-        qemu_aio_wait();
+        main_loop_wait(false);
     }
 
     *total = qiov->size;
@@ -306,7 +307,7 @@ static int do_aio_multiwrite(BlockRequest* reqs, int num_reqs, int *total)
     }
 
     while (async_ret.num_done < num_reqs) {
-        qemu_aio_wait();
+        main_loop_wait(false);
     }
 
     return async_ret.error < 0 ? async_ret.error : 1;
@@ -1793,6 +1794,7 @@ int main(int argc, char **argv)
         exit(1);
     }
 
+    qemu_init_main_loop();
     bdrv_init();
 
     /* initialize commands */
-- 
1.7.7.1


>From ac2958b0992dcd155ab76885cac044a9fec5cc06 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Tue, 18 Oct 2011 12:20:44 +0200
Subject: [PATCH 2/2] qemu-img: run commands in coroutine context

The new function qemu_coroutine_schedule is in qemu-coroutine-lock.c
since it's where the other dependencies on the main loop reside.

Not-tested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 main-loop.h           |   20 ++++++--------------
 qemu-coroutine-lock.c |   24 ++++++++++++++++++++++++
 qemu-coroutine.h      |    8 ++++++++
 qemu-img.c            |   40 ++++++++++++++++++++++++++++++++--------
 4 files changed, 70 insertions(+), 22 deletions(-)

diff --git a/main-loop.h b/main-loop.h
index f971013..4e99485 100644
--- a/main-loop.h
+++ b/main-loop.h
@@ -59,24 +59,16 @@ int qemu_init_main_loop(void);
  * It is sometimes useful to put a whole program in a coroutine.  In this
  * case, the coroutine actually should be started from within the main loop,
  * so that the main loop can run whenever the coroutine yields.  To do this,
- * you can use a bottom half to enter the coroutine as soon as the main loop
- * starts:
+ * you can use qemu_coroutine_schedule.
  *
- *     void enter_co_bh(void *opaque) {
- *         QEMUCoroutine *co = opaque;
- *         qemu_coroutine_enter(co, NULL);
- *     }
- *
- *     ...
- *     QEMUCoroutine *co = qemu_coroutine_create(coroutine_entry);
- *     QEMUBH *start_bh = qemu_bh_new(enter_co_bh, co);
- *     qemu_bh_schedule(start_bh);
- *     while (...) {
+ *     int ret = -1;
+ *     Coroutine *co = qemu_coroutine_create(coroutine_entry);
+ *     qemu_init_main_loop();
+ *     qemu_coroutine_schedule(co, &ret);
+ *     while (ret == -1) {
  *         main_loop_wait(false);
  *     }
  *
- * (In the future we may provide a wrapper for this).
- *
  * @nonblocking: Whether the caller should block until an event occurs.
  */
 int main_loop_wait(int nonblocking);
diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c
index 26ad76b..45a2fc2 100644
--- a/qemu-coroutine-lock.c
+++ b/qemu-coroutine-lock.c
@@ -169,3 +169,27 @@ void qemu_co_rwlock_wrlock(CoRwlock *lock)
     }
     lock->writer = true;
 }
+
+typedef struct {
+    QEMUBH *bh;
+    Coroutine *co;
+    void *opaque;
+} CoroutineBHArgs;
+
+static void qemu_coroutine_schedule_bh(void *opaque)
+{
+    CoroutineBHArgs args = *(CoroutineBHArgs *)opaque;
+    g_free(opaque);
+    qemu_bh_delete(args.bh);
+    qemu_coroutine_enter(args.co, args.opaque);
+}
+
+void qemu_coroutine_schedule(Coroutine *co, void *opaque)
+{
+    CoroutineBHArgs *args = g_malloc(sizeof(CoroutineBHArgs));
+    args->bh = qemu_bh_new(qemu_coroutine_schedule_bh, args);
+    args->co = co;
+    args->opaque = opaque;
+    qemu_bh_schedule(args->bh);
+}
+
diff --git a/qemu-coroutine.h b/qemu-coroutine.h
index 8a55fe1..8cdef58 100644
--- a/qemu-coroutine.h
+++ b/qemu-coroutine.h
@@ -199,4 +199,12 @@ void qemu_co_rwlock_wrlock(CoRwlock *lock);
  */
 void qemu_co_rwlock_unlock(CoRwlock *lock);
 
+/**
+ * Transfer control to a coroutine from a bottom half
+ *
+ * The opaque argument is passed as the argument to the entry point when
+ * entering the coroutine for the first time.  It is subsequently ignored.
+ */
+void qemu_coroutine_schedule(Coroutine *coroutine, void *opaque);
+
 #endif /* QEMU_COROUTINE_H */
diff --git a/qemu-img.c b/qemu-img.c
index 01cc0d3..2123476 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -24,6 +24,8 @@
 #include "qemu-common.h"
 #include "qemu-option.h"
 #include "qemu-error.h"
+#include "qemu-coroutine.h"
+#include "main-loop.h"
 #include "osdep.h"
 #include "sysemu.h"
 #include "block_int.h"
@@ -1674,27 +1676,49 @@ static const img_cmd_t img_cmds[] = {
     { NULL, NULL, },
 };
 
-int main(int argc, char **argv)
+typedef struct {
+    int argc;
+    char **argv;
+    int ret;
+} MainArgs;
+
+static void main_co_entry(void *opaque)
 {
+    MainArgs *args = opaque;
     const img_cmd_t *cmd;
     const char *cmdname;
 
-    error_set_progname(argv[0]);
-
-    bdrv_init();
-    if (argc < 2)
+    if (args->argc < 2)
         help();
-    cmdname = argv[1];
-    argc--; argv++;
+    cmdname = args->argv[1];
+    args->argc--; args->argv++;
 
     /* find the command */
     for(cmd = img_cmds; cmd->name != NULL; cmd++) {
         if (!strcmp(cmdname, cmd->name)) {
-            return cmd->handler(argc, argv);
+            args->ret = cmd->handler(args->argc, args->argv);
+            return;
         }
     }
 
     /* not found */
     help();
+    args->ret = 0;
+}
+
+#define NOT_DONE 0x7fffffff
+
+int main(int argc, char **argv)
+{
+    error_set_progname(argv[0]);
+    MainArgs args = { .argc = argc, .argv = argv, .ret = NOT_DONE };
+    Coroutine *co = qemu_coroutine_create(main_co_entry);
+
+    qemu_init_main_loop();
+    bdrv_init();
+    qemu_coroutine_schedule(co, &args);
+    while (args.ret == NOT_DONE) {
+        main_loop_wait(false);
+    }
     return 0;
 }
-- 
1.7.7.1