On Thu, Jan 24, 2013 at 12:55:09PM -0700, Jim Fehlig wrote:
> 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 <jfehlig@xxxxxxxx>
> 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;
> }
>
ACK, I assume you tested with the older libxl, right ?
Daniel
--
Daniel Veillard | Open Source and Standards, Red Hat
veillard@xxxxxxxxxx | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list