[RFC Patch 2/2] tools/xc_domain_getinfo: Implement sane interface for single domain information.

[Andrew Cooper <andrew.cooper3@citrix.com>]

Now that xc_domain_getinfo_many() exists, implement a rather more sane
interface for xc_domain_getinfo(), under the name xc_domain_getinfo_single(),
which will only return success if the information is for the requested domid.

As can be seen from the patch, only 3 of the in-tree callers correctly check
whether the information they have refers to the correct domain.

There are out-of-tree callers of xc_domain_getinfo, so removing the API is a
little antisocial at this point.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 tools/console/client/main.c                        |    3 +-
 tools/console/daemon/io.c                          |    6 +-
 tools/debugger/kdd/kdd-xen.c                       |    2 +-
 tools/libxc/xc_core.c                              |    2 +-
 tools/libxc/xc_cpuid_x86.c                         |    2 +-
 tools/libxc/xc_domain.c                            |   96 +++++++++++++-------
 tools/libxc/xc_domain_save.c                       |    4 +-
 tools/libxc/xc_offline_page.c                      |    2 +-
 tools/libxc/xc_pagetab.c                           |    3 +-
 tools/libxc/xc_resume.c                            |    4 +-
 tools/libxc/xenctrl.h                              |   12 +++
 tools/misc/xen-hvmcrash.c                          |    4 +-
 tools/misc/xen-lowmemd.c                           |    2 +-
 .../python/xen/lowlevel/checkpoint/libcheckpoint.c |    5 +-
 tools/xenstore/xenstored_domain.c                  |    5 +-
 tools/xentrace/xenctx.c                            |    4 +-
 16 files changed, 97 insertions(+), 59 deletions(-)

diff --git a/tools/console/client/main.c b/tools/console/client/main.c
index 523fc23..ec1211d 100644
--- a/tools/console/client/main.c
+++ b/tools/console/client/main.c
@@ -339,7 +339,8 @@ int main(int argc, char **argv)
 		xc_interface *xc_handle = xc_interface_open(0,0,0);
 		if (xc_handle == NULL)
 			err(errno, "Could not open xc interface");
-		xc_domain_getinfo(xc_handle, domid, 1, &xcinfo);
+		if (xc_domain_getinfo_single(xc_handle, domid, &xcinfo) != 0)
+			err(errno, "Unable to get domain information");
 		/* default to pv console for pv guests and serial for hvm guests */
 		if (xcinfo.hvm)
 			type = CONSOLE_SERIAL;
diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
index ba9a2bd..0f37188 100644
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -251,13 +251,9 @@ static void buffer_advance(struct buffer *buffer, size_t len)
 
 static bool domain_is_valid(int domid)
 {
-	bool ret;
 	xc_dominfo_t info;
 
-	ret = (xc_domain_getinfo(xc, domid, 1, &info) == 1 &&
-	       info.domid == domid);
-		
-	return ret;
+	return xc_domain_getinfo_single(xc, domid, &info) == 0;
 }
 
 static int create_hv_log(void)
diff --git a/tools/debugger/kdd/kdd-xen.c b/tools/debugger/kdd/kdd-xen.c
index 4fbea7d..9ebc3f1 100644
--- a/tools/debugger/kdd/kdd-xen.c
+++ b/tools/debugger/kdd/kdd-xen.c
@@ -593,7 +593,7 @@ kdd_guest *kdd_guest_init(char *arg, FILE *log, int verbosity)
     g->domid = domid;
 
     /* Check that the domain exists and is HVM */
