From owner-freebsd-xen@FreeBSD.ORG Wed Jul 6 04:14:10 2011 Return-Path: Delivered-To: freebsd-xen@freebsd.org Received: from mx2.freebsd.org (mx2.freebsd.org [IPv6:2001:4f8:fff6::35]) by hub.freebsd.org (Postfix) with ESMTP id 52A5D106566B for ; Wed, 6 Jul 2011 04:14:10 +0000 (UTC) (envelope-from cperciva@freebsd.org) Received: from xps.daemonology.net (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx2.freebsd.org (Postfix) with SMTP id EFD8714E214 for ; Wed, 6 Jul 2011 04:14:09 +0000 (UTC) Received: (qmail 6161 invoked from network); 6 Jul 2011 04:14:09 -0000 Received: from unknown (HELO xps.daemonology.net) (127.0.0.1) by localhost with SMTP; 6 Jul 2011 04:14:09 -0000 Message-ID: <4E13E111.9070005@freebsd.org> Date: Tue, 05 Jul 2011 21:14:09 -0700 From: Colin Percival User-Agent: Mozilla/5.0 (X11; U; FreeBSD amd64; en-US; rv:1.9.1.16) Gecko/20101220 Thunderbird/3.0.11 MIME-Version: 1.0 To: gibbs@freebsd.org, "freebsd-xen@freebsd.org" References: <4DF18EE5.5090704@freebsd.org> <4DF27EFF.5060005@FreeBSD.org> <4E13B6E9.9070202@freebsd.org> <4E13C2A4.5040202@FreeBSD.org> <4E13CB8F.8000009@freebsd.org> In-Reply-To: <4E13CB8F.8000009@freebsd.org> X-Enigmail-Version: 1.0.1 Content-Type: multipart/mixed; boundary="------------030903070809000902080104" Cc: Subject: Re: breakage in blkfront with ring_pages > 1 X-BeenThere: freebsd-xen@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion of the freebsd port to xen - implementation and usage List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Jul 2011 04:14:10 -0000 This is a multi-part message in MIME format. --------------030903070809000902080104 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit On 07/05/11 19:42, Colin Percival wrote: > On 07/05/11 19:04, Justin T. Gibbs wrote: >> On 7/5/11 7:14 PM, Colin Percival wrote: >>> Maybe the right option is to have a loader tunable dev.xn.linuxback to >>> control which version of the protocol is used? >> >> What a mess. > > Yep. Mess or not, shall I go ahead with having a loader tunable control this, > or can you think of a better solution? Does anyone object to the attached patch? It keeps the differing behaviour to a minimum -- we MUST set ring-ref with a FreeBSD blkback, and we MUST NOT set it with a linux blkback -- but otherwise errs in the direction of setting more variables than are needed, to maximize the possibility of a future blkback being compatible with both blkback_is_linux=0 and blkback_is_linux=1. -- Colin Percival Security Officer, FreeBSD | freebsd.org | The power to serve Founder / author, Tarsnap | tarsnap.com | Online backups for the truly paranoid --------------030903070809000902080104 Content-Type: text/x-diff; name="blkfront.c.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="blkfront.c.patch" Index: sys/dev/xen/blkfront/blkfront.c =================================================================== --- sys/dev/xen/blkfront/blkfront.c (revision 223815) +++ sys/dev/xen/blkfront/blkfront.c (working copy) @@ -616,7 +616,16 @@ if (setup_blkring(sc) != 0) return; + /* Different backends use different names for this variable. */ error = xs_printf(XST_NIL, node_path, + "num-ring-pages","%u", sc->ring_pages); + if (error) { + xenbus_dev_fatal(sc->xb_dev, error, + "writing %s/num-ring-pages", + node_path); + return; + } + error = xs_printf(XST_NIL, node_path, "ring-pages","%u", sc->ring_pages); if (error) { xenbus_dev_fatal(sc->xb_dev, error, @@ -673,6 +682,9 @@ xenbus_set_state(sc->xb_dev, XenbusStateInitialised); } +static int blkback_is_linux = 0; +TUNABLE_INT("dev.xbd.blkback_is_linux", &blkback_is_linux); + static int setup_blkring(struct xb_softc *sc) { @@ -702,14 +714,16 @@ return (error); } } - error = xs_printf(XST_NIL, xenbus_get_node(sc->xb_dev), - "ring-ref","%u", sc->ring_ref[0]); - if (error) { - xenbus_dev_fatal(sc->xb_dev, error, "writing %s/ring-ref", - xenbus_get_node(sc->xb_dev)); - return (error); + if (!blkback_is_linux || sc->ring_pages == 1) { + error = xs_printf(XST_NIL, xenbus_get_node(sc->xb_dev), + "ring-ref","%u", sc->ring_ref[0]); + if (error) { + xenbus_dev_fatal(sc->xb_dev, error, "writing %s/ring-ref", + xenbus_get_node(sc->xb_dev)); + return (error); + } } - for (i = 1; i < sc->ring_pages; i++) { + for (i = 0; i < sc->ring_pages; i++) { char ring_ref_name[]= "ring_refXX"; snprintf(ring_ref_name, sizeof(ring_ref_name), "ring-ref%u", i); --------------030903070809000902080104--