xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0 of 4 V2] xl: guest reboot cleanups
@ 2012-06-29  8:14 Ian Campbell
  2012-06-29  8:14 ` [PATCH 1 of 4 V2] libxl: initialise cpupoolinfo in libxl__domain_scheduler Ian Campbell
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Ian Campbell @ 2012-06-29  8:14 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson, Dario Faggioli

These are miscellaneous fixes I noticed while tracking down the guest
reboot failures in light 13302, previously posted as 'xl: fix guest
reboot failures'.

The reboot issue is already fixed but these remained outstanding. I've
rebased to current tip and fixed the trivial conflict in cpupool_info.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1 of 4 V2] libxl: initialise cpupoolinfo in libxl__domain_scheduler
  2012-06-29  8:14 [PATCH 0 of 4 V2] xl: guest reboot cleanups Ian Campbell
@ 2012-06-29  8:14 ` Ian Campbell
  2012-06-29  8:14 ` [PATCH 2 of 4 V2] libxl: correct type of cpupool variable Ian Campbell
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Ian Campbell @ 2012-06-29  8:14 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson, Dario Faggioli

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1340956631 -3600
# Node ID d12ad3958ca787f565ba0fe67d7e97706314e46d
# Parent  850af6e1985ed0ed0393846c1aae749e2742de8a
libxl: initialise cpupoolinfo in libxl__domain_scheduler

If libxl_cpupool_info fails then we would call
libxl_cpupoolinfo_dispose on an uninitialised struct, and possibly
free an invalid pointer.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 850af6e1985e -r d12ad3958ca7 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Fri Jun 29 08:57:11 2012 +0100
+++ b/tools/libxl/libxl_dom.c	Fri Jun 29 08:57:11 2012 +0100
@@ -81,6 +81,7 @@ libxl_scheduler libxl__domain_scheduler(
     if (cpupool < 0)
         return sched;
 
+    libxl_cpupoolinfo_init(&poolinfo);
     rc = libxl_cpupool_info(CTX, &poolinfo, cpupool);
     if (rc < 0)
         goto out;

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2 of 4 V2] libxl: correct type of cpupool variable
  2012-06-29  8:14 [PATCH 0 of 4 V2] xl: guest reboot cleanups Ian Campbell
  2012-06-29  8:14 ` [PATCH 1 of 4 V2] libxl: initialise cpupoolinfo in libxl__domain_scheduler Ian Campbell
@ 2012-06-29  8:14 ` Ian Campbell
  2012-06-29  8:14 ` [PATCH 3 of 4 V2] libxl: log on failure in cpupool_info and libxl__domain_cpupool Ian Campbell
  2012-06-29  8:14 ` [PATCH 4 of 4 V2] xl: initialise domid to an explicitly invalid value Ian Campbell
  3 siblings, 0 replies; 5+ messages in thread
From: Ian Campbell @ 2012-06-29  8:14 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson, Dario Faggioli

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1340956631 -3600
# Node ID 763756d2590c84816782e2ffc2102c774366ed50
# Parent  d12ad3958ca787f565ba0fe67d7e97706314e46d
libxl: correct type of cpupool variable.

libxl__domain_cpupool returns int and can return ERROR_* so we need to
use a signed type.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r d12ad3958ca7 -r 763756d2590c tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Fri Jun 29 08:57:11 2012 +0100
+++ b/tools/libxl/libxl_dom.c	Fri Jun 29 08:57:11 2012 +0100
@@ -73,7 +73,7 @@ int libxl__domain_cpupool(libxl__gc *gc,
 
 libxl_scheduler libxl__domain_scheduler(libxl__gc *gc, uint32_t domid)
 {
-    uint32_t cpupool = libxl__domain_cpupool(gc, domid);
+    int cpupool = libxl__domain_cpupool(gc, domid);
     libxl_cpupoolinfo poolinfo;
     libxl_scheduler sched = LIBXL_SCHEDULER_UNKNOWN;
     int rc;

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 3 of 4 V2] libxl: log on failure in cpupool_info and libxl__domain_cpupool
  2012-06-29  8:14 [PATCH 0 of 4 V2] xl: guest reboot cleanups Ian Campbell
  2012-06-29  8:14 ` [PATCH 1 of 4 V2] libxl: initialise cpupoolinfo in libxl__domain_scheduler Ian Campbell
  2012-06-29  8:14 ` [PATCH 2 of 4 V2] libxl: correct type of cpupool variable Ian Campbell
@ 2012-06-29  8:14 ` Ian Campbell
  2012-06-29  8:14 ` [PATCH 4 of 4 V2] xl: initialise domid to an explicitly invalid value Ian Campbell
  3 siblings, 0 replies; 5+ messages in thread
From: Ian Campbell @ 2012-06-29  8:14 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson, Dario Faggioli

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1340956699 -3600
# Node ID 426b2f7429a792b9e54cbb8439d357c8edbd350a
# Parent  763756d2590c84816782e2ffc2102c774366ed50
libxl: log on failure in cpupool_info and libxl__domain_cpupool

Also in cpupool_info propagate the failure value from
libxl_cpumap_alloc.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 763756d2590c -r 426b2f7429a7 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Fri Jun 29 08:57:11 2012 +0100
+++ b/tools/libxl/libxl.c	Fri Jun 29 08:58:19 2012 +0100
@@ -571,16 +571,27 @@ static int cpupool_info(libxl__gc *gc,
 
     xcinfo = xc_cpupool_getinfo(CTX->xch, poolid);
     if (xcinfo == NULL)
+    {
+        LOGE(ERROR, "failed to get info for cpupool%d\n", poolid);
         return ERROR_FAIL;
+    }
 
     if (exact && xcinfo->cpupool_id != poolid)
+    {
+        LOG(ERROR, "got info for cpupool%d, wanted cpupool%d\n",
+            xcinfo->cpupool_id, poolid);
         goto out;
+    }
 
     info->poolid = xcinfo->cpupool_id;
     info->sched = xcinfo->sched_id;
     info->n_dom = xcinfo->n_dom;
-    if (libxl_cpumap_alloc(CTX, &info->cpumap, 0))
+    rc = libxl_cpumap_alloc(CTX, &info->cpumap, 0);
+    if (rc)
+    {
+        LOG(ERROR, "unable to allocate cpumap %d\n", rc);
         goto out;
+    }
     memcpy(info->cpumap.map, xcinfo->cpumap, info->cpumap.size);
 
     rc = 0;
diff -r 763756d2590c -r 426b2f7429a7 tools/libxl/libxl_dom.c
--- a/tools/libxl/libxl_dom.c	Fri Jun 29 08:57:11 2012 +0100
+++ b/tools/libxl/libxl_dom.c	Fri Jun 29 08:58:19 2012 +0100
@@ -64,10 +64,15 @@ int libxl__domain_cpupool(libxl__gc *gc,
 
     ret = xc_domain_getinfolist(CTX->xch, domid, 1, &info);
     if (ret != 1)
+    {
+        LOGE(ERROR, "getinfolist failed %d\n", ret);
         return ERROR_FAIL;
+    }
     if (info.domain != domid)
+    {
+        LOGE(ERROR, "got info for dom%d, wanted dom%d\n", info.domain, domid);
         return ERROR_FAIL;
-
+    }
     return info.cpupool;
 }

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 4 of 4 V2] xl: initialise domid to an explicitly invalid value
  2012-06-29  8:14 [PATCH 0 of 4 V2] xl: guest reboot cleanups Ian Campbell
                   ` (2 preceding siblings ...)
  2012-06-29  8:14 ` [PATCH 3 of 4 V2] libxl: log on failure in cpupool_info and libxl__domain_cpupool Ian Campbell
@ 2012-06-29  8:14 ` Ian Campbell
  3 siblings, 0 replies; 5+ messages in thread
