From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] oxenstored: fix short-write issue Date: Tue, 27 Oct 2015 17:21:39 +0000 Message-ID: <562FB2A3.8070603@citrix.com> References: <1445965809-5144-1-git-send-email-wei.liu2@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Zr7wP-0003Ik-BY for xen-devel@lists.xenproject.org; Tue, 27 Oct 2015 17:21:45 +0000 In-Reply-To: <1445965809-5144-1-git-send-email-wei.liu2@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: Wei Liu , Xen-devel Cc: Samuel Thibault , Ian Jackson , Ian Campbell , David Scott List-Id: xen-devel@lists.xenproject.org On 27/10/15 17:10, Wei Liu wrote: > When oxenstored wrote to the ring, it wrote a chunk of contiguous data. > Originally when it tried to write across ring boundary, it returned a > short-write when there is still room. That led to stalling mini-os's > xenstore thread at times. > > Fix this by calling write function for a second time when the first > write completes partially. > > Signed-off-by: Samuel Thibault > Signed-off-by: Wei Liu > --- > Cc: David Scott > Cc: Ian Campbell > Cc: Ian Jackson > > Ian, backport candidate for as far as you can manage. > --- > tools/ocaml/libs/xb/xb.ml | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/tools/ocaml/libs/xb/xb.ml b/tools/ocaml/libs/xb/xb.ml > index 50944b5..0730d13 100644 > --- a/tools/ocaml/libs/xb/xb.ml > +++ b/tools/ocaml/libs/xb/xb.ml > @@ -91,10 +91,12 @@ let write_fd back con s len = > Unix.write back.fd s 0 len > > let write_mmap back con s len = > - let ws = Xs_ring.write back.mmap s len in > - if ws > 0 then > + let ws = ref (Xs_ring.write back.mmap s len) in > + if !ws < len then > + ws := !ws + Xs_ring.write back.mmap (String.sub s !ws (len - !ws)) (len - !ws); This is surely a TOCTOU, as the second attempted write could return short as well. The correct behaviour would be for Xs_ring.write to return the actual number of bytes it put into the ring, even if this is shorter than len. ~Andrew