Re: [libvirt] [PATCH 1/6] libxl: Fix handling of timeouts

[Jim Fehlig]

Jim Fehlig wrote:
> xen-unstable commit XXXX makes changes wrt modifying and deregistering
> timeouts.
>
> First, timeout modify callbacks will only be invoked with an
> abs_t of {0,0}, i.e. make the timeout fire immediately.  Prior to this
> commit, timeout modify callbacks were never invoked.
>
> Second, timeout deregister hooks will no longer be called.
>
> This patch makes changes in the libvirt libxl driver that should be
> compatible before and after commit XXXX.
>
> While at it, fix a potential overflow in the timeout register callback.
> ---
>
> 'commit XXXX' will be replaced with a proper commit id once committed
> to xen-unstable.
>   

libxl patch as been committed to xen-unstable now, changeset 26469. 
I've updated this patch accordingly.

Regards,
Jim



>From 451c84284fa1b4f311f2ceb052c8e9f25ffa0cf0 Mon Sep 17 00:00:00 2001
From: Jim Fehlig <jfeh...@suse.com>
Date: Mon, 21 Jan 2013 09:59:28 -0700
Subject: [PATCH 1/6] libxl: Fix handling of timeouts

xen-unstable changeset 26469 makes changes wrt modifying and deregistering
timeouts.

First, timeout modify callbacks will only be invoked with an
abs_t of {0,0}, i.e. make the timeout fire immediately.  Prior to this
commit, timeout modify callbacks were never invoked.

Second, timeout deregister hooks will no longer be called.

This patch makes changes in the libvirt libxl driver that should be
compatible before and after changeset 26469.

While at it, fix a potential overflow in the timeout register callback.
---
 src/libxl/libxl_driver.c | 39 ++++++++++++++++++++++++++++++---------
 1 file changed, 30 insertions(+), 9 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index a8c4cae..77026fc 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -186,7 +186,13 @@ libxlTimerCallback(int timer ATTRIBUTE_UNUSED, void *timer_info)
 {
     struct libxlOSEventHookTimerInfo *info = timer_info;
 
+    /* libxl expects the event to be deregistered when calling
+       libxl_osevent_occurred_timeout, but we dont want the event info
+       destroyed.  Disable the timeout and only remove it after returning
+       from libxl. */
+    virEventUpdateTimeout(info->id, -1);
     libxl_osevent_occurred_timeout(info->priv->ctx, info->xl_priv);
+    virEventRemoveTimeout(info->id);
 }
 
 static void
@@ -202,6 +208,8 @@ libxlTimeoutRegisterEventHook(void *priv,
                               void *xl_priv)
 {
     struct timeval now;
+    struct timeval res;
+    static struct timeval zero;
     struct libxlOSEventHookTimerInfo *timer_info;
     int timeout, timer_id;
 
@@ -211,8 +219,15 @@ libxlTimeoutRegisterEventHook(void *priv,
     }
 
     gettimeofday(&now, NULL);
-    timeout = (abs_t.tv_usec - now.tv_usec) / 1000;
-    timeout += (abs_t.tv_sec - now.tv_sec) * 1000;
+    timersub(&abs_t, &now, &res);
+    /* Ensure timeout is not overflowed */
+    if (timercmp(&res, &zero, <)) {
+        timeout = 0;
+    } else if (res.tv_sec > INT_MAX / 1000) {
+        timeout = INT_MAX;
+    } else {
+        timeout = res.tv_sec * 1000 + (res.tv_usec + 999) / 1000;
+    }
     timer_id = virEventAddTimeout(timeout, libxlTimerCallback,
                                   timer_info, libxlTimerInfoFree);
     if (timer_id < 0) {
@@ -227,19 +242,25 @@ libxlTimeoutRegisterEventHook(void *priv,
     return 0;
 }
 
+/*
+ * Note:  There are two changes wrt timeouts starting with xen-unstable
+ * changeset 26469:
+ *
+ * 1. Timeout modify callbacks will only be invoked with an abs_t of {0,0},
+ * i.e. make the timeout fire immediately.  Prior to this commit, timeout
+ * modify callbacks were never invoked.
+ *
+ * 2. Timeout deregister hooks will no longer be called.
+ */
 static int
 libxlTimeoutModifyEventHook(void *priv ATTRIBUTE_UNUSED,
                             void **hndp,
-                            struct timeval abs_t)
+                            struct timeval abs_t ATTRIBUTE_UNUSED)
 {
-    struct timeval now;
-    int timeout;
     struct libxlOSEventHookTimerInfo *timer_info = *hndp;
 
-    gettimeofday(&now, NULL);
-    timeout = (abs_t.tv_usec - now.tv_usec) / 1000;
-    timeout += (abs_t.tv_sec - now.tv_sec) * 1000;
-    virEventUpdateTimeout(timer_info->id, timeout);
+    /* Make the timeout fire */
+    virEventUpdateTimeout(timer_info->id, 0);
     return 0;
 }
 
-- 
1.8.0.1


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list