[Qemu-devel] [PATCH] util: merge main-loop.c and iohandler.c

Paolo Bonzini posted 1 patch 4 years, 9 months ago
Test docker-clang@ubuntu passed
Test asan passed
Test docker-mingw@fedora passed
Test FreeBSD passed
Test s390x passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1562952875-53702-1-git-send-email-pbonzini@redhat.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>
util/Makefile.objs |   2 +-
util/iohandler.c   | 135 -----------------------------------------------------
util/main-loop.c   | 110 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 111 insertions(+), 136 deletions(-)
delete mode 100644 util/iohandler.c
[Qemu-devel] [PATCH] util: merge main-loop.c and iohandler.c
Posted by Paolo Bonzini 4 years, 9 months ago
main-loop.c has a dependency on iohandler.c, and everything breaks
if that dependency is instead satisfied by stubs/iohandler.c.
Just put everything in the same file to avoid strange dependencies
on the order of files in util-obj-y.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 util/Makefile.objs |   2 +-
 util/iohandler.c   | 135 -----------------------------------------------------
 util/main-loop.c   | 110 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 111 insertions(+), 136 deletions(-)
 delete mode 100644 util/iohandler.c

diff --git a/util/Makefile.objs b/util/Makefile.objs
index 3817820..a817ced 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -2,7 +2,7 @@ util-obj-y = osdep.o cutils.o unicode.o qemu-timer-common.o
 util-obj-y += bufferiszero.o
 util-obj-y += lockcnt.o
 util-obj-y += aiocb.o async.o aio-wait.o thread-pool.o qemu-timer.o
-util-obj-y += main-loop.o iohandler.o
+util-obj-y += main-loop.o
 main-loop.o-cflags := $(SLIRP_CFLAGS)
 util-obj-$(call lnot,$(CONFIG_ATOMIC64)) += atomic64.o
 util-obj-$(CONFIG_POSIX) += aio-posix.o
diff --git a/util/iohandler.c b/util/iohandler.c
deleted file mode 100644
index bece882..0000000
--- a/util/iohandler.c
+++ /dev/null
@@ -1,135 +0,0 @@
-/*
- * QEMU System Emulator - managing I/O handler
- *
- * Copyright (c) 2003-2008 Fabrice Bellard
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
- * THE SOFTWARE.
- */
-
-#include "qemu/osdep.h"
-#include "qapi/error.h"
-#include "qemu/queue.h"
-#include "block/aio.h"
-#include "qemu/main-loop.h"
-
-#ifndef _WIN32
-#include <sys/wait.h>
-#endif
-
-/* This context runs on top of main loop. We can't reuse qemu_aio_context
- * because iohandlers mustn't be polled by aio_poll(qemu_aio_context). */
-static AioContext *iohandler_ctx;
-
-static void iohandler_init(void)
-{
-    if (!iohandler_ctx) {
-        iohandler_ctx = aio_context_new(&error_abort);
-    }
-}
-
-AioContext *iohandler_get_aio_context(void)
-{
-    iohandler_init();
-    return iohandler_ctx;
-}
-
-GSource *iohandler_get_g_source(void)
-{
-    iohandler_init();
-    return aio_get_g_source(iohandler_ctx);
-}
-
-void qemu_set_fd_handler(int fd,
-                         IOHandler *fd_read,
-                         IOHandler *fd_write,
-                         void *opaque)
-{
-    iohandler_init();
-    aio_set_fd_handler(iohandler_ctx, fd, false,
-                       fd_read, fd_write, NULL, opaque);
-}
-
-void event_notifier_set_handler(EventNotifier *e,
-                                EventNotifierHandler *handler)
-{
-    iohandler_init();
-    aio_set_event_notifier(iohandler_ctx, e, false,
-                           handler, NULL);
-}
-
-/* reaping of zombies.  right now we're not passing the status to
-   anyone, but it would be possible to add a callback.  */
-#ifndef _WIN32
-typedef struct ChildProcessRecord {
-    int pid;
-    QLIST_ENTRY(ChildProcessRecord) next;
-} ChildProcessRecord;
-
-static QLIST_HEAD(, ChildProcessRecord) child_watches =
-    QLIST_HEAD_INITIALIZER(child_watches);
-
-static QEMUBH *sigchld_bh;
-
-static void sigchld_handler(int signal)
-{
-    qemu_bh_schedule(sigchld_bh);
-}
-
-static void sigchld_bh_handler(void *opaque)
-{
-    ChildProcessRecord *rec, *next;
-
-    QLIST_FOREACH_SAFE(rec, &child_watches, next, next) {
-        if (waitpid(rec->pid, NULL, WNOHANG) == rec->pid) {
-            QLIST_REMOVE(rec, next);
-            g_free(rec);
-        }
-    }
-}
-
-static void qemu_init_child_watch(void)
-{
-    struct sigaction act;
-    sigchld_bh = qemu_bh_new(sigchld_bh_handler, NULL);
-
-    memset(&act, 0, sizeof(act));
-    act.sa_handler = sigchld_handler;
-    act.sa_flags = SA_NOCLDSTOP;
-    sigaction(SIGCHLD, &act, NULL);
-}
-
-int qemu_add_child_watch(pid_t pid)
-{
-    ChildProcessRecord *rec;
-
-    if (!sigchld_bh) {
-        qemu_init_child_watch();
-    }
-
-    QLIST_FOREACH(rec, &child_watches, next) {
-        if (rec->pid == pid) {
-            return 1;
-        }
-    }
-    rec = g_malloc0(sizeof(ChildProcessRecord));
-    rec->pid = pid;
-    QLIST_INSERT_HEAD(&child_watches, rec, next);
-    return 0;
-}
-#endif
diff --git a/util/main-loop.c b/util/main-loop.c
index a9f4e8d..e3eaa55 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -32,6 +32,11 @@
 #include "qemu/main-loop.h"
 #include "block/aio.h"
 #include "qemu/error-report.h"
