Patchwork [v3,6/6] qemu_calculate_timeout: increase minimum timeout to 1h

login
register
mail settings
Submitter Stefano Stabellini
Date Feb. 10, 2012, 5:07 p.m.
Message ID <alpine.DEB.2.00.1202101644250.7456@kaball-desktop>
Download mbox | patch
Permalink /patch/140694/
State New
Headers show

Comments

Stefano Stabellini - Feb. 10, 2012, 5:07 p.m.
On Fri, 10 Feb 2012, Jan Kiszka wrote:
> On 2012-02-10 12:18, Paolo Bonzini wrote:
> > On 02/10/2012 12:19 PM, Stefano Stabellini wrote:
> >> I think you are right and the right thing to do would be blocking
> >> indefinitely.
> >> However if slirp doesn't support it, we could have a timeout of 1000 if
> >> CONFIG_SLIRP, otherwise block indefinitely.
> > 
> > You could add a similar hack to qemu_bh_update_timeout for
> > slirp_update_timeout.
> 
> Real solutions would be preferred, but I know that the code is hairy. In
> any case, please no CONFIG_SLIRP code forks.

I tried to follow your suggestions, what do you guys think about the
approach of the patch below?

---


main_loop_wait: block indefinitely

- remove qemu_calculate_timeout;

- explicitly size timeout to uint32_t;

- introduce slirp_update_timeout;

- pass NULL as timeout argument to select in case timeout is the maximum
value;

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Paul Brook - Feb. 10, 2012, 11:34 p.m.
> +#ifdef CONFIG_SLIRP
> +static inline void slirp_update_timeout(uint32_t *timeout)
> +{
> +    *timeout = MIN(1000, *timeout);
> +}
> +#else
> +static inline void slirp_update_timeout(uint32_t *timeout) { }
> +#endif

Shouldn't we be testing whether slirp is actually in use? I doubt many people 
go to the effort of rebuilding without SLIRP support.

Paul

Patch

diff --git a/async.c b/async.c
index 332d511..ecdaf15 100644
--- a/async.c
+++ b/async.c
@@ -120,7 +120,7 @@  void qemu_bh_delete(QEMUBH *bh)
     bh->deleted = 1;
 }
 
-void qemu_bh_update_timeout(int *timeout)
+void qemu_bh_update_timeout(uint32_t *timeout)
 {
     QEMUBH *bh;
 
diff --git a/main-loop.c b/main-loop.c
index 692381c..4a105e9 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -366,7 +366,7 @@  void qemu_del_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque)
     }
 }
 
-static void os_host_main_loop_wait(int *timeout)
+static void os_host_main_loop_wait(uint32_t *timeout)
 {
     int ret, ret2, i;
     PollingEntry *pe;
@@ -410,7 +410,7 @@  static void os_host_main_loop_wait(int *timeout)
     *timeout = 0;
 }
 #else
-static inline void os_host_main_loop_wait(int *timeout)
+static inline void os_host_main_loop_wait(uint32_t *timeout)
 {
 }
 #endif
@@ -419,21 +419,17 @@  int main_loop_wait(int nonblocking)
 {
     fd_set rfds, wfds, xfds;
     int ret, nfds;
-    struct timeval tv;
-    int timeout;
+    struct timeval tv, *tvarg = NULL;
+    uint32_t timeout = UINT32_MAX;
 
     if (nonblocking) {
         timeout = 0;
     } else {
-        timeout = qemu_calculate_timeout();
         qemu_bh_update_timeout(&timeout);
     }
 
     os_host_main_loop_wait(&timeout);
 
-    tv.tv_sec = timeout / 1000;
-    tv.tv_usec = (timeout % 1000) * 1000;
-
     /* poll any events */
     /* XXX: separate device handlers from system ones */
     nfds = -1;
@@ -442,16 +438,23 @@  int main_loop_wait(int nonblocking)
     FD_ZERO(&xfds);
 
 #ifdef CONFIG_SLIRP
+    slirp_update_timeout(&timeout);
     slirp_select_fill(&nfds, &rfds, &wfds, &xfds);
 #endif
     qemu_iohandler_fill(&nfds, &rfds, &wfds, &xfds);
     glib_select_fill(&nfds, &rfds, &wfds, &xfds, &tv);
 
+    if (timeout < UINT32_MAX) {
+        tvarg = &tv;
+        tv.tv_sec = timeout / 1000;
+        tv.tv_usec = (timeout % 1000) * 1000;
+    }
+
     if (timeout > 0) {
         qemu_mutex_unlock_iothread();
     }
 
-    ret = select(nfds + 1, &rfds, &wfds, &xfds, &tv);
+    ret = select(nfds + 1, &rfds, &wfds, &xfds, tvarg);
 
     if (timeout > 0) {
         qemu_mutex_lock_iothread();
diff --git a/main-loop.h b/main-loop.h
index f971013..22c0dc9 100644
--- a/main-loop.h
+++ b/main-loop.h
@@ -352,6 +352,6 @@  void qemu_iohandler_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds, int rc
 
 void qemu_bh_schedule_idle(QEMUBH *bh);
 int qemu_bh_poll(void);
-void qemu_bh_update_timeout(int *timeout);
+void qemu_bh_update_timeout(uint32_t *timeout);
 
 #endif
diff --git a/qemu-timer.c b/qemu-timer.c
index de20852..3e1ce08 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -821,8 +821,3 @@  fail:
     return err;
 }
 
-int qemu_calculate_timeout(void)
-{
-    return 1000;
-}
-
diff --git a/qemu-timer.h b/qemu-timer.h
index de17f3b..f1386d5 100644
--- a/qemu-timer.h
+++ b/qemu-timer.h
@@ -62,7 +62,6 @@  uint64_t qemu_timer_expire_time_ns(QEMUTimer *ts);
 void qemu_run_all_timers(void);
 int qemu_alarm_pending(void);
 void configure_alarms(char const *opt);
-int qemu_calculate_timeout(void);
 void init_clocks(void);
 int init_timer_alarm(void);
 
diff --git a/slirp/libslirp.h b/slirp/libslirp.h
index 890fd86..6af7534 100644
--- a/slirp/libslirp.h
+++ b/slirp/libslirp.h
@@ -42,4 +42,13 @@  void slirp_socket_recv(Slirp *slirp, struct in_addr guest_addr,
 size_t slirp_socket_can_recv(Slirp *slirp, struct in_addr guest_addr,
                              int guest_port);
 
+#ifdef CONFIG_SLIRP
+static inline void slirp_update_timeout(uint32_t *timeout)
+{
+    *timeout = MIN(1000, *timeout);
+}
+#else
+static inline void slirp_update_timeout(uint32_t *timeout) { }
+#endif
+
 #endif