-    if (xc_domain_getinfo(xch, domid, 1, &info) != 1 || !info.hvm)
+    if (xc_domain_getinfo_single(xch, domid, &info) != 0 || !info.hvm)
         goto err;
 
     snprintf(g->id, (sizeof g->id) - 1, 
diff --git a/tools/libxc/xc_core.c b/tools/libxc/xc_core.c
index 4207eed..66f7313 100644
--- a/tools/libxc/xc_core.c
+++ b/tools/libxc/xc_core.c
@@ -491,7 +491,7 @@ xc_domain_dumpcore_via_callback(xc_interface *xch,
         goto out;
     }
 
-    if ( xc_domain_getinfo(xch, domid, 1, &info) != 1 )
+    if ( xc_domain_getinfo_single(xch, domid, &info) != 0 )
     {
         PERROR("Could not get info for domain");
         goto out;
diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index fa47787..d18f45e 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -557,7 +557,7 @@ static int xc_cpuid_policy(
 {
     xc_dominfo_t        info;
 
-    if ( xc_domain_getinfo(xch, domid, 1, &info) == 0 )
+    if ( xc_domain_getinfo_single(xch, domid, &info) != 0 )
         return -EINVAL;
 
     if ( info.hvm )
diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index e49dfc2..da65b2c 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -270,6 +270,69 @@ out:
     return ret;
 }
 
+static void xc_domaininfo_to_dominfo(const xc_domaininfo_t *src,
+                                     xc_dominfo_t *dst)
+{
+    dst->dying    = !!(src->flags & XEN_DOMINF_dying);
+    dst->shutdown = !!(src->flags & XEN_DOMINF_shutdown);
+    dst->paused   = !!(src->flags & XEN_DOMINF_paused);
+    dst->blocked  = !!(src->flags & XEN_DOMINF_blocked);
+    dst->running  = !!(src->flags & XEN_DOMINF_running);
+    dst->hvm      = !!(src->flags & XEN_DOMINF_hvm_guest);
+    dst->debugged = !!(src->flags & XEN_DOMINF_debugged);
+
+    dst->shutdown_reason =
+        (src->flags>>XEN_DOMINF_shutdownshift) &
+        XEN_DOMINF_shutdownmask;
+
+    if ( dst->shutdown && (dst->shutdown_reason == SHUTDOWN_crash) )
+    {
+        dst->shutdown = 0;
+        dst->crashed  = 1;
+    }
+
+    dst->ssidref  = src->ssidref;
+    dst->nr_pages = src->tot_pages;
+    dst->nr_outstanding_pages = src->outstanding_pages;
+    dst->nr_shared_pages = src->shr_pages;
+    dst->nr_paged_pages = src->paged_pages;
+    dst->max_memkb = src->max_pages << (PAGE_SHIFT-10);
+    dst->shared_info_frame = src->shared_info_frame;
+    dst->cpu_time = src->cpu_time;
+    dst->nr_online_vcpus = src->nr_online_vcpus;
+    dst->max_vcpu_id = src->max_vcpu_id;
+    dst->cpupool = src->cpupool;
+
+    memcpy(dst->handle, src->handle,
+           sizeof(xen_domain_handle_t));
+}
+
+int xc_domain_getinfo_single(xc_interface *xch,
+                             uint32_t domid,
+                             xc_dominfo_t *info)
+{
+    DECLARE_DOMCTL;
+    int rc = 0;
+
+    domctl.cmd = XEN_DOMCTL_getdomaininfo;
+    domctl.domain = (domid_t)domid;
+
+    rc = do_domctl(xch, &domctl);
+    if ( rc )
+        return rc;
+
+    if ( domctl.domain != domid ) {
+        errno = ESRCH;
+        return -1;
+    }
+
+    memset(info, 0, sizeof *info);
+
+    info->domid = domctl.domain;
+    xc_domaininfo_to_dominfo(&domctl.u.getdomaininfo, info);
+
+    return 0;
+}
 
 int xc_domain_getinfo_many(xc_interface *xch,
                            uint32_t first_domid,
@@ -291,38 +354,7 @@ int xc_domain_getinfo_many(xc_interface *xch,
             break;
         info->domid      = (uint16_t)domctl.domain;
 
-        info->dying    = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_dying);
-        info->shutdown = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_shutdown);
-        info->paused   = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_paused);
-        info->blocked  = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_blocked);
-        info->running  = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_running);
-        info->hvm      = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_hvm_guest);
-        info->debugged = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_debugged);
-
-        info->shutdown_reason =
-            (domctl.u.getdomaininfo.flags>>XEN_DOMINF_shutdownshift) &
-            XEN_DOMINF_shutdownmask;
-
-        if ( info->shutdown && (info->shutdown_reason == SHUTDOWN_crash) )
-        {
-            info->shutdown = 0;
-            info->crashed  = 1;
-        }
-
-        info->ssidref  = domctl.u.getdomaininfo.ssidref;
-        info->nr_pages = domctl.u.getdomaininfo.tot_pages;
-        info->nr_outstanding_pages = domctl.u.getdomaininfo.outstanding_pages;
-        info->nr_shared_pages = domctl.u.getdomaininfo.shr_pages;
-        info->nr_paged_pages = domctl.u.getdomaininfo.paged_pages;
-        info->max_memkb = domctl.u.getdomaininfo.max_pages << (PAGE_SHIFT-10);
-        info->shared_info_frame = domctl.u.getdomaininfo.shared_info_frame;
-        info->cpu_time = domctl.u.getdomaininfo.cpu_time;
-        info->nr_online_vcpus = domctl.u.getdomaininfo.nr_online_vcpus;
-        info->max_vcpu_id = domctl.u.getdomaininfo.max_vcpu_id;
-        info->cpupool = domctl.u.getdomaininfo.cpupool;
-
-        memcpy(info->handle, domctl.u.getdomaininfo.handle,
-               sizeof(xen_domain_handle_t));
+        xc_domaininfo_to_dominfo(&domctl.u.getdomaininfo, info);
 
         next_domid = (uint16_t)domctl.domain + 1;
         info++;