+#include "qemu/queue.h"
+
+#ifndef _WIN32
+#include <sys/wait.h>
+#endif
 
 #ifndef _WIN32
 
@@ -525,3 +530,108 @@ QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque)
 {
     return aio_bh_new(qemu_aio_context, cb, opaque);
 }
+
+/*
+ * Functions to operate on the I/O handler AioContext.
+ * This context runs on top of main loop. We can't reuse qemu_aio_context
+ * because iohandlers mustn't be polled by aio_poll(qemu_aio_context).
+ */
+static AioContext *iohandler_ctx;
+
+static void iohandler_init(void)
+{
+    if (!iohandler_ctx) {
+        iohandler_ctx = aio_context_new(&error_abort);
+    }
+}
+
+AioContext *iohandler_get_aio_context(void)
+{
+    iohandler_init();
+    return iohandler_ctx;
+}
+
+GSource *iohandler_get_g_source(void)
+{
+    iohandler_init();
+    return aio_get_g_source(iohandler_ctx);
+}
+
+void qemu_set_fd_handler(int fd,
+                         IOHandler *fd_read,
+                         IOHandler *fd_write,
+                         void *opaque)
+{
+    iohandler_init();
+    aio_set_fd_handler(iohandler_ctx, fd, false,
+                       fd_read, fd_write, NULL, opaque);
+}
+
+void event_notifier_set_handler(EventNotifier *e,
+                                EventNotifierHandler *handler)
+{
+    iohandler_init();
+    aio_set_event_notifier(iohandler_ctx, e, false,
+                           handler, NULL);
+}
+
+/* reaping of zombies.  right now we're not passing the status to
+   anyone, but it would be possible to add a callback.  */
+#ifndef _WIN32
+typedef struct ChildProcessRecord {
+    int pid;
+    QLIST_ENTRY(ChildProcessRecord) next;
+} ChildProcessRecord;
+
+static QLIST_HEAD(, ChildProcessRecord) child_watches =
+    QLIST_HEAD_INITIALIZER(child_watches);
+
+static QEMUBH *sigchld_bh;
+
+static void sigchld_handler(int signal)
+{
+    qemu_bh_schedule(sigchld_bh);
+}
+
+static void sigchld_bh_handler(void *opaque)
+{
+    ChildProcessRecord *rec, *next;
+
+    QLIST_FOREACH_SAFE(rec, &child_watches, next, next) {
+        if (waitpid(rec->pid, NULL, WNOHANG) == rec->pid) {
+            QLIST_REMOVE(rec, next);
+            g_free(rec);
+        }
+    }
+}
+
+static void qemu_init_child_watch(void)
+{
+    struct sigaction act;
+    sigchld_bh = qemu_bh_new(sigchld_bh_handler, NULL);
+
+    memset(&act, 0, sizeof(act));
+    act.sa_handler = sigchld_handler;
+    act.sa_flags = SA_NOCLDSTOP;
+    sigaction(SIGCHLD, &act, NULL);
+}
+
+int qemu_add_child_watch(pid_t pid)
+{
+    ChildProcessRecord *rec;
+
+    if (!sigchld_bh) {
+        qemu_init_child_watch();
+    }
+
+    QLIST_FOREACH(rec, &child_watches, next) {
+        if (rec->pid == pid) {
+            return 1;
+        }
+    }
+    rec = g_malloc0(sizeof(ChildProcessRecord));
+    rec->pid = pid;
+    QLIST_INSERT_HEAD(&child_watches, rec, next);
+    return 0;
+}
+#endif
-- 
1.8.3.1


