[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH 2/4] libxl: remove list of timer registrations from libxlDomainObjPrivate



Due to some misunderstanding of requirements libxl places on timer
handling, I introduced the half-brained idea of maintaining a list
of timeouts that the driver could force to expire before freeing a
libxlDomainObjPrivate (and hence libxl_ctx).  But testing all
the latest versions of Xen supported by the libxl driver (4.2.3,
4.3.1, 4.4.0 RC3), I see that libxl will handle this just fine and
there is no need to force expiration behind libxl's back.  Indeed it
may be harmful to do so.

This patch removes the timer list, allowing libxl to handle cleanup
of its timer registrations.

Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
---
 src/libxl/libxl_conf.h   |  5 +---
 src/libxl/libxl_domain.c | 72 +++---------------------------------------------
 src/libxl/libxl_domain.h |  8 +-----
 src/libxl/libxl_driver.c |  3 +-
 4 files changed, 7 insertions(+), 81 deletions(-)

diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index f743541..ca7bc7d 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -1,7 +1,7 @@
 /*
  * libxl_conf.h: libxl configuration management
  *
- * Copyright (C) 2011-2013 SUSE LINUX Products GmbH, Nuernberg, Germany.
+ * Copyright (C) 2011-2014 SUSE LINUX Products GmbH, Nuernberg, Germany.
  * Copyright (C) 2011 Univention GmbH.
  *
  * This library is free software; you can redistribute it and/or
@@ -115,9 +115,6 @@ struct _libxlDriverPrivate {
     virSysinfoDefPtr hostsysinfo;
 };
 
-typedef struct _libxlEventHookInfo libxlEventHookInfo;
-typedef libxlEventHookInfo *libxlEventHookInfoPtr;
-
 # define LIBXL_SAVE_MAGIC "libvirt-xml\n \0 \r"
 # define LIBXL_SAVE_VERSION 1
 
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 7efc13b..fbd6cab 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -34,37 +34,9 @@
 #define VIR_FROM_THIS VIR_FROM_LIBXL
 
 
-/* Append an event registration to the list of registrations */
-#define LIBXL_EV_REG_APPEND(head, add)                 \
-    do {                                               \
-        libxlEventHookInfoPtr temp;                    \
-        if (head) {                                    \
-            temp = head;                               \
-            while (temp->next)                         \
-                temp = temp->next;                     \
-            temp->next = add;                          \
-        } else {                                       \
-            head = add;                                \
-        }                                              \
-    } while (0)
-
-/* Remove an event registration from the list of registrations */
-#define LIBXL_EV_REG_REMOVE(head, del)                 \
-    do {                                               \
-        libxlEventHookInfoPtr temp;                    \
-        if (head == del) {                             \
-            head = head->next;                         \
-        } else {                                       \
-            temp = head;                               \
-            while (temp->next && temp->next != del)    \
-                temp = temp->next;                     \
-            if (temp->next) {                          \
-                temp->next = del->next;                \
-            }                                          \
-        }                                              \
-    } while (0)
-
 /* Object used to store info related to libxl event registrations */
+typedef struct _libxlEventHookInfo libxlEventHookInfo;
+typedef libxlEventHookInfo *libxlEventHookInfoPtr;
 struct _libxlEventHookInfo {
     libxlEventHookInfoPtr next;
     libxlDomainObjPrivatePtr priv;
@@ -214,12 +186,7 @@ libxlDomainObjTimerCallback(int timer ATTRIBUTE_UNUSED, 
void *timer_info)
     virObjectUnlock(p);
     libxl_osevent_occurred_timeout(p->ctx, info->xl_priv);
     virObjectLock(p);
-    /*
-     * Timeout could have been freed while the lock was dropped.
-     * Only remove it from the list if it still exists.
-     */
-    if (virEventRemoveTimeout(info->id) == 0)
-        LIBXL_EV_REG_REMOVE(p->timerRegistrations, info);
+    virEventRemoveTimeout(info->id);
     virObjectUnlock(p);
 }
 
@@ -265,9 +232,6 @@ libxlDomainObjTimeoutRegisterEventHook(void *priv,
         return -1;
     }
 
-    virObjectLock(info->priv);
-    LIBXL_EV_REG_APPEND(info->priv->timerRegistrations, info);
-    virObjectUnlock(info->priv);
     *hndp = info;
 
     return 0;
@@ -306,12 +270,7 @@ libxlDomainObjTimeoutDeregisterEventHook(void *priv 
ATTRIBUTE_UNUSED,
     libxlDomainObjPrivatePtr p = info->priv;
 
     virObjectLock(p);
-    /*
-     * Only remove the timeout from the list if removal from the
-     * event loop is successful.
-     */
-    if (virEventRemoveTimeout(info->id) == 0)
-        LIBXL_EV_REG_REMOVE(p->timerRegistrations, info);
+    virEventRemoveTimeout(info->id);
     virObjectUnlock(p);
 }
 
@@ -443,26 +402,3 @@ cleanup:
     VIR_FREE(log_file);
     return ret;
 }
-
-void
-libxlDomainObjRegisteredTimeoutsCleanup(libxlDomainObjPrivatePtr priv)
-{
-    libxlEventHookInfoPtr info;
-
-    virObjectLock(priv);
-    info = priv->timerRegistrations;
-    while (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(priv->ctx, info->xl_priv);
-        virEventRemoveTimeout(info->id);
-        info = info->next;
-    }
-    priv->timerRegistrations = NULL;
-    virObjectUnlock(priv);
-}
diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
index e4695ef..8565820 100644
--- a/src/libxl/libxl_domain.h
+++ b/src/libxl/libxl_domain.h
@@ -1,7 +1,7 @@
 /*
  * libxl_domain.h: libxl domain object private state
  *
- * Copyright (C) 2011-2013 SUSE LINUX Products GmbH, Nuernberg, Germany.
+ * Copyright (C) 2011-2014 SUSE LINUX Products GmbH, Nuernberg, Germany.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -43,9 +43,6 @@ struct _libxlDomainObjPrivate {
     /* console */
     virChrdevsPtr devs;
     libxl_evgen_domain_death *deathW;
-
-    /* list of libxl timeout registrations */
-    libxlEventHookInfoPtr timerRegistrations;
 };
 
 
@@ -56,7 +53,4 @@ extern virDomainDefParserConfig libxlDomainDefParserConfig;
 int
 libxlDomainObjPrivateInitCtx(virDomainObjPtr vm);
 
-void
-libxlDomainObjRegisteredTimeoutsCleanup(libxlDomainObjPrivatePtr priv);
-
 #endif /* LIBXL_DOMAIN_H */
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index cb3deec..d639011 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -2,7 +2,7 @@
  * libxl_driver.c: core driver methods for managing libxenlight domains
  *
  * Copyright (C) 2006-2014 Red Hat, Inc.
- * Copyright (C) 2011-2013 SUSE LINUX Products GmbH, Nuernberg, Germany.
+ * Copyright (C) 2011-2014 SUSE LINUX Products GmbH, Nuernberg, Germany.
  * Copyright (C) 2011 Univention GmbH.
  *
  * This library is free software; you can redistribute it and/or
@@ -313,7 +313,6 @@ libxlVmCleanup(libxlDriverPrivatePtr driver,
         vm->newDef = NULL;
     }
 
-    libxlDomainObjRegisteredTimeoutsCleanup(priv);
     virObjectUnref(cfg);
 }
 
-- 
1.8.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.