diff mbox series

[v6,1/6] libxl: add infrastructure to track and query 'recent' domids

Message ID 20200219093754.2924-2-pdurrant@amazon.com (mailing list archive)
State Superseded
Headers show
Series xl/libxl: domid allocation/preservation changes | expand

Commit Message

Paul Durrant Feb. 19, 2020, 9:37 a.m. UTC
A domid is considered recent if the domain it represents was destroyed
less than a specified number of seconds ago. For debugging and/or testing
purposes the number can be set using the environment variable
LIBXL_DOMID_REUSE_TIMEOUT. If the variable does not exist then a default
value of 60s is used.

Whenever a domain is destroyed, a time-stamped record will be written into
a history file (/var/run/xen/domid-history). To avoid the history file
growing too large, any records with time-stamps that indicate that the
age of a domid has exceeded the re-use timeout will also be purged.

A new utility function, libxl__is_recent_domid(), has been added. This
function reads the same history file checking whether a specified domid
has a record that does not exceed the re-use timeout. Since this utility
function does not write to the file, no records are actually purged by it.

NOTE: The history file is purged on boot to it is safe to use
      CLOCK_MONOTONIC as a time source.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wl@xen.org>
Cc: Anthony PERARD <anthony.perard@citrix.com>

v6:
 _ Addressed further comments from Ian

v5:
 - Re-work file manipulation some more
 - Add more error checks

v4:
 - Use new generalised libxl__flock
 - Don't read and write the same file
 - Use 'recent' rather than 'retired'
 - Add code into xen-init-dom0 to delete an old history file at boot

v2:
 - New in v2
---
 tools/helpers/xen-init-dom0.c |  30 +++++
 tools/libxl/libxl.h           |   7 ++
 tools/libxl/libxl_domain.c    | 222 ++++++++++++++++++++++++++++++++++
 tools/libxl/libxl_internal.c  |  10 ++
 tools/libxl/libxl_internal.h  |  14 +++
 5 files changed, 283 insertions(+)

Comments

Ian Jackson Feb. 20, 2020, 4:19 p.m. UTC | #1
Paul Durrant writes ("[PATCH v6 1/6] libxl: add infrastructure to track and query 'recent' domids"):
> A domid is considered recent if the domain it represents was destroyed
> less than a specified number of seconds ago. For debugging and/or testing
> purposes the number can be set using the environment variable
> LIBXL_DOMID_REUSE_TIMEOUT. If the variable does not exist then a default
> value of 60s is used.
...

Quoting only the parts which are neither specific to the particular
function, nor calls to the functions into which common code has
currently been moved:

