All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wei Liu <wei.liu2@citrix.com>
To: Jonathan Davies <jonathan.davies@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Jon Ludlam <jonathan.ludlam@citrix.com>,
	Euan Harris <euan.harris@citrix.com>,
	David Scott <dave@recoil.org>,
	xen-devel@lists.xenproject.org,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>
Subject: Re: [PATCH 3/7] oxenstored: refactor request processing
Date: Wed, 30 Mar 2016 16:53:55 +0100	[thread overview]
Message-ID: <20160330155355.GJ5656@citrix.com> (raw)
In-Reply-To: <20160330154658.GF3316@citrix.com>

On Wed, Mar 30, 2016 at 04:46:58PM +0100, Jonathan Davies wrote:
[...]
> > >> Andrew's guess was close, but the wrong way around -- please could you try the
> > >> following with the older compiler?
> > >> 
> > >> let req = {Packet.tid=tid; Packet.rid=rid; Packet.ty=ty; Packet.data=data} in
> > >> 
> > >> I was using a syntactic feature of OCaml called 'field punning' which is
> > >> generally considered good practice and makes for more readable code. It looks
> > >> like this feature was introduced in OCaml 3.12.0 (dating from 2010), which is
> > >> consistent with Boris' findings.
> > >> 
> > >> What's the policy here -- is there a defined version of the OCaml compiler which
> > >> tools/ocaml needs to be able to compile with?
> > > 
> > > It is not explicitly listed in README or INSTALL. The ocaml tools
> > > maintainer (Dave in this case) is welcome to provide the minimum version
> > > required.
> > > 
> > > Meanwhile, I don't think we should break existing build without pinning
> > > down the minimum required version first, so we should fix Boris's
> > > breakage.  The fix seems simple enough anyway.
> > 
> > It looks like the fix is small and easy — I think this is good for now.
> > 
> > Let’s postpone requiring a later OCaml version until we really need a feature only present in a later version. I suspect this will happen eventually, probably when we try to add a dependency (e.g. from the Mirage world) which requires 4.02+.
> 
> OK; sounds sensible.
> 
> Since the original patch has already been committed to staging, I presume you'd
> like me to formally submit a standalone patch that fixes this issue. I will post
> it separately.
> 

Yes, a standalone patch to fix the issue is fine.

Wei.

> If I'm wrong and you'd like me to post a v2 of the whole series, let me know.
> 
> Thanks,
> Jonathan

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

  reply	other threads:[~2016-03-30 15:53 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-17 17:51 [PATCH 0/7] oxenstored: improve transaction conflict handling Jonathan Davies
2016-03-17 17:51 ` [PATCH 1/7] oxenstored: refactor putting response on wire Jonathan Davies
2016-03-17 17:51 ` [PATCH 2/7] oxenstored: remove some unused parameters Jonathan Davies
2016-03-17 17:51 ` [PATCH 3/7] oxenstored: refactor request processing Jonathan Davies
2016-03-24 22:22   ` Boris Ostrovsky
2016-03-24 22:49     ` Andrew Cooper
2016-03-24 23:57       ` Boris Ostrovsky
2016-03-29  9:08         ` Jonathan Davies
2016-03-29 12:45           ` Boris Ostrovsky
2016-03-29 16:38           ` Wei Liu
2016-03-29 19:41             ` David Scott
2016-03-30 15:46               ` Jonathan Davies
2016-03-30 15:53                 ` Wei Liu [this message]
2016-03-17 17:51 ` [PATCH 4/7] oxenstored: keep track of each transaction's operations Jonathan Davies
2016-03-17 17:51 ` [PATCH 5/7] oxenstored: move functions that process simple operations Jonathan Davies
2016-03-17 17:51 ` [PATCH 6/7] oxenstored: replay transaction upon conflict Jonathan Davies
2016-03-17 17:51 ` [PATCH 7/7] oxenstored: log request and response during transaction replay Jonathan Davies
2016-03-18 14:33 ` [PATCH 0/7] oxenstored: improve transaction conflict handling Konrad Rzeszutek Wilk
2016-03-18 16:21   ` Jonathan Davies
2016-03-18 16:36   ` Wei Liu
2016-03-19 11:30     ` David Scott

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160330155355.GJ5656@citrix.com \
    --to=wei.liu2@citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=boris.ostrovsky@oracle.com \
    --cc=dave@recoil.org \
    --cc=euan.harris@citrix.com \
    --cc=jonathan.davies@citrix.com \
    --cc=jonathan.ludlam@citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.