Re: [Xen-devel] [RFC Patch 1/2] tools/xc_domain_getinfo: Fix callsites looking for more than 1 domain.

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Mon, 2013-08-12 at 18:31 +0100, Andrew Cooper wrote:
> xc_domain_getinfo has an absolutely insane interface, leading to it being
> misused by accident at almost all callsites.
> 
> This patch renames the current xc_domain_getinfo() sideways to
> xc_domain_getinfo_many(), and provides a #define for compilation
> compatibility.

libxc doesn't have any API guarantees so we don't need to do this for
that reason. Of course renaming makes it easier to validate that you
have caught all the callsites.

FWIW libxc has a few existing functions which are called foo_exact
rather than foo_single.

> 
> The callsites which are actually using the ability to find more than a
> specific domain are updated to to the _many() variant, while all other
> callsites use the compatibility #define for now.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> CC: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
> CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> ---
>  tools/console/daemon/io.c         |    2 +-
>  tools/libxc/xc_domain.c           |    8 ++++----
>  tools/libxc/xenctrl.h             |   13 ++++++++-----
>  tools/python/xen/lowlevel/xc/xc.c |    2 +-
>  tools/xenmon/xenbaked.c           |    2 +-
>  5 files changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
> index 250550a..ba9a2bd 100644
> --- a/tools/console/daemon/io.c
> +++ b/tools/console/daemon/io.c
> @@ -758,7 +758,7 @@ static void enum_domains(void)
>  
>       enum_pass++;
>  
> -     while (xc_domain_getinfo(xc, domid, 1, &dominfo) == 1) {
> +     while (xc_domain_getinfo_many(xc, domid, 1, &dominfo) == 1) {
>               dom = lookup_domain(dominfo.domid);
>               if (dominfo.dying) {
>                       if (dom)
> diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
> index 3257e2a..e49dfc2 100644
> --- a/tools/libxc/xc_domain.c
> +++ b/tools/libxc/xc_domain.c
> @@ -271,10 +271,10 @@ out:
>  }
>  
> 
> -int xc_domain_getinfo(xc_interface *xch,
> -                      uint32_t first_domid,
> -                      unsigned int max_doms,
> -                      xc_dominfo_t *info)
> +int xc_domain_getinfo_many(xc_interface *xch,
> +                           uint32_t first_domid,
> +                           unsigned int max_doms,
> +                           xc_dominfo_t *info)
>  {
>      unsigned int nr_doms;
>      uint32_t next_domid = first_domid;
> diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
> index 388a9c3..86abec7 100644
> --- a/tools/libxc/xenctrl.h
> +++ b/tools/libxc/xenctrl.h
> @@ -576,11 +576,14 @@ int xc_vcpu_getaffinity(xc_interface *xch,
>   *            the enumerated domains.
>   * @return the number of domains enumerated or -1 on error
>   */
> -int xc_domain_getinfo(xc_interface *xch,
> -                      uint32_t first_domid,
> -                      unsigned int max_doms,
> -                      xc_dominfo_t *info);
> -
> +int xc_domain_getinfo_many(xc_interface *xch,
> +                           uint32_t first_domid,
> +                           unsigned int max_doms,
> +                           xc_dominfo_t *info);
> +
> +/* Compatability for the moment for out of tree users.  External callers
> + * should move over to using an explicit _single() or _many() */
> +#define xc_domain_getinfo(x, f, m, i) xc_domain_getinfo_many((x), (f), (m), 
> (i))
>  
>  /**
>   * This function will set the execution context for the specified vcpu.
> diff --git a/tools/python/xen/lowlevel/xc/xc.c 
> b/tools/python/xen/lowlevel/xc/xc.c
> index e611b24..2d30890 100644
> --- a/tools/python/xen/lowlevel/xc/xc.c
> +++ b/tools/python/xen/lowlevel/xc/xc.c
> @@ -326,7 +326,7 @@ static PyObject *pyxc_domain_getinfo(XcObject *self,
>      if (info == NULL)
>          return PyErr_NoMemory();
>  
> -    nr_doms = xc_domain_getinfo(self->xc_handle, first_dom, max_doms, info);
> +    nr_doms = xc_domain_getinfo_many(self->xc_handle, first_dom, max_doms, 
> info);
>  
>      if (nr_doms < 0)
>      {
> diff --git a/tools/xenmon/xenbaked.c b/tools/xenmon/xenbaked.c
> index 985f1dd..d9c9bbe 100644
> --- a/tools/xenmon/xenbaked.c
> +++ b/tools/xenmon/xenbaked.c
> @@ -794,7 +794,7 @@ static int indexof(int domid)
>  
>      // call domaininfo hypercall to try and garbage collect unused entries
>      xc_handle = xc_interface_open(0,0,0);
> -    ndomains = xc_domain_getinfo(xc_handle, 0, NDOMAINS, dominfo);
> +    ndomains = xc_domain_getinfo_many(xc_handle, 0, NDOMAINS, dominfo);
>      xc_interface_close(xc_handle);
>  
>      // for each domain in our data, look for it in the system dominfo 
> structure



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