Patchwork qemu: stop polling non-system fds on vmstop

login
register
mail settings
Submitter Michael S. Tsirkin
Date Nov. 1, 2010, 10:25 a.m.
Message ID <20101101102505.GA28220@redhat.com>
Download mbox | patch
Permalink /patch/69755/
State New
Headers show

Comments

Michael S. Tsirkin - Nov. 1, 2010, 10:25 a.m.
We should not run any devices while the VM is stopped:
DMA into memory while VM is stopped makes it
hard to debug migration (consequitive saves
result in different files);  Sending while vm is stopped
has an even worse effect as it confuses the bridges
so that they do not know where to send packets.

The following patch addresses this by splitting device and system
io handlers into separate lists. Device handlers are stopped
on vmstop, system handlers keep running.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 aio.c            |   32 +++++++++++--
 cmd.c            |    8 ++--
 cpus.c           |    2 +-
 migration-exec.c |    4 +-
 migration-fd.c   |    4 +-
 migration-tcp.c  |    8 ++--
 migration-unix.c |    8 ++--
 migration.c      |    8 ++--
 qemu-aio.h       |    7 +++
 qemu-char.h      |    5 ++
 qemu-kvm.c       |    2 +-
 qemu-tool.c      |    9 ++++
 vl.c             |  136 +++++++++++++++++++++++++++++++++++-------------------
 13 files changed, 160 insertions(+), 73 deletions(-)

Patch

diff --git a/aio.c b/aio.c
index 2f08655..0d50c87 100644
--- a/aio.c
+++ b/aio.c
@@ -52,14 +52,15 @@  static AioHandler *find_aio_handler(int fd)
     return NULL;
 }
 
-int qemu_aio_set_fd_handler(int fd,
+static int qemu_aio_assign_fd_handler(int fd,
                             IOHandler *io_read,
                             IOHandler *io_write,
                             AioFlushHandler *io_flush,
                             AioProcessQueue *io_process_queue,
-                            void *opaque)
+                            void *opaque, bool system)
 {
     AioHandler *node;
+    int r;
 
     node = find_aio_handler(fd);
 
@@ -93,11 +94,34 @@  int qemu_aio_set_fd_handler(int fd,
         node->opaque = opaque;
     }
 
-    qemu_set_fd_handler2(fd, NULL, io_read, io_write, opaque);
-
+    r = system ? qemu_set_fd_handler2(fd, NULL, io_read, io_write, opaque) :
+        qemu_set_system_fd_handler(fd, NULL, io_read, io_write, opaque);
+    assert(!r);
     return 0;
 }
 
+int qemu_aio_set_fd_handler(int fd,
+                            IOHandler *io_read,
+                            IOHandler *io_write,
+                            AioFlushHandler *io_flush,
+                            AioProcessQueue *io_process_queue,
+                            void *opaque)
+{
+    return qemu_aio_assign_fd_handler(fd, io_read, io_write, io_flush,
+                                      io_process_queue, opaque, false);
+}
+
+int qemu_aio_set_system_fd_handler(int fd,
+                            IOHandler *io_read,
+                            IOHandler *io_write,
+                            AioFlushHandler *io_flush,
+                            AioProcessQueue *io_process_queue,
+                            void *opaque)
+{
+    return qemu_aio_assign_fd_handler(fd, io_read, io_write, io_flush,
+                                      io_process_queue, opaque, true);
+}
+
 void qemu_aio_flush(void)
 {
     AioHandler *node;
diff --git a/cmd.c b/cmd.c
index db2c9c4..26dcfe4 100644
--- a/cmd.c
+++ b/cmd.c
@@ -154,7 +154,7 @@  static void prep_fetchline(void *opaque)
 {
     int *fetchable = opaque;
 
-    qemu_aio_set_fd_handler(STDIN_FILENO, NULL, NULL, NULL, NULL, NULL);
+    qemu_aio_set_system_fd_handler(STDIN_FILENO, NULL, NULL, NULL, NULL, NULL);
     *fetchable= 1;
 }
 
@@ -202,8 +202,8 @@  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_aio_set_system_fd_handler(STDIN_FILENO, prep_fetchline,
+                                           NULL, NULL, NULL, &fetchable);
             prompted = 1;
         }
 
@@ -228,7 +228,7 @@  command_loop(void)
         prompted = 0;
         fetchable = 0;
 	}
