From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH v5 7/8] libxl/libxc: Move libxl_get_numainfo()'s hypercall buffer management to libxc Date: Fri, 20 Mar 2015 10:31:39 -0400 Message-ID: <550C2F4B.8080508@oracle.com> References: <1426802044-19444-1-git-send-email-boris.ostrovsky@oracle.com> <1426802044-19444-8-git-send-email-boris.ostrovsky@oracle.com> <1426859767.21742.194.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1426859767.21742.194.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: elena.ufimtseva@oracle.com, wei.liu2@citrix.com, stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, dario.faggioli@citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org, jbeulich@suse.com, keir@xen.org List-Id: xen-devel@lists.xenproject.org On 03/20/2015 09:56 AM, Ian Campbell wrote: > On Thu, 2015-03-19 at 17:54 -0400, Boris Ostrovsky wrote: >> diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c >> index 411128e..607ae61 100644 >> --- a/tools/libxc/xc_misc.c >> +++ b/tools/libxc/xc_misc.c >> @@ -209,22 +209,49 @@ out: >> return ret; >> } >> >> -int xc_numainfo(xc_interface *xch, >> - xc_numainfo_t *put_info) >> +int xc_numainfo(xc_interface *xch, unsigned *max_nodes, >> + xc_meminfo_t *meminfo, uint32_t *distance) >> { >> int ret; >> DECLARE_SYSCTL; >> + DECLARE_HYPERCALL_BOUNCE(meminfo, *max_nodes * sizeof(*meminfo), >> + XC_HYPERCALL_BUFFER_BOUNCE_OUT); >> + DECLARE_HYPERCALL_BOUNCE(distance, >> + *max_nodes * *max_nodes * sizeof(*distance), >> + XC_HYPERCALL_BUFFER_BOUNCE_OUT); >> >> - sysctl.cmd = XEN_SYSCTL_numainfo; >> + if (meminfo && distance) { >> + if ((ret = xc_hypercall_bounce_pre(xch, meminfo))) >> + goto out; >> + if ((ret = xc_hypercall_bounce_pre(xch, distance))) >> + goto out; > Same comment about handling NULL as before. > > In addition what if only one of meminfo and distance is NULL? Is that > valid or do you need a !!meminfo ^ !!distance check? I want to treat this as as an error here, which is why I have } else if (meminfo || distance) { errno = EINVAL; return -1; } because the hypervisor will only attempt to copy numainfo things when both are valid. Otherwise (i.e. even if only one is a NULL) it will assume that this is a request for size. The alternative would be to add another error there, which I decided not to do. -boris > > Rests looks ok. >