From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH XEN v6 14/32] tools: Remove xc_map_foreign_batch Date: Fri, 11 Dec 2015 15:42:23 +0000 Message-ID: <1449848543.30975.54.camel@citrix.com> References: <1449141675.4424.125.camel@citrix.com> <1449141749-14940-1-git-send-email-ian.campbell@citrix.com> <1449141749-14940-15-git-send-email-ian.campbell@citrix.com> <5669985E.1020208@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5669985E.1020208@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: George Dunlap , ian.jackson@eu.citrix.com, wei.liu2@citrix.com, xen-devel@lists.xen.org Cc: George Dunlap List-Id: xen-devel@lists.xenproject.org On Thu, 2015-12-10 at 15:21 +0000, George Dunlap wrote: > On 03/12/15 11:22, Ian Campbell wrote: > > It can trivially be replaced by xc_map_foreign_pages which is the > > interface I want to move to going forward (by standardising on _bulk > > but handling err=NULL as _pages does). > > > > The callers of _batch are checking a mixture of a NULL return or > > looking to see if the top nibble of the (usually sole) mfn they pass > > has been modified to be non-zero to detect errors. _pages never > > modifies the mfn it was given (it's const) and returns NULL on > > failure, so adjust the error handling where necessary. Some callers > > use a copy of the mfn array, for reuse on failure with _batch, which > > is no longer necessary as _pages doesn't modify the array, however I > > haven't cleaned that up here. > > > > This reduces the twist maze of xc_map_foreign_* by one, which will be > > useful when trying to come up with an underlying stable interface. > > > > NetBSD and Solaris implemented xc_map_foreign_bulk in terms of > > xc_map_foreign_batch via a compat layer, so xc_map_foreign_batch > > becomes an internal osdep for them. New OS ports should always > > implement xc_map_foreign_bulk instead. > > "New OS ports should always implement xc_map_foreign_pages instead"? "It's complicated" ;-). New ports should indeed implement the bulk interface, which will in a later patch become the API which libxenforeignmemory provides. Then the _pages style interface comes via a that API providing a mode which is compatible with that way of use (tolerating passing a NULL err array). Given that the advice here is going to be superceded pretty soon in this series, I think I will therefore just drop that final sentence. > Other than that: > > Acked-by: George Dunlap I'll take the liberty of assuming this still applies with the change discussed above, please let me know if not. Cheers, Ian.