Re: [Xen-devel] [PATCH] tools: libxl: do not set the PoD target on ARM

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2014-01-16 at 15:27 +0000, Ian Campbell wrote:
> ARM does not implemented PoD and so returns ENOSYS from XENMEM_set_pod_target.
> 
> The correct solution here would be to check for ENOSYS in libxl, unfortunately
> xc_domain_set_pod_target suffers from the same broken error reporting as the
> rest of libxc and throws away the errno.
> 
> So for now conditionally define xc_domain_set_pod_target to return success
> (which is what PoD does if nothing needs doing). xc_domain_get_pod_target sets
> errno==-1 and returns -1, which matches the broken error reporting of the
> existing function. It appears to have no in tree callers in any case.
> 
> The conditional should be removed once libxc has been fixed.
> 
> This makes ballooning (xl mem-set) work for ARM domains.
> 
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> Cc: george.dunlap@xxxxxxxxxx
> ---
> I'd be generally wary of modifying the error handling in a piecemeal way, but
> certainly doing so for 4.4 now would be inapropriate.
> 
> IIRC Ian J was planning a thorough sweep of the libxc error paths in 4.5 time
> frame, at which point this conditional stuff could be dropped.
> 
> In terms of the 4.4 release, obviously ballooning would be very nice to have
> for ARM guests, on the other hand I'm aware that while the patch is fairly
> small/contained and safe it is also pretty skanky and likely wouldn't be
> accepted outside of the rc period.

George -- what do you think of this?

> ---
>  tools/libxc/xc_domain.c |   28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
> index c2fdd74..e1d1bec 100644
> --- a/tools/libxc/xc_domain.c
> +++ b/tools/libxc/xc_domain.c
> @@ -986,6 +986,12 @@ out:
>      return rc;
>  }
>  
> +/* Currently only implemented on x86. This cannot be handled in the
> + * caller, e.g. by looking for errno==ENOSYS because of the broken
> + * error reporting style. Once this is fixed then this condition can
> + * be removed.
> + */
> +#if defined(__i386__)||defined(__x86_64__)
>  static int xc_domain_pod_target(xc_interface *xch,
>                                  int op,
>                                  uint32_t domid,
> @@ -1055,6 +1061,28 @@ int xc_domain_get_pod_target(xc_interface *xch,
>                                  pod_cache_pages,
>                                  pod_entries);
>  }
> +#else
> +int xc_domain_set_pod_target(xc_interface *xch,
> +                             uint32_t domid,
> +                             uint64_t target_pages,
> +                             uint64_t *tot_pages,
> +                             uint64_t *pod_cache_pages,
> +                             uint64_t *pod_entries)
> +{
> +    return 0;
> +}
> +int xc_domain_get_pod_target(xc_interface *xch,
> +                             uint32_t domid,
> +                             uint64_t *tot_pages,
> +                             uint64_t *pod_cache_pages,
> +                             uint64_t *pod_entries)
> +{
> +    /* On x86 (above) xc_domain_pod_target will incorrectly return -1
> +     * with errno==-1 on error. Do the same for least surprise. */
> +    errno = -1;
> +    return -1;
> +}
> +#endif
>  
>  int xc_domain_max_vcpus(xc_interface *xch, uint32_t domid, unsigned int max)
>  {



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