-    qemu_aio_set_fd_handler(STDIN_FILENO, NULL, NULL, NULL, NULL, NULL);
+    qemu_aio_set_system_fd_handler(STDIN_FILENO, NULL, NULL, NULL, NULL, NULL);
 }
 
 /* from libxcmd/input.c */
diff --git a/cpus.c b/cpus.c
index b16fbe1..819cc9e 100644
--- a/cpus.c
+++ b/cpus.c
@@ -209,7 +209,7 @@  static int qemu_event_init(void)
     if (err < 0)
         goto fail;
 
-    qemu_set_fd_handler2(fds[0], NULL, qemu_event_read, NULL,
+    qemu_set_system_fd_handler(fds[0], NULL, qemu_event_read, NULL,
                          (void *)(unsigned long)fds[0]);
 
     io_thread_fd = fds[1];
diff --git a/migration-exec.c b/migration-exec.c
index 14718dd..a8026bf 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -123,7 +123,7 @@  static void exec_accept_incoming_migration(void *opaque)
     QEMUFile *f = opaque;
 
     process_incoming_migration(f);
-    qemu_set_fd_handler2(qemu_stdio_fd(f), NULL, NULL, NULL, NULL);
+    qemu_set_system_fd_handler(qemu_stdio_fd(f), NULL, NULL, NULL, NULL);
     qemu_fclose(f);
 }
 
@@ -138,7 +138,7 @@  int exec_start_incoming_migration(const char *command)
         return -errno;
     }
 
-    qemu_set_fd_handler2(qemu_stdio_fd(f), NULL,
+    qemu_set_system_fd_handler(qemu_stdio_fd(f), NULL,
 			 exec_accept_incoming_migration, NULL, f);
 
     return 0;
diff --git a/migration-fd.c b/migration-fd.c
index 6d14505..e7f8c1f 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -106,7 +106,7 @@  static void fd_accept_incoming_migration(void *opaque)
     QEMUFile *f = opaque;
 
     process_incoming_migration(f);
-    qemu_set_fd_handler2(qemu_stdio_fd(f), NULL, NULL, NULL, NULL);
+    qemu_set_system_fd_handler(qemu_stdio_fd(f), NULL, NULL, NULL, NULL);
     qemu_fclose(f);
 }
 
@@ -124,7 +124,7 @@  int fd_start_incoming_migration(const char *infd)
         return -errno;
     }
 
-    qemu_set_fd_handler2(fd, NULL, fd_accept_incoming_migration, NULL, f);
+    qemu_set_system_fd_handler(fd, NULL, fd_accept_incoming_migration, NULL, f);
 
     return 0;
 }
diff --git a/migration-tcp.c b/migration-tcp.c
index b55f419..7ea0f5b 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -66,7 +66,7 @@  static void tcp_wait_for_connect(void *opaque)
         return;
     }
 
-    qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
+    qemu_set_system_fd_handler(s->fd, NULL, NULL, NULL, NULL);
 
     if (val == 0)
         migrate_fd_connect(s);
@@ -123,7 +123,7 @@  MigrationState *tcp_start_outgoing_migration(Monitor *mon,
             ret = -(s->get_error(s));
 
         if (ret == -EINPROGRESS || ret == -EWOULDBLOCK)
-            qemu_set_fd_handler2(s->fd, NULL, NULL, tcp_wait_for_connect, s);
+            qemu_set_system_fd_handler(s->fd, NULL, NULL, tcp_wait_for_connect, s);
     } while (ret == -EINTR);
 
     if (ret < 0 && ret != -EINPROGRESS && ret != -EWOULDBLOCK) {
@@ -165,7 +165,7 @@  static void tcp_accept_incoming_migration(void *opaque)
 out:
     close(c);
 out2:
-    qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL);
+    qemu_set_system_fd_handler(s, NULL, NULL, NULL, NULL);
     close(s);
 }
 
@@ -193,7 +193,7 @@  int tcp_start_incoming_migration(const char *host_port)
     if (listen(s, 1) == -1)
         goto err;
 
-    qemu_set_fd_handler2(s, NULL, tcp_accept_incoming_migration, NULL,
+    qemu_set_system_fd_handler(s, NULL, tcp_accept_incoming_migration, NULL,
                          (void *)(unsigned long)s);
 
     return 0;
diff --git a/migration-unix.c b/migration-unix.c
index 57232c0..e32fe6b 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -65,7 +65,7 @@  static void unix_wait_for_connect(void *opaque)
         return;
     }
 