diff --git a/tools/libxc/xc_domain_save.c b/tools/libxc/xc_domain_save.c
index fbc15e9..b42c1c5 100644
--- a/tools/libxc/xc_domain_save.c
+++ b/tools/libxc/xc_domain_save.c
@@ -365,7 +365,7 @@ static int suspend_and_state(int (*suspend)(void*), void* data,
         return -1;
     }
 
-    if ( (xc_domain_getinfo(xch, dom, 1, info) != 1) ||
+    if ( (xc_domain_getinfo_single(xch, dom, info) != 0) ||
          !info->shutdown || (info->shutdown_reason != SHUTDOWN_suspend) )
     {
         ERROR("Domain not in suspended state");
@@ -919,7 +919,7 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iter
         return 1;
     }
 
-    if ( xc_domain_getinfo(xch, dom, 1, &info) != 1 )
+    if ( xc_domain_getinfo_single(xch, dom, &info) != 0 )
     {
         PERROR("Could not get domain info");
         return 1;
diff --git a/tools/libxc/xc_offline_page.c b/tools/libxc/xc_offline_page.c
index 36b9812..0405a3f 100644
--- a/tools/libxc/xc_offline_page.c
+++ b/tools/libxc/xc_offline_page.c
@@ -557,7 +557,7 @@ int xc_exchange_page(xc_interface *xch, int domid, xen_pfn_t mfn)
     uint32_t status;
     xen_pfn_t new_mfn, gpfn;
 
-    if ( xc_domain_getinfo(xch, domid, 1, &info) != 1 )
+    if ( xc_domain_getinfo_single(xch, domid, &info) != 0 )
     {
         ERROR("Could not get domain info");
         return -EFAULT;
diff --git a/tools/libxc/xc_pagetab.c b/tools/libxc/xc_pagetab.c
index 27c4e9f..387139b 100644
--- a/tools/libxc/xc_pagetab.c
+++ b/tools/libxc/xc_pagetab.c
@@ -35,8 +35,7 @@ unsigned long xc_translate_foreign_address(xc_interface *xch, uint32_t dom,
     int size, level, pt_levels = 2;
     void *map;
 
-    if (xc_domain_getinfo(xch, dom, 1, &dominfo) != 1 
-        || dominfo.domid != dom)
+    if (xc_domain_getinfo_single(xch, dom, &dominfo) != 0)
         return 0;
 
     /* What kind of paging are we dealing with? */
diff --git a/tools/libxc/xc_resume.c b/tools/libxc/xc_resume.c
index 1c43ec6..30de9bc 100644
--- a/tools/libxc/xc_resume.c
+++ b/tools/libxc/xc_resume.c
@@ -46,7 +46,7 @@ static int modify_returncode(xc_interface *xch, uint32_t domid)
     struct domain_info_context *dinfo = &_dinfo;
     int rc;
 
-    if ( xc_domain_getinfo(xch, domid, 1, &info) != 1 )
+    if ( xc_domain_getinfo_single(xch, domid, &info) != 0 )
     {
         PERROR("Could not get domain info");
         return -1;
@@ -131,7 +131,7 @@ static int xc_domain_resume_any(xc_interface *xch, uint32_t domid)
     xen_pfn_t *p2m = NULL;
 #endif
 
-    if ( xc_domain_getinfo(xch, domid, 1, &info) != 1 )
+    if ( xc_domain_getinfo_single(xch, domid, &info) != 0 )
     {
         PERROR("Could not get domain info");
         return rc;
diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
index 86abec7..6f795e1 100644
--- a/tools/libxc/xenctrl.h
+++ b/tools/libxc/xenctrl.h
@@ -562,6 +562,18 @@ int xc_vcpu_getaffinity(xc_interface *xch,
                         xc_cpumap_t cpumap);
 
 /**
+ * This function will return information about a single domain.
+ *
+ * @parm xch a handle to an open hypervisor interface
+ * @parm domid the domain to get information information from.
+ * @parm info an xc_dominfo_t structure to hold the information.
+ * @return 0 on success or -1 on error
+ */
+int xc_domain_getinfo_single(xc_interface *xch,
+                             uint32_t domid,
+                             xc_dominfo_t *info);
+
+/**
  * This function will return information about one or more domains. It is
  * designed to iterate over the list of domains. If a single domain is
  * requested, this function will return the next domain in the list - if
diff --git a/tools/misc/xen-hvmcrash.c b/tools/misc/xen-hvmcrash.c
index 4f0dabc..bdd60fc 100644
--- a/tools/misc/xen-hvmcrash.c
+++ b/tools/misc/xen-hvmcrash.c
@@ -66,8 +66,8 @@ main(int argc, char **argv)
         exit(1);
     }
 
-    ret = xc_domain_getinfo(xch, domid, 1, &dominfo);
-    if (ret < 0) {
+    ret = xc_domain_getinfo_single(xch, domid, &dominfo);
+    if (ret != 0) {
         perror("xc_domain_getinfo");
         exit(1);
     }
diff --git a/tools/misc/xen-lowmemd.c b/tools/misc/xen-lowmemd.c
index 82ffd75..d5b6eb0 100644
--- a/tools/misc/xen-lowmemd.c
+++ b/tools/misc/xen-lowmemd.c
@@ -57,7 +57,7 @@ void handle_low_mem(void)
         return;
     diff = THRESHOLD_PG - free_pages; 
 
-    if (xc_domain_getinfo(xch, 0, 1, &dom0_info) < 1)
+    if (xc_domain_getinfo_single(xch, 0, &dom0_info) != 0)
     {
         perror("Failed to get dom0 info");
         return;
diff --git a/tools/python/xen/lowlevel/checkpoint/libcheckpoint.c b/tools/python/xen/lowlevel/checkpoint/libcheckpoint.c
index 01c0d47..cacdfee 100644
--- a/tools/python/xen/lowlevel/checkpoint/libcheckpoint.c
+++ b/tools/python/xen/lowlevel/checkpoint/libcheckpoint.c
@@ -97,7 +97,7 @@ int checkpoint_open(checkpoint_state* s, unsigned int domid)
        return -1;
     }
 
-    if (xc_domain_getinfo(s->xch, s->domid, 1, &dominfo) < 0) {
+    if (xc_domain_getinfo_single(s->xch, s->domid, &dominfo) != 0) {
        checkpoint_close(s);
        s->errstr = "could not get domain info";
 
@@ -428,8 +428,7 @@ static int check_shutdown(checkpoint_state* s) {
 	    break;
     }
 
-    if (xc_domain_getinfo(s->xch, s->domid, 1, &info) != 1
-	|| info.domid != s->domid) {
+    if (xc_domain_getinfo_single(s->xch, s->domid, &info) != 0) {
 	snprintf(errbuf, sizeof(errbuf),
 		 "error getting info for domain %u", s->domid);
 	s->errstr = errbuf;
diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
index bf83d58..e626593 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -217,9 +217,8 @@ static void domain_cleanup(void)
 	int notify = 0;
 
 	list_for_each_entry_safe(domain, tmp, &domains, list) {
-		if (xc_domain_getinfo(*xc_handle, domain->domid, 1,
-				      &dominfo) == 1 &&
-		    dominfo.domid == domain->domid) {
+		if (xc_domain_getinfo_single(*xc_handle, domain->domid,
+				      &dominfo) == 0) {
 			if ((dominfo.crashed || dominfo.shutdown)
 			    && !domain->shutdown) {
 				domain->shutdown = 1;
diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c
index 060e480..e1ca581 100644
--- a/tools/xentrace/xenctx.c
+++ b/tools/xentrace/xenctx.c
@@ -896,8 +896,8 @@ int main(int argc, char **argv)
         exit(-1);
     }
 
-    ret = xc_domain_getinfo(xenctx.xc_handle, xenctx.domid, 1, &xenctx.dominfo);
-    if (ret < 0) {
+    ret = xc_domain_getinfo_single(xenctx.xc_handle, xenctx.domid, &xenctx.dominfo);
+    if (ret != 0) {
         perror("xc_domain_getinfo");
         exit(-1);
     }
-- 
1.7.10.4