Date: Tue, 05 Jul 2011 21:14:09 -0700 From: Colin Percival <cperciva@freebsd.org> To: gibbs@freebsd.org, "freebsd-xen@freebsd.org" <freebsd-xen@freebsd.org> Subject: Re: breakage in blkfront with ring_pages > 1 Message-ID: <4E13E111.9070005@freebsd.org> In-Reply-To: <4E13CB8F.8000009@freebsd.org> References: <4DF18EE5.5090704@freebsd.org> <4DF27EFF.5060005@FreeBSD.org> <4E13B6E9.9070202@freebsd.org> <4E13C2A4.5040202@FreeBSD.org> <4E13CB8F.8000009@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4E13E111.9070005>