-    qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
+    qemu_set_system_fd_handler(s->fd, NULL, NULL, NULL, NULL);
 
     if (val == 0)
         migrate_fd_connect(s);
@@ -118,7 +118,7 @@  MigrationState *unix_start_outgoing_migration(Monitor *mon,
 	    ret = -(s->get_error(s));
 
         if (ret == -EINPROGRESS || ret == -EWOULDBLOCK)
-	    qemu_set_fd_handler2(s->fd, NULL, NULL, unix_wait_for_connect, s);
+	    qemu_set_system_fd_handler(s->fd, NULL, NULL, unix_wait_for_connect, s);
     } while (ret == -EINTR);
 
     if (ret < 0 && ret != -EINPROGRESS && ret != -EWOULDBLOCK) {
@@ -171,7 +171,7 @@  static void unix_accept_incoming_migration(void *opaque)
     process_incoming_migration(f);
     qemu_fclose(f);
 out:
-    qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL);
+    qemu_set_system_fd_handler(s, NULL, NULL, NULL, NULL);
     close(s);
     close(c);
 }
@@ -203,7 +203,7 @@  int unix_start_incoming_migration(const char *path)
         goto err;
     }
 
-    qemu_set_fd_handler2(sock, NULL, unix_accept_incoming_migration, NULL,
+    qemu_set_system_fd_handler(sock, NULL, unix_accept_incoming_migration, NULL,
 			 (void *)(unsigned long)sock);
 
     return 0;
diff --git a/migration.c b/migration.c
index 468d517..18546ee 100644
--- a/migration.c
+++ b/migration.c
@@ -273,7 +273,7 @@  int migrate_fd_cleanup(FdMigrationState *s)
 {
     int ret = 0;
 
-    qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
+    qemu_set_system_fd_handler(s->fd, NULL, NULL, NULL, NULL);
 
     if (s->file) {
         DPRINTF("closing file\n");
@@ -300,7 +300,7 @@  void migrate_fd_put_notify(void *opaque)
 {
     FdMigrationState *s = opaque;
 
-    qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
+    qemu_set_system_fd_handler(s->fd, NULL, NULL, NULL, NULL);
     qemu_file_put_notify(s->file);
 }
 
@@ -317,7 +317,7 @@  ssize_t migrate_fd_put_buffer(void *opaque, const void *data, size_t size)
         ret = -(s->get_error(s));
 
     if (ret == -EAGAIN) {
-        qemu_set_fd_handler2(s->fd, NULL, NULL, migrate_fd_put_notify, s);
+        qemu_set_system_fd_handler(s->fd, NULL, NULL, migrate_fd_put_notify, s);
     } else if (ret < 0) {
         if (s->mon) {
             monitor_resume(s->mon);
@@ -445,6 +445,6 @@  int migrate_fd_close(void *opaque)
 {
     FdMigrationState *s = opaque;
 
-    qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
+    qemu_set_system_fd_handler(s->fd, NULL, NULL, NULL, NULL);
     return s->close(s);
 }
diff --git a/qemu-aio.h b/qemu-aio.h
index 3bdd749..4223d34 100644
--- a/qemu-aio.h
+++ b/qemu-aio.h
@@ -56,4 +56,11 @@  int qemu_aio_set_fd_handler(int fd,
                             AioProcessQueue *io_process_queue,
                             void *opaque);
 
+/* Same but for system handlers, which run when VM is stopped. */
+int qemu_aio_set_system_fd_handler(int fd,
+                            IOHandler *io_read,
+                            IOHandler *io_write,
+                            AioFlushHandler *io_flush,
+                            AioProcessQueue *io_process_queue,
+                            void *opaque);
 #endif
diff --git a/qemu-char.h b/qemu-char.h
index 18ad12b..377b760 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -111,4 +111,9 @@  int qemu_set_fd_handler(int fd,
                         IOHandler *fd_read,
                         IOHandler *fd_write,
                         void *opaque);
+int qemu_set_system_fd_handler(int fd,
+                         IOCanReadHandler *fd_read_poll,
+                         IOHandler *fd_read,
+                         IOHandler *fd_write,
+                         void *opaque);
 #endif
diff --git a/qemu-kvm.c b/qemu-kvm.c
index 625f286..9caddcf 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -1621,7 +1621,7 @@  int kvm_main_loop(void)
 
     fcntl(sigfd, F_SETFL, O_NONBLOCK);
 
-    qemu_set_fd_handler2(sigfd, NULL, sigfd_handler, NULL,
+    qemu_set_system_fd_handler(sigfd, NULL, sigfd_handler, NULL,
                          (void *)(unsigned long) sigfd);
 
     pthread_cond_broadcast(&qemu_system_cond);
diff --git a/qemu-tool.c b/qemu-tool.c
index b39af86..eff2dab 100644
--- a/qemu-tool.c
+++ b/qemu-tool.c
@@ -111,6 +111,15 @@  int qemu_set_fd_handler2(int fd,
     return 0;
 }
 
+int qemu_set_system_fd_handler(int fd,
+                               IOCanReadHandler *fd_read_poll,
+                               IOHandler *fd_read,
+                               IOHandler *fd_write,
+                               void *opaque)
+{
+    return 0;
+}
+
 int64_t qemu_get_clock(QEMUClock *clock)
 {
     qemu_timeval tv;
diff --git a/vl.c b/vl.c
index 42617c2..123a0e3 100644
--- a/vl.c
+++ b/vl.c
@@ -958,34 +958,36 @@  typedef struct IOHandlerRecord {
     QLIST_ENTRY(IOHandlerRecord) next;
 } IOHandlerRecord;
 
-static QLIST_HEAD(, IOHandlerRecord) io_handlers =
-    QLIST_HEAD_INITIALIZER(io_handlers);
+typedef QLIST_HEAD(, IOHandlerRecord) IOHandlerRecordList;
+static IOHandlerRecordList device_io_handlers =
+    QLIST_HEAD_INITIALIZER(device_io_handlers);
 
+static IOHandlerRecordList system_io_handlers =
+    QLIST_HEAD_INITIALIZER(system_io_handlers);
 
-/* XXX: fd_read_poll should be suppressed, but an API change is
-   necessary in the character devices to suppress fd_can_read(). */
-int qemu_set_fd_handler2(int fd,
-                         IOCanReadHandler *fd_read_poll,
-                         IOHandler *fd_read,
-                         IOHandler *fd_write,
-                         void *opaque)
+static int qemu_assign_fd_handler(int fd,
+                                  IOCanReadHandler *fd_read_poll,
+                                  IOHandler *fd_read,
+                                  IOHandler *fd_write,
+                                  void *opaque,
+                                  IOHandlerRecordList *list)
 {
     IOHandlerRecord *ioh;
 
     if (!fd_read && !fd_write) {
-        QLIST_FOREACH(ioh, &io_handlers, next) {
+        QLIST_FOREACH(ioh, list, next) {
             if (ioh->fd == fd) {
                 ioh->deleted = 1;
                 break;
             }
         }
     } else {
-        QLIST_FOREACH(ioh, &io_handlers, next) {
+        QLIST_FOREACH(ioh, list, next) {
             if (ioh->fd == fd)
                 goto found;
         }
         ioh = qemu_mallocz(sizeof(IOHandlerRecord));
-        QLIST_INSERT_HEAD(&io_handlers, ioh, next);
+        QLIST_INSERT_HEAD(list, ioh, next);
     found:
         ioh->fd = fd;
         ioh->fd_read_poll = fd_read_poll;
@@ -998,6 +1000,19 @@  int qemu_set_fd_handler2(int fd,
     return 0;
 }
 
+
+/* XXX: fd_read_poll should be suppressed, but an API change is
+   necessary in the character devices to suppress fd_can_read(). */
+int qemu_set_fd_handler2(int fd,
+                         IOCanReadHandler *fd_read_poll,
+                         IOHandler *fd_read,
+                         IOHandler *fd_write,
+                         void *opaque)
+{
+    return qemu_assign_fd_handler(fd, fd_read_poll, fd_read, fd_write, opaque,
+                                  &device_io_handlers);
+}
+
 int qemu_set_fd_handler(int fd,
                         IOHandler *fd_read,
                         IOHandler *fd_write,
@@ -1006,6 +1021,16 @@  int qemu_set_fd_handler(int fd,
     return qemu_set_fd_handler2(fd, NULL, fd_read, fd_write, opaque);
 }
 
+int qemu_set_system_fd_handler(int fd,
+                         IOCanReadHandler *fd_read_poll,
+                         IOHandler *fd_read,
+                         IOHandler *fd_write,
+                         void *opaque)
+{
+    return qemu_assign_fd_handler(fd, fd_read_poll, fd_read, fd_write, opaque,
+                                  &system_io_handlers);
+}
+
 /***********************************************************/
 /* machine registration */
 
@@ -1242,9 +1267,52 @@  void qemu_system_powerdown_request(void)
     qemu_notify_event();
 }
 
-void main_loop_wait(int nonblocking)
+static inline int get_ioh_fds(IOHandlerRecordList *list,
+                              int nfds, fd_set *rfds, fd_set *wfds)
 {
     IOHandlerRecord *ioh;
+    QLIST_FOREACH(ioh, list, next) {
+        if (ioh->deleted)
+            continue;
+        if (ioh->fd_read &&
+            (!ioh->fd_read_poll ||
+             ioh->fd_read_poll(ioh->opaque) != 0)) {
+            FD_SET(ioh->fd, rfds);
+            if (ioh->fd > nfds)
+                nfds = ioh->fd;
+        }
+        if (ioh->fd_write) {
+            FD_SET(ioh->fd, wfds);
+            if (ioh->fd > nfds)
+                nfds = ioh->fd;
+        }
+    }
+    return nfds;
+}
+
+static inline void call_ioh_fds(IOHandlerRecordList *list,
+                                  fd_set *rfds, fd_set *wfds)
+{
+    IOHandlerRecord *ioh, *pioh;
+
+    QLIST_FOREACH_SAFE(ioh, list, next, pioh) {
+        if (ioh->deleted) {
+            QLIST_REMOVE(ioh, next);
+            qemu_free(ioh);
+            continue;
+        }
+        if (ioh->fd_read && FD_ISSET(ioh->fd, rfds)) {
+            ioh->fd_read(ioh->opaque);
+            if (!(ioh->fd_read_poll && ioh->fd_read_poll(ioh->opaque)))
+                FD_CLR(ioh->fd, rfds);
+        }
+        if (ioh->fd_write && FD_ISSET(ioh->fd, wfds)) {
+            ioh->fd_write(ioh->opaque);
+        }
+    }
+}
+void main_loop_wait(int nonblocking)
+{
     fd_set rfds, wfds, xfds;
     int ret, nfds;
     struct timeval tv;
@@ -1260,26 +1328,13 @@  void main_loop_wait(int nonblocking)
     os_host_main_loop_wait(&timeout);
 
     /* poll any events */
-    /* XXX: separate device handlers from system ones */
     nfds = -1;
     FD_ZERO(&rfds);
     FD_ZERO(&wfds);
     FD_ZERO(&xfds);
-    QLIST_FOREACH(ioh, &io_handlers, next) {
-        if (ioh->deleted)
-            continue;
-        if (ioh->fd_read &&
-            (!ioh->fd_read_poll ||
-             ioh->fd_read_poll(ioh->opaque) != 0)) {
-            FD_SET(ioh->fd, &rfds);
-            if (ioh->fd > nfds)
-                nfds = ioh->fd;
-        }
-        if (ioh->fd_write) {
-            FD_SET(ioh->fd, &wfds);
-            if (ioh->fd > nfds)
-                nfds = ioh->fd;
-        }
+    nfds = get_ioh_fds(&system_io_handlers, nfds, &rfds, &wfds);
+    if (vm_running) {
+        nfds = get_ioh_fds(&device_io_handlers, nfds, &rfds, &wfds);
     }
 
     tv.tv_sec = timeout / 1000;
@@ -1291,22 +1346,9 @@  void main_loop_wait(int nonblocking)
     ret = select(nfds + 1, &rfds, &wfds, &xfds, &tv);
     qemu_mutex_lock_iothread();
     if (ret > 0) {
-        IOHandlerRecord *pioh;
-
-        QLIST_FOREACH_SAFE(ioh, &io_handlers, next, pioh) {
-            if (ioh->deleted) {
-                QLIST_REMOVE(ioh, next);
-                qemu_free(ioh);
-                continue;
-            }
-            if (ioh->fd_read && FD_ISSET(ioh->fd, &rfds)) {
-                ioh->fd_read(ioh->opaque);
-                if (!(ioh->fd_read_poll && ioh->fd_read_poll(ioh->opaque)))
-                    FD_CLR(ioh->fd, &rfds);
-            }
-            if (ioh->fd_write && FD_ISSET(ioh->fd, &wfds)) {
-                ioh->fd_write(ioh->opaque);
-            }
+        call_ioh_fds(&system_io_handlers, &rfds, &wfds);
+        if (vm_running) {
+            call_ioh_fds(&device_io_handlers, &rfds, &wfds);
         }
     }