Re: [Qemu-devel] [PATCH] util: merge main-loop.c and iohandler.c
Posted by Marc-André Lureau 4 years, 9 months ago
On Fri, Jul 12, 2019 at 9:34 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> main-loop.c has a dependency on iohandler.c, and everything breaks
> if that dependency is instead satisfied by stubs/iohandler.c.
> Just put everything in the same file to avoid strange dependencies
> on the order of files in util-obj-y.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

that can help indeed, thanks Paolo

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  util/Makefile.objs |   2 +-
>  util/iohandler.c   | 135 -----------------------------------------------------
>  util/main-loop.c   | 110 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 111 insertions(+), 136 deletions(-)
>  delete mode 100644 util/iohandler.c
>
> diff --git a/util/Makefile.objs b/util/Makefile.objs
> index 3817820..a817ced 100644
> --- a/util/Makefile.objs
> +++ b/util/Makefile.objs
> @@ -2,7 +2,7 @@ util-obj-y = osdep.o cutils.o unicode.o qemu-timer-common.o
>  util-obj-y += bufferiszero.o
>  util-obj-y += lockcnt.o
>  util-obj-y += aiocb.o async.o aio-wait.o thread-pool.o qemu-timer.o
> -util-obj-y += main-loop.o iohandler.o
> +util-obj-y += main-loop.o
>  main-loop.o-cflags := $(SLIRP_CFLAGS)
>  util-obj-$(call lnot,$(CONFIG_ATOMIC64)) += atomic64.o
>  util-obj-$(CONFIG_POSIX) += aio-posix.o
> diff --git a/util/iohandler.c b/util/iohandler.c
> deleted file mode 100644
> index bece882..0000000
> --- a/util/iohandler.c
> +++ /dev/null
> @@ -1,135 +0,0 @@
> -/*
> - * QEMU System Emulator - managing I/O handler
> - *
> - * Copyright (c) 2003-2008 Fabrice Bellard
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a copy
> - * of this software and associated documentation files (the "Software"), to deal
> - * in the Software without restriction, including without limitation the rights
> - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> - * copies of the Software, and to permit persons to whom the Software is
> - * furnished to do so, subject to the following conditions:
> - *
> - * The above copyright notice and this permission notice shall be included in
> - * all copies or substantial portions of the Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> - * THE SOFTWARE.
> - */
> -
> -#include "qemu/osdep.h"
> -#include "qapi/error.h"
> -#include "qemu/queue.h"
> -#include "block/aio.h"
> -#include "qemu/main-loop.h"
> -
> -#ifndef _WIN32
> -#include <sys/wait.h>
> -#endif
> -
> -/* This context runs on top of main loop. We can't reuse qemu_aio_context
> - * because iohandlers mustn't be polled by aio_poll(qemu_aio_context). */
> -static AioContext *iohandler_ctx;
> -
> -static void iohandler_init(void)
> -{
> -    if (!iohandler_ctx) {
> -        iohandler_ctx = aio_context_new(&error_abort);
> -    }
> -}
> -
> -AioContext *iohandler_get_aio_context(void)
> -{
> -    iohandler_init();
> -    return iohandler_ctx;
> -}
> -
> -GSource *iohandler_get_g_source(void)
> -{
> -    iohandler_init();
> -    return aio_get_g_source(iohandler_ctx);
> -}
> -
> -void qemu_set_fd_handler(int fd,
> -                         IOHandler *fd_read,
> -                         IOHandler *fd_write,
> -                         void *opaque)
> -{
> -    iohandler_init();
> -    aio_set_fd_handler(iohandler_ctx, fd, false,
> -                       fd_read, fd_write, NULL, opaque);
> -}
> -
> -void event_notifier_set_handler(EventNotifier *e,
> -                                EventNotifierHandler *handler)
> -{
> -    iohandler_init();
> -    aio_set_event_notifier(iohandler_ctx, e, false,
> -                           handler, NULL);
> -}
> -
> -/* reaping of zombies.  right now we're not passing the status to
> -   anyone, but it would be possible to add a callback.  */
> -#ifndef _WIN32
> -typedef struct ChildProcessRecord {
> -    int pid;
> -    QLIST_ENTRY(ChildProcessRecord) next;
> -} ChildProcessRecord;
> -
> -static QLIST_HEAD(, ChildProcessRecord) child_watches =
> -    QLIST_HEAD_INITIALIZER(child_watches);
> -
> -static QEMUBH *sigchld_bh;
> -
> -static void sigchld_handler(int signal)
> -{
> -    qemu_bh_schedule(sigchld_bh);
> -}
> -
> -static void sigchld_bh_handler(void *opaque)
> -{
> -    ChildProcessRecord *rec, *next;
> -
> -    QLIST_FOREACH_SAFE(rec, &child_watches, next, next) {
> -        if (waitpid(rec->pid, NULL, WNOHANG) == rec->pid) {
> -            QLIST_REMOVE(rec, next);
> -            g_free(rec);
> -        }
> -    }
> -}
> -
> -static void qemu_init_child_watch(void)
> -{
> -    struct sigaction act;
> -    sigchld_bh = qemu_bh_new(sigchld_bh_handler, NULL);
> -
> -    memset(&act, 0, sizeof(act));
> -    act.sa_handler = sigchld_handler;
> -    act.sa_flags = SA_NOCLDSTOP;
> -    sigaction(SIGCHLD, &act, NULL);
> -}
> -
> -int qemu_add_child_watch(pid_t pid)
> -{
> -    ChildProcessRecord *rec;
> -
> -    if (!sigchld_bh) {
> -        qemu_init_child_watch();
> -    }
> -
> -    QLIST_FOREACH(rec, &child_watches, next) {
> -        if (rec->pid == pid) {
> -            return 1;
> -        }
> -    }
> -    rec = g_malloc0(sizeof(ChildProcessRecord));
> -    rec->pid = pid;
> -    QLIST_INSERT_HEAD(&child_watches, rec, next);
> -    return 0;
> -}
> -#endif
> diff --git a/util/main-loop.c b/util/main-loop.c
> index a9f4e8d..e3eaa55 100644
> --- a/util/main-loop.c
> +++ b/util/main-loop.c
> @@ -32,6 +32,11 @@
>  #include "qemu/main-loop.h"
>  #include "block/aio.h"
>  #include "qemu/error-report.h"
> +#include "qemu/queue.h"
> +
> +#ifndef _WIN32
> +#include <sys/wait.h>
> +#endif
>
>  #ifndef _WIN32
>
> @@ -525,3 +530,108 @@ QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque)
>  {
>      return aio_bh_new(qemu_aio_context, cb, opaque);
>  }
> +
> +/*
> + * Functions to operate on the I/O handler AioContext.
> + * This context runs on top of main loop. We can't reuse qemu_aio_context
> + * because iohandlers mustn't be polled by aio_poll(qemu_aio_context).
> + */
> +static AioContext *iohandler_ctx;
> +
> +static void iohandler_init(void)
> +{
> +    if (!iohandler_ctx) {
> +        iohandler_ctx = aio_context_new(&error_abort);
> +    }
> +}
> +
> +AioContext *iohandler_get_aio_context(void)
> +{
> +    iohandler_init();
> +    return iohandler_ctx;
> +}
> +
> +GSource *iohandler_get_g_source(void)
> +{
> +    iohandler_init();
> +    return aio_get_g_source(iohandler_ctx);
> +}
> +
> +void qemu_set_fd_handler(int fd,
> +                         IOHandler *fd_read,
> +                         IOHandler *fd_write,
> +                         void *opaque)
> +{
> +    iohandler_init();
> +    aio_set_fd_handler(iohandler_ctx, fd, false,
> +                       fd_read, fd_write, NULL, opaque);
> +}
> +
> +void event_notifier_set_handler(EventNotifier *e,
> +                                EventNotifierHandler *handler)
> +{
> +    iohandler_init();
> +    aio_set_event_notifier(iohandler_ctx, e, false,
> +                           handler, NULL);
> +}
> +
> +/* reaping of zombies.  right now we're not passing the status to
> +   anyone, but it would be possible to add a callback.  */
> +#ifndef _WIN32
> +typedef struct ChildProcessRecord {
> +    int pid;
> +    QLIST_ENTRY(ChildProcessRecord) next;
> +} ChildProcessRecord;
> +
> +static QLIST_HEAD(, ChildProcessRecord) child_watches =
> +    QLIST_HEAD_INITIALIZER(child_watches);
> +
> +static QEMUBH *sigchld_bh;
> +
> +static void sigchld_handler(int signal)
> +{
> +    qemu_bh_schedule(sigchld_bh);
> +}
> +
> +static void sigchld_bh_handler(void *opaque)
> +{
> +    ChildProcessRecord *rec, *next;
> +
> +    QLIST_FOREACH_SAFE(rec, &child_watches, next, next) {
> +        if (waitpid(rec->pid, NULL, WNOHANG) == rec->pid) {
> +            QLIST_REMOVE(rec, next);
> +            g_free(rec);
> +        }
> +    }
> +}
> +
> +static void qemu_init_child_watch(void)
> +{
> +    struct sigaction act;
> +    sigchld_bh = qemu_bh_new(sigchld_bh_handler, NULL);
> +
> +    memset(&act, 0, sizeof(act));
> +    act.sa_handler = sigchld_handler;
> +    act.sa_flags = SA_NOCLDSTOP;
> +    sigaction(SIGCHLD, &act, NULL);
> +}
> +
> +int qemu_add_child_watch(pid_t pid)
> +{
> +    ChildProcessRecord *rec;
> +
> +    if (!sigchld_bh) {
> +        qemu_init_child_watch();
> +    }
> +
> +    QLIST_FOREACH(rec, &child_watches, next) {
> +        if (rec->pid == pid) {
> +            return 1;
> +        }
> +    }
> +    rec = g_malloc0(sizeof(ChildProcessRecord));
> +    rec->pid = pid;
> +    QLIST_INSERT_HEAD(&child_watches, rec, next);
> +    return 0;
> +}
> +#endif
> --
> 1.8.3.1
>
>


-- 
Marc-André Lureau