From: Ian Campbell @ 2012-06-29  8:14 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson, Dario Faggioli

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1340956702 -3600
# Node ID 77993401a44fad90f1c3f75d28d497e0551c8530
# Parent  426b2f7429a792b9e54cbb8439d357c8edbd350a
xl: initialise domid to an explicitly invalid value

also ensure it is invalid whenever we destroy the domain.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r 426b2f7429a7 -r 77993401a44f tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Fri Jun 29 08:58:19 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Fri Jun 29 08:58:22 2012 +0100
@@ -68,7 +68,8 @@ libxl_ctx *ctx;
 xlchild children[child_max];
 
 /* when we operate on a domain, it is this one: */
-static uint32_t domid;
+#define INVALID_DOMID ~0
+static uint32_t domid = INVALID_DOMID;
 static const char *common_domname;
 static int fd_lock = -1;
 
@@ -1396,6 +1397,7 @@ static int handle_domain_death(uint32_t 
     case LIBXL_ACTION_ON_SHUTDOWN_DESTROY:
         LOG("Domain %d needs to be cleaned up: destroying the domain", domid);
         libxl_domain_destroy(ctx, domid);
+        domid = INVALID_DOMID;
         break;
 
     case LIBXL_ACTION_ON_SHUTDOWN_COREDUMP_DESTROY:
@@ -1458,6 +1460,12 @@ static int preserve_domain(uint32_t domi
     LOG("Preserving domain %d %s with suffix%s", domid, d_config->c_info.name, stime);
     rc = libxl_domain_preserve(ctx, domid, &d_config->c_info, stime, new_uuid);
 
+    /*
+     * Although domid still exists it is no longer the one we are concerned
+     * with.
+     */
+    domid = INVALID_DOMID;
+
     return rc == 0 ? 1 : 0;
 }
 
@@ -1745,7 +1753,7 @@ static int create_domain(struct domain_c
         goto out;
 
 start:
-    domid = -1;
+    assert(domid == INVALID_DOMID);
 
     rc = acquire_lock();
     if (rc < 0)
@@ -1994,8 +2002,10 @@ start:
 
 error_out:
     release_lock();
-    if (libxl_domid_valid_guest(domid))
+    if (libxl_domid_valid_guest(domid)) {
         libxl_domain_destroy(ctx, domid);
+        domid = INVALID_DOMID;
+    }
 
 out:
     if (logfile != 2)

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2012-06-29  8:14 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-29  8:14 [PATCH 0 of 4 V2] xl: guest reboot cleanups Ian Campbell
2012-06-29  8:14 ` [PATCH 1 of 4 V2] libxl: initialise cpupoolinfo in libxl__domain_scheduler Ian Campbell
2012-06-29  8:14 ` [PATCH 2 of 4 V2] libxl: correct type of cpupool variable Ian Campbell
2012-06-29  8:14 ` [PATCH 3 of 4 V2] libxl: log on failure in cpupool_info and libxl__domain_cpupool Ian Campbell
2012-06-29  8:14 ` [PATCH 4 of 4 V2] xl: initialise domid to an explicitly invalid value Ian Campbell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).