> +static int libxl__mark_domid_recent(libxl__gc *gc, uint32_t domid)
> +{
+    long timeout = libxl__get_domid_reuse_timeout();
...
> +    if (clock_gettime(CLOCK_MONOTONIC, &ts)) {
> +        LOGED(ERROR, domid, "failed to get time");
> +        goto out;
> +    }
...
> +        if (ts.tv_sec - sec > timeout)
> +            continue; /* Ignore expired entries */

> +int libxl__is_domid_recent(libxl__gc *gc, uint32_t domid, bool *recent)
> +{
> +    long timeout = libxl__get_domid_reuse_timeout();
...
> +    if (clock_gettime(CLOCK_MONOTONIC, &ts)) {
> +        LOGED(ERROR, domid, "failed to get time");
> +        goto out;
> +    }
...
> +        if (val == domid && ts.tv_sec - sec <= timeout) {

I'm afraid I am still making style comments:

IMO the reuse timeout call and the clock_gettime call should be put in
libxl__open_domid_history; and the time filtering check should be
folded into libxl__read_recent.

In my review of v4 I wrote:

  Do you think this can be combined somehow ?  Possibilities seem to
  include: explicit read_recent_{init,entry,finish} functions; a single
  function with a "per-entry" callback; same but with a macro.  If you
  do that you can also have it have handle the "file does not exist"
  special case.

You've provided the read_recent_entry function but the "init" function
libxl__open_domid_history does too little.  This series seems to be
moving towards what I called read_recent_{init,entry,finish} (which
should probably have the timestamp and FILE* in a struct together) but
it seems to be doing so quite slowly.

In your factored out functions you generally do this:

   int some_function(){
      r = do_the_thing();
      if (r == 0) return 0;

      LOGE(....)
      return ERROR_FAIL;
    }

This structure is not ideal because:

 - It makes it hard to extend this function to do more, later.
   For example, refactoring the clock_gettime call into
   what is now libxl__open_domid_history would involve reorganising
   the function.

 - It encourages vacuous log messages whose content is mainly in the
   function and line number framing:
       +    LOGE(ERROR, "failed");
       +    return ERROR_FAIL;
       +}
   rather than
       if (!*f) {
           LOGE(ERROR, "failed to open recent domid file `%s'", path);
           rc = ERROR_FAIL;
           goto out;
       }
   (and the latter is to be preferred).

 - It is nonstandard.  See ERROR_HANDLING in CODING_STYLE.

> +    ret = fclose(nf);

This should be called `r', not `ret'.  See CODING_STYLE.

Sorry that some of the other code which you are having to edit here
sets a bad example.  (See the apology at the top of CODING_STYLE.)
(Existing uses of `ret' in libxl are sometimes a syscall return value
and sometimes a libxl error code, which is one reason that name is now
deprecated.)

> +static int libxl__replace_domid_history(libxl__gc *gc, char *new)
> +{

For the record: it was not necessary to break this out into its own
function, because there is only one call site, so open-coding it would
not duplicate anything.  On the other hand if you think it is clearer,
I have no objection.

I think the actual behaviour is correct now but I would like to read
it again when it is in the conventional style.

Thanks
Ian.
Durrant, Paul Feb. 20, 2020, 4:36 p.m. UTC | #2
> -----Original Message-----
> From: Ian Jackson <ian.jackson@citrix.com>
> Sent: 20 February 2020 16:20
> To: Durrant, Paul <pdurrant@amazon.co.uk>
> Cc: xen-devel@lists.xenproject.org; Wei Liu <wl@xen.org>; Anthony Perard
> <anthony.perard@citrix.com>
> Subject: Re: [PATCH v6 1/6] libxl: add infrastructure to track and query
> 'recent' domids
> 
> Paul Durrant writes ("[PATCH v6 1/6] libxl: add infrastructure to track
> and query 'recent' domids"):
> > A domid is considered recent if the domain it represents was destroyed
> > less than a specified number of seconds ago. For debugging and/or
> testing
> > purposes the number can be set using the environment variable
> > LIBXL_DOMID_REUSE_TIMEOUT. If the variable does not exist then a default
> > value of 60s is used.
> ...
> 
> Quoting only the parts which are neither specific to the particular
> function, nor calls to the functions into which common code has
> currently been moved:
> 
> > +static int libxl__mark_domid_recent(libxl__gc *gc, uint32_t domid)
> > +{
> +    long timeout = libxl__get_domid_reuse_timeout();
> ...
> > +    if (clock_gettime(CLOCK_MONOTONIC, &ts)) {
> > +        LOGED(ERROR, domid, "failed to get time");
> > +        goto out;
> > +    }
> ...
> > +        if (ts.tv_sec - sec > timeout)
> > +            continue; /* Ignore expired entries */
> 
> > +int libxl__is_domid_recent(libxl__gc *gc, uint32_t domid, bool *recent)
> > +{
> > +    long timeout = libxl__get_domid_reuse_timeout();
> ...
> > +    if (clock_gettime(CLOCK_MONOTONIC, &ts)) {
> > +        LOGED(ERROR, domid, "failed to get time");
> > +        goto out;
> > +    }
> ...
> > +        if (val == domid && ts.tv_sec - sec <= timeout) {
> 
> I'm afraid I am still making style comments:
> 
> IMO the reuse timeout call and the clock_gettime call should be put in
> libxl__open_domid_history; and the time filtering check should be
> folded into libxl__read_recent.

Ok. I was having a hard time guessing just exactly what you're looking for. I think that makes it a little clearer.

> 
> In my review of v4 I wrote:
> 
>   Do you think this can be combined somehow ?  Possibilities seem to
>   include: explicit read_recent_{init,entry,finish} functions; a single
>   function with a "per-entry" callback; same but with a macro.  If you
>   do that you can also have it have handle the "file does not exist"
>   special case.
> 
> You've provided the read_recent_entry function but the "init" function
> libxl__open_domid_history does too little.  This series seems to be
> moving towards what I called read_recent_{init,entry,finish} (which
> should probably have the timestamp and FILE* in a struct together) but
> it seems to be doing so quite slowly.

Now again I'm not sure *exactly* what you want me to do, but I'll have another guess.

> 
> In your factored out functions you generally do this:
> 
>    int some_function(){
>       r = do_the_thing();
>       if (r == 0) return 0;
> 
>       LOGE(....)
>       return ERROR_FAIL;
>     }
> 
> This structure is not ideal because:
> 
>  - It makes it hard to extend this function to do more, later.
>    For example, refactoring the clock_gettime call into
>    what is now libxl__open_domid_history would involve reorganising
>    the function.

Ok, but it makes the code shorter done the way I have it and I don't really see why any necessary future re-organisation would be such a problem.

> 
>  - It encourages vacuous log messages whose content is mainly in the
>    function and line number framing:
>        +    LOGE(ERROR, "failed");
>        +    return ERROR_FAIL;
>        +}
>    rather than
>        if (!*f) {
>            LOGE(ERROR, "failed to open recent domid file `%s'", path);
>            rc = ERROR_FAIL;
>            goto out;
>        }
>    (and the latter is to be preferred).

But by asking me to put the error handling inside the sub-functions I lost context such as 'path' which was present in the previous structure. I could pass in an argument purely for the benefit of a log function but that seems excessive. I guess I will pull the error logging out again, but that seemed to be against your requirement to de-duplicate code.

> 
>  - It is nonstandard.  See ERROR_HANDLING in CODING_STYLE.
> 
> > +    ret = fclose(nf);
> 
> This should be called `r', not `ret'.  See CODING_STYLE.

Ok, I clearly didn't pick up all the subtleties.

> 
> Sorry that some of the other code which you are having to edit here
> sets a bad example.  (See the apology at the top of CODING_STYLE.)
> (Existing uses of `ret' in libxl are sometimes a syscall return value
> and sometimes a libxl error code, which is one reason that name is now
> deprecated.)
> 
> > +static int libxl__replace_domid_history(libxl__gc *gc, char *new)
> > +{
> 
> For the record: it was not necessary to break this out into its own
> function, because there is only one call site, so open-coding it would
> not duplicate anything.  On the other hand if you think it is clearer,
> I have no objection.
> 
> I think the actual behaviour is correct now but I would like to read
> it again when it is in the conventional style.
> 

I will spin it again shortly.

  Paul
Ian Jackson Feb. 20, 2020, 4:45 p.m. UTC | #3
Durrant, Paul writes ("RE: [PATCH v6 1/6] libxl: add infrastructure to track and query 'recent' domids"):
> [Ian:]
> > IMO the reuse timeout call and the clock_gettime call should be put in
> > libxl__open_domid_history; and the time filtering check should be
> > folded into libxl__read_recent.
> 
> Ok. I was having a hard time guessing just exactly what you're looking for. I think that makes it a little clearer.
...
> > In my review of v4 I wrote:
> > 
> >   Do you think this can be combined somehow ?  Possibilities seem to
> >   include: explicit read_recent_{init,entry,finish} functions; a single
> >   function with a "per-entry" callback; same but with a macro.  If you
> >   do that you can also have it have handle the "file does not exist"
> >   special case.
> > 
> > You've provided the read_recent_entry function but the "init" function
> > libxl__open_domid_history does too little.  This series seems to be
> > moving towards what I called read_recent_{init,entry,finish} (which
> > should probably have the timestamp and FILE* in a struct together) but
> > it seems to be doing so quite slowly.
> 
> Now again I'm not sure *exactly* what you want me to do, but I'll have another guess.

Maybe it would be better for us to try to come to a shared
understanding before you do another respin...

> >  - It encourages vacuous log messages whose content is mainly in the
> >    function and line number framing:
> >        +    LOGE(ERROR, "failed");
> >        +    return ERROR_FAIL;
> >        +}
> >    rather than
> >        if (!*f) {
> >            LOGE(ERROR, "failed to open recent domid file `%s'", path);
> >            rc = ERROR_FAIL;
> >            goto out;
> >        }
> >    (and the latter is to be preferred).
> 
> But by asking me to put the error handling inside the sub-functions I lost context such as 'path' which was present in the previous structure. I could pass in an argument purely for the benefit of a log function but that seems excessive. I guess I will pull the error logging out again, but that seemed to be against your requirement to de-duplicate code.

I think the path needs to be passed into these functions.  This is why
I think the functions need to take a struct* as an argument, for their
shared state (including the path and the other stuff).

Thanks,
Ian.
Durrant, Paul Feb. 20, 2020, 4:54 p.m. UTC | #4
> -----Original Message-----
> From: Ian Jackson <ian.jackson@citrix.com>
> Sent: 20 February 2020 16:46
> To: Durrant, Paul <pdurrant@amazon.co.uk>
> Cc: xen-devel@lists.xenproject.org; Wei Liu <wl@xen.org>; Anthony Perard
> <anthony.perard@citrix.com>
> Subject: RE: [PATCH v6 1/6] libxl: add infrastructure to track and query
> 'recent' domids
> 
> Durrant, Paul writes ("RE: [PATCH v6 1/6] libxl: add infrastructure to
> track and query 'recent' domids"):
> > [Ian:]
> > > IMO the reuse timeout call and the clock_gettime call should be put in
> > > libxl__open_domid_history; and the time filtering check should be
> > > folded into libxl__read_recent.
> >
> > Ok. I was having a hard time guessing just exactly what you're looking
> for. I think that makes it a little clearer.
> ...
> > > In my review of v4 I wrote:
> > >
> > >   Do you think this can be combined somehow ?  Possibilities seem to
> > >   include: explicit read_recent_{init,entry,finish} functions; a
> single
> > >   function with a "per-entry" callback; same but with a macro.  If you
> > >   do that you can also have it have handle the "file does not exist"
> > >   special case.
> > >
> > > You've provided the read_recent_entry function but the "init" function
> > > libxl__open_domid_history does too little.  This series seems to be
> > > moving towards what I called read_recent_{init,entry,finish} (which
> > > should probably have the timestamp and FILE* in a struct together) but
> > > it seems to be doing so quite slowly.
> >
> > Now again I'm not sure *exactly* what you want me to do, but I'll have
> another guess.
> 
> Maybe it would be better for us to try to come to a shared
> understanding before you do another respin...
> 

Not being co-located makes this somewhat tricky; I think it will basically still come down to me writing some code and then you saying whether that's what you meant... unless you can write some (pseudo-)code to illustrate? I think, from what you say below, I might now have a better idea of what you want so let's have one more go-around of me writing the code first :-)

> > >  - It encourages vacuous log messages whose content is mainly in the
> > >    function and line number framing:
> > >        +    LOGE(ERROR, "failed");
> > >        +    return ERROR_FAIL;
> > >        +}
> > >    rather than
> > >        if (!*f) {
> > >            LOGE(ERROR, "failed to open recent domid file `%s'", path);
> > >            rc = ERROR_FAIL;
> > >            goto out;
> > >        }
> > >    (and the latter is to be preferred).
> >
> > But by asking me to put the error handling inside the sub-functions I
> lost context such as 'path' which was present in the previous structure. I
> could pass in an argument purely for the benefit of a log function but
> that seems excessive. I guess I will pull the error logging out again, but
> that seemed to be against your requirement to de-duplicate code.
> 
> I think the path needs to be passed into these functions.  This is why
> I think the functions need to take a struct* as an argument, for their
> shared state (including the path and the other stuff).
> 

Ok, if that's the style you prefer I'll re-structure it that way.

  Paul

> Thanks,
> Ian.
Ian Jackson Feb. 20, 2020, 5:19 p.m. UTC | #5
Durrant, Paul writes ("RE: [PATCH v6 1/6] libxl: add infrastructure to track and query 'recent' domids"):
> Not being co-located makes this somewhat tricky; I think it will basically still come down to me writing some code and then you saying whether that's what you meant... unless you can write some (pseudo-)code to illustrate? I think, from what you say below, I might now have a better idea of what you want so let's have one more go-around of me writing the code first :-)

OK.

> [Ian:]
> > I think the path needs to be passed into these functions.  This is why
> > I think the functions need to take a struct* as an argument, for their
> > shared state (including the path and the other stuff).
> 
> Ok, if that's the style you prefer I'll re-structure it that way.

My bottom line on this aspect of code structure is that I want each
thing to be written only once.

Thanks,
Ian.
diff mbox series

Patch

diff --git a/tools/helpers/xen-init-dom0.c b/tools/helpers/xen-init-dom0.c
index a1e5729458..56f69ab66f 100644
--- a/tools/helpers/xen-init-dom0.c
+++ b/tools/helpers/xen-init-dom0.c
@@ -12,6 +12,32 @@ 
 #define DOMNAME_PATH   "/local/domain/0/name"
 #define DOMID_PATH     "/local/domain/0/domid"
 
+int clear_domid_history(void)
+{
+    int rc = 1;
+    xentoollog_logger_stdiostream *logger;
+    libxl_ctx *ctx;
+
+    logger = xtl_createlogger_stdiostream(stderr, XTL_ERROR, 0);
+    if (!logger)
+        return 1;
+
+    if (libxl_ctx_alloc(&ctx, LIBXL_VERSION, 0,
+                        (xentoollog_logger *)logger)) {
+        fprintf(stderr, "cannot init libxl context\n");
+        goto outlog;
+    }
+
+    if (!libxl_clear_domid_history(ctx))
+        rc = 0;
+
+    libxl_ctx_free(ctx);
+
+outlog:
+    xtl_logger_destroy((xentoollog_logger *)logger);
+    return rc;
+}
+
 int main(int argc, char **argv)
 {
     int rc;
@@ -70,6 +96,10 @@  int main(int argc, char **argv)
     if (rc)
         goto out;
 
+    rc = clear_domid_history();
+    if (rc)
+        goto out;
+
     /* Write xenstore entries. */
     if (!xs_write(xsh, XBT_NULL, DOMID_PATH, "0", strlen("0"))) {
         fprintf(stderr, "cannot set domid for Dom0\n");
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index fde8548847..80ae110a52 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -2679,6 +2679,13 @@  static inline int libxl_qemu_monitor_command_0x041200(libxl_ctx *ctx,
 
 #include <libxl_event.h>
 
+/*
+ * This function is for use only during host initialisation. If it is
+ * invoked on a host with running domains, or concurrent libxl
+ * processes then the system may malfuntion.
+ */
+int libxl_clear_domid_history(libxl_ctx *ctx);
+
 #endif /* LIBXL_H */
 
 /*
diff --git a/tools/libxl/libxl_domain.c b/tools/libxl/libxl_domain.c
index 973fc1434d..53f90cb555 100644
--- a/tools/libxl/libxl_domain.c
+++ b/tools/libxl/libxl_domain.c
@@ -1268,6 +1268,226 @@  static void dm_destroy_cb(libxl__egc *egc,
     libxl__devices_destroy(egc, &dis->drs);
 }
 
+static unsigned int libxl__get_domid_reuse_timeout(void)
+{
+    const char *env_timeout = getenv("LIBXL_DOMID_REUSE_TIMEOUT");
+
+    return env_timeout ? strtol(env_timeout, NULL, 0) :
+        LIBXL_DOMID_REUSE_TIMEOUT;
+}
+
+char *libxl__domid_history_path(libxl__gc *gc, const char *suffix)
+{
+    return GCSPRINTF("%s/domid-history%s", libxl__run_dir_path(),
+                     suffix ?: "");
+}
+
+int libxl_clear_domid_history(libxl_ctx *ctx)
+{
+    GC_INIT(ctx);
+    char *path;
+    int rc = ERROR_FAIL;
+
+    path = libxl__domid_history_path(gc, NULL);
+    if (!path)
+        goto out;
+
+    if (unlink(path) < 0 && errno != ENOENT) {
+        LOGE(ERROR, "failed to remove '%s'\n", path);
+        goto out;
+    }
+
+    rc = 0;
+
+out:
+    GC_FREE;
+    return rc;
+}
+
+static int libxl__read_recent(libxl__gc *gc, FILE *f, unsigned long *sec,
+                              unsigned int *domid)
+{
+    if (!f) {
+        *domid = INVALID_DOMID;
+        return 0;
+    }
+
+    for (;;) {
+        int n = fscanf(f, "%lu %u", sec, domid);
+
+        if (n == EOF) {
+            if (ferror(f)) {
+                LOGE(ERROR, "failed");
+                return ERROR_FAIL;
+            }
+
+            *domid = INVALID_DOMID;
+            break;
+        } else if (n == 2 && libxl_domid_valid_guest(*domid)) {
+            break;
+        }
+    }
+
+    return 0;
+}
+
+static int libxl__write_recent(libxl__gc *gc, FILE *f, unsigned long sec,
+                               unsigned int domid)
+{
+    int n = fprintf(f, "%lu %u\n", sec, domid);
+
+    if (n >= 0) return 0;
+
+    LOGE(ERROR, "failed");
+    return ERROR_FAIL;
+}
+
+static int libxl__open_domid_history(libxl__gc *gc, FILE **f)
+{
+    char *path = libxl__domid_history_path(gc, NULL);
+
+    *f = fopen(path, "r");
+    if (*f || errno == ENOENT) return 0;
+
+    LOGE(ERROR, "failed to open '%s'", path);
+    return ERROR_FAIL;
+}
+
+static int libxl__close_domid_history(libxl__gc *gc, FILE **f)
+{
+    int ret;
+
+    if (!*f) return 0;
+
+    ret = fclose(*f);
+    *f = NULL;
+    if (!ret) return 0;
+
+    LOGE(ERROR, "failed");
+    return ERROR_FAIL;
+}
+
+static int libxl__replace_domid_history(libxl__gc *gc, char *new)
+{
+    char *path = libxl__domid_history_path(gc, NULL);
+    int ret = rename(new, path);
+
+    if (!ret) return 0;
+
+    LOGE(ERROR, "failed to rename '%s' -> '%s'", new, path);
+    return ERROR_FAIL;
+}
+
+static int libxl__mark_domid_recent(libxl__gc *gc, uint32_t domid)
+{
+    long timeout = libxl__get_domid_reuse_timeout();
+    libxl__flock *lock;
+    char *new;
+    FILE *of = NULL, *nf = NULL;
+    struct timespec ts;
+    int ret, rc;
+
+    lock = libxl__lock_domid_history(gc);
+    if (!lock) {
+        LOGED(ERROR, domid, "failed to acquire lock");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    rc = libxl__open_domid_history(gc, &of);
+    if (rc) goto out;
+
+    new = libxl__domid_history_path(gc, ".new");
+    nf = fopen(new, "a");
+    if (!nf) {
+        LOGED(ERROR, domid, "failed to open '%s'", new);
+        goto out;
+    }
+
+    if (clock_gettime(CLOCK_MONOTONIC, &ts)) {
+        LOGED(ERROR, domid, "failed to get time");
+        goto out;
+    }
+
+    for (;;) {
+        unsigned long sec;
+        unsigned int val;
+
+        rc = libxl__read_recent(gc, of, &sec, &val);
+        if (rc) goto out;
+
+        if (val == INVALID_DOMID) /* EOF */
+            break;
+
+        if (ts.tv_sec - sec > timeout)
+            continue; /* Ignore expired entries */
+
+        rc = libxl__write_recent(gc, nf, sec, val);
+        if (rc) goto out;
+    }
+
+    rc = libxl__write_recent(gc, nf, ts.tv_sec, domid);
+    if (rc) goto out;
+
+    ret = fclose(nf);
+    nf = NULL;
+    if (ret == EOF) {
+        LOGED(ERROR, domid, "failed to close '%s'", new);
+        goto out;
+    }
+
+    rc = libxl__close_domid_history(gc, &of);
+    if (rc) goto out;
+
+    rc = libxl__replace_domid_history(gc, new);
+
+out:
+    if (nf) fclose(nf);
+    if (of) fclose(of);
+    if (lock) libxl__unlock_file(lock);
+
+    return rc;
+}
+
+int libxl__is_domid_recent(libxl__gc *gc, uint32_t domid, bool *recent)
+{
+    long timeout = libxl__get_domid_reuse_timeout();
+    FILE *f;
+    struct timespec ts;
+    int rc;
+
+    rc = libxl__open_domid_history(gc, &f);
+    if (rc) goto out;
+
+    if (clock_gettime(CLOCK_MONOTONIC, &ts)) {
+        LOGED(ERROR, domid, "failed to get time");
+        goto out;
+    }
+
+    *recent = false;
+    for (;;) {
+        unsigned long sec;
+        unsigned int val;
+
+        rc = libxl__read_recent(gc, f, &sec, &val);
+        if (rc) goto out;
+
+        if (val == INVALID_DOMID) /* EOF */
+            break;
+
+        if (val == domid && ts.tv_sec - sec <= timeout) {
+            *recent = true;
+            break;
+        }
+    }
+
+    rc = libxl__close_domid_history(gc, &f);
+
+out:
+    if (f) fclose(f);
+    return rc;
+}
+
 static void devices_destroy_cb(libxl__egc *egc,
                                libxl__devices_remove_state *drs,
                                int rc)
@@ -1331,6 +1551,8 @@  static void devices_destroy_cb(libxl__egc *egc,
         if (!ctx->xch) goto badchild;
 
         if (!dis->soft_reset) {
+            rc = libxl__mark_domid_recent(gc, domid);
+            if (rc) goto badchild;
             rc = xc_domain_destroy(ctx->xch, domid);
         } else {
             rc = xc_domain_pause(ctx->xch, domid);
diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index 211236dc99..bbd4c6cba9 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -504,6 +504,16 @@  libxl__flock *libxl__lock_domain_userdata(libxl__gc *gc, uint32_t domid)
     return lock;
 }
 
+libxl__flock *libxl__lock_domid_history(libxl__gc *gc)
+{
+    const char *lockfile;
+
+    lockfile = libxl__domid_history_path(gc, ".lock");
+    if (!lockfile) return NULL;
+
+    return libxl__lock_file(gc, lockfile);
+}
+
 int libxl__get_domain_configuration(libxl__gc *gc, uint32_t domid,
                                     libxl_domain_config *d_config)
 {
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 4936446069..43e5885d1e 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -4263,6 +4263,8 @@  _hidden void libxl__remus_teardown(libxl__egc *egc,
 _hidden void libxl__remus_restore_setup(libxl__egc *egc,
                                         libxl__domain_create_state *dcs);
 
+_hidden char *libxl__domid_history_path(libxl__gc *gc,
+                                        const char *suffix);
 
 /*
  * Convenience macros.
@@ -4661,6 +4663,7 @@  libxl__flock *libxl__lock_file(libxl__gc *gc, const char *filename);
 void libxl__unlock_file(libxl__flock *lock);
 
 libxl__flock *libxl__lock_domain_userdata(libxl__gc *gc, uint32_t domid);
+libxl__flock *libxl__lock_domid_history(libxl__gc *gc);
 
 /*
  * Retrieve / store domain configuration from / to libxl private
@@ -4799,6 +4802,17 @@  _hidden int libxl__domain_pvcontrol(libxl__egc *egc,
                                     libxl__xswait_state *pvcontrol,
                                     domid_t domid, const char *cmd);
 
+/*
+ * Maximum number of seconds after desctruction then a domid remains
+ * 'recent'. Recent domids are not allowed to be re-used. This can be
+ * overidden, for debugging purposes, by the environment variable of the
+ * same name.
+ */
+#define LIBXL_DOMID_REUSE_TIMEOUT 60
+
+/* Check whether a domid is recent */
+int libxl__is_domid_recent(libxl__gc *gc, uint32_t domid, bool *recent);
+
 #endif
 
 /*