Re: [PATCH 3/6] libxc: use xc_vcpu_getinfo() instead of calling do_domctl()

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

On 12/07/13 17:48, Dario Faggioli wrote:
> The wrapper is there already, so better use it in place of
> all the stuff required to issue a call to do_domctl() for
> XEN_DOMCTL_getdomaininfo.
>
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> ---
>  tools/libxc/xc_dom_boot.c       |   14 ++++++--------
>  tools/libxc/xc_domain_restore.c |    7 +++----
>  tools/libxc/xc_private.c        |    8 +++-----
>  3 files changed, 12 insertions(+), 17 deletions(-)
>
> diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c
> index cf509fa..69b5c9c 100644
> --- a/tools/libxc/xc_dom_boot.c
> +++ b/tools/libxc/xc_dom_boot.c
> @@ -197,8 +197,8 @@ void *xc_dom_boot_domU_map(struct xc_dom_image *dom, xen_pfn_t pfn,
>  
>  int xc_dom_boot_image(struct xc_dom_image *dom)
>  {
> -    DECLARE_DOMCTL;
>      DECLARE_HYPERCALL_BUFFER(vcpu_guest_context_any_t, ctxt);
> +    xc_dominfo_t info;
>      int rc;
>  
>      ctxt = xc_hypercall_buffer_alloc(dom->xch, ctxt, sizeof(*ctxt));
> @@ -212,23 +212,21 @@ int xc_dom_boot_image(struct xc_dom_image *dom)
>          return rc;
>  
>      /* collect some info */
> -    domctl.cmd = XEN_DOMCTL_getdomaininfo;
> -    domctl.domain = dom->guest_domid;
> -    rc = do_domctl(dom->xch, &domctl);
> -    if ( rc != 0 )
> +    rc = xc_domain_getinfo(dom->xch, dom->guest_domid, 1, &info);
> +    if ( rc != 1 )
>      {
>          xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
>                       "%s: getdomaininfo failed (rc=%d)", __FUNCTION__, rc);
>          return rc;

With the rc from do_domctl() different to the rc from
xc_domain_getinfo(), this error print and return is possibly wrong.  An
rc of 0 is a now a valid failure case, meaning "I cant find any domains".

~Andrew

>      }
> -    if ( domctl.domain != dom->guest_domid )
> +    if ( info.domid != dom->guest_domid )
>      {
>          xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
>                       "%s: Huh? domid mismatch (%d != %d)", __FUNCTION__,
> -                     domctl.domain, dom->guest_domid);
> +                     info.domid, dom->guest_domid);
>          return -1;
>      }
> -    dom->shared_info_mfn = domctl.u.getdomaininfo.shared_info_frame;
> +    dom->shared_info_mfn = info.shared_info_frame;
>  
>      /* sanity checks */
>      if ( !xc_dom_compat_check(dom) )
> diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c
> index 41a63cb..b418963 100644
> --- a/tools/libxc/xc_domain_restore.c
> +++ b/tools/libxc/xc_domain_restore.c
> @@ -1406,6 +1406,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
>                        struct restore_callbacks *callbacks)
>  {
>      DECLARE_DOMCTL;
> +    xc_dominfo_t info;
>      int rc = 1, frc, i, j, n, m, pae_extended_cr3 = 0, ext_vcpucontext = 0;
>      int vcpuextstate = 0;
>      uint32_t vcpuextstate_size = 0;
> @@ -1562,14 +1563,12 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
>             ROUNDUP(MAX_BATCH_SIZE * sizeof(xen_pfn_t), PAGE_SHIFT)); 
>  
>      /* Get the domain's shared-info frame. */
> -    domctl.cmd = XEN_DOMCTL_getdomaininfo;
> -    domctl.domain = (domid_t)dom;
> -    if ( xc_domctl(xch, &domctl) < 0 )
> +    if ( xc_domain_getinfo(xch, (domid_t)dom, 1, &info) != 1 )
>      {
>          PERROR("Could not get information on new domain");
>          goto out;
>      }
> -    shared_info_frame = domctl.u.getdomaininfo.shared_info_frame;
> +    shared_info_frame = info.shared_info_frame;
>  
>      /* Mark all PFNs as invalid; we allocate on demand */
>      for ( pfn = 0; pfn < dinfo->p2m_size; pfn++ )
> diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c
> index acaf9e0..a260257 100644
> --- a/tools/libxc/xc_private.c
> +++ b/tools/libxc/xc_private.c
> @@ -609,11 +609,9 @@ int xc_get_pfn_list(xc_interface *xch,
>  
>  long xc_get_tot_pages(xc_interface *xch, uint32_t domid)
>  {
> -    DECLARE_DOMCTL;
> -    domctl.cmd = XEN_DOMCTL_getdomaininfo;
> -    domctl.domain = (domid_t)domid;
> -    return (do_domctl(xch, &domctl) < 0) ?
> -        -1 : domctl.u.getdomaininfo.tot_pages;
> +    xc_dominfo_t info;
> +    return (xc_domain_getinfo(xch, domid, 1, &info) != 1) ?
> +        -1 : info.nr_pages;
>  }
>  
>  int xc_copy_to_domain_page(xc_interface *xch,
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel