On Fri, Sep 11, 2015 at 02:41:02PM +0100, Daniel P. Berrange wrote:
> The xenXMConfigCacheRefresh method scans /etc/xen and loads
> all config files it finds. It then scans its internal hash
> table and purges any (previously) loaded config files whose
> refresh timestamp does not match the timestamp recorded at
> the start of xenXMConfigCacheRefresh(). There is unfortunately
> a subtle flaw in this, because if loading the config files
> takes longer than 1 second, some of the config files will
> have a refresh timestamp that is 1 or more seconds different
> (newer) than is checked for. So we immediately purge a bunch
> of valid config files we just loaded.
>
> To avoid this flaw, we must pass the timestamp we record at
> the start of xenXMConfigCacheRefresh() into the
> xenXMConfigCacheAddFile() method, instead of letting the
> latter call time(NULL) again.
>
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
> src/xen/xen_inotify.c | 10 ++++++----
> src/xen/xm_internal.c | 7 +++----
> src/xen/xm_internal.h | 2 +-
> 3 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/src/xen/xen_inotify.c b/src/xen/xen_inotify.c
> index cd1e2df..d81a35d 100644
> --- a/src/xen/xen_inotify.c
> +++ b/src/xen/xen_inotify.c
> @@ -217,11 +217,11 @@ xenInotifyRemoveDomainConfigInfo(virConnectPtr conn, const char *fname)
> }
>
> static int
> -xenInotifyAddDomainConfigInfo(virConnectPtr conn, const char *fname)
> +xenInotifyAddDomainConfigInfo(virConnectPtr conn, const char *fname, time_t now)
> {
> xenUnifiedPrivatePtr priv = conn->privateData;
> return priv->useXenConfigCache ?
> - xenXMConfigCacheAddFile(conn, fname) :
> + xenXMConfigCacheAddFile(conn, fname, now) :
> xenInotifyXendDomainsDirAddEntry(conn, fname);
> }
>
> @@ -238,6 +238,7 @@ xenInotifyEvent(int watch ATTRIBUTE_UNUSED,
> char *tmp, *name;
> virConnectPtr conn = data;
> xenUnifiedPrivatePtr priv = NULL;
> + time_t now = time(NULL);
>
> VIR_DEBUG("got inotify event");
>
> @@ -300,7 +301,7 @@ xenInotifyEvent(int watch ATTRIBUTE_UNUSED,
> }
> } else if (e->mask & (IN_CREATE | IN_CLOSE_WRITE | IN_MOVED_TO)) {
> virObjectEventPtr event;
> - if (xenInotifyAddDomainConfigInfo(conn, fname) < 0) {
> + if (xenInotifyAddDomainConfigInfo(conn, fname, now) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> "%s", _("Error adding file to config cache"));
> goto cleanup;
> @@ -344,6 +345,7 @@ xenInotifyOpen(virConnectPtr conn,
> char *path;
> xenUnifiedPrivatePtr priv = conn->privateData;
> int direrr;
> + time_t now = time(NULL);
>
> virCheckFlags(VIR_CONNECT_RO, -1);
>
> @@ -374,7 +376,7 @@ xenInotifyOpen(virConnectPtr conn,
> return -1;
> }
>
> - if (xenInotifyAddDomainConfigInfo(conn, path) < 0) {
> + if (xenInotifyAddDomainConfigInfo(conn, path, now) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> "%s", _("Error adding file to config list"));
> closedir(dh);
> diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
> index 59b1cd4..07b9eb4 100644
> --- a/src/xen/xm_internal.c
> +++ b/src/xen/xm_internal.c
> @@ -191,15 +191,14 @@ xenXMConfigCacheRemoveFile(virConnectPtr conn, const char *filename)
> * calling this function
> */
> int
> -xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename)
> +xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename, time_t now)
> {
> xenUnifiedPrivatePtr priv = conn->privateData;
> xenXMConfCachePtr entry;
> struct stat st;
> int newborn = 0;
> - time_t now = time(NULL);
>
> - VIR_DEBUG("Adding file %s", filename);
> + VIR_DEBUG("Adding file %s %lld", filename, (long long)now);
>
> /* Get modified time */
> if ((stat(filename, &st) < 0)) {
> @@ -373,7 +372,7 @@ xenXMConfigCacheRefresh(virConnectPtr conn)
>
> /* If we already have a matching entry and it is not
> modified, then carry on to next one*/
> - if (xenXMConfigCacheAddFile(conn, path) < 0) {
> + if (xenXMConfigCacheAddFile(conn, path, now) < 0) {
> /* Ignoring errors, since a lot of stuff goes wrong in /etc/xen */
> }
>
> diff --git a/src/xen/xm_internal.h b/src/xen/xm_internal.h
> index 25b4fd5..0246895 100644
> --- a/src/xen/xm_internal.h
> +++ b/src/xen/xm_internal.h
> @@ -31,7 +31,7 @@
> # include "domain_conf.h"
>
> int xenXMConfigCacheRefresh (virConnectPtr conn);
> -int xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename);
> +int xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename, time_t now);
> int xenXMConfigCacheRemoveFile(virConnectPtr conn, const char *filename);
>
> int xenXMOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags);
Good one :-)
